Skip to content

Fix absolute timerange history crash #78

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions packages/grafana-data/src/datetime/rangeutil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,9 +198,14 @@ export const describeTimeRangeAbbreviation = (range: TimeRange, timeZone?: TimeZ
return parsed ? timeZoneAbbrevation(parsed, { timeZone }) : '';
};

export const convertRawToRange = (raw: RawTimeRange, timeZone?: TimeZone, fiscalYearStartMonth?: number): TimeRange => {
const from = dateTimeParse(raw.from, { roundUp: false, timeZone, fiscalYearStartMonth });
const to = dateTimeParse(raw.to, { roundUp: true, timeZone, fiscalYearStartMonth });
export const convertRawToRange = (
raw: RawTimeRange,
timeZone?: TimeZone,
fiscalYearStartMonth?: number,
format?: string
): TimeRange => {
const from = dateTimeParse(raw.from, { roundUp: false, timeZone, fiscalYearStartMonth, format });
const to = dateTimeParse(raw.to, { roundUp: true, timeZone, fiscalYearStartMonth, format });

if (dateMath.isMathString(raw.from) || dateMath.isMathString(raw.to)) {
return { from, to, raw };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ const getStyles = stylesFactory((theme: GrafanaTheme2, isReversed, hideQuickRang
flex-direction: column;
border-right: ${isReversed ? 'none' : `1px solid ${theme.colors.border.weak}`};
width: ${!hideQuickRanges ? '50%' : '100%'};
overflow: hidden;
overflow: auto;
order: ${isReversed ? 1 : 0};
`,
rightSide: css`
Expand Down
41 changes: 23 additions & 18 deletions public/app/core/components/TimePicker/TimePickerWithHistory.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import React, { CSSProperties, FC, useEffect, useRef } from 'react';
// eslint-disable-next-line no-restricted-imports
import { useDispatch, useSelector } from 'react-redux';

import { TimeRange, isDateTime, rangeUtil, TimeZone, toUtc } from '@grafana/data';
import { TimeRange, isDateTime, rangeUtil } from '@grafana/data';
import { TimeRangePickerProps, TimeRangePicker, useTheme2 } from '@grafana/ui';
import { FnGlobalState, updatePartialFnStates } from 'app/core/reducers/fn-slice';
import { StoreState } from 'app/types';
Expand All @@ -14,6 +14,15 @@ const LOCAL_STORAGE_KEY = 'grafana.dashboard.timepicker.history';

interface Props extends Omit<TimeRangePickerProps, 'history' | 'theme'> {}

// Simplified object to store in local storage
interface TimePickerHistoryItem {
from: string;
to: string;
}

// We should only be storing TimePickerHistoryItem, but in the past we also stored TimeRange
type LSTimePickerHistoryItem = TimePickerHistoryItem | TimeRange;

const FnText: React.FC = () => {
const { FNDashboard } = useSelector<StoreState, FnGlobalState>(({ fnGlobalState }) => fnGlobalState);
const theme = useTheme2();
Expand All @@ -24,23 +33,26 @@ const FnText: React.FC = () => {
};

export const TimePickerWithHistory: FC<Props> = (props) => (
<LocalStorageValueProvider<TimeRange[]> storageKey={LOCAL_STORAGE_KEY} defaultValue={[]}>
{(values, onSaveToStore) => {
return <Picker values={values} onSaveToStore={onSaveToStore} pickerProps={props} />;
<LocalStorageValueProvider<LSTimePickerHistoryItem[]> storageKey={LOCAL_STORAGE_KEY} defaultValue={[]}>
{(rawValues, onSaveToStore) => {
return <Picker rawValues={rawValues} onSaveToStore={onSaveToStore} pickerProps={props} />;
}}
</LocalStorageValueProvider>
);

export interface PickerProps {
values: TimeRange[];
onSaveToStore: (value: TimeRange[]) => void;
rawValues: LSTimePickerHistoryItem[];
onSaveToStore: (value: LSTimePickerHistoryItem[]) => void;
pickerProps: Props;
}

export const Picker: FC<PickerProps> = ({ values, onSaveToStore, pickerProps }) => {
export const Picker: FC<PickerProps> = ({ rawValues, onSaveToStore, pickerProps }) => {
const { fnGlobalTimeRange } = useSelector<StoreState, FnGlobalState>(({ fnGlobalState }) => fnGlobalState);
const dispatch = useDispatch();

const values = migrateHistory(rawValues);
const history = deserializeHistory(values);

const didMountRef = useRef(false);
useEffect(() => {
/* The condition below skips the first run of useeffect that happens when this component gets mounted */
Expand All @@ -62,7 +74,7 @@ export const Picker: FC<PickerProps> = ({ values, onSaveToStore, pickerProps })
<TimeRangePicker
{...pickerProps}
value={fnGlobalTimeRange || pickerProps.value}
history={convertIfJson(values)}
history={history}
onChange={(value) => {
onAppendToHistory(value, values, onSaveToStore);
pickerProps.onChange(value);
Expand All @@ -72,16 +84,9 @@ export const Picker: FC<PickerProps> = ({ values, onSaveToStore, pickerProps })
);
};

function convertIfJson(history: TimeRange[]): TimeRange[] {
return history.map((time) => {
if (isDateTime(time.from)) {
return time;
}
});
}

function deserializeHistory(values: TimePickerHistoryItem[], timeZone: TimeZone | undefined): TimeRange[] {
return values.map((item) => rangeUtil.convertRawToRange(item, timeZone));
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 migrateHistory(values: LSTimePickerHistoryItem[]): TimePickerHistoryItem[] {
Expand Down