Skip to content

Commit 7e79d07

Browse files
narsaynorathnarsaynorath
authored
feat(widget-builder): Cache builder state between dataset changes (#92122)
Either restore the old state if it exists, or push the dataset change through the original way the widget builder hook works. Note: We have to purposely ignore the `title` and `description` fields because a user should not have those values change if they return to a cached state with a different title/description.
1 parent 6bda3fc commit 7e79d07

File tree

3 files changed

+294
-8
lines changed

3 files changed

+294
-8
lines changed

‎static/app/views/dashboards/widgetBuilder/components/datasetSelector.tsx

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,16 @@ import useOrganization from 'sentry/utils/useOrganization';
1111
import {WidgetType} from 'sentry/views/dashboards/types';
1212
import {SectionHeader} from 'sentry/views/dashboards/widgetBuilder/components/common/sectionHeader';
1313
import {useWidgetBuilderContext} from 'sentry/views/dashboards/widgetBuilder/contexts/widgetBuilderContext';
14+
import {useCacheBuilderState} from 'sentry/views/dashboards/widgetBuilder/hooks/useCacheBuilderState';
1415
import useDashboardWidgetSource from 'sentry/views/dashboards/widgetBuilder/hooks/useDashboardWidgetSource';
1516
import useIsEditingWidget from 'sentry/views/dashboards/widgetBuilder/hooks/useIsEditingWidget';
16-
import {BuilderStateAction} from 'sentry/views/dashboards/widgetBuilder/hooks/useWidgetBuilderState';
1717

1818
function WidgetBuilderDatasetSelector() {
1919
const organization = useOrganization();
20-
const {state, dispatch} = useWidgetBuilderContext();
20+
const {state} = useWidgetBuilderContext();
2121
const source = useDashboardWidgetSource();
2222
const isEditing = useIsEditingWidget();
23+
const {cacheBuilderState, restoreOrSetBuilderState} = useCacheBuilderState();
2324

2425
const datasetChoices: Array<RadioOption<WidgetType>> = [];
2526
datasetChoices.push([WidgetType.ERRORS, t('Errors')]);
@@ -48,17 +49,21 @@ function WidgetBuilderDatasetSelector() {
4849
label={t('Dataset')}
4950
value={state.dataset ?? WidgetType.ERRORS}
5051
choices={datasetChoices}
51-
onChange={(newValue: WidgetType) => {
52-
dispatch({
53-
type: BuilderStateAction.SET_DATASET,
54-
payload: newValue,
55-
});
52+
onChange={(newDataset: WidgetType) => {
53+
// Set the current dataset state in local storage for recovery
54+
// when the user navigates back to this dataset
55+
cacheBuilderState(state.dataset ?? WidgetType.ERRORS);
56+
57+
// Restore the builder state for the new dataset
58+
// or set the dataset if there is no cached state
59+
restoreOrSetBuilderState(newDataset);
60+
5661
trackAnalytics('dashboards_views.widget_builder.change', {
5762
from: source,
5863
widget_type: state.dataset ?? '',
5964
builder_version: WidgetBuilderVersion.SLIDEOUT,
6065
field: 'dataSet',
61-
value: newValue,
66+
value: newDataset,
6267
new_widget: !isEditing,
6368
organization,
6469
});

‎static/app/views/dashboards/widgetBuilder/hooks/useCacheBuilderState.spec.tsx

Lines changed: 204 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,204 @@
1+
import {LocationFixture} from 'sentry-fixture/locationFixture';
2+
3+
import {renderHook} from 'sentry-test/reactTestingLibrary';
4+
5+
import {useLocation} from 'sentry/utils/useLocation';
6+
import {DisplayType, WidgetType} from 'sentry/views/dashboards/types';
7+
import {
8+
useWidgetBuilderContext,
9+
WidgetBuilderProvider,
10+
} from 'sentry/views/dashboards/widgetBuilder/contexts/widgetBuilderContext';
11+
import {
12+
BuilderStateAction,
13+
type WidgetBuilderState,
14+
} from 'sentry/views/dashboards/widgetBuilder/hooks/useWidgetBuilderState';
15+
import {convertBuilderStateToWidget} from 'sentry/views/dashboards/widgetBuilder/utils/convertBuilderStateToWidget';
16+
17+
import {useCacheBuilderState} from './useCacheBuilderState';
18+
19+
jest.mock('sentry/utils/useNavigate', () => ({
20+
useNavigate: jest.fn(),
21+
}));
22+
jest.mock('sentry/utils/useLocation');
23+
24+
jest.mock('sentry/views/dashboards/widgetBuilder/contexts/widgetBuilderContext', () => ({
25+
useWidgetBuilderContext: jest.fn(),
26+
WidgetBuilderProvider: jest.requireActual(
27+
'sentry/views/dashboards/widgetBuilder/contexts/widgetBuilderContext'
28+
).WidgetBuilderProvider,
29+
}));
30+
31+
const mockUseWidgetBuilderContext = jest.mocked(useWidgetBuilderContext);
32+
const mockUseLocation = jest.mocked(useLocation);
33+
34+
function Wrapper({children}: {children: React.ReactNode}) {
35+
return <WidgetBuilderProvider>{children}</WidgetBuilderProvider>;
36+
}
37+
38+
describe('useCacheBuilderState', () => {
39+
let mockLocalStorage: Record<string, string>;
40+
41+
beforeEach(() => {
42+
mockLocalStorage = {};
43+
mockUseWidgetBuilderContext.mockReturnValue({
44+
state: {},
45+
dispatch: jest.fn(),
46+
});
47+
mockUseLocation.mockReturnValue(LocationFixture());
48+
49+
Storage.prototype.getItem = jest.fn(key => mockLocalStorage[key] ?? null);
50+
Storage.prototype.setItem = jest.fn((key, value) => {
51+
mockLocalStorage[key] = value;
52+
});
53+
Storage.prototype.removeItem = jest.fn(key => {
54+
delete mockLocalStorage[key];
55+
});
56+
});
57+
58+
afterEach(() => {
59+
jest.restoreAllMocks();
60+
});
61+
62+
it('caches builder state to localStorage', () => {
63+
const cachedWidget: WidgetBuilderState = {
64+
dataset: WidgetType.ERRORS,
65+
displayType: DisplayType.LINE,
66+
yAxis: [
67+
{
68+
function: ['count', '', undefined, undefined],
69+
kind: 'function',
70+
},
71+
],
72+
query: ['this is a test query'],
73+
};
74+
mockUseWidgetBuilderContext.mockReturnValue({
75+
state: cachedWidget,
76+
dispatch: jest.fn(),
77+
});
78+
79+
const {result} = renderHook(() => useCacheBuilderState(), {
80+
wrapper: Wrapper,
81+
});
82+
83+
result.current.cacheBuilderState(WidgetType.ERRORS);
84+
85+
// Verify state was saved to localStorage
86+
expect(localStorage.setItem).toHaveBeenCalledWith(
87+
'dashboards:widget-builder:dataset:error-events',
88+
JSON.stringify(convertBuilderStateToWidget(cachedWidget))
89+
);
90+
91+
result.current.restoreOrSetBuilderState(WidgetType.ERRORS);
92+
93+
expect(localStorage.getItem).toHaveBeenCalledWith(
94+
'dashboards:widget-builder:dataset:error-events'
95+
);
96+
});
97+
98+
it('restores builder state from localStorage when available', () => {
99+
const cachedWidget: WidgetBuilderState = {
100+
title: 'error widget title',
101+
description: 'error widget description',
102+
dataset: WidgetType.ERRORS,
103+
displayType: DisplayType.LINE,
104+
yAxis: [
105+
{
106+
function: ['count', '', undefined, undefined],
107+
kind: 'function',
108+
},
109+
],
110+
query: ['this is a test query'],
111+
};
112+
const currentWidget: WidgetBuilderState = {
113+
title: 'issue widget title',
114+
description: 'issue widget description',
115+
dataset: WidgetType.ISSUE,
116+
displayType: DisplayType.TABLE,
117+
query: ['issue.id:123'],
118+
fields: [
119+
{
120+
field: 'issue',
121+
kind: 'field',
122+
},
123+
],
124+
};
125+
const mockDispatch = jest.fn();
126+
mockUseWidgetBuilderContext.mockReturnValue({
127+
state: currentWidget,
128+
dispatch: mockDispatch,
129+
});
130+
// Add cached widget to the localStorage
131+
localStorage.setItem(
132+
'dashboards:widget-builder:dataset:error-events',
133+
JSON.stringify(convertBuilderStateToWidget(cachedWidget))
134+
);
135+
136+
const {result} = renderHook(() => useCacheBuilderState(), {
137+
wrapper: Wrapper,
138+
});
139+
140+
// Call the restore helper on the cached dataset
141+
result.current.restoreOrSetBuilderState(WidgetType.ERRORS);
142+
143+
expect(mockDispatch).toHaveBeenCalledWith({
144+
type: BuilderStateAction.SET_STATE,
145+
146+
// the yAxis gets converted to a string when used with this payload
147+
payload: expect.objectContaining({
148+
...cachedWidget,
149+
yAxis: ['count()'],
150+
title: 'issue widget title', // The title was not overridden
151+
description: 'issue widget description', // The description was not overridden
152+
}),
153+
});
154+
});
155+
156+
it('plainly sets the new dataset when no cached state exists', () => {
157+
const cachedWidget: WidgetBuilderState = {
158+
dataset: WidgetType.ERRORS,
159+
displayType: DisplayType.LINE,
160+
yAxis: [
161+
{
162+
function: ['count', '', undefined, undefined],
163+
kind: 'function',
164+
},
165+
],
166+
query: ['this is a test query'],
167+
};
168+
const currentWidget: WidgetBuilderState = {
169+
dataset: WidgetType.ISSUE,
170+
displayType: DisplayType.TABLE,
171+
query: ['issue.id:123'],
172+
fields: [
173+
{
174+
field: 'issue',
175+
kind: 'field',
176+
},
177+
],
178+
};
179+
const mockDispatch = jest.fn();
180+
mockUseWidgetBuilderContext.mockReturnValue({
181+
state: currentWidget,
182+
dispatch: mockDispatch,
183+
});
184+
// Add cached widget to the localStorage, this will not be the one
185+
// used in the test to test that a cache miss falls back to the plain
186+
// dataset change
187+
localStorage.setItem(
188+
'dashboards:widget-builder:dataset:error-events',
189+
JSON.stringify(convertBuilderStateToWidget(cachedWidget))
190+
);
191+
192+
const {result} = renderHook(() => useCacheBuilderState(), {
193+
wrapper: Wrapper,
194+
});
195+
196+
// Call the restore helper on the cached dataset
197+
result.current.restoreOrSetBuilderState(WidgetType.TRANSACTIONS);
198+
199+
expect(mockDispatch).toHaveBeenCalledWith({
200+
type: BuilderStateAction.SET_DATASET,
201+
payload: WidgetType.TRANSACTIONS,
202+
});
203+
});
204+
});

‎static/app/views/dashboards/widgetBuilder/hooks/useCacheBuilderState.tsx

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
import {useCallback, useEffect} from 'react';
2+
3+
import createStorage from 'sentry/utils/createStorage';
4+
import type {WidgetType} from 'sentry/views/dashboards/types';
5+
import {useWidgetBuilderContext} from 'sentry/views/dashboards/widgetBuilder/contexts/widgetBuilderContext';
6+
import {BuilderStateAction} from 'sentry/views/dashboards/widgetBuilder/hooks/useWidgetBuilderState';
7+
import {convertBuilderStateToWidget} from 'sentry/views/dashboards/widgetBuilder/utils/convertBuilderStateToWidget';
8+
import {convertWidgetToBuilderStateParams} from 'sentry/views/dashboards/widgetBuilder/utils/convertWidgetToBuilderStateParams';
9+
10+
const WIDGET_BUILDER_DATASET_STATE_KEY = 'dashboards:widget-builder:dataset';
11+
12+
const STORAGE = createStorage(() => window.sessionStorage);
13+
14+
function cleanUpDatasetState() {
15+
for (let i = 0; i < STORAGE.length; i++) {
16+
const key = STORAGE.key(i);
17+
if (key?.startsWith(WIDGET_BUILDER_DATASET_STATE_KEY)) {
18+
STORAGE.removeItem(key);
19+
}
20+
}
21+
}
22+
23+
/**
24+
* This hook is used to cache the builder state for the given dataset
25+
* and restore it when the user navigates back to the same dataset.
26+
*/
27+
export function useCacheBuilderState() {
28+
const {state, dispatch} = useWidgetBuilderContext();
29+
30+
useEffect(() => {
31+
// Remove all cached dataset states when the component mounts
32+
// to prevent stale data from being used.
33+
cleanUpDatasetState();
34+
35+
return cleanUpDatasetState;
36+
}, []);
37+
38+
const cacheBuilderState = useCallback(
39+
(dataset: WidgetType) => {
40+
STORAGE.setItem(
41+
`${WIDGET_BUILDER_DATASET_STATE_KEY}:${dataset}`,
42+
JSON.stringify(convertBuilderStateToWidget(state))
43+
);
44+
},
45+
[state]
46+
);
47+
48+
// Checks if there is a cached builder state for the given dataset
49+
// and restores it if it exists. Otherwise, it sets the dataset.
50+
const restoreOrSetBuilderState = useCallback(
51+
(nextDataset: WidgetType) => {
52+
const previousDatasetState = STORAGE.getItem(
53+
`${WIDGET_BUILDER_DATASET_STATE_KEY}:${nextDataset}`
54+
);
55+
if (previousDatasetState) {
56+
const builderState = convertWidgetToBuilderStateParams(
57+
JSON.parse(previousDatasetState)
58+
);
59+
dispatch({
60+
type: BuilderStateAction.SET_STATE,
61+
payload: {...builderState, title: state.title, description: state.description},
62+
});
63+
} else {
64+
dispatch({
65+
type: BuilderStateAction.SET_DATASET,
66+
payload: nextDataset,
67+
});
68+
}
69+
},
70+
[dispatch, state.title, state.description]
71+
);
72+
73+
return {
74+
cacheBuilderState,
75+
restoreOrSetBuilderState,
76+
};
77+
}

0 commit comments

Comments
 (0)