fix(explore): Replace url search params only if current page is Explore (#20972)

* fix(explore): Replace url search params only if current page is Explore

* Add unit test
This commit is contained in:
Kamil Gabryjelski
2022-08-05 16:59:52 +02:00
committed by GitHub
parent 499a28f599
commit 9350bbafee
3 changed files with 44 additions and 14 deletions

View File

@@ -153,7 +153,7 @@ const ExploreChartPanel = ({
const [showDatasetModal, setShowDatasetModal] = useState(false);
const metaDataRegistry = getChartMetadataRegistry();
const { useLegacyApi } = metaDataRegistry.get(vizType);
const { useLegacyApi } = metaDataRegistry.get(vizType) ?? {};
const vizTypeNeedsDataset =
useLegacyApi && datasource.type !== DatasourceType.Table;
// added boolean column to below show boolean so that the errors aren't overlapping

View File

@@ -84,9 +84,21 @@ fetchMock.put('glob:*/api/v1/explore/form_data*', { key });
fetchMock.get('glob:*/api/v1/explore/form_data*', {});
fetchMock.get('glob:*/favstar/slice*', { count: 0 });
const renderWithRouter = (withKey?: boolean) => {
const path = '/explore/';
const defaultPath = '/explore/';
const renderWithRouter = ({
withKey,
overridePathname,
}: {
withKey?: boolean;
overridePathname?: string;
} = {}) => {
const path = overridePathname ?? defaultPath;
const search = withKey ? `?form_data_key=${key}&dataset_id=1` : '';
Object.defineProperty(window, 'location', {
get() {
return { pathname: path, search };
},
});
return render(
<MemoryRouter initialEntries={[`${path}${search}`]}>
<Route path={path}>
@@ -123,7 +135,7 @@ test('generates a new form_data param when none is available', async () => {
test('generates a different form_data param when one is provided and is mounting', async () => {
const replaceState = jest.spyOn(window.history, 'replaceState');
await waitFor(() => renderWithRouter(true));
await waitFor(() => renderWithRouter({ withKey: true }));
expect(replaceState).not.toHaveBeenLastCalledWith(
0,
expect.anything(),
@@ -144,7 +156,7 @@ test('reuses the same form_data param when updating', async () => {
});
const replaceState = jest.spyOn(window.history, 'replaceState');
const pushState = jest.spyOn(window.history, 'pushState');
await waitFor(() => renderWithRouter());
await waitFor(() => renderWithRouter({ withKey: true }));
expect(replaceState.mock.calls.length).toBe(1);
userEvent.click(screen.getByText('Update chart'));
await waitFor(() => expect(pushState.mock.calls.length).toBe(1));
@@ -153,3 +165,18 @@ test('reuses the same form_data param when updating', async () => {
pushState.mockRestore();
getChartControlPanelRegistry().remove('table');
});
test('doesnt call replaceState when pathname is not /explore', async () => {
getChartMetadataRegistry().registerValue(
'table',
new ChartMetadata({
name: 'fake table',
thumbnail: '.png',
useLegacyApi: false,
}),
);
const replaceState = jest.spyOn(window.history, 'replaceState');
await waitFor(() => renderWithRouter({ overridePathname: '/dashboard' }));
expect(replaceState).not.toHaveBeenCalled();
replaceState.mockRestore();
});

View File

@@ -206,15 +206,18 @@ const updateHistory = debounce(
);
stateModifier = 'pushState';
}
const url = mountExploreUrl(
standalone ? URL_PARAMS.standalone.name : null,
{
[URL_PARAMS.formDataKey.name]: key,
...additionalParam,
},
force,
);
window.history[stateModifier](payload, title, url);
// avoid race condition in case user changes route before explore updates the url
if (window.location.pathname.startsWith('/explore')) {
const url = mountExploreUrl(
standalone ? URL_PARAMS.standalone.name : null,
{
[URL_PARAMS.formDataKey.name]: key,
...additionalParam,
},
force,
);
window.history[stateModifier](payload, title, url);
}
} catch (e) {
logging.warn('Failed at altering browser history', e);
}