From d7b44f8172d7814290b2ea147ebea4368f22a67b Mon Sep 17 00:00:00 2001 From: Anton Dosov Date: Sat, 18 Apr 2020 10:34:17 +0200 Subject: [PATCH] [Drilldowns] Fix back button by removing panels from url in dashboard in view mode (#62415) * remove panels from url * review nits Co-authored-by: Elastic Machine --- .../application/dashboard_state_manager.ts | 19 ++++++-- src/plugins/dashboard/public/types.ts | 8 ++++ .../apps/dashboard/dashboard_back_button.ts | 47 +++++++++++++++++++ test/functional/apps/dashboard/index.js | 1 + .../apps/maps/embeddable/dashboard.js | 11 +++++ 5 files changed, 82 insertions(+), 4 deletions(-) create mode 100644 test/functional/apps/dashboard/dashboard_back_button.ts diff --git a/src/plugins/dashboard/public/application/dashboard_state_manager.ts b/src/plugins/dashboard/public/application/dashboard_state_manager.ts index 13ba3c6d0b60d4..b03ea95069a3da 100644 --- a/src/plugins/dashboard/public/application/dashboard_state_manager.ts +++ b/src/plugins/dashboard/public/application/dashboard_state_manager.ts @@ -34,6 +34,7 @@ import { FilterUtils } from './lib/filter_utils'; import { DashboardAppState, DashboardAppStateDefaults, + DashboardAppStateInUrl, DashboardAppStateTransitions, SavedDashboardPanel, } from '../types'; @@ -165,11 +166,12 @@ export class DashboardStateManager { }); // setup state syncing utils. state container will be synced with url into `this.STATE_STORAGE_KEY` query param - this.stateSyncRef = syncState({ + this.stateSyncRef = syncState({ storageKey: this.STATE_STORAGE_KEY, stateContainer: { ...this.stateContainer, - set: (state: DashboardAppState | null) => { + get: () => this.toUrlState(this.stateContainer.get()), + set: (state: DashboardAppStateInUrl | null) => { // sync state required state container to be able to handle null // overriding set() so it could handle null coming from url if (state) { @@ -558,9 +560,9 @@ export class DashboardStateManager { */ private saveState({ replace }: { replace: boolean }): boolean { // schedules setting current state to url - this.kbnUrlStateStorage.set( + this.kbnUrlStateStorage.set( this.STATE_STORAGE_KEY, - this.stateContainer.get() + this.toUrlState(this.stateContainer.get()) ); // immediately forces scheduled updates and changes location return this.kbnUrlStateStorage.flush({ replace }); @@ -620,4 +622,13 @@ export class DashboardStateManager { const current = _.omit(this.stateContainer.get(), propsToIgnore); return !_.isEqual(initial, current); } + + private toUrlState(state: DashboardAppState): DashboardAppStateInUrl { + if (state.viewMode === ViewMode.VIEW) { + const { panels, ...stateWithoutPanels } = state; + return stateWithoutPanels; + } + + return state; + } } diff --git a/src/plugins/dashboard/public/types.ts b/src/plugins/dashboard/public/types.ts index 7bccd3de6eca84..d96d2cdf75626d 100644 --- a/src/plugins/dashboard/public/types.ts +++ b/src/plugins/dashboard/public/types.ts @@ -152,6 +152,14 @@ export type DashboardAppStateDefaults = DashboardAppState & { description?: string; }; +/** + * In URL panels are optional, + * Panels are not added to the URL when in "view" mode + */ +export type DashboardAppStateInUrl = Omit & { + panels?: SavedDashboardPanel[]; +}; + export interface DashboardAppStateTransitions { set: ( state: DashboardAppState diff --git a/test/functional/apps/dashboard/dashboard_back_button.ts b/test/functional/apps/dashboard/dashboard_back_button.ts new file mode 100644 index 00000000000000..8a488c1780fcc5 --- /dev/null +++ b/test/functional/apps/dashboard/dashboard_back_button.ts @@ -0,0 +1,47 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import expect from '@kbn/expect'; +import { FtrProviderContext } from '../../ftr_provider_context'; + +export default function({ getService, getPageObjects }: FtrProviderContext) { + const esArchiver = getService('esArchiver'); + const kibanaServer = getService('kibanaServer'); + const PageObjects = getPageObjects(['dashboard', 'header', 'common', 'visualize', 'timePicker']); + const browser = getService('browser'); + + describe('dashboard back button', () => { + before(async () => { + await esArchiver.loadIfNeeded('dashboard/current/kibana'); + await kibanaServer.uiSettings.replace({ + defaultIndex: '0bf35f60-3dc9-11e8-8660-4d65aa086b3c', + }); + await PageObjects.common.navigateToApp('dashboard'); + await PageObjects.dashboard.preserveCrossAppState(); + }); + + it('after navigation from listing page to dashboard back button works', async () => { + await PageObjects.dashboard.gotoDashboardLandingPage(); + await PageObjects.dashboard.loadSavedDashboard('dashboard with everything'); + await PageObjects.dashboard.waitForRenderComplete(); + await browser.goBack(); + expect(await PageObjects.dashboard.onDashboardLandingPage()).to.be(true); + }); + }); +} diff --git a/test/functional/apps/dashboard/index.js b/test/functional/apps/dashboard/index.js index 5e96a55b190149..6666ccc57d5845 100644 --- a/test/functional/apps/dashboard/index.js +++ b/test/functional/apps/dashboard/index.js @@ -55,6 +55,7 @@ export default function({ getService, loadTestFile }) { loadTestFile(require.resolve('./dashboard_options')); loadTestFile(require.resolve('./data_shared_attributes')); loadTestFile(require.resolve('./embed_mode')); + loadTestFile(require.resolve('./dashboard_back_button')); // Note: This one must be last because it unloads some data for one of its tests! // No, this isn't ideal, but loading/unloading takes so much time and these are all bunched diff --git a/x-pack/test/functional/apps/maps/embeddable/dashboard.js b/x-pack/test/functional/apps/maps/embeddable/dashboard.js index 43d5cccb209058..9027bb5309ff83 100644 --- a/x-pack/test/functional/apps/maps/embeddable/dashboard.js +++ b/x-pack/test/functional/apps/maps/embeddable/dashboard.js @@ -13,6 +13,7 @@ export default function({ getPageObjects, getService }) { const dashboardPanelActions = getService('dashboardPanelActions'); const inspector = getService('inspector'); const testSubjects = getService('testSubjects'); + const browser = getService('browser'); describe('embed in dashboard', () => { before(async () => { @@ -111,5 +112,15 @@ export default function({ getPageObjects, getService }) { const afterRefreshTimerTimestamp = await getRequestTimestamp(); expect(beforeRefreshTimerTimestamp).not.to.equal(afterRefreshTimerTimestamp); }); + + // see https://github.com/elastic/kibana/issues/61596 on why it is specific to maps + it("dashboard's back button should navigate to previous page", async () => { + await PageObjects.common.navigateToApp('dashboard'); + await PageObjects.dashboard.preserveCrossAppState(); + await PageObjects.dashboard.loadSavedDashboard('map embeddable example'); + await PageObjects.dashboard.waitForRenderComplete(); + await browser.goBack(); + expect(await PageObjects.dashboard.onDashboardLandingPage()).to.be(true); + }); }); }