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

Fix dashboard to refresh visualizations when the refresh button is clicked #27353

Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,6 @@ app.directive('dashboardApp', function ($injector) {
$scope.indexPatterns = dashboardStateManager.getPanelIndexPatterns();
};

$scope.$watch('model.query', $scope.updateQueryAndFetch);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing this line does prevent the duplicate network call, but it also makes it so that if you change the query in the URL (and not in the query bar), then saved searches aren't updated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, added a test that would have caught that and confirmed it fails:
screen shot 2018-12-18 at 2 43 59 pm


$scope.$listenAndDigestAsync(timefilter, 'fetch', () => {
dashboardStateManager.handleTimeChange(timefilter.getTime());
// Currently discover relies on this logic to re-fetch. We need to refactor it to rely instead on the
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 @@ -220,6 +221,7 @@ export class DashboardStateManager {
)) {
store.dispatch(updateFilters(dashboardFilters));
}
store.dispatch(requestReload());
}

_handleStoreChanges() {
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 @@ -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