public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

TimePickerWithHistory: Improve type handling when reading history from localStorage (#105859)

Browse Source 
* TimePickerWithHistory: Improve migrateHistory type safety handling and validation
This commit is contained in:
Yunwen Zheng
2025-05-27 14:00:47 -04:00
committed by GitHub
parent c84388f550
commit 8b6103fc67
2 changed files with 88 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files
public/app/core/components/TimePicker/TimePickerWithHistory.test.tsx
+16 -15
View File
@@ -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(<TimePickerWithHistory value={timeRange} {...props} />);
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 () => {
public/app/core/components/TimePicker/TimePickerWithHistory.tsx
+72 -27
View File
@@ -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<TimeRangePickerProps, 'history' | 'theme'> {}
@@ -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 (
<LocalStorageValueProvider<LSTimePickerHistoryItem[]> storageKey={LOCAL_STORAGE_KEY} defaultValue={[]}>
{(rawValues, onSaveToStore) => {
const values = migrateHistory(rawValues);
const history = deserializeHistory(values);
<LocalStorageValueProvider<unknown> storageKey={LOCAL_STORAGE_KEY} defaultValue={[]}>
{(values, onSaveToStore) => {
const validHistory = getValidHistory(values);
const history = deserializeHistory(validHistory);
return (
<TimeRangePicker
{...props}
history={history}
onChange={(value) => {
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();
}