From 6de2c8b0e5ebf4676f39fb6927ed6e371a3d1878 Mon Sep 17 00:00:00 2001
From: Alexander Polyankin <alexander.polyankin@metabase.com>
Date: Mon, 26 Sep 2022 18:41:03 +0300
Subject: [PATCH] Do not allow to enter an empty relative time interval
(#25542)
---
frontend/src/metabase/lib/time.ts | 13 +++--
.../pickers/DatePicker/RelativeDatePicker.tsx | 21 ++++---
.../RelativeDatePicker.unit.spec.tsx | 58 +++++++++++++++++++
frontend/test/metabase/lib/time.unit.spec.js | 28 ++++-----
4 files changed, 90 insertions(+), 30 deletions(-)
create mode 100644 frontend/src/metabase/query_builder/components/filters/pickers/DatePicker/RelativeDatePicker.unit.spec.tsx
diff --git a/frontend/src/metabase/lib/time.ts b/frontend/src/metabase/lib/time.ts
index 5322bb42769..ad9d4f9a618 100644
--- a/frontend/src/metabase/lib/time.ts
+++ b/frontend/src/metabase/lib/time.ts
@@ -74,15 +74,16 @@ function addAbbreviatedLocale() {
moment.locale(initialLocale);
}
-export function checkIfTimeSpanTooGreat(
- amount: DurationInputArg1,
- key: DurationInputArg2,
-) {
+export function isValidTimeInterval(interval: number, unit: DurationInputArg2) {
+ if (!interval) {
+ return false;
+ }
+
const now = moment();
- const newTime = moment().add(amount, key);
+ const newTime = moment().add(interval, unit);
const diff = now.diff(newTime, "years");
- return Number.isNaN(diff);
+ return !Number.isNaN(diff);
}
export function formatFrame(frame: "first" | "last" | "mid") {
diff --git a/frontend/src/metabase/query_builder/components/filters/pickers/DatePicker/RelativeDatePicker.tsx b/frontend/src/metabase/query_builder/components/filters/pickers/DatePicker/RelativeDatePicker.tsx
index df96953a6e1..8383f1e1f18 100644
--- a/frontend/src/metabase/query_builder/components/filters/pickers/DatePicker/RelativeDatePicker.tsx
+++ b/frontend/src/metabase/query_builder/components/filters/pickers/DatePicker/RelativeDatePicker.tsx
@@ -1,4 +1,3 @@
-/* eslint-disable react/prop-types */
import React from "react";
import { t } from "ttag";
import { assoc } from "icepick";
@@ -13,7 +12,7 @@ import {
setStartingFrom,
toTimeInterval,
} from "metabase/lib/query_time";
-import { checkIfTimeSpanTooGreat, getRelativeTime } from "metabase/lib/time";
+import { isValidTimeInterval } from "metabase/lib/time";
import TippyPopover from "metabase/components/Popover/TippyPopover";
import Filter from "metabase-lib/lib/queries/structured/Filter";
@@ -31,8 +30,8 @@ type RelativeDatePickerProps = {
className?: string;
filter: Filter;
onFilterChange: (filter: any[]) => void;
- formatter: (value: number) => number;
- offsetFormatter: (value: number) => number;
+ formatter?: (value: number) => number;
+ offsetFormatter?: (value: number) => number;
primaryColor?: string;
reverseIconDirection?: boolean;
supportsExpressions?: boolean;
@@ -100,14 +99,14 @@ function getCurrentIntervalName(filter: Filter) {
return null;
}
-const OptionsContent: React.FC<OptionsContentProps> = ({
+const OptionsContent = ({
filter,
primaryColor,
onFilterChange,
reverseIconDirection,
setOptionsVisible,
supportsExpressions,
-}) => {
+}: OptionsContentProps) => {
const options = filter[4] || {};
const includeCurrent = !!options["include-current"];
const currentString = getCurrentString(filter);
@@ -152,7 +151,7 @@ const OptionsContent: React.FC<OptionsContentProps> = ({
);
};
-const RelativeDatePicker: React.FC<RelativeDatePickerProps> = props => {
+const RelativeDatePicker = (props: RelativeDatePickerProps) => {
const {
filter,
onFilterChange,
@@ -175,15 +174,15 @@ const RelativeDatePicker: React.FC<RelativeDatePickerProps> = props => {
);
const handleChangeDateNumericInput = (newIntervals: number) => {
- const timeSpanTooGreat = checkIfTimeSpanTooGreat(newIntervals, unit);
- const valueToUse = timeSpanTooGreat ? intervals : newIntervals;
+ const isValid = isValidTimeInterval(newIntervals, unit);
+ const valueToUse = isValid ? newIntervals : Math.abs(intervals);
onFilterChange(setRelativeDatetimeValue(filter, formatter(valueToUse)));
};
const handleChangeUnitInput = (newUnit: DurationInputArg2) => {
- const timeSpanTooGreat = checkIfTimeSpanTooGreat(intervals, newUnit);
- const unitToUse = timeSpanTooGreat ? unit : newUnit;
+ const isValid = isValidTimeInterval(intervals, newUnit);
+ const unitToUse = isValid ? newUnit : unit;
onFilterChange(setRelativeDatetimeUnit(filter, unitToUse));
};
diff --git a/frontend/src/metabase/query_builder/components/filters/pickers/DatePicker/RelativeDatePicker.unit.spec.tsx b/frontend/src/metabase/query_builder/components/filters/pickers/DatePicker/RelativeDatePicker.unit.spec.tsx
new file mode 100644
index 00000000000..f53cb1931db
--- /dev/null
+++ b/frontend/src/metabase/query_builder/components/filters/pickers/DatePicker/RelativeDatePicker.unit.spec.tsx
@@ -0,0 +1,58 @@
+import React from "react";
+import { render, screen } from "@testing-library/react";
+import userEvent from "@testing-library/user-event";
+import Filter from "metabase-lib/lib/queries/structured/Filter";
+import { PastPicker, NextPicker } from "./RelativeDatePicker";
+
+describe("PastPicker", () => {
+ it("should change a filter", () => {
+ const filter = new Filter(["time-interval", null, -10, "month"]);
+ const onFilterChange = jest.fn();
+
+ render(<PastPicker filter={filter} onFilterChange={onFilterChange} />);
+ typeByDisplayValue("10", "20");
+
+ const expectedFilter = ["time-interval", null, -20, "month"];
+ expect(onFilterChange).toHaveBeenCalledWith(expectedFilter);
+ });
+
+ it("should not allow to enter an empty time interval", () => {
+ const filter = new Filter(["time-interval", null, -10, "month"]);
+ const onFilterChange = jest.fn();
+
+ render(<PastPicker filter={filter} onFilterChange={onFilterChange} />);
+ typeByDisplayValue("10", "0");
+
+ expect(onFilterChange).toHaveBeenCalledWith(filter);
+ });
+});
+
+describe("NextPicker", () => {
+ it("should change a filter", () => {
+ const filter = new Filter(["time-interval", null, 10, "month"]);
+ const onFilterChange = jest.fn();
+
+ render(<NextPicker filter={filter} onFilterChange={onFilterChange} />);
+ typeByDisplayValue("10", "20");
+
+ const expectedFilter = ["time-interval", null, 20, "month"];
+ expect(onFilterChange).toHaveBeenCalledWith(expectedFilter);
+ });
+
+ it("should not allow to enter an empty time interval", () => {
+ const filter = new Filter(["time-interval", null, 10, "month"]);
+ const onFilterChange = jest.fn();
+
+ render(<NextPicker filter={filter} onFilterChange={onFilterChange} />);
+ typeByDisplayValue("10", "0");
+
+ expect(onFilterChange).toHaveBeenCalledWith(filter);
+ });
+});
+
+const typeByDisplayValue = (label: string, value: string) => {
+ const input = screen.getByDisplayValue(label);
+ userEvent.clear(input);
+ userEvent.type(input, value);
+ userEvent.tab();
+};
diff --git a/frontend/test/metabase/lib/time.unit.spec.js b/frontend/test/metabase/lib/time.unit.spec.js
index 99d0e6db508..c23fd401c27 100644
--- a/frontend/test/metabase/lib/time.unit.spec.js
+++ b/frontend/test/metabase/lib/time.unit.spec.js
@@ -1,13 +1,13 @@
import moment from "moment-timezone";
import {
- checkIfTimeSpanTooGreat,
- parseTime,
- parseTimestamp,
getRelativeTimeAbbreviated,
- msToSeconds,
- msToMinutes,
- msToHours,
hoursToSeconds,
+ isValidTimeInterval,
+ msToHours,
+ msToMinutes,
+ msToSeconds,
+ parseTime,
+ parseTimestamp,
} from "metabase/lib/time";
describe("time", () => {
@@ -153,15 +153,17 @@ describe("time", () => {
});
});
- describe("checkIfTimeSpanTooGreat", () => {
- it(`returns false for small time spans`, () => {
- const isTimeSpanTooGreat = checkIfTimeSpanTooGreat(10, "days");
- expect(isTimeSpanTooGreat).toBeFalsy();
+ describe("isValidTimeInterval", () => {
+ it(`is not valid for 0 time span`, () => {
+ expect(isValidTimeInterval(0, "days")).toBeFalsy();
+ });
+
+ it(`is valid for small time spans`, () => {
+ expect(isValidTimeInterval(10, "days")).toBeTruthy();
});
- it(`returns truthy for large time spans`, () => {
- const isTimeSpanTooGreat = checkIfTimeSpanTooGreat(1000000000, "years");
- expect(isTimeSpanTooGreat).toBeTruthy();
+ it(`is not valid for large time spans`, () => {
+ expect(isValidTimeInterval(1000000000, "years")).toBeFalsy();
});
});
});
--
GitLab