Skip to content

Commit

Permalink
Fix dashboard to refresh visualizations when the refresh button is cl…
Browse files Browse the repository at this point in the history
…icked (#27353)

* Fix dashboard to refresh visualizations when the refresh button is clicked

* Suggestions

* Fix tests

* Catch bug with a new test to ensure saved searches update when query in url is modified

* Add a test that would have caught the initial problem

* Final fixes that should get ci passing and the bug fixed

* Move requestReload out of _pushFiltersToState

* Fix comparison
  • Loading branch information
stacey-gammon committed Dec 18, 2018
1 parent 1e7740a commit b2576f5
Show file tree
Hide file tree
Showing 15 changed files with 134 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export enum EmbeddableActionTypeKeys {
SET_STAGED_FILTER = 'SET_STAGED_FILTER',
CLEAR_STAGED_FILTERS = 'CLEAR_STAGED_FILTERS',
EMBEDDABLE_ERROR = 'EMBEDDABLE_ERROR',
REQUEST_RELOAD = 'REQUEST_RELOAD',
}

export interface EmbeddableIsInitializingAction
Expand All @@ -58,6 +59,8 @@ export interface SetStagedFilterAction

export interface ClearStagedFiltersAction
extends KibanaAction<EmbeddableActionTypeKeys.CLEAR_STAGED_FILTERS, undefined> {}
export interface RequestReload
extends KibanaAction<EmbeddableActionTypeKeys.REQUEST_RELOAD, undefined> {}

export interface EmbeddableErrorActionPayload {
error: string | object;
Expand Down Expand Up @@ -88,6 +91,8 @@ export const embeddableError = createAction<EmbeddableErrorActionPayload>(
EmbeddableActionTypeKeys.EMBEDDABLE_ERROR
);

export const requestReload = createAction(EmbeddableActionTypeKeys.REQUEST_RELOAD);

/**
* The main point of communication from the embeddable to the dashboard. Any time state in the embeddable
* changes, this function will be called. The data is then extracted from EmbeddableState and stored in
Expand Down
12 changes: 10 additions & 2 deletions src/legacy/core_plugins/kibana/public/dashboard/dashboard_app.js
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,16 @@ app.directive('dashboardApp', function ($injector) {
};

$scope.updateQueryAndFetch = function (query) {
$scope.model.query = migrateLegacyQuery(query);
dashboardStateManager.applyFilters($scope.model.query, filterBar.getFilters());
const oldQuery = $scope.model.query;
if (_.isEqual(oldQuery, query)) {
// The user can still request a reload in the query bar, even if the
// query is the same, and in that case, we have to explicitly ask for
// a reload, since no state changes will cause it.
dashboardStateManager.requestReload();
} else {
$scope.model.query = migrateLegacyQuery(query);
dashboardStateManager.applyFilters($scope.model.query, filterBar.getFilters());
}
$scope.refresh();
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import {
updateFilters,
updateQuery,
closeContextMenu,
requestReload,
} from './actions';
import { stateMonitorFactory } from 'ui/state_management/state_monitor_factory';
import { createPanelState } from './panel';
Expand Down Expand Up @@ -222,6 +223,10 @@ export class DashboardStateManager {
}
}

requestReload() {
store.dispatch(requestReload());
}

_handleStoreChanges() {
let dirty = false;
if (!this._areStoreAndAppStatePanelsEqual()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,10 @@ class DashboardPanelUi extends React.Component {
this.embeddable.onContainerStateChanged(nextProps.containerState);
}

if (this.embeddable && nextProps.lastReloadRequestTime !== this.props.lastReloadRequestTime) {
this.embeddable.reload();
}

return nextProps.error !== this.props.error ||
nextProps.initialized !== this.props.initialized;
}
Expand Down Expand Up @@ -177,6 +181,7 @@ DashboardPanelUi.propTypes = {
embeddableFactory: PropTypes.shape({
create: PropTypes.func,
}).isRequired,
lastReloadRequestTime: PropTypes.number.isRequired,
embeddableStateChanged: PropTypes.func.isRequired,
embeddableIsInitialized: PropTypes.func.isRequired,
embeddableError: PropTypes.func.isRequired,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ function getProps(props = {}) {
viewOnlyMode: false,
destroy: () => {},
initialized: true,
lastReloadRequestTime: 0,
embeddableIsInitialized: () => {},
embeddableIsInitializing: () => {},
embeddableStateChanged: () => {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,15 @@ const mapStateToProps = ({ dashboard }, { embeddableFactory, panelId }) => {
} else {
error = (embeddable && getEmbeddableError(dashboard, panelId)) || '';
}
const lastReloadRequestTime = embeddable ? embeddable.lastReloadRequestTime : 0;
const initialized = embeddable ? getEmbeddableInitialized(dashboard, panelId) : false;
return {
error,
viewOnlyMode: getFullScreenMode(dashboard) || getViewMode(dashboard) === DashboardViewMode.VIEW,
containerState: getContainerState(dashboard, panelId),
initialized,
panel: getPanel(dashboard, panelId)
panel: getPanel(dashboard, panelId),
lastReloadRequestTime,
};
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ const embeddableIsInitializing = (
initialized: false,
metadata: {},
stagedFilter: undefined,
lastReloadRequestTime: 0,
},
});

Expand Down Expand Up @@ -88,6 +89,16 @@ const deleteEmbeddable = (embeddables: EmbeddablesMap, panelId: PanelId): Embedd
return embeddablesCopy;
};

const setReloadRequestTime = (
embeddables: EmbeddablesMap,
lastReloadRequestTime: number
): EmbeddablesMap => {
return _.mapValues<EmbeddablesMap>(embeddables, embeddable => ({
...embeddable,
lastReloadRequestTime,
}));
};

export const embeddablesReducer: Reducer<EmbeddablesMap> = (
embeddables = {},
action
Expand All @@ -105,6 +116,8 @@ export const embeddablesReducer: Reducer<EmbeddablesMap> = (
return embeddableError(embeddables, action.payload);
case PanelActionTypeKeys.DELETE_PANEL:
return deleteEmbeddable(embeddables, action.payload);
case EmbeddableActionTypeKeys.REQUEST_RELOAD:
return setReloadRequestTime(embeddables, new Date().getTime());
default:
return embeddables;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ export interface EmbeddableReduxState {
readonly error?: string | object;
readonly initialized: boolean;
readonly stagedFilter?: object;
/**
* Timestamp of the last time this embeddable was requested to reload.
*/
readonly lastReloadRequestTime: number;
}

export interface PanelsMap {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,12 @@ export class VisualizeEmbeddable extends Embeddable {
}
}

public reload() {
if (this.handler) {
this.handler.reload();
}
}

/**
* Retrieve the panel title for this panel from the container state.
* This will either return the overwritten panel title or the visualization title.
Expand Down
9 changes: 9 additions & 0 deletions src/ui/public/embeddable/embeddable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ interface EmbeddableOptions {
render?: (domNode: HTMLElement, containerState: ContainerState) => void;
destroy?: () => void;
onContainerStateChanged?: (containerState: ContainerState) => void;
reload?: () => void;
}

export abstract class Embeddable {
Expand All @@ -78,6 +79,10 @@ export abstract class Embeddable {
if (options.onContainerStateChanged) {
this.onContainerStateChanged = options.onContainerStateChanged;
}

if (options.reload) {
this.reload = options.reload;
}
}

public abstract onContainerStateChanged(containerState: ContainerState): void;
Expand All @@ -99,4 +104,8 @@ export abstract class Embeddable {
public destroy(): void {
return;
}

public reload(): void {
return;
}
}
14 changes: 7 additions & 7 deletions src/ui/public/visualize/loader/embedded_visualize_handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,13 @@ export class EmbeddedVisualizeHandler {
this.listeners.removeListener(RENDER_COMPLETE_EVENT, listener);
}

/**
* Force the fetch of new data and renders the chart again.
*/
public reload = () => {
this.fetchAndRender(true);
};

private onRenderCompleteListener = () => {
this.listeners.emit(RENDER_COMPLETE_EVENT);
this.element.removeAttribute(LOADING_ATTRIBUTE);
Expand Down Expand Up @@ -365,13 +372,6 @@ export class EmbeddedVisualizeHandler {
this.fetchAndRender();
};

/**
* Force the fetch of new data and renders the chart again.
*/
private reload = () => {
this.fetchAndRender(true);
};

private fetch = (forceFetch: boolean = false) => {
this.dataLoaderParams.aggs = this.vis.getAggConfig();
this.dataLoaderParams.forceFetch = forceFetch;
Expand Down
43 changes: 43 additions & 0 deletions test/functional/apps/dashboard/_dashboard_query_bar.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* 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 'expect.js';

export default function ({ getService, getPageObjects }) {
const esArchiver = getService('esArchiver');
const dashboardExpect = getService('dashboardExpect');
const queryBar = getService('queryBar');
const PageObjects = getPageObjects(['dashboard', 'discover']);

describe('dashboard query bar', async () => {
before(async () => {
await PageObjects.dashboard.loadSavedDashboard('dashboard with filter');
});

it('causes panels to reload when refresh is clicked', async () => {
await esArchiver.unload('dashboard/current/data');

await queryBar.clickQuerySubmitButton();
const headers = await PageObjects.discover.getColumnHeaders();
expect(headers.length).to.be(0);

await dashboardExpect.pieSliceCount(0);
});
});
}
14 changes: 14 additions & 0 deletions test/functional/apps/dashboard/_dashboard_state.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,20 @@ export default function ({ getService, getPageObjects }) {
expect(headers[1]).to.be('agent');
});

it('Saved search will update when the query is changed in the URL', async () => {
const currentQuery = await queryBar.getQueryString();
expect(currentQuery).to.equal('');
const currentUrl = await browser.getCurrentUrl();
const newUrl = currentUrl.replace('query:%27%27', 'query:%27abc12345678910%27');
// Don't add the timestamp to the url or it will cause a hard refresh and we want to test a
// soft refresh.
await browser.get(newUrl.toString(), false);
await PageObjects.header.waitUntilLoadingHasFinished();

const headers = await PageObjects.discover.getColumnHeaders();
expect(headers.length).to.be(0);
});

it('Tile map with no changes will update with visualization changes', async () => {
await PageObjects.dashboard.gotoDashboardLandingPage();

Expand Down
5 changes: 5 additions & 0 deletions test/functional/apps/dashboard/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ export default function ({ getService, loadTestFile, getPageObjects }) {
loadTestFile(require.resolve('./_dashboard_options'));
loadTestFile(require.resolve('./_data_shared_attributes'));
loadTestFile(require.resolve('./_embed_mode'));

// 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
// to improve efficiency...
loadTestFile(require.resolve('./_dashboard_query_bar'));
});

describe('using current data', function () {
Expand Down
4 changes: 4 additions & 0 deletions test/functional/services/query_bar.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ export function QueryBarProvider({ getService, getPageObjects }) {
await PageObjects.header.waitUntilLoadingHasFinished();
}

async clickQuerySubmitButton() {
await testSubjects.click('querySubmitButton');
}

}

return new QueryBar();
Expand Down

0 comments on commit b2576f5

Please sign in to comment.