From e7988dbb5adacfcb6bad862018485067672a11d7 Mon Sep 17 00:00:00 2001 From: "Grot (@grafanabot)" <43478413+grafanabot@users.noreply.github.com> Date: Tue, 6 Sep 2022 20:05:18 +0200 Subject: [PATCH] Dashboard: Unsaved changes warning fixes (#54706) (#54803) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Quick test of alternative method * change order of if condition again for better readability * Think I have something that is working * Update (cherry picked from commit e5c32c8cc9ef18041dd25df0d3b5db860128aefd) Co-authored-by: Torkel Ödegaard --- .../components/PanelEditor/PanelEditor.tsx | 24 +----------- .../PanelEditor/state/actions.test.ts | 39 +++++++++++++++++++ .../components/PanelEditor/state/actions.ts | 14 +++++-- .../dashboard/state/DashboardModel.ts | 6 +++ .../features/dashboard/state/PanelModel.ts | 4 +- public/app/features/library-panels/utils.ts | 1 + public/app/features/panel/state/actions.ts | 1 + 7 files changed, 61 insertions(+), 28 deletions(-) diff --git a/public/app/features/dashboard/components/PanelEditor/PanelEditor.tsx b/public/app/features/dashboard/components/PanelEditor/PanelEditor.tsx index 7663394bb07..cb392911a76 100644 --- a/public/app/features/dashboard/components/PanelEditor/PanelEditor.tsx +++ b/public/app/features/dashboard/components/PanelEditor/PanelEditor.tsx @@ -6,7 +6,7 @@ import { Subscription } from 'rxjs'; import { FieldConfigSource, GrafanaTheme2 } from '@grafana/data'; import { selectors } from '@grafana/e2e-selectors'; -import { isFetchError, locationService } from '@grafana/runtime'; +import { locationService } from '@grafana/runtime'; import { HorizontalGroup, InlineSwitch, @@ -31,12 +31,6 @@ import { PanelOptionsChangedEvent, ShowModalReactEvent } from 'app/types/events' import { notifyApp } from '../../../../core/actions'; import { UnlinkModal } from '../../../library-panels/components/UnlinkModal/UnlinkModal'; import { isPanelModelLibraryPanel } from '../../../library-panels/guard'; -import { getLibraryPanelConnectedDashboards } from '../../../library-panels/state/api'; -import { - createPanelLibraryErrorNotification, - createPanelLibrarySuccessNotification, - saveAndRefreshLibraryPanel, -} from '../../../library-panels/utils'; import { getVariablesByKey } from '../../../variables/state/selectors'; import { DashboardPanel } from '../../dashgrid/DashboardPanel'; import { DashboardModel, PanelModel } from '../../state'; @@ -154,22 +148,6 @@ export class PanelEditorUnconnected extends PureComponent { return; } - const connectedDashboards = await getLibraryPanelConnectedDashboards(this.props.panel.libraryPanel.uid); - if ( - connectedDashboards.length === 0 || - (connectedDashboards.length === 1 && connectedDashboards.includes(this.props.dashboard.id)) - ) { - try { - await saveAndRefreshLibraryPanel(this.props.panel, this.props.dashboard.meta.folderId!); - this.props.notifyApp(createPanelLibrarySuccessNotification('Library panel saved')); - } catch (err) { - if (isFetchError(err)) { - this.props.notifyApp(createPanelLibraryErrorNotification(`Error saving library panel: "${err.statusText}"`)); - } - } - return; - } - this.setState({ showSaveLibraryPanelModal: true }); }; diff --git a/public/app/features/dashboard/components/PanelEditor/state/actions.test.ts b/public/app/features/dashboard/components/PanelEditor/state/actions.test.ts index 64881c59fa4..13e5d7f78fe 100644 --- a/public/app/features/dashboard/components/PanelEditor/state/actions.test.ts +++ b/public/app/features/dashboard/components/PanelEditor/state/actions.test.ts @@ -165,6 +165,45 @@ describe('panelEditor actions', () => { expect(sourcePanel.configRev).toEqual(0); }); + it('should apply changes when dashboard was saved from panel edit', async () => { + const sourcePanel = new PanelModel({ id: 12, type: 'graph' }); + sourcePanel.plugin = getPanelPlugin({}); + sourcePanel.plugin.angularPanelCtrl = undefined; + + const dashboard = new DashboardModel({ + panels: [{ id: 12, type: 'graph' }], + }); + + const panel = dashboard.initEditPanel(sourcePanel); + + const state: PanelEditorState = { + ...initialState(), + getPanel: () => panel, + getSourcePanel: () => sourcePanel, + }; + + panel.setProperty('title', 'new title'); + panel.configRev = 0; + panel.hasSavedPanelEditChange = true; + + await thunkTester({ + panelEditor: state, + panels: {}, + dashboard: { + getModel: () => dashboard, + }, + }) + .givenThunk(exitPanelEditor) + .whenThunkIsDispatched(); + + expect(sourcePanel.configRev).toEqual(1); + + await new Promise((r) => setTimeout(r, 30)); + + // expect configRev to be reset to 0 as it was saved + expect(sourcePanel.hasChanged).toEqual(false); + }); + it('should apply changes when leaving panel edit with angular panel', async () => { const sourcePanel = new PanelModel({ id: 12, type: 'graph' }); sourcePanel.plugin = getPanelPlugin({}); diff --git a/public/app/features/dashboard/components/PanelEditor/state/actions.ts b/public/app/features/dashboard/components/PanelEditor/state/actions.ts index 3290ce66189..320f78de20d 100644 --- a/public/app/features/dashboard/components/PanelEditor/state/actions.ts +++ b/public/app/features/dashboard/components/PanelEditor/state/actions.ts @@ -116,10 +116,7 @@ export function exitPanelEditor(): ThunkResult { dashboard.exitPanelEditor(); } - // For angular panels we always commit as panel.hasChanged will not have picked up changes done from angular - const commitChanges = !shouldDiscardChanges && (panel.hasChanged || panel.isAngularPlugin()); - - if (commitChanges) { + if (hasPanelChangedInPanelEdit(panel) && !shouldDiscardChanges) { const modifiedSaveModel = panel.getSaveModel(); const sourcePanel = getSourcePanel(); const panelTypeChanged = sourcePanel.type !== panel.type; @@ -143,6 +140,11 @@ export function exitPanelEditor(): ThunkResult { setTimeout(() => { sourcePanel.getQueryRunner().useLastResultFrom(panel.getQueryRunner()); sourcePanel.render(); + + // If all changes where saved then reset configRev after applying changes + if (panel.hasSavedPanelEditChange && !panel.hasChanged) { + sourcePanel.configRev = 0; + } }, 20); } @@ -151,6 +153,10 @@ export function exitPanelEditor(): ThunkResult { }; } +function hasPanelChangedInPanelEdit(panel: PanelModel) { + return panel.hasChanged || panel.hasSavedPanelEditChange || panel.isAngularPlugin(); +} + export function updatePanelEditorUIState(uiState: Partial): ThunkResult { return (dispatch, getStore) => { const nextState = { ...getStore().panelEditor.ui, ...uiState }; diff --git a/public/app/features/dashboard/state/DashboardModel.ts b/public/app/features/dashboard/state/DashboardModel.ts index 23362488653..0e397e28d3b 100644 --- a/public/app/features/dashboard/state/DashboardModel.ts +++ b/public/app/features/dashboard/state/DashboardModel.ts @@ -494,6 +494,12 @@ export class DashboardModel implements TimeModel { for (const panel of this.panels) { panel.configRev = 0; } + + if (this.panelInEdit) { + // Remember that we have a saved a change in panel editor so we apply it when leaving panel edit + this.panelInEdit.hasSavedPanelEditChange = this.panelInEdit.configRev > 0; + this.panelInEdit.configRev = 0; + } } hasUnsavedChanges() { diff --git a/public/app/features/dashboard/state/PanelModel.ts b/public/app/features/dashboard/state/PanelModel.ts index b2d949cee4e..e0904edb409 100644 --- a/public/app/features/dashboard/state/PanelModel.ts +++ b/public/app/features/dashboard/state/PanelModel.ts @@ -61,6 +61,7 @@ const notPersistedProperties: { [str: string]: boolean } = { queryRunner: true, replaceVariables: true, configRev: true, + hasSavedPanelEditChange: true, getDisplayTitle: true, dataSupport: true, key: true, @@ -161,7 +162,7 @@ export class PanelModel implements DataConfigSource, IPanelModel { links?: DataLink[]; declare transparent: boolean; - libraryPanel?: { uid: undefined; name: string } | PanelModelLibraryPanel; + libraryPanel?: { uid: undefined; name: string; version?: number } | PanelModelLibraryPanel; autoMigrateFrom?: string; @@ -170,6 +171,7 @@ export class PanelModel implements DataConfigSource, IPanelModel { isEditing = false; isInView = false; configRev = 0; // increments when configs change + hasSavedPanelEditChange?: boolean; hasRefreshed?: boolean; cacheTimeout?: string | null; cachedPluginOptions: Record = {}; diff --git a/public/app/features/library-panels/utils.ts b/public/app/features/library-panels/utils.ts index c9e2b8a9bc6..40428ffec0f 100644 --- a/public/app/features/library-panels/utils.ts +++ b/public/app/features/library-panels/utils.ts @@ -45,6 +45,7 @@ function updatePanelModelWithUpdate(panel: PanelModel, updated: LibraryElementDT libraryPanel: toPanelModelLibraryPanel(updated), title: panel.title, }); + panel.hasSavedPanelEditChange = true; panel.refresh(); } diff --git a/public/app/features/panel/state/actions.ts b/public/app/features/panel/state/actions.ts index 1bfb01dac2b..ebb5792f363 100644 --- a/public/app/features/panel/state/actions.ts +++ b/public/app/features/panel/state/actions.ts @@ -148,6 +148,7 @@ export function changeToLibraryPanel(panel: PanelModel, libraryPanel: LibraryEle } panel.configRev = 0; + panel.hasSavedPanelEditChange = true; panel.refresh(); panel.events.publish(PanelQueriesChangedEvent);