From e6f2b10a36127635f20dbb622264fc154e12c055 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Wed, 2 Jun 2021 12:24:19 +0200 Subject: [PATCH] ChangeTracker: Unified unsaved changes handling with library panels (#34989) * UnsavedChanges: Move Change tracker to use Prompt * Fix a lot of race conditions and stacking of changes in onConfirm and onDismiss * Listen to save event * add missing delay argument * migrated the change tracker unit tests * Updated snapshot * Removed unessary action * removed updateSourcePanel * Fix hiding save library panel modal prompt when clicking discard * change saved libray panel title and buttons so they are a bit different as Prompt and when used from save button * Fixed issue with saving new dashboard * Now all scenarios work * increase wait time * Fixed one more race condition --- .../grafana-e2e/src/flows/addDashboard.ts | 1 + .../DashboardPrompt/DashboardPrompt.test.tsx} | 80 +++--- .../DashboardPrompt/DashboardPrompt.tsx | 234 ++++++++++++++++++ .../components/PanelEditor/PanelEditor.tsx | 72 +++--- .../components/PanelEditor/state/actions.ts | 55 +--- .../SaveDashboard/UnsavedChangesModal.tsx | 8 +- .../SaveDashboard/useDashboardSave.tsx | 3 +- .../dashboard/containers/DashboardPage.tsx | 3 + .../__snapshots__/DashboardPage.test.tsx.snap | 214 ++++++++++++++++ .../dashboard/services/ChangeTracker.ts | 161 ------------ .../services/__mocks__/ChangeTracker.ts | 9 - .../dashboard/state/DashboardModel.ts | 2 +- .../dashboard/state/initDashboard.test.ts | 1 - .../features/dashboard/state/initDashboard.ts | 3 - .../SaveLibraryPanelModal.tsx | 20 +- 15 files changed, 538 insertions(+), 328 deletions(-) rename public/app/features/dashboard/{services/ChangeTracker.test.ts => components/DashboardPrompt/DashboardPrompt.test.tsx} (53%) create mode 100644 public/app/features/dashboard/components/DashboardPrompt/DashboardPrompt.tsx delete mode 100644 public/app/features/dashboard/services/ChangeTracker.ts delete mode 100644 public/app/features/dashboard/services/__mocks__/ChangeTracker.ts diff --git a/packages/grafana-e2e/src/flows/addDashboard.ts b/packages/grafana-e2e/src/flows/addDashboard.ts index 1dc1807c034..9bde10d2295 100644 --- a/packages/grafana-e2e/src/flows/addDashboard.ts +++ b/packages/grafana-e2e/src/flows/addDashboard.ts @@ -77,6 +77,7 @@ export const addDashboard = (config?: Partial) => { return e2e() .url() + .should('contain', '/d/') .then((url: string) => { const uid = getDashboardUid(url); diff --git a/public/app/features/dashboard/services/ChangeTracker.test.ts b/public/app/features/dashboard/components/DashboardPrompt/DashboardPrompt.test.tsx similarity index 53% rename from public/app/features/dashboard/services/ChangeTracker.test.ts rename to public/app/features/dashboard/components/DashboardPrompt/DashboardPrompt.test.tsx index a7175184df9..3d0145e0a0b 100644 --- a/public/app/features/dashboard/services/ChangeTracker.test.ts +++ b/public/app/features/dashboard/components/DashboardPrompt/DashboardPrompt.test.tsx @@ -1,7 +1,7 @@ -import { ChangeTracker } from './ChangeTracker'; -import { DashboardModel } from '../state/DashboardModel'; -import { PanelModel } from '../state/PanelModel'; -import { setContextSrv } from '../../../core/services/context_srv'; +import { DashboardModel } from '../../state/DashboardModel'; +import { PanelModel } from '../../state/PanelModel'; +import { setContextSrv } from '../../../../core/services/context_srv'; +import { hasChanges, ignoreChanges } from './DashboardPrompt'; function getDefaultDashboardModel(): DashboardModel { return new DashboardModel({ @@ -32,129 +32,125 @@ function getTestContext() { const contextSrv: any = { isSignedIn: true, isEditor: true }; setContextSrv(contextSrv); const dash: any = getDefaultDashboardModel(); - const tracker = new ChangeTracker(); const original: any = dash.getSaveModelClone(); - return { dash, tracker, original, contextSrv }; + return { dash, original, contextSrv }; } -describe('ChangeTracker', () => { +describe('DashboardPrompt', () => { it('No changes should not have changes', () => { - const { tracker, original, dash } = getTestContext(); - expect(tracker.hasChanges(dash, original)).toBe(false); + const { original, dash } = getTestContext(); + expect(hasChanges(dash, original)).toBe(false); }); it('Simple change should be registered', () => { - const { tracker, original, dash } = getTestContext(); + const { original, dash } = getTestContext(); dash.title = 'google'; - expect(tracker.hasChanges(dash, original)).toBe(true); + expect(hasChanges(dash, original)).toBe(true); }); it('Should ignore a lot of changes', () => { - const { tracker, original, dash } = getTestContext(); + const { original, dash } = getTestContext(); dash.time = { from: '1h' }; dash.refresh = true; dash.schemaVersion = 10; - expect(tracker.hasChanges(dash, original)).toBe(false); + expect(hasChanges(dash, original)).toBe(false); }); it('Should ignore .iteration changes', () => { - const { tracker, original, dash } = getTestContext(); + const { original, dash } = getTestContext(); dash.iteration = new Date().getTime() + 1; - expect(tracker.hasChanges(dash, original)).toBe(false); + expect(hasChanges(dash, original)).toBe(false); }); it('Should ignore row collapse change', () => { - const { tracker, original, dash } = getTestContext(); + const { original, dash } = getTestContext(); dash.toggleRow(dash.panels[1]); - expect(tracker.hasChanges(dash, original)).toBe(false); + expect(hasChanges(dash, original)).toBe(false); }); it('Should ignore panel legend changes', () => { - const { tracker, original, dash } = getTestContext(); + const { original, dash } = getTestContext(); dash.panels[0].legend.sortDesc = true; dash.panels[0].legend.sort = 'avg'; - expect(tracker.hasChanges(dash, original)).toBe(false); + expect(hasChanges(dash, original)).toBe(false); }); it('Should ignore panel repeats', () => { - const { tracker, original, dash } = getTestContext(); + const { original, dash } = getTestContext(); dash.panels.push(new PanelModel({ repeatPanelId: 10 })); - expect(tracker.hasChanges(dash, original)).toBe(false); + expect(hasChanges(dash, original)).toBe(false); }); describe('ignoreChanges', () => { describe('when called without original dashboard', () => { it('then it should return true', () => { - const { tracker, dash } = getTestContext(); - expect(tracker.ignoreChanges(dash, null)).toBe(true); + const { dash } = getTestContext(); + expect(ignoreChanges(dash, null)).toBe(true); }); }); describe('when called without current dashboard', () => { it('then it should return true', () => { - const { tracker, original } = getTestContext(); - expect(tracker.ignoreChanges((null as unknown) as DashboardModel, original)).toBe(true); + const { original } = getTestContext(); + expect(ignoreChanges((null as unknown) as DashboardModel, original)).toBe(true); }); }); describe('when called without meta in current dashboard', () => { it('then it should return true', () => { - const { tracker, original, dash } = getTestContext(); - expect(tracker.ignoreChanges({ ...dash, meta: undefined }, original)).toBe(true); + const { original, dash } = getTestContext(); + expect(ignoreChanges({ ...dash, meta: undefined }, original)).toBe(true); }); }); describe('when called for a viewer without save permissions', () => { it('then it should return true', () => { - const { tracker, original, dash, contextSrv } = getTestContext(); + const { original, dash, contextSrv } = getTestContext(); contextSrv.isEditor = false; - expect(tracker.ignoreChanges({ ...dash, meta: { canSave: false } }, original)).toBe(true); + expect(ignoreChanges({ ...dash, meta: { canSave: false } }, original)).toBe(true); }); }); describe('when called for a viewer with save permissions', () => { it('then it should return undefined', () => { - const { tracker, original, dash, contextSrv } = getTestContext(); + const { original, dash, contextSrv } = getTestContext(); contextSrv.isEditor = false; - expect(tracker.ignoreChanges({ ...dash, meta: { canSave: true } }, original)).toBe(undefined); + expect(ignoreChanges({ ...dash, meta: { canSave: true } }, original)).toBe(undefined); }); }); describe('when called for an user that is not signed in', () => { it('then it should return true', () => { - const { tracker, original, dash, contextSrv } = getTestContext(); + const { original, dash, contextSrv } = getTestContext(); contextSrv.isSignedIn = false; - expect(tracker.ignoreChanges({ ...dash, meta: { canSave: true } }, original)).toBe(true); + expect(ignoreChanges({ ...dash, meta: { canSave: true } }, original)).toBe(true); }); }); describe('when called with fromScript', () => { it('then it should return true', () => { - const { tracker, original, dash } = getTestContext(); + const { original, dash } = getTestContext(); expect( - tracker.ignoreChanges({ ...dash, meta: { canSave: true, fromScript: true, fromFile: undefined } }, original) + ignoreChanges({ ...dash, meta: { canSave: true, fromScript: true, fromFile: undefined } }, original) ).toBe(true); }); }); describe('when called with fromFile', () => { it('then it should return true', () => { - const { tracker, original, dash } = getTestContext(); + const { original, dash } = getTestContext(); expect( - tracker.ignoreChanges({ ...dash, meta: { canSave: true, fromScript: undefined, fromFile: true } }, original) + ignoreChanges({ ...dash, meta: { canSave: true, fromScript: undefined, fromFile: true } }, original) ).toBe(true); }); }); describe('when called with canSave but without fromScript and fromFile', () => { it('then it should return false', () => { - const { tracker, original, dash } = getTestContext(); + const { original, dash } = getTestContext(); expect( - tracker.ignoreChanges( - { ...dash, meta: { canSave: true, fromScript: undefined, fromFile: undefined } }, - original - ) + ignoreChanges({ ...dash, meta: { canSave: true, fromScript: undefined, fromFile: undefined } }, original) ).toBe(undefined); }); }); diff --git a/public/app/features/dashboard/components/DashboardPrompt/DashboardPrompt.tsx b/public/app/features/dashboard/components/DashboardPrompt/DashboardPrompt.tsx new file mode 100644 index 00000000000..7dbca6b78ca --- /dev/null +++ b/public/app/features/dashboard/components/DashboardPrompt/DashboardPrompt.tsx @@ -0,0 +1,234 @@ +import { locationService } from '@grafana/runtime'; +import { appEvents } from 'app/core/core'; +import { contextSrv } from 'app/core/services/context_srv'; +import React, { useEffect, useState } from 'react'; +import { Prompt } from 'react-router-dom'; +import { DashboardModel } from '../../state/DashboardModel'; +import { each, filter, find } from 'lodash'; +import angular from 'angular'; +import { UnsavedChangesModal } from '../SaveDashboard/UnsavedChangesModal'; +import * as H from 'history'; +import { SaveLibraryPanelModal } from 'app/features/library-panels/components/SaveLibraryPanelModal/SaveLibraryPanelModal'; +import { PanelModelWithLibraryPanel } from 'app/features/library-panels/types'; +import { useDispatch } from 'react-redux'; +import { discardPanelChanges } from '../PanelEditor/state/actions'; +import { DashboardSavedEvent } from 'app/types/events'; + +export interface Props { + dashboard: DashboardModel; +} + +interface State { + original: object | null; + originalPath?: string; + modal: PromptModal | null; + blockedLocation?: H.Location | null; +} + +enum PromptModal { + UnsavedChangesModal, + SaveLibraryPanelModal, +} + +export const DashboardPrompt = React.memo(({ dashboard }: Props) => { + const [state, setState] = useState({ original: null, modal: null }); + const dispatch = useDispatch(); + const { original, originalPath, blockedLocation, modal } = state; + + useEffect(() => { + // This timeout delay is to wait for panels to load and migrate scheme before capturing the original state + // This is to minimize unsaved changes warnings due to automatic schema migrations + const timeoutId = setTimeout(() => { + const originalPath = locationService.getLocation().pathname; + const original = dashboard.getSaveModelClone(); + + setState({ originalPath, original, modal: null }); + }, 1000); + + return () => { + clearTimeout(timeoutId); + }; + }, [dashboard]); + + // Handle saved events + useEffect(() => { + const savedEventUnsub = appEvents.subscribe(DashboardSavedEvent, () => { + const original = dashboard.getSaveModelClone(); + const originalPath = locationService.getLocation().pathname; + setState({ originalPath, original, modal: null }); + + if (blockedLocation) { + moveToBlockedLocationAfterReactStateUpdate(blockedLocation); + } + }); + + return () => savedEventUnsub.unsubscribe(); + }, [dashboard, blockedLocation]); + + const onHistoryBlock = (location: H.Location) => { + const panelInEdit = dashboard.panelInEdit; + const search = new URLSearchParams(location.search); + + // Are we leaving panel edit & library panel? + if (panelInEdit && panelInEdit.libraryPanel && panelInEdit.hasChanged && !search.has('editPanel')) { + setState({ ...state, modal: PromptModal.SaveLibraryPanelModal, blockedLocation: location }); + return false; + } + + // Are we still on the same dashboard? + if (originalPath === location.pathname || !original) { + return true; + } + + if (ignoreChanges(dashboard, original)) { + return true; + } + + if (!hasChanges(dashboard, original)) { + return true; + } + + setState({ ...state, modal: PromptModal.UnsavedChangesModal, blockedLocation: location }); + return false; + }; + + const onHideModalAndMoveToBlockedLocation = () => { + setState({ ...state, modal: null }); + moveToBlockedLocationAfterReactStateUpdate(blockedLocation); + }; + + return ( + <> + + {modal === PromptModal.UnsavedChangesModal && ( + {}} // Handled by DashboardSavedEvent above + onDiscard={() => { + // Clear original will allow us to leave without unsaved changes prompt + setState({ ...state, original: null, modal: null }); + moveToBlockedLocationAfterReactStateUpdate(blockedLocation); + }} + onDismiss={() => { + setState({ ...state, modal: null, blockedLocation: null }); + }} + /> + )} + {modal === PromptModal.SaveLibraryPanelModal && ( + { + dispatch(discardPanelChanges()); + setState({ ...state, modal: null }); + moveToBlockedLocationAfterReactStateUpdate(blockedLocation); + }} + onDismiss={() => { + setState({ ...state, modal: null, blockedLocation: null }); + }} + /> + )} + + ); +}); + +function moveToBlockedLocationAfterReactStateUpdate(location?: H.Location | null) { + if (location) { + setTimeout(() => locationService.push(location!), 10); + } +} + +/** + * For some dashboards and users changes should be ignored * + */ +export function ignoreChanges(current: DashboardModel, original: object | null) { + if (!original) { + return true; + } + + // Ignore changes if the user has been signed out + if (!contextSrv.isSignedIn) { + return true; + } + + if (!current || !current.meta) { + return true; + } + + const { canSave, fromScript, fromFile } = current.meta; + if (!contextSrv.isEditor && !canSave) { + return true; + } + + return !canSave || fromScript || fromFile; +} + +/** + * Remove stuff that should not count in diff + */ +function cleanDashboardFromIgnoredChanges(dashData: any) { + // need to new up the domain model class to get access to expand / collapse row logic + const model = new DashboardModel(dashData); + + // Expand all rows before making comparison. This is required because row expand / collapse + // change order of panel array and panel positions. + model.expandRows(); + + const dash = model.getSaveModelClone(); + + // ignore time and refresh + dash.time = 0; + dash.refresh = 0; + dash.schemaVersion = 0; + dash.timezone = 0; + + // ignore iteration property + delete dash.iteration; + + dash.panels = filter(dash.panels, (panel) => { + if (panel.repeatPanelId) { + return false; + } + + // remove scopedVars + panel.scopedVars = undefined; + + // ignore panel legend sort + if (panel.legend) { + delete panel.legend.sort; + delete panel.legend.sortDesc; + } + + return true; + }); + + // ignore template variable values + each(dash.getVariables(), (variable: any) => { + variable.current = null; + variable.options = null; + variable.filters = null; + }); + + return dash; +} + +export function hasChanges(current: DashboardModel, original: any) { + const currentClean = cleanDashboardFromIgnoredChanges(current.getSaveModelClone()); + const originalClean = cleanDashboardFromIgnoredChanges(original); + + const currentTimepicker: any = find((currentClean as any).nav, { type: 'timepicker' }); + const originalTimepicker: any = find((originalClean as any).nav, { type: 'timepicker' }); + + if (currentTimepicker && originalTimepicker) { + currentTimepicker.now = originalTimepicker.now; + } + + const currentJson = angular.toJson(currentClean); + const originalJson = angular.toJson(originalClean); + console.log('current', currentJson); + console.log('originalJson', originalJson); + + return currentJson !== originalJson; +} diff --git a/public/app/features/dashboard/components/PanelEditor/PanelEditor.tsx b/public/app/features/dashboard/components/PanelEditor/PanelEditor.tsx index fcf4e26ada4..79402b77bca 100644 --- a/public/app/features/dashboard/components/PanelEditor/PanelEditor.tsx +++ b/public/app/features/dashboard/components/PanelEditor/PanelEditor.tsx @@ -1,6 +1,5 @@ import React, { PureComponent } from 'react'; import { connect, ConnectedProps } from 'react-redux'; -import { Prompt } from 'react-router-dom'; import AutoSizer from 'react-virtualized-auto-sizer'; import { css, cx } from '@emotion/css'; import { Subscription } from 'rxjs'; @@ -29,14 +28,7 @@ import { SplitPaneWrapper } from 'app/core/components/SplitPaneWrapper/SplitPane import { SaveDashboardModalProxy } from '../SaveDashboard/SaveDashboardModalProxy'; import { DashboardPanel } from '../../dashgrid/DashboardPanel'; -import { - exitPanelEditor, - discardPanelChanges, - initPanelEditor, - panelEditorCleanUp, - updatePanelEditorUIState, - updateSourcePanel, -} from './state/actions'; +import { discardPanelChanges, initPanelEditor, panelEditorCleanUp, updatePanelEditorUIState } from './state/actions'; import { updateTimeZoneForSession } from 'app/features/profile/state/reducers'; import { toggleTableView } from './state/reducers'; @@ -62,6 +54,7 @@ import { } from '../../../library-panels/utils'; import { notifyApp } from '../../../../core/actions'; import { PanelEditorTableView } from './PanelEditorTableView'; +import { PanelModelWithLibraryPanel } from 'app/features/library-panels/types'; interface OwnProps { dashboard: DashboardModel; @@ -85,8 +78,6 @@ const mapStateToProps = (state: StoreState) => { const mapDispatchToProps = { initPanelEditor, - exitPanelEditor, - updateSourcePanel, panelEditorCleanUp, discardPanelChanges, updatePanelEditorUIState, @@ -99,9 +90,17 @@ const connector = connect(mapStateToProps, mapDispatchToProps); type Props = OwnProps & ConnectedProps; +interface State { + showSaveLibraryPanelModal?: boolean; +} + export class PanelEditorUnconnected extends PureComponent { private eventSubs?: Subscription; + state: State = { + showSaveLibraryPanelModal: false, + }; + componentDidMount() { this.props.initPanelEditor(this.props.sourcePanel, this.props.dashboard); } @@ -164,7 +163,6 @@ export class PanelEditorUnconnected extends PureComponent { ) { try { await saveAndRefreshLibraryPanel(this.props.panel, this.props.dashboard.meta.folderId!); - this.props.updateSourcePanel(this.props.panel); this.props.notifyApp(createPanelLibrarySuccessNotification('Library panel saved')); } catch (err) { this.props.notifyApp(createPanelLibraryErrorNotification(`Error saving library panel: "${err.statusText}"`)); @@ -172,22 +170,7 @@ export class PanelEditorUnconnected extends PureComponent { return; } - appEvents.publish( - new ShowModalReactEvent({ - component: SaveLibraryPanelModal, - props: { - panel: this.props.panel, - folderId: this.props.dashboard.meta.folderId, - isOpen: true, - onConfirm: () => { - // need to update the source panel here so that when - // the user exits the panel editor they aren't prompted to save again - this.props.updateSourcePanel(this.props.panel); - }, - onDiscard: this.onDiscard, - }, - }) - ); + this.setState({ showSaveLibraryPanelModal: true }); }; onChangeTab = (tab: PanelEditorTab) => { @@ -428,8 +411,16 @@ export class PanelEditorUnconnected extends PureComponent { ); } + onGoBackToDashboard = () => { + locationService.partial({ editPanel: null, tab: null }); + }; + + onConfirmAndDismissLibarayPanelModel = () => { + this.setState({ showSaveLibraryPanelModal: false }); + }; + render() { - const { dashboard, initDone, updatePanelEditorUIState, uiState, exitPanelEditor } = this.props; + const { dashboard, initDone, updatePanelEditorUIState, uiState } = this.props; const styles = getStyles(config.theme, this.props); if (!initDone) { @@ -438,19 +429,7 @@ export class PanelEditorUnconnected extends PureComponent { return (
- { - const searchParams = new URLSearchParams(location.search); - if (!this.props.panel.libraryPanel || !this.props.panel.hasChanged || searchParams.has('editPanel')) { - return true; - } - - exitPanelEditor(); - return false; - }} - /> - + {this.renderEditorActions()}
@@ -462,6 +441,15 @@ export class PanelEditorUnconnected extends PureComponent { rightPaneVisible={uiState.isPanelOptionsVisible} />
+ {this.state.showSaveLibraryPanelModal && ( + + )}
); } diff --git a/public/app/features/dashboard/components/PanelEditor/state/actions.ts b/public/app/features/dashboard/components/PanelEditor/state/actions.ts index b7a309fb47f..08bf2eaf49d 100644 --- a/public/app/features/dashboard/components/PanelEditor/state/actions.ts +++ b/public/app/features/dashboard/components/PanelEditor/state/actions.ts @@ -1,7 +1,5 @@ import { DashboardModel, PanelModel } from '../../../state'; import { ThunkResult } from 'app/types'; -import { appEvents } from 'app/core/core'; -import { SaveLibraryPanelModal } from 'app/features/library-panels/components/SaveLibraryPanelModal/SaveLibraryPanelModal'; import { closeCompleted, PANEL_EDITOR_UI_STATE_STORAGE_KEY, @@ -14,7 +12,6 @@ import { cleanUpEditPanel, panelModelAndPluginReady } from '../../../state/reduc import store from 'app/core/store'; import { pick } from 'lodash'; import { locationService } from '@grafana/runtime'; -import { ShowModalReactEvent } from '../../../../../types/events'; export function initPanelEditor(sourcePanel: PanelModel, dashboard: DashboardModel): ThunkResult { return (dispatch) => { @@ -29,19 +26,6 @@ export function initPanelEditor(sourcePanel: PanelModel, dashboard: DashboardMod }; } -export function updateSourcePanel(sourcePanel: PanelModel): ThunkResult { - return (dispatch, getStore) => { - const { getPanel } = getStore().panelEditor; - - dispatch( - updateEditorInitState({ - panel: getPanel(), - sourcePanel, - }) - ); - }; -} - export function discardPanelChanges(): ThunkResult { return async (dispatch, getStore) => { const { getPanel } = getStore().panelEditor; @@ -52,43 +36,12 @@ export function discardPanelChanges(): ThunkResult { export function exitPanelEditor(): ThunkResult { return async (dispatch, getStore) => { - const dashboard = getStore().dashboard.getModel(); - const { getPanel, shouldDiscardChanges } = getStore().panelEditor; - const onConfirm = () => locationService.partial({ editPanel: null, tab: null }); - - const panel = getPanel(); - const onDiscard = () => { - dispatch(discardPanelChanges()); - onConfirm(); - }; - - if (shouldDiscardChanges || !panel.libraryPanel) { - onConfirm(); - return; - } - - if (!panel.hasChanged) { - onConfirm(); - return; - } - - appEvents.publish( - new ShowModalReactEvent({ - component: SaveLibraryPanelModal, - props: { - panel, - folderId: dashboard!.meta.folderId, - isOpen: true, - onConfirm, - onDiscard, - }, - }) - ); + locationService.partial({ editPanel: null, tab: null }); }; } -function updateDuplicateLibraryPanels(modifiedPanel: PanelModel, dashboard: DashboardModel, dispatch: any) { - if (modifiedPanel.libraryPanel?.uid === undefined) { +function updateDuplicateLibraryPanels(modifiedPanel: PanelModel, dashboard: DashboardModel | null, dispatch: any) { + if (modifiedPanel.libraryPanel?.uid === undefined || !dashboard) { return; } @@ -132,7 +85,7 @@ export function panelEditorCleanUp(): ThunkResult { const sourcePanel = getSourcePanel(); const panelTypeChanged = sourcePanel.type !== panel.type; - updateDuplicateLibraryPanels(panel, dashboard!, dispatch); + updateDuplicateLibraryPanels(panel, dashboard, dispatch); // restore the source panel ID before we update source panel modifiedSaveModel.id = sourcePanel.id; diff --git a/public/app/features/dashboard/components/SaveDashboard/UnsavedChangesModal.tsx b/public/app/features/dashboard/components/SaveDashboard/UnsavedChangesModal.tsx index b50b921ca3d..fa0eaa4cf19 100644 --- a/public/app/features/dashboard/components/SaveDashboard/UnsavedChangesModal.tsx +++ b/public/app/features/dashboard/components/SaveDashboard/UnsavedChangesModal.tsx @@ -32,13 +32,7 @@ export const UnsavedChangesModal: React.FC = ({ - diff --git a/public/app/features/dashboard/components/SaveDashboard/useDashboardSave.tsx b/public/app/features/dashboard/components/SaveDashboard/useDashboardSave.tsx index e0faa2f22fe..7b607eb8d85 100644 --- a/public/app/features/dashboard/components/SaveDashboard/useDashboardSave.tsx +++ b/public/app/features/dashboard/components/SaveDashboard/useDashboardSave.tsx @@ -31,12 +31,11 @@ export const useDashboardSave = (dashboard: DashboardModel) => { appEvents.publish(new DashboardSavedEvent()); appEvents.emit(AppEvents.alertSuccess, ['Dashboard saved']); - // Using global locationService because save modals are rendered as a separate React tree const currentPath = locationService.getLocation().pathname; const newUrl = locationUtil.stripBaseFromUrl(state.value.url); if (newUrl !== currentPath) { - locationService.replace(newUrl); + setTimeout(() => locationService.replace(newUrl)); } } }, [dashboard, state]); diff --git a/public/app/features/dashboard/containers/DashboardPage.tsx b/public/app/features/dashboard/containers/DashboardPage.tsx index b74e1bb86d6..11a401e63de 100644 --- a/public/app/features/dashboard/containers/DashboardPage.tsx +++ b/public/app/features/dashboard/containers/DashboardPage.tsx @@ -29,6 +29,7 @@ import { getKioskMode } from 'app/core/navigation/kiosk'; import { GrafanaTheme2, UrlQueryValue } from '@grafana/data'; import { DashboardLoading } from '../components/DashboardLoading/DashboardLoading'; import { DashboardFailed } from '../components/DashboardLoading/DashboardFailed'; +import { DashboardPrompt } from '../components/DashboardPrompt/DashboardPrompt'; export interface DashboardPageRouteParams { uid?: string; @@ -320,6 +321,8 @@ export class UnthemedDashboardPage extends PureComponent { )} + +
+
@@ -499,6 +606,113 @@ exports[`DashboardPage When dashboard has editview url state should render setti title="My dashboard" />
+
diff --git a/public/app/features/dashboard/services/ChangeTracker.ts b/public/app/features/dashboard/services/ChangeTracker.ts deleted file mode 100644 index f3e3fed8d75..00000000000 --- a/public/app/features/dashboard/services/ChangeTracker.ts +++ /dev/null @@ -1,161 +0,0 @@ -import { each, filter, find } from 'lodash'; -import { DashboardModel } from '../state/DashboardModel'; -import { contextSrv } from 'app/core/services/context_srv'; -import { appEvents } from 'app/core/app_events'; -import { UnsavedChangesModal } from '../components/SaveDashboard/UnsavedChangesModal'; -import { DashboardSavedEvent, ShowModalReactEvent } from '../../../types/events'; -import { locationService } from '@grafana/runtime'; -import angular from 'angular'; - -export class ChangeTracker { - init(dashboard: DashboardModel, originalCopyDelay: number) { - let original: object | null = null; - let originalPath = locationService.getLocation().pathname; - - // register events - const savedEventUnsub = appEvents.subscribe(DashboardSavedEvent, () => { - original = dashboard.getSaveModelClone(); - originalPath = locationService.getLocation().pathname; - }); - - if (originalCopyDelay && !dashboard.meta.fromExplore) { - setTimeout(() => { - // wait for different services to patch the dashboard (missing properties) - original = dashboard.getSaveModelClone(); - }, originalCopyDelay); - } else { - original = dashboard.getSaveModelClone(); - } - - const history = locationService.getHistory(); - - const blockUnsub = history.block((location) => { - if (originalPath === location.pathname) { - return; - } - - if (this.ignoreChanges(dashboard, original)) { - return; - } - - if (!this.hasChanges(dashboard, original!)) { - return; - } - - appEvents.publish( - new ShowModalReactEvent({ - component: UnsavedChangesModal, - props: { - dashboard: dashboard, - onSaveSuccess: () => { - original = dashboard.getSaveModelClone(); - history.push(location); - }, - onDiscard: () => { - original = dashboard.getSaveModelClone(); - history.push(location); - }, - }, - }) - ); - - return false; - }); - - const historyListenUnsub = history.listen((location) => { - if (originalPath !== location.pathname) { - blockUnsub(); - historyListenUnsub(); - savedEventUnsub.unsubscribe(); - } - }); - } - - // for some dashboards and users - // changes should be ignored - ignoreChanges(current: DashboardModel, original: object | null) { - if (!original) { - return true; - } - - // Ignore changes if the user has been signed out - if (!contextSrv.isSignedIn) { - return true; - } - - if (!current || !current.meta) { - return true; - } - - const { canSave, fromScript, fromFile } = current.meta; - if (!contextSrv.isEditor && !canSave) { - return true; - } - - return !canSave || fromScript || fromFile; - } - - // remove stuff that should not count in diff - cleanDashboardFromIgnoredChanges(dashData: any) { - // need to new up the domain model class to get access to expand / collapse row logic - const model = new DashboardModel(dashData); - - // Expand all rows before making comparison. This is required because row expand / collapse - // change order of panel array and panel positions. - model.expandRows(); - - const dash = model.getSaveModelClone(); - - // ignore time and refresh - dash.time = 0; - dash.refresh = 0; - dash.schemaVersion = 0; - dash.timezone = 0; - - // ignore iteration property - delete dash.iteration; - - dash.panels = filter(dash.panels, (panel) => { - if (panel.repeatPanelId) { - return false; - } - - // remove scopedVars - panel.scopedVars = undefined; - - // ignore panel legend sort - if (panel.legend) { - delete panel.legend.sort; - delete panel.legend.sortDesc; - } - - return true; - }); - - // ignore template variable values - each(dash.getVariables(), (variable: any) => { - variable.current = null; - variable.options = null; - variable.filters = null; - }); - - return dash; - } - - hasChanges(current: DashboardModel, original: any) { - const currentClean = this.cleanDashboardFromIgnoredChanges(current.getSaveModelClone()); - const originalClean = this.cleanDashboardFromIgnoredChanges(original); - - const currentTimepicker: any = find((currentClean as any).nav, { type: 'timepicker' }); - const originalTimepicker: any = find((originalClean as any).nav, { type: 'timepicker' }); - - if (currentTimepicker && originalTimepicker) { - currentTimepicker.now = originalTimepicker.now; - } - - const currentJson = angular.toJson(currentClean); - const originalJson = angular.toJson(originalClean); - - return currentJson !== originalJson; - } -} diff --git a/public/app/features/dashboard/services/__mocks__/ChangeTracker.ts b/public/app/features/dashboard/services/__mocks__/ChangeTracker.ts deleted file mode 100644 index 01ba3631311..00000000000 --- a/public/app/features/dashboard/services/__mocks__/ChangeTracker.ts +++ /dev/null @@ -1,9 +0,0 @@ -import { DashboardModel } from '../../state/DashboardModel'; - -export class ChangeTracker { - initCalled = false; - - init(dashboard: DashboardModel, originalCopyDelay: number) { - this.initCalled = true; - } -} diff --git a/public/app/features/dashboard/state/DashboardModel.ts b/public/app/features/dashboard/state/DashboardModel.ts index 9f9205d430a..b359b21dcbb 100644 --- a/public/app/features/dashboard/state/DashboardModel.ts +++ b/public/app/features/dashboard/state/DashboardModel.ts @@ -374,10 +374,10 @@ export class DashboardModel { } exitPanelEditor() { - getTimeSrv().resumeAutoRefresh(); this.panelInEdit!.destroy(); this.panelInEdit = undefined; this.refreshIfChangeAffectsAllPanels(); + getTimeSrv().resumeAutoRefresh(); } setChangeAffectsAllPanels() { diff --git a/public/app/features/dashboard/state/initDashboard.test.ts b/public/app/features/dashboard/state/initDashboard.test.ts index 470c4ad491e..f08ed3ff1d4 100644 --- a/public/app/features/dashboard/state/initDashboard.test.ts +++ b/public/app/features/dashboard/state/initDashboard.test.ts @@ -38,7 +38,6 @@ jest.mock('app/core/services/context_srv', () => ({ user: { orgId: 1, orgName: 'TestOrg' }, }, })); -jest.mock('app/features/dashboard/services/ChangeTracker'); variableAdapters.register(createConstantVariableAdapter()); const mockStore = configureMockStore([thunk]); diff --git a/public/app/features/dashboard/state/initDashboard.ts b/public/app/features/dashboard/state/initDashboard.ts index 03419a2fa1c..6075f4d1bda 100644 --- a/public/app/features/dashboard/state/initDashboard.ts +++ b/public/app/features/dashboard/state/initDashboard.ts @@ -23,7 +23,6 @@ import { initVariablesTransaction } from '../../variables/state/actions'; import { emitDashboardViewEvent } from './analyticsProcessor'; import { dashboardWatcher } from 'app/features/live/dashboard/dashboardWatcher'; import { locationService } from '@grafana/runtime'; -import { ChangeTracker } from '../services/ChangeTracker'; import { createDashboardQueryRunner } from '../../query/state/DashboardQueryRunner/DashboardQueryRunner'; export interface InitDashboardArgs { @@ -174,7 +173,6 @@ export function initDashboard(args: InitDashboardArgs): ThunkResult { // init services const timeSrv: TimeSrv = getTimeSrv(); const dashboardSrv: DashboardSrv = getDashboardSrv(); - const changeTracker = new ChangeTracker(); timeSrv.init(dashboard); const runner = createDashboardQueryRunner({ dashboard, timeSrv }); @@ -208,7 +206,6 @@ export function initDashboard(args: InitDashboardArgs): ThunkResult { dashboard.autoFitPanels(window.innerHeight, queryParams.kiosk); } - changeTracker.init(dashboard, 2000); keybindingSrv.setupDashboardBindings(dashboard); } catch (err) { dispatch(notifyApp(createErrorNotification('Dashboard init failed', err))); diff --git a/public/app/features/library-panels/components/SaveLibraryPanelModal/SaveLibraryPanelModal.tsx b/public/app/features/library-panels/components/SaveLibraryPanelModal/SaveLibraryPanelModal.tsx index 6840f948983..6829868aea2 100644 --- a/public/app/features/library-panels/components/SaveLibraryPanelModal/SaveLibraryPanelModal.tsx +++ b/public/app/features/library-panels/components/SaveLibraryPanelModal/SaveLibraryPanelModal.tsx @@ -9,7 +9,7 @@ import { getModalStyles } from '../../styles'; interface Props { panel: PanelModelWithLibraryPanel; folderId: number; - isOpen: boolean; + isUnsavedPrompt?: boolean; onConfirm: () => void; onDismiss: () => void; onDiscard: () => void; @@ -18,7 +18,7 @@ interface Props { export const SaveLibraryPanelModal: React.FC = ({ panel, folderId, - isOpen, + isUnsavedPrompt, onDismiss, onConfirm, onDiscard, @@ -52,11 +52,12 @@ export const SaveLibraryPanelModal: React.FC = ({ const styles = useStyles(getModalStyles); const discardAndClose = useCallback(() => { onDiscard(); - onDismiss(); - }, [onDiscard, onDismiss]); + }, [onDiscard]); + + const title = isUnsavedPrompt ? 'Unsaved library panel changes' : 'Save library panel'; return ( - +

{'This update will affect '} @@ -95,14 +96,15 @@ export const SaveLibraryPanelModal: React.FC = ({ - + {isUnsavedPrompt && ( + + )}