Dashboard: avoid and correct invalid date ranges (i.e. to < from) (#34543)

* Dashboard: invert invalid date ranges from URL

* Dashboard: discard invalid date ranges from inputs

* Dashboard: move time range reset

* Dashboard: simplify undefined dashboard verification

* Dashboard: show form error on invalid date range

* Dashboard: rename function to isRangeInvalid

* Dashboard: refactor invalid check functions

* Dashboard: different error messages for date picker

* Dashboard: add date tests to TimeRangeForm

packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimeRangeForm.test.tsx

@@ -96,4 +96,56 @@ describe('TimeRangeForm', () => {
     expect(from).toHaveClass('react-calendar__tile--rangeStart');
     expect(to).toHaveClass('react-calendar__tile--rangeEnd');
   });
+
+  describe('dates error handling', () => {
+    it('should show error on invalid dates', () => {
+      const invalidTimeRange: TimeRange = {
+        from: dateTimeParse('foo', { timeZone: 'utc' }),
+        to: dateTimeParse('2021-06-19 23:59:00', { timeZone: 'utc' }),
+        raw: {
+          from: 'foo',
+          to: '2021-06-19 23:59:00',
+        },
+      };
+      const { getAllByRole } = setup(invalidTimeRange, 'Asia/Tokyo');
+      const error = getAllByRole('alert');
+
+      expect(error).toHaveLength(1);
+      expect(error[0]).toBeVisible();
+      expect(error[0]).toHaveTextContent('Please enter a past date or "now"');
+    });
+
+    it('should show error on invalid range', () => {
+      const invalidTimeRange: TimeRange = {
+        from: dateTimeParse('2021-06-19 00:00:00', { timeZone: 'utc' }),
+        to: dateTimeParse('2021-06-17 23:59:00', { timeZone: 'utc' }),
+        raw: {
+          from: '2021-06-19 00:00:00',
+          to: '2021-06-17 23:59:00',
+        },
+      };
+      const { getAllByRole } = setup(invalidTimeRange, 'Asia/Tokyo');
+      const error = getAllByRole('alert');
+
+      expect(error[0]).toBeVisible();
+      expect(error[0]).toHaveTextContent('"From" can\'t be after "To"');
+    });
+
+    it('should not show range error when "to" is invalid', () => {
+      const invalidTimeRange: TimeRange = {
+        from: dateTimeParse('2021-06-19 00:00:00', { timeZone: 'utc' }),
+        to: dateTimeParse('foo', { timeZone: 'utc' }),
+        raw: {
+          from: '2021-06-19 00:00:00',
+          to: 'foo',
+        },
+      };
+      const { getAllByRole } = setup(invalidTimeRange, 'Asia/Tokyo');
+      const error = getAllByRole('alert');
+
+      expect(error).toHaveLength(1);
+      expect(error[0]).toBeVisible();
+      expect(error[0]).toHaveTextContent('Please enter a past date or "now"');
+    });
+  });
 });

packages/grafana-ui/src/components/DateTimePickers/TimeRangePicker/TimeRangeForm.tsx

@@ -28,21 +28,27 @@ interface Props {
 interface InputState {
   value: string;
   invalid: boolean;
+  errorMessage: string;
 }
 
-const errorMessage = 'Please enter a past date or "now"';
+const ERROR_MESSAGES = {
+  default: 'Please enter a past date or "now"',
+  range: '"From" can\'t be after "To"',
+};
 
 export const TimeRangeForm: React.FC<Props> = (props) => {
   const { value, isFullscreen = false, timeZone, onApply: onApplyFromProps, isReversed } = props;
+  const [fromValue, toValue] = valueToState(value.raw.from, value.raw.to, timeZone);
 
-  const [from, setFrom] = useState<InputState>(valueToState(value.raw.from, false, timeZone));
-  const [to, setTo] = useState<InputState>(valueToState(value.raw.to, true, timeZone));
+  const [from, setFrom] = useState<InputState>(fromValue);
+  const [to, setTo] = useState<InputState>(toValue);
   const [isOpen, setOpen] = useState(false);
 
   // Synchronize internal state with external value
   useEffect(() => {
-    setFrom(valueToState(value.raw.from, false, timeZone));
-    setTo(valueToState(value.raw.to, true, timeZone));
+    const [fromValue, toValue] = valueToState(value.raw.from, value.raw.to, timeZone);
+    setFrom(fromValue);
+    setTo(toValue);
   }, [value.raw.from, value.raw.to, timeZone]);
 
   const onOpen = useCallback(
@@ -79,9 +85,10 @@ export const TimeRangeForm: React.FC<Props> = (props) => {
   );
 
   const onChange = useCallback(
-    (from: DateTime, to: DateTime) => {
-      setFrom(valueToState(from, false, timeZone));
-      setTo(valueToState(to, true, timeZone));
+    (from: DateTime | string, to: DateTime | string) => {
+      const [fromValue, toValue] = valueToState(from, to, timeZone);
+      setFrom(fromValue);
+      setTo(toValue);
     },
     [timeZone]
   );
@@ -90,21 +97,21 @@ export const TimeRangeForm: React.FC<Props> = (props) => {
 
   return (
     <>
-      <Field label="From" invalid={from.invalid} error={errorMessage}>
+      <Field label="From" invalid={from.invalid} error={from.errorMessage}>
         <Input
           onClick={(event) => event.stopPropagation()}
           onFocus={onFocus}
-          onChange={(event) => setFrom(eventToState(event, false, timeZone))}
+          onChange={(event) => onChange(event.currentTarget.value, to.value)}
           addonAfter={icon}
           aria-label={selectors.components.TimePicker.fromField}
           value={from.value}
         />
       </Field>
-      <Field label="To" invalid={to.invalid} error={errorMessage}>
+      <Field label="To" invalid={to.invalid} error={to.errorMessage}>
         <Input
           onClick={(event) => event.stopPropagation()}
           onFocus={onFocus}
-          onChange={(event) => setTo(eventToState(event, true, timeZone))}
+          onChange={(event) => onChange(from.value, event.currentTarget.value)}
           addonAfter={icon}
           aria-label={selectors.components.TimePicker.toField}
           value={to.value}
@@ -129,14 +136,34 @@ export const TimeRangeForm: React.FC<Props> = (props) => {
   );
 };
 
-function eventToState(event: FormEvent<HTMLInputElement>, roundup?: boolean, timeZone?: TimeZone): InputState {
-  return valueToState(event.currentTarget.value, roundup, timeZone);
+function isRangeInvalid(from: string, to: string, timezone?: string): boolean {
+  const raw: RawTimeRange = { from, to };
+  const timeRange = rangeUtil.convertRawToRange(raw, timezone);
+  const valid = timeRange.from.isSame(timeRange.to) || timeRange.from.isBefore(timeRange.to);
+
+  return !valid;
 }
 
-function valueToState(raw: DateTime | string, roundup?: boolean, timeZone?: TimeZone): InputState {
-  const value = valueAsString(raw, timeZone);
-  const invalid = !isValid(value, roundup, timeZone);
-  return { value, invalid };
+function valueToState(
+  rawFrom: DateTime | string,
+  rawTo: DateTime | string,
+  timeZone?: TimeZone
+): [InputState, InputState] {
+  const fromValue = valueAsString(rawFrom, timeZone);
+  const toValue = valueAsString(rawTo, timeZone);
+  const fromInvalid = !isValid(fromValue, false, timeZone);
+  const toInvalid = !isValid(toValue, true, timeZone);
+  // If "To" is invalid, we should not check the range anyways
+  const rangeInvalid = isRangeInvalid(fromValue, toValue, timeZone) && !toInvalid;
+
+  return [
+    {
+      value: fromValue,
+      invalid: fromInvalid || rangeInvalid,
+      errorMessage: rangeInvalid && !fromInvalid ? ERROR_MESSAGES.range : ERROR_MESSAGES.default,
+    },
+    { value: toValue, invalid: toInvalid, errorMessage: ERROR_MESSAGES.default },
+  ];
 }
 
 function valueAsString(value: DateTime | string, timeZone?: TimeZone): string {

public/app/features/dashboard/services/TimeSrv.test.ts

@@ -163,6 +163,28 @@ describe('timeSrv', () => {
         expect(time.from.valueOf()).toEqual(1410337640000);
         expect(time.to.valueOf()).toEqual(1410337650000);
       });
+
+      it('corrects inverted from/to dates in ms', () => {
+        locationService.push('/d/id?from=1621436828909&to=1621436818909');
+
+        timeSrv = new TimeSrv(new ContextSrvStub() as any);
+
+        timeSrv.init(_dashboard);
+        const time = timeSrv.timeRange();
+        expect(time.from.valueOf()).toEqual(1621436818909);
+        expect(time.to.valueOf()).toEqual(1621436828909);
+      });
+
+      it('corrects inverted from/to dates as relative times', () => {
+        locationService.push('/d/id?from=now&to=now-1h');
+
+        timeSrv = new TimeSrv(new ContextSrvStub() as any);
+
+        timeSrv.init(_dashboard);
+        const time = timeSrv.timeRange();
+        expect(time.raw.from).toBe('now-1h');
+        expect(time.raw.to).toBe('now');
+      });
     });
   });
 

public/app/features/dashboard/services/TimeSrv.ts

@@ -60,6 +60,18 @@ export class TimeSrv {
     // remember time at load so we can go back to it
     this.timeAtLoad = cloneDeep(this.time);
 
+    const range = rangeUtil.convertRawToRange(this.time, this.dashboard?.getTimezone());
+
+    if (range.to.isBefore(range.from)) {
+      this.setTime(
+        {
+          from: range.raw.to,
+          to: range.raw.from,
+        },
+        false
+      );
+    }
+
     if (this.refresh) {
       this.setAutoRefresh(this.refresh);
     }