Skip to content
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

[Backport 2.19] Fix add to dashboard after saving (with unit tests) #9312

Open
wants to merge 1 commit into
base: 2.19
Choose a base branch
from
Open
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
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 @@
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 @@
osdUrlStateStorage
);

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

Check warning on line 136 in src/plugins/dashboard/public/application/utils/create_dashboard_app_state.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/dashboard/public/application/utils/create_dashboard_app_state.tsx#L136

Added line #L136 was not covered by tests
// start syncing the appState with the ('_a') url
startStateSync();
return { stateContainer, stopStateSync, stopSyncingQueryServiceStateWithUrl };
Expand All @@ -147,15 +149,26 @@
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 });

Check warning on line 164 in src/plugins/dashboard/public/application/utils/create_dashboard_app_state.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/dashboard/public/application/utils/create_dashboard_app_state.tsx#L163-L164

Added lines #L163 - L164 were not covered by tests
if (changed) {
scopedHistory.replace({

Check warning on line 166 in src/plugins/dashboard/public/application/utils/create_dashboard_app_state.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/dashboard/public/application/utils/create_dashboard_app_state.tsx#L166

Added line #L166 was not covered by tests
...scopedHistory.location,
state: previousState,
});
}
return changed;

Check warning on line 171 in src/plugins/dashboard/public/application/utils/create_dashboard_app_state.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/dashboard/public/application/utils/create_dashboard_app_state.tsx#L171

Added line #L171 was not covered by tests
};

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
Loading