Skip to content

Commit

Permalink
Fix add to dashboard after saving (with unit tests) (#9072)
Browse files Browse the repository at this point in the history
* Preserve Dashboards scoped history state across URL updates

Signed-off-by: Nick Steinbaugh <[email protected]>

* update/add unit tests

Signed-off-by: Tony Lee <[email protected]>

* Changeset file for PR #9072 created/updated

* update unit tests

Signed-off-by: Tony Lee <[email protected]>

* update unit tests

Signed-off-by: Tony Lee <[email protected]>

* use scopedHistoryMock in unit tests

Signed-off-by: Tony Lee <[email protected]>

---------

Signed-off-by: Nick Steinbaugh <[email protected]>
Signed-off-by: Tony Lee <[email protected]>
Co-authored-by: Nick Steinbaugh <[email protected]>
Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
  • Loading branch information
3 people authored Jan 30, 2025
1 parent 85e2187 commit 95cfb51
Show file tree
Hide file tree
Showing 6 changed files with 182 additions and 6 deletions.
2 changes: 2 additions & 0 deletions changelogs/fragments/9072.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
fix:
- Preserve location state at dashboard app startup to fix adding a new visualization ([#9072](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/9072))

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { createDashboardServicesMock } from './mocks';
import { SavedObjectDashboard } from '../..';
import { syncQueryStateWithUrl } from 'src/plugins/data/public';
import { ViewMode } from 'src/plugins/embeddable/public';
import { scopedHistoryMock } from '../../../../../core/public/mocks';

const mockStartStateSync = jest.fn();
const mockStopStateSync = jest.fn();
Expand Down Expand Up @@ -48,7 +49,7 @@ const { createStateContainer, syncState } = jest.requireMock(
const osdUrlStateStorage = ({
set: jest.fn(),
get: jest.fn(() => ({ linked: false })),
flush: jest.fn(),
flush: jest.fn().mockReturnValue(true),
} as unknown) as IOsdUrlStateStorage;

describe('createDashboardGlobalAndAppState', () => {
Expand Down Expand Up @@ -148,13 +149,47 @@ describe('updateStateUrl', () => {
...dashboardAppStateStub,
viewMode: ViewMode.VIEW,
};
updateStateUrl({ osdUrlStateStorage, state: dashboardAppState, replace: true });

test('update URL to not contain panels', () => {
const { panels, ...statesWithoutPanels } = dashboardAppState;

const basePath = '/base';
const history = scopedHistoryMock.create({
pathname: basePath,
});

updateStateUrl({
osdUrlStateStorage,
state: dashboardAppState,
scopedHistory: history,
replace: true,
});

expect(osdUrlStateStorage.set).toHaveBeenCalledWith('_a', statesWithoutPanels, {
replace: true,
});
expect(osdUrlStateStorage.flush).toHaveBeenCalledWith({ replace: true });
});

test('preserve Dashboards scoped history state', () => {
const basePath = '/base';
const someState = { some: 'state' };
const history = scopedHistoryMock.create({
pathname: basePath,
state: someState,
});
const { location } = history;
const replaceSpy = jest.spyOn(history, 'replace');

const changed = updateStateUrl({
osdUrlStateStorage,
state: dashboardAppState,
scopedHistory: history,
replace: true,
});

expect(history.location.state).toEqual(someState);
expect(changed).toBe(true);
expect(replaceSpy).toHaveBeenCalledWith({ ...location, state: someState });
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { ScopedHistory } from 'src/core/public';
import { migrateAppState } from '../utils/migrate_app_state';
import {
IOsdUrlStateStorage,
Expand Down Expand Up @@ -40,13 +41,14 @@ export const createDashboardGlobalAndAppState = ({
opensearchDashboardsVersion,
usageCollection,
history,
scopedHistory,
data: { query },
} = services;

/*
/*
Function migrateAppState() does two things
1. Migrate panel before version 7.3.0 to the 7.3.0 panel structure.
There are no changes to the panel structure after version 7.3.0 to the current
There are no changes to the panel structure after version 7.3.0 to the current
OpenSearch version so no need to migrate panels that are version 7.3.0 or higher
2. Update the version number on each panel to the current version.
*/
Expand Down Expand Up @@ -131,7 +133,7 @@ export const createDashboardGlobalAndAppState = ({
osdUrlStateStorage
);

updateStateUrl({ osdUrlStateStorage, state: initialState, replace: true });
updateStateUrl({ osdUrlStateStorage, state: initialState, scopedHistory, replace: true });
// start syncing the appState with the ('_a') url
startStateSync();
return { stateContainer, stopStateSync, stopSyncingQueryServiceStateWithUrl };
Expand All @@ -147,15 +149,26 @@ export const createDashboardGlobalAndAppState = ({
export const updateStateUrl = ({
osdUrlStateStorage,
state,
scopedHistory,
replace,
}: {
osdUrlStateStorage: IOsdUrlStateStorage;
state: DashboardAppState;
scopedHistory: ScopedHistory;
replace: boolean;
}) => {
osdUrlStateStorage.set(APP_STATE_STORAGE_KEY, toUrlState(state), { replace });
// immediately forces scheduled updates and changes location
return osdUrlStateStorage.flush({ replace });
// scoped history state is preserved to allow embeddable state transfer
const previousState = scopedHistory.location.state;
const changed = osdUrlStateStorage.flush({ replace });
if (changed) {
scopedHistory.replace({
...scopedHistory.location,
state: previousState,
});
}
return changed;
};

const toUrlState = (state: DashboardAppState): DashboardAppStateInUrl => {
Expand Down
4 changes: 4 additions & 0 deletions src/plugins/dashboard/public/application/utils/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import { dashboardPluginMock } from '../../../../dashboard/public/mocks';
import { usageCollectionPluginMock } from '../../../../usage_collection/public/mocks';
import { embeddablePluginMock } from '../../../../embeddable/public/mocks';
import { DashboardServices } from '../../types';
import { scopedHistoryMock } from '../../../../../core/public/mocks';
import { ScopedHistory } from '../../../../../core/public';

export const createDashboardServicesMock = () => {
const coreStartMock = coreMock.createStart();
Expand All @@ -18,6 +20,7 @@ export const createDashboardServicesMock = () => {
const usageCollection = usageCollectionPluginMock.createSetupContract();
const embeddable = embeddablePluginMock.createStartContract();
const opensearchDashboardsVersion = '3.0.0';
const scopedHistory = (scopedHistoryMock.create() as unknown) as ScopedHistory;

return ({
...coreStartMock,
Expand All @@ -27,6 +30,7 @@ export const createDashboardServicesMock = () => {
replace: jest.fn(),
location: { pathname: '' },
},
scopedHistory,
dashboardConfig: {
getHideWriteControls: jest.fn(),
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ export const useDashboardAppAndGlobalState = ({
usageCollection,
opensearchDashboardsVersion,
osdUrlStateStorage,
scopedHistory,
} = services;
const hideWriteControls = dashboardConfig.getHideWriteControls();
const stateDefaults = migrateAppState(
Expand Down Expand Up @@ -136,6 +137,7 @@ export const useDashboardAppAndGlobalState = ({
const updated = updateStateUrl({
osdUrlStateStorage,
state: stateContainer.getState(),
scopedHistory,
replace,
});

Expand Down

0 comments on commit 95cfb51

Please sign in to comment.