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

[Drilldowns] Fix back button by removing panels from url in dashboard in view mode #62415

Merged
merged 4 commits into from
Apr 18, 2020
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import { FilterUtils } from './lib/filter_utils';
import {
DashboardAppState,
DashboardAppStateDefaults,
DashboardAppStateInUrl,
DashboardAppStateTransitions,
SavedDashboardPanel,
} from '../types';
Expand Down Expand Up @@ -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<DashboardAppState>({
this.stateSyncRef = syncState<DashboardAppStateInUrl>({
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) {
Expand Down Expand Up @@ -558,9 +560,9 @@ export class DashboardStateManager {
*/
private saveState({ replace }: { replace: boolean }): boolean {
// schedules setting current state to url
this.kbnUrlStateStorage.set<DashboardAppState>(
this.kbnUrlStateStorage.set<DashboardAppStateInUrl>(
this.STATE_STORAGE_KEY,
this.stateContainer.get()
this.toUrlState(this.stateContainer.get())
);
// immediately forces scheduled updates and changes location
return this.kbnUrlStateStorage.flush({ replace });
Expand Down Expand Up @@ -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;
}
}
8 changes: 8 additions & 0 deletions src/plugins/dashboard/public/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<DashboardAppState, 'panels'> & {
panels?: SavedDashboardPanel[];
};

export interface DashboardAppStateTransitions {
set: (
state: DashboardAppState
Expand Down
47 changes: 47 additions & 0 deletions test/functional/apps/dashboard/dashboard_back_button.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
}
1 change: 1 addition & 0 deletions test/functional/apps/dashboard/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions x-pack/test/functional/apps/maps/embeddable/dashboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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);
});
});
}