Skip to content

Commit bd58f2c

Browse files
TCL735TCL735
authored
feat: make custom time ranges smarter (#6070)
* feat: make custom time ranges smarter * test: update handleCustomTime unit tests to use shared constants * chore: clarify that time range values must be in RFC 3339 format * refactor: use inline regex to remove spaces and now() call * fix: update Date picker validation to interpret numbers as duration * fix: reject number timestamps when handling custom time for Giraffe
1 parent a00c85f commit bd58f2c

File tree

4 files changed

+268
-86
lines changed

4 files changed

+268
-86
lines changed

‎src/shared/components/dateRangePicker/NewDatePicker.tsx‎

Lines changed: 42 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,9 @@ const DatePickerMenu: FC<Props> = ({onCollapse, timeRange, timeRangeLabel}) => {
7777
collapse()
7878
}
7979

80+
const durationRegExp = /([0-9]+)(y|mo|w|d|h|ms|s|m|us|µs|ns)$/g
81+
8082
const validateInput = value => {
81-
const durationRegExp = /([0-9]+)(y|mo|w|d|h|ms|s|m|us|µs|ns)$/g
8283
return (
8384
isValidDatepickerFormat(value) ||
8485
!!value.match(durationRegExp) ||
@@ -89,7 +90,9 @@ const DatePickerMenu: FC<Props> = ({onCollapse, timeRange, timeRangeLabel}) => {
8990

9091
const handleSetStartDate = event => {
9192
const value = event.target.value
92-
if (validateInput(value)) {
93+
if (!value) {
94+
setInputStartErrorMessage('from field required')
95+
} else if (validateInput(value)) {
9396
if (inputStartErrorMessage !== NBSP) {
9497
setInputStartErrorMessage(NBSP)
9598
}
@@ -183,45 +186,45 @@ const DatePickerMenu: FC<Props> = ({onCollapse, timeRange, timeRangeLabel}) => {
183186
}
184187

185188
const handleApplyTimeRange = collapse => {
186-
// no start or end date
187-
if (inputStartDate == null && inputEndDate == null) {
188-
setInputStartErrorMessage('from field required')
189-
}
190-
if (validateInput(inputStartDate) && inputEndDate == null) {
191-
setRange({
192-
lower: inputStartDate,
193-
upper: 'now()',
194-
type: 'custom',
195-
})
196-
resetCalendar()
197-
collapse()
198-
return
199-
}
200-
// valid start date, valid end date
201-
if (validateInput(inputStartDate) && validateInput(inputEndDate)) {
202-
setRange({
203-
lower: inputStartDate,
204-
upper: inputEndDate,
205-
type: 'custom',
206-
})
207-
resetCalendar()
208-
collapse()
209-
return
210-
}
211-
// valid start date, invalid end date
212-
if (validateInput(inputStartDate) && !validateInput(inputEndDate)) {
213-
setInputEndErrorMessage('Invalid Format')
214-
return
189+
const isInputStartDateDuration =
190+
!inputStartDate.match(durationRegExp) && !isNaN(Number(inputStartDate))
191+
const isInputEndDateDuration =
192+
inputEndDate &&
193+
!inputEndDate.match(durationRegExp) &&
194+
!isNaN(Number(inputEndDate))
195+
196+
if (isInputStartDateDuration || isInputEndDateDuration) {
197+
if (isInputStartDateDuration) {
198+
setInputStartDate(`${inputStartDate}ms`)
199+
}
200+
if (isInputEndDateDuration) {
201+
setInputEndDate(`${inputEndDate}ms`)
202+
}
203+
} else if (validateInput(inputStartDate)) {
204+
if (!inputEndDate) {
205+
setRange({
206+
lower: inputStartDate,
207+
upper: 'now()',
208+
type: 'custom',
209+
})
210+
resetCalendar()
211+
collapse()
212+
} else if (validateInput(inputEndDate)) {
213+
setRange({
214+
lower: inputStartDate,
215+
upper: inputEndDate,
216+
type: 'custom',
217+
})
218+
resetCalendar()
219+
collapse()
220+
}
215221
}
216-
// invalid start date, valid end date
217-
if (!validateInput(inputStartDate) && validateInput(inputEndDate)) {
222+
223+
if (!validateInput(inputStartDate)) {
218224
setInputStartErrorMessage('Invalid Format')
219-
return
220225
}
221-
if (!validateInput(inputStartDate) && !validateInput(inputEndDate)) {
222-
setInputStartErrorMessage('Invalid Format')
226+
if (!validateInput(inputEndDate)) {
223227
setInputEndErrorMessage('Invalid Format')
224-
return
225228
}
226229
}
227230

@@ -272,12 +275,10 @@ const DatePickerMenu: FC<Props> = ({onCollapse, timeRange, timeRangeLabel}) => {
272275
tooltipContents={
273276
<>
274277
Use a relative duration (now(), -1h, -5m),{'\n'}
275-
absolute time (2022-08-28 14:26:00),{'\n'}
276-
or integer (Unix timestamp in seconds,{'\n'}
277-
like 1567029600).&nbsp;
278+
or absolute time (2022-08-28 14:26:00).
278279
</>
279280
}
280-
tooltipStyle={{maxWidth: 285}}
281+
tooltipStyle={{maxWidth: 290}}
281282
/>
282283
</InputLabel>
283284
<Form.Element

‎src/shared/utils/duration.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {Duration, DurationUnit} from 'src/types/ast'
88
import {TIME_RANGE_FORMAT} from 'src/shared/constants/timeRanges'
99
import {createDateTimeFormatter} from 'src/utils/datetime/formatters'
1010

11-
export const removeSpacesAndNow = (input: string): string =>
11+
export const removeSpacesNowAndMinus = (input: string): string =>
1212
input.replace(/\s/g, '').replace(/now\(\)-/, '')
1313

1414
export const isDurationWithNowParseable = (lower: string): boolean => {
@@ -17,7 +17,7 @@ export const isDurationWithNowParseable = (lower: string): boolean => {
1717
return false
1818
}
1919
// warning! Using string.match(regex) here instead of regex.test(string) because regex.test() modifies the regex object, and can lead to unexpected behavior
20-
const removedLower = removeSpacesAndNow(lower)
20+
const removedLower = removeSpacesNowAndMinus(lower)
2121

2222
return !!removedLower.match(durationRegExp)
2323
}
@@ -126,7 +126,7 @@ export const timeRangeToDuration = (timeRange: TimeRange): string => {
126126
throw new Error('cannot convert time range to duration')
127127
}
128128

129-
return removeSpacesAndNow(timeRange.lower)
129+
return removeSpacesNowAndMinus(timeRange.lower)
130130
}
131131

132132
export const convertTimeRangeToCustom = (

‎src/timeMachine/selectors/index.test.ts‎

Lines changed: 172 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
1-
import {getStartTime, getEndTime} from 'src/timeMachine/selectors/index'
1+
import {
2+
getStartTime,
3+
getEndTime,
4+
handleCustomTime,
5+
} from 'src/timeMachine/selectors/index'
26
import moment from 'moment'
37

48
import {
@@ -10,48 +14,178 @@ import {
1014
const custom = 'custom' as 'custom'
1115

1216
describe('TimeMachine.Selectors.Index', () => {
13-
const thirty = moment()
14-
.subtract(30, 'days')
15-
.subtract(moment().isDST() ? 1 : 0, 'hours') // added to account for DST
16-
.valueOf()
17-
it(`getStartTime should return ${thirty} when lower is now() - 30d`, () => {
18-
expect(getStartTime(pastThirtyDaysTimeRange)).toBeGreaterThanOrEqual(thirty)
19-
})
17+
describe('getStartTime and getEndTime', () => {
18+
const thirty = moment()
19+
.subtract(30, 'days')
20+
.subtract(moment().isDST() ? 1 : 0, 'hours') // added to account for DST
21+
.valueOf()
22+
it(`getStartTime should return ${thirty} when lower is now() - 30d`, () => {
23+
expect(getStartTime(pastThirtyDaysTimeRange)).toBeGreaterThanOrEqual(
24+
thirty
25+
)
26+
})
2027

21-
const hour = moment().subtract(1, 'hours').valueOf()
22-
it(`getStartTime should return ${hour} when lower is now() - 1h`, () => {
23-
expect(getStartTime(pastHourTimeRange)).toBeGreaterThanOrEqual(hour)
24-
})
28+
const hour = moment().subtract(1, 'hours').valueOf()
29+
it(`getStartTime should return ${hour} when lower is now() - 1h`, () => {
30+
expect(getStartTime(pastHourTimeRange)).toBeGreaterThanOrEqual(hour)
31+
})
2532

26-
const fifteen = moment().subtract(15, 'minutes').valueOf()
27-
it(`getStartTime should return ${hour} when lower is now() - 1h`, () => {
28-
expect(getStartTime(pastFifteenMinTimeRange)).toBeGreaterThanOrEqual(
29-
fifteen
30-
)
31-
})
33+
const fifteen = moment().subtract(15, 'minutes').valueOf()
34+
it(`getStartTime should return ${hour} when lower is now() - 1h`, () => {
35+
expect(getStartTime(pastFifteenMinTimeRange)).toBeGreaterThanOrEqual(
36+
fifteen
37+
)
38+
})
3239

33-
const date = '2019-01-01'
34-
const newYears = new Date(date).valueOf()
35-
it(`getStartTime should return ${newYears} when lower is ${date}`, () => {
36-
const timeRange = {
37-
type: custom,
38-
lower: date,
39-
upper: date,
40-
}
41-
expect(getStartTime(timeRange)).toEqual(newYears)
42-
})
40+
const date = '2019-01-01'
41+
const newYears = new Date(date).valueOf()
42+
it(`getStartTime should return ${newYears} when lower is ${date}`, () => {
43+
const timeRange = {
44+
type: custom,
45+
lower: date,
46+
upper: date,
47+
}
48+
expect(getStartTime(timeRange)).toEqual(newYears)
49+
})
50+
51+
it(`getEndTime should return ${newYears} when lower is ${date}`, () => {
52+
const timeRange = {
53+
type: custom,
54+
lower: date,
55+
upper: date,
56+
}
57+
expect(getEndTime(timeRange)).toEqual(newYears)
58+
})
4359

44-
it(`getEndTime should return ${newYears} when lower is ${date}`, () => {
45-
const timeRange = {
46-
type: custom,
47-
lower: date,
48-
upper: date,
49-
}
50-
expect(getEndTime(timeRange)).toEqual(newYears)
60+
const now = new Date().valueOf()
61+
it(`getEndTime should return ${now} when upper is null and lower includes now()`, () => {
62+
expect(getEndTime(pastThirtyDaysTimeRange)).toBeGreaterThanOrEqual(now)
63+
})
5164
})
5265

53-
const now = new Date().valueOf()
54-
it(`getEndTime should return ${now} when upper is null and lower includes now()`, () => {
55-
expect(getEndTime(pastThirtyDaysTimeRange)).toBeGreaterThanOrEqual(now)
66+
describe('handleCustomTime', () => {
67+
const now = new Date()
68+
const threeDays = 1000 * 60 * 60 * 24 * 3
69+
const twentyFourHours = 1000 * 60 * 60 * 24
70+
const fifteenMinutes = 1000 * 60 * 15
71+
const oneMillisecond = 1
72+
73+
it('can parse just "now"', () => {
74+
expect(handleCustomTime('now()', now)).toEqual(now.getTime())
75+
})
76+
77+
it('can parse now() and a duration', () => {
78+
expect(handleCustomTime('now() - 3d', now)).toEqual(
79+
now.getTime() - threeDays
80+
)
81+
expect(handleCustomTime('now() + 3d', now)).toEqual(
82+
now.getTime() + threeDays
83+
)
84+
85+
expect(handleCustomTime('now() - 24h', now)).toEqual(
86+
now.getTime() - twentyFourHours
87+
)
88+
expect(handleCustomTime('now() + 24h', now)).toEqual(
89+
now.getTime() + twentyFourHours
90+
)
91+
92+
expect(handleCustomTime('now() - 15m', now)).toEqual(
93+
now.getTime() - fifteenMinutes
94+
)
95+
expect(handleCustomTime('now() + 15m', now)).toEqual(
96+
now.getTime() + fifteenMinutes
97+
)
98+
99+
expect(handleCustomTime('now() - 1ms', now)).toEqual(
100+
now.getTime() - oneMillisecond
101+
)
102+
expect(handleCustomTime('now() + 1ms', now)).toEqual(
103+
now.getTime() + oneMillisecond
104+
)
105+
})
106+
107+
it('can parse a duration without now()', () => {
108+
expect(handleCustomTime('-3d', now)).toEqual(now.getTime() - threeDays)
109+
expect(handleCustomTime('+3d', now)).toEqual(now.getTime() + threeDays)
110+
111+
expect(handleCustomTime('-24h', now)).toEqual(
112+
now.getTime() - twentyFourHours
113+
)
114+
expect(handleCustomTime('+24h', now)).toEqual(
115+
now.getTime() + twentyFourHours
116+
)
117+
118+
expect(handleCustomTime('-15m', now)).toEqual(
119+
now.getTime() - fifteenMinutes
120+
)
121+
expect(handleCustomTime('+15m', now)).toEqual(
122+
now.getTime() + fifteenMinutes
123+
)
124+
125+
expect(handleCustomTime('-1ms', now)).toEqual(
126+
now.getTime() - oneMillisecond
127+
)
128+
expect(handleCustomTime('+1ms', now)).toEqual(
129+
now.getTime() + oneMillisecond
130+
)
131+
})
132+
133+
it('can ignore spaces around now and the sign correctly', () => {
134+
expect(handleCustomTime(' now()', now)).toEqual(now.getTime())
135+
expect(handleCustomTime('now() ', now)).toEqual(now.getTime())
136+
expect(handleCustomTime(' now() ', now)).toEqual(now.getTime())
137+
138+
expect(handleCustomTime(' now() - 3d', now)).toEqual(
139+
now.getTime() - threeDays
140+
)
141+
expect(handleCustomTime('now()+ 3d ', now)).toEqual(
142+
now.getTime() + threeDays
143+
)
144+
145+
expect(handleCustomTime('- 24h', now)).toEqual(
146+
now.getTime() - twentyFourHours
147+
)
148+
expect(handleCustomTime(' + 24h', now)).toEqual(
149+
now.getTime() + twentyFourHours
150+
)
151+
152+
expect(handleCustomTime(' 15m', now)).toEqual(
153+
now.getTime() + fifteenMinutes
154+
)
155+
expect(handleCustomTime('15m ', now)).toEqual(
156+
now.getTime() + fifteenMinutes
157+
)
158+
expect(handleCustomTime(' 15m ', now)).toEqual(
159+
now.getTime() + fifteenMinutes
160+
)
161+
})
162+
163+
it('can parse a JavaScript timestamp as a string', () => {
164+
let specificTimeString: string = '2022-10-14T20:33:01.433Z'
165+
expect(handleCustomTime(specificTimeString, now)).toEqual(
166+
new Date(specificTimeString).getTime()
167+
)
168+
169+
specificTimeString = '2022/07/04'
170+
expect(handleCustomTime(specificTimeString, now)).toEqual(
171+
new Date(specificTimeString).getTime()
172+
)
173+
174+
specificTimeString = '2022-10-06 00:00'
175+
expect(handleCustomTime(specificTimeString, now)).toEqual(
176+
new Date(specificTimeString).getTime()
177+
)
178+
})
179+
180+
it('rejects numbers as invalid time', () => {
181+
let numberString = '1666029415'
182+
expect(() => handleCustomTime(numberString, now)).toThrowError()
183+
184+
numberString = '0'
185+
expect(() => handleCustomTime(numberString, now)).toThrowError()
186+
187+
numberString = '-4'
188+
expect(() => handleCustomTime(numberString, now)).toThrowError()
189+
})
56190
})
57191
})

0 commit comments

Comments
 (0)