From 1f4a04436c63c672bc7035349d3cfca6cdbb41b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Sat, 22 Feb 2020 21:08:42 +0100 Subject: [PATCH] NewPanelEditor: Fixed cleanup that could cause crash (#22384) * NewPanelEditor: Fixed cleanup that could cause crash * Fixed unit test --- public/app/core/constants.ts | 2 ++ .../AddPanelWidget/AddPanelWidget.test.tsx | 2 +- .../components/AddPanelWidget/AddPanelWidget.tsx | 6 +++--- .../components/PanelEditor/state/actions.test.ts | 6 ++++-- .../components/PanelEditor/state/actions.ts | 2 ++ .../components/PanelEditor/state/reducers.ts | 6 ++++-- .../app/features/dashboard/state/PanelModel.ts | 3 ++- public/app/features/dashboard/state/reducers.ts | 16 ++++++++++------ 8 files changed, 28 insertions(+), 15 deletions(-) diff --git a/public/app/core/constants.ts b/public/app/core/constants.ts index db2f796cf16..08b268c7fc1 100644 --- a/public/app/core/constants.ts +++ b/public/app/core/constants.ts @@ -10,3 +10,5 @@ export const MIN_PANEL_HEIGHT = GRID_CELL_HEIGHT * 3; export const LS_PANEL_COPY_KEY = 'panel-copy'; export const PANEL_BORDER = 2; + +export const EDIT_PANEL_ID = 23763571993; diff --git a/public/app/features/dashboard/components/AddPanelWidget/AddPanelWidget.test.tsx b/public/app/features/dashboard/components/AddPanelWidget/AddPanelWidget.test.tsx index 6f09d1faede..2d567f5f4da 100644 --- a/public/app/features/dashboard/components/AddPanelWidget/AddPanelWidget.test.tsx +++ b/public/app/features/dashboard/components/AddPanelWidget/AddPanelWidget.test.tsx @@ -7,7 +7,7 @@ const setup = (propOverrides?: object) => { const props: Props = { dashboard: {} as DashboardModel, panel: {} as PanelModel, - addPanelToDashboard: jest.fn() as any, + addPanel: jest.fn() as any, }; Object.assign(props, propOverrides); diff --git a/public/app/features/dashboard/components/AddPanelWidget/AddPanelWidget.tsx b/public/app/features/dashboard/components/AddPanelWidget/AddPanelWidget.tsx index b6057096dab..085ad5c6393 100644 --- a/public/app/features/dashboard/components/AddPanelWidget/AddPanelWidget.tsx +++ b/public/app/features/dashboard/components/AddPanelWidget/AddPanelWidget.tsx @@ -10,7 +10,7 @@ import store from 'app/core/store'; // Store import { store as reduxStore } from 'app/store/store'; import { updateLocation } from 'app/core/actions'; -import { addPanelToDashboard } from 'app/features/dashboard/state/reducers'; +import { addPanel } from 'app/features/dashboard/state/reducers'; // Types import { DashboardModel, PanelModel } from '../../state'; import { LS_PANEL_COPY_KEY } from 'app/core/constants'; @@ -23,7 +23,7 @@ export interface OwnProps { } export interface DispatchProps { - addPanelToDashboard: typeof addPanelToDashboard; + addPanel: typeof addPanel; } export type Props = OwnProps & DispatchProps; @@ -197,6 +197,6 @@ export class AddPanelWidgetUnconnected extends React.Component { } } -const mapDispatchToProps: MapDispatchToProps = { addPanelToDashboard }; +const mapDispatchToProps: MapDispatchToProps = { addPanel }; export const AddPanelWidget = connect(null, mapDispatchToProps)(AddPanelWidgetUnconnected); 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 b0c993e693d..390c698cf53 100644 --- a/public/app/features/dashboard/components/PanelEditor/state/actions.test.ts +++ b/public/app/features/dashboard/components/PanelEditor/state/actions.test.ts @@ -2,6 +2,7 @@ import { thunkTester } from '../../../../../../test/core/thunk/thunkTester'; import { initialState } from './reducers'; import { initPanelEditor, panelEditorCleanUp } from './actions'; import { PanelEditorStateNew, closeCompleted } from './reducers'; +import { cleanUpEditPanel } from '../../../state/reducers'; import { PanelModel, DashboardModel } from '../../../state'; describe('panelEditor actions', () => { @@ -51,8 +52,9 @@ describe('panelEditor actions', () => { .givenThunk(panelEditorCleanUp) .whenThunkIsDispatched(); - expect(dispatchedActions.length).toBe(1); - expect(dispatchedActions[0].type).toBe(closeCompleted.type); + expect(dispatchedActions.length).toBe(2); + expect(dispatchedActions[0].type).toBe(cleanUpEditPanel.type); + expect(dispatchedActions[1].type).toBe(closeCompleted.type); expect(sourcePanel.getOptions()).toEqual({ prop: true }); expect(sourcePanel.id).toEqual(12); }); diff --git a/public/app/features/dashboard/components/PanelEditor/state/actions.ts b/public/app/features/dashboard/components/PanelEditor/state/actions.ts index f8cb6bbc890..9d4a996749e 100644 --- a/public/app/features/dashboard/components/PanelEditor/state/actions.ts +++ b/public/app/features/dashboard/components/PanelEditor/state/actions.ts @@ -9,6 +9,7 @@ import { setPanelEditorUIState, PANEL_EDITOR_UI_STATE_STORAGE_KEY, } from './reducers'; +import { cleanUpEditPanel } from '../../../state/reducers'; import store from '../../../../../core/store'; export function initPanelEditor(sourcePanel: PanelModel, dashboard: DashboardModel): ThunkResult { @@ -50,6 +51,7 @@ export function panelEditorCleanUp(): ThunkResult { dashboard.exitPanelEditor(); querySubscription.unsubscribe(); + dispatch(cleanUpEditPanel()); dispatch(closeCompleted()); }; } diff --git a/public/app/features/dashboard/components/PanelEditor/state/reducers.ts b/public/app/features/dashboard/components/PanelEditor/state/reducers.ts index 2e889eeb212..72b6987b044 100644 --- a/public/app/features/dashboard/components/PanelEditor/state/reducers.ts +++ b/public/app/features/dashboard/components/PanelEditor/state/reducers.ts @@ -15,11 +15,13 @@ export const DEFAULT_PANEL_EDITOR_UI_STATE: PanelEditorUIState = { }; export interface PanelEditorUIState { + /* Visualization options pane visibility */ isPanelOptionsVisible: boolean; - // annotating as number or string since size can be expressed as static value or percentage + /* Pixels or percentage */ rightPaneSize: number | string; - // annotating as number or string since size can be expressed as static value or percentage + /* Pixels or percentage */ topPaneSize: number | string; + /* Visualization size mode */ mode: DisplayMode; } diff --git a/public/app/features/dashboard/state/PanelModel.ts b/public/app/features/dashboard/state/PanelModel.ts index 4b85ef3b83c..838fcde51e7 100644 --- a/public/app/features/dashboard/state/PanelModel.ts +++ b/public/app/features/dashboard/state/PanelModel.ts @@ -14,6 +14,7 @@ import { ScopedVars, } from '@grafana/data'; import { AngularComponent } from '@grafana/runtime'; +import { EDIT_PANEL_ID } from 'app/core/constants'; import config from 'app/core/config'; @@ -357,7 +358,7 @@ export class PanelModel { const sourceModel = this.getSaveModel(); // Temporary id for the clone, restored later in redux action when changes are saved - sourceModel.id = 23763571993; + sourceModel.id = EDIT_PANEL_ID; const clone = new PanelModel(sourceModel); const sourceQueryRunner = this.getQueryRunner(); diff --git a/public/app/features/dashboard/state/reducers.ts b/public/app/features/dashboard/state/reducers.ts index f48ed64af7c..ff1178d1657 100644 --- a/public/app/features/dashboard/state/reducers.ts +++ b/public/app/features/dashboard/state/reducers.ts @@ -7,6 +7,7 @@ import { PanelState, QueriesToUpdateOnDashboardLoad, } from 'app/types'; +import { EDIT_PANEL_ID } from 'app/core/constants'; import { processAclItems } from 'app/core/utils/acl'; import { panelEditorReducer } from '../panel_editor/state/reducers'; import { panelEditorReducerNew } from '../components/PanelEditor/state/reducers'; @@ -64,6 +65,7 @@ const dashbardSlice = createSlice({ state.getModel = () => null; } + state.panels = {}; state.initPhase = DashboardInitPhase.NotStarted; state.isInitSlow = false; state.initError = null; @@ -77,7 +79,12 @@ const dashbardSlice = createSlice({ panelModelAndPluginReady: (state: DashboardState, action: PayloadAction) => { updatePanelState(state, action.payload.panelId, { plugin: action.payload.plugin }); }, - addPanelToDashboard: (state, action: PayloadAction) => {}, + cleanUpEditPanel: (state, action: PayloadAction) => { + delete state.panels[EDIT_PANEL_ID]; + }, + addPanel: (state, action: PayloadAction) => { + state.panels[action.payload.id] = { pluginId: action.payload.type }; + }, }, }); @@ -94,10 +101,6 @@ export interface PanelModelAndPluginReadyPayload { plugin: PanelPlugin; } -export interface AddPanelPayload { - panel: PanelModel; -} - export const { loadDashboardPermissions, dashboardInitFetching, @@ -109,7 +112,8 @@ export const { setDashboardQueriesToUpdateOnLoad, clearDashboardQueriesToUpdateOnLoad, panelModelAndPluginReady, - addPanelToDashboard, + addPanel, + cleanUpEditPanel, } = dashbardSlice.actions; export const dashboardReducer = dashbardSlice.reducer;