diff --git a/public/app/core/components/TimePicker/TimePickerWithHistory.test.tsx b/public/app/core/components/TimePicker/TimePickerWithHistory.test.tsx
index 8558e5764e5..59abd9f9375 100644
--- a/public/app/core/components/TimePicker/TimePickerWithHistory.test.tsx
+++ b/public/app/core/components/TimePicker/TimePickerWithHistory.test.tsx
@@ -13,18 +13,6 @@ describe('TimePickerWithHistory', () => {
const getApplyButton = () => screen.getByRole('button', { name: 'Apply time range' });
const LOCAL_STORAGE_KEY = 'grafana.dashboard.timepicker.history';
- const OLD_LOCAL_STORAGE = [
- {
- from: '2022-12-03T00:00:00.000Z',
- to: '2022-12-03T23:59:59.000Z',
- raw: { from: '2022-12-03T00:00:00.000Z', to: '2022-12-03T23:59:59.000Z' },
- },
- {
- from: '2022-12-02T00:00:00.000Z',
- to: '2022-12-02T23:59:59.000Z',
- raw: { from: '2022-12-02T00:00:00.000Z', to: '2022-12-02T23:59:59.000Z' },
- },
- ];
const NEW_LOCAL_STORAGE = [
{ from: '2022-12-03T00:00:00.000Z', to: '2022-12-03T23:59:59.000Z' },
@@ -52,15 +40,28 @@ describe('TimePickerWithHistory', () => {
expect(screen.getByText(/It looks like you haven't used this time picker before/i)).toBeInTheDocument();
});
- it('Should load with old TimeRange history', async () => {
- window.localStorage.setItem(LOCAL_STORAGE_KEY, JSON.stringify(OLD_LOCAL_STORAGE));
+ it('Should load with valid time picker history only', async () => {
+ // TimePickerWithHistory only accepts TimePickerHistoryItem objects, invalid history items should be ignored
+ const BAD_LOCAL_STORAGE = [
+ { from: '2022-12-03T00:00:00.000Z', to: '2022-12-03T23:59:59.000Z' }, // valid
+ {
+ from: '2022-12-01T00:00:00.000Z',
+ to: '2022-12-01T23:59:59.000Z',
+ raw: { from: '2022-12-01T00:00:00.000Z', to: '022-12-01T23:59:59.000Z' }, // Invalid, because it has raw property which doesn't match TimePickerHistoryItem
+ },
+ {}, // Invalid, because empty
+ { from: null, to: null }, // Invalid, because both value are null
+ { from: '2022-12-04T00:00:00.000Z', to: null }, // Invalid because one value is null
+ ];
+ window.localStorage.setItem(LOCAL_STORAGE_KEY, JSON.stringify(BAD_LOCAL_STORAGE));
const timeRange = getDefaultTimeRange();
render();
await userEvent.click(screen.getByLabelText(/Time range selected/));
expect(screen.getByText(/2022-12-03 00:00:00 to 2022-12-03 23:59:59/i)).toBeInTheDocument();
- expect(screen.queryByText(/2022-12-02 00:00:00 to 2022-12-02 23:59:59/i)).toBeInTheDocument();
+ expect(screen.queryByText(/2022-12-01 00:00:00 to 2022-12-01 23:59:59/i)).not.toBeInTheDocument();
+ expect(screen.queryByText(/2022-12-04 00:00:00 to 2022-12-04 23:59:59/i)).not.toBeInTheDocument();
});
it('Should load with new TimePickerHistoryItem history', async () => {
diff --git a/public/app/core/components/TimePicker/TimePickerWithHistory.tsx b/public/app/core/components/TimePicker/TimePickerWithHistory.tsx
index 204f971efc0..4487eb1378a 100644
--- a/public/app/core/components/TimePicker/TimePickerWithHistory.tsx
+++ b/public/app/core/components/TimePicker/TimePickerWithHistory.tsx
@@ -1,6 +1,6 @@
import { uniqBy } from 'lodash';
-import { AppEvents, TimeRange, isDateTime, rangeUtil } from '@grafana/data';
+import { AppEvents, DateTime, TimeRange, isDateTime, rangeUtil } from '@grafana/data';
import { useTranslate } from '@grafana/i18n';
import { TimeRangePickerProps, TimeRangePicker } from '@grafana/ui';
import appEvents from 'app/core/app_events';
@@ -8,6 +8,7 @@ import appEvents from 'app/core/app_events';
import { LocalStorageValueProvider } from '../LocalStorageValueProvider';
const LOCAL_STORAGE_KEY = 'grafana.dashboard.timepicker.history';
+const MAX_HISTORY_ITEMS = 4;
interface Props extends Omit {}
@@ -17,24 +18,21 @@ interface TimePickerHistoryItem {
to: string;
}
-// We should only be storing TimePickerHistoryItem, but in the past we also stored TimeRange
-type LSTimePickerHistoryItem = TimePickerHistoryItem | TimeRange;
-
export const TimePickerWithHistory = (props: Props) => {
const { t } = useTranslate();
return (
- storageKey={LOCAL_STORAGE_KEY} defaultValue={[]}>
- {(rawValues, onSaveToStore) => {
- const values = migrateHistory(rawValues);
- const history = deserializeHistory(values);
+ storageKey={LOCAL_STORAGE_KEY} defaultValue={[]}>
+ {(values, onSaveToStore) => {
+ const validHistory = getValidHistory(values);
+ const history = deserializeHistory(validHistory);
return (
{
- onAppendToHistory(value, values, onSaveToStore);
+ onAppendToHistory(value, validHistory, onSaveToStore);
props.onChange(value);
}}
onError={(error?: string) =>
@@ -50,21 +48,26 @@ export const TimePickerWithHistory = (props: Props) => {
);
};
-function deserializeHistory(values: TimePickerHistoryItem[]): TimeRange[] {
- // The history is saved in UTC and with the default date format, so we need to pass those values to the convertRawToRange
- return values.map((item) => rangeUtil.convertRawToRange(item, 'utc', undefined, 'YYYY-MM-DD HH:mm:ss'));
+function getValidHistory(values: unknown): TimePickerHistoryItem[] {
+ const result: TimePickerHistoryItem[] = [];
+
+ if (!Array.isArray(values)) {
+ return result;
+ }
+ // Check if the values are already in the correct format
+
+ for (let item of values) {
+ const parsed = getValidHistoryItem(item);
+ if (parsed) {
+ result.push(parsed);
+ }
+ }
+
+ return result;
}
-function migrateHistory(values: LSTimePickerHistoryItem[]): TimePickerHistoryItem[] {
- return values.map((item) => {
- const fromValue = typeof item.from === 'string' ? item.from : item.from.toISOString();
- const toValue = typeof item.to === 'string' ? item.to : item.to.toISOString();
-
- return {
- from: fromValue,
- to: toValue,
- };
- });
+export function deserializeHistory(values: TimePickerHistoryItem[]): TimeRange[] {
+ return values.map((item) => rangeUtil.convertRawToRange(item, 'utc', undefined, 'YYYY-MM-DD HH:mm:ss'));
}
function onAppendToHistory(
@@ -72,24 +75,66 @@ function onAppendToHistory(
values: TimePickerHistoryItem[],
onSaveToStore: (values: TimePickerHistoryItem[]) => void
) {
- if (!isAbsolute(newTimeRange)) {
+ if (!isAbsoluteTimeRange(newTimeRange)) {
+ // If the time range is not absolute, do not append it to history, ex: last 5 minutes
return;
}
// Convert DateTime objects to strings
const toAppend = {
- from: typeof newTimeRange.raw.from === 'string' ? newTimeRange.raw.from : newTimeRange.raw.from.toISOString(),
- to: typeof newTimeRange.raw.to === 'string' ? newTimeRange.raw.to : newTimeRange.raw.to.toISOString(),
+ from: convertToISOString(newTimeRange.raw.from),
+ to: convertToISOString(newTimeRange.raw.to),
};
const toStore = limit([toAppend, ...values]);
onSaveToStore(toStore);
}
-function isAbsolute(value: TimeRange): boolean {
+function isAbsoluteTimeRange(value: TimeRange): boolean {
return isDateTime(value.raw.from) || isDateTime(value.raw.to);
}
function limit(value: TimePickerHistoryItem[]): TimePickerHistoryItem[] {
- return uniqBy(value, (v) => v.from + v.to).slice(0, 4);
+ return uniqBy(value, (v) => v.from + v.to).slice(0, MAX_HISTORY_ITEMS);
+}
+
+/**
+ * Check if the value is a valid TimePickerHistoryItem. If it doesn't match the format exactly, it will return false.
+ * @returns true if the value match exactly to TimePickerHistoryItem, false otherwise
+ */
+export function getValidHistoryItem(value: unknown): TimePickerHistoryItem | null {
+ // First check if it's a valid object
+ if (typeof value !== 'object' || value === null) {
+ return null;
+ }
+
+ // Check if it has exactly two properties
+ if (Object.keys(value).length !== 2) {
+ return null;
+ }
+
+ // Check if it has the required properties
+ if (!('from' in value) || !('to' in value)) {
+ return null;
+ }
+
+ const { from, to } = value;
+ // Check if both properties are strings
+ if (typeof from === 'string' && typeof to === 'string') {
+ return { from, to };
+ }
+
+ return null;
+}
+
+function convertToISOString(value: DateTime | string): string {
+ if (typeof value === 'string') {
+ return value;
+ }
+
+ if (!value?.toISOString) {
+ throw console.error('Invalid DateTime object passed to convertToISOString');
+ }
+
+ return value.toISOString();
}