Dashboard: Refactor dashboard reducer & actions (#22021)

* Dashboard: Refactor dashboard reducer & actions

* Dashboard: minor refactoring

* Minor cleanup

public/app/features/dashboard/containers/DashboardPage.test.tsx

@@ -2,13 +2,13 @@ import React from 'react';
 import { shallow, ShallowWrapper } from 'enzyme';
 import { DashboardPage, mapStateToProps, Props, State } from './DashboardPage';
 import { DashboardModel } from '../state';
-import { cleanUpDashboard } from '../state/actions';
+import { cleanUpDashboard } from '../state/reducers';
 import {
   mockToolkitActionCreator,
   mockToolkitActionCreatorWithoutPayload,
   ToolkitActionCreatorWithoutPayloadMockType,
 } from 'test/core/redux/mocks';
-import { DashboardInitPhase, DashboardRouteInfo } from 'app/types';
+import { DashboardInitPhase, DashboardRouteInfo, MutableDashboard } from 'app/types';
 import { notifyApp, updateLocation } from 'app/core/actions';
 
 jest.mock('app/features/dashboard/components/DashboardSettings/SettingsCtrl', () => ({}));
@@ -271,7 +271,9 @@ describe('DashboardPage', () => {
           edit: false,
         },
       },
-      dashboard: {},
+      dashboard: {
+        getModel: () => null as MutableDashboard,
+      },
     } as any);
 
     expect(props.urlFullscreen).toBe(true);
@@ -287,7 +289,9 @@ describe('DashboardPage', () => {
           edit: 'true',
         },
       },
-      dashboard: {},
+      dashboard: {
+        getModel: () => null as MutableDashboard,
+      },
     } as any);
 
     expect(props.urlFullscreen).toBe(false);

public/app/features/dashboard/containers/DashboardPage.tsx

@@ -17,7 +17,7 @@ import { CustomScrollbar, Alert, Portal } from '@grafana/ui';
 
 // Redux
 import { initDashboard } from '../state/initDashboard';
-import { cleanUpDashboard } from '../state/actions';
+import { cleanUpDashboard } from '../state/reducers';
 import { notifyApp, updateLocation } from 'app/core/actions';
 // Types
 import {
@@ -349,7 +349,7 @@ export const mapStateToProps = (state: StoreState) => ({
   initPhase: state.dashboard.initPhase,
   isInitSlow: state.dashboard.isInitSlow,
   initError: state.dashboard.initError,
-  dashboard: state.dashboard.model as DashboardModel,
+  dashboard: state.dashboard.getModel() as DashboardModel,
   inspectTab: state.location.query.tab,
 });
 

public/app/features/dashboard/containers/SoloPanelPage.tsx

@@ -100,7 +100,7 @@ const mapStateToProps = (state: StoreState) => ({
   urlSlug: state.location.routeParams.slug,
   urlType: state.location.routeParams.type,
   urlPanelId: state.location.query.panelId,
-  dashboard: state.dashboard.model as DashboardModel,
+  dashboard: state.dashboard.getModel() as DashboardModel,
 });
 
 const mapDispatchToProps = {

public/app/features/dashboard/state/actions.ts

@@ -1,42 +1,12 @@
 // Services & Utils
-import { createAction } from '@reduxjs/toolkit';
 import { getBackendSrv } from '@grafana/runtime';
 import { createSuccessNotification } from 'app/core/copy/appNotification';
 // Actions
 import { loadPluginDashboards } from '../../plugins/state/actions';
+import { loadDashboardPermissions } from './reducers';
 import { notifyApp } from 'app/core/actions';
 // Types
-import {
+import { DashboardAcl, DashboardAclUpdateDTO, NewDashboardAclItem, PermissionLevel, ThunkResult } from 'app/types';
-  DashboardAcl,
-  DashboardAclDTO,
-  DashboardAclUpdateDTO,
-  DashboardInitError,
-  MutableDashboard,
-  NewDashboardAclItem,
-  PermissionLevel,
-  ThunkResult,
-} from 'app/types';
-import { DataQuery } from '@grafana/data';
-
-export const loadDashboardPermissions = createAction<DashboardAclDTO[]>('dashboard/loadDashboardPermissions');
-
-export const dashboardInitFetching = createAction('dashboard/dashboardInitFetching');
-
-export const dashboardInitServices = createAction('dashboard/dashboardInitServices');
-
-export const dashboardInitSlow = createAction('dashboard/dashboardInitSlow');
-
-export const dashboardInitCompleted = createAction<MutableDashboard>('dashboard/dashboardInitCompleted');
-
-/*
- * Unrecoverable init failure (fetch or model creation failed)
- */
-export const dashboardInitFailed = createAction<DashboardInitError>('dashboard/dashboardInitFailed');
-
-/*
- * When leaving dashboard, resets state
- * */
-export const cleanUpDashboard = createAction('dashboard/cleanUpDashboard');
 
 export function getDashboardPermissions(id: number): ThunkResult<void> {
   return async dispatch => {
@@ -54,21 +24,6 @@ function toUpdateItem(item: DashboardAcl): DashboardAclUpdateDTO {
   };
 }
 
-interface SetDashboardQueriesToUpdatePayload {
-  panelId: number;
-  queries: DataQuery[];
-}
-
-export const clearDashboardQueriesToUpdate = createAction('dashboard/clearDashboardQueriesToUpdate');
-export const setDashboardQueriesToUpdate = createAction<SetDashboardQueriesToUpdatePayload>(
-  'dashboard/setDashboardQueriesToUpdate'
-);
-export const setDashboardQueriesToUpdateOnLoad = (panelId: number, queries: DataQuery[]): ThunkResult<void> => {
-  return async dispatch => {
-    await dispatch(setDashboardQueriesToUpdate({ panelId, queries }));
-  };
-};
-
 export function updateDashboardPermission(
   dashboardId: number,
   itemToUpdate: DashboardAcl,

public/app/features/dashboard/state/initDashboard.test.ts

@@ -3,7 +3,7 @@ import thunk from 'redux-thunk';
 import { initDashboard, InitDashboardArgs } from './initDashboard';
 import { DashboardRouteInfo } from 'app/types';
 import { getBackendSrv } from 'app/core/services/backend_srv';
-import { dashboardInitCompleted, dashboardInitFetching, dashboardInitServices } from './actions';
+import { dashboardInitCompleted, dashboardInitFetching, dashboardInitServices } from './reducers';
 import { updateLocation } from '../../../core/actions';
 
 jest.mock('app/core/services/backend_srv');
@@ -108,12 +108,7 @@ function describeInitScenario(description: string, scenarioFn: ScenarioFn) {
         location: {
           query: {},
         },
-        dashboard: {
+        dashboard: {},
-          modifiedQueries: {
-            panelId: undefined,
-            queries: undefined,
-          },
-        },
         user: {},
         explore: {
           left: {

public/app/features/dashboard/state/initDashboard.ts

@@ -18,8 +18,8 @@ import {
   dashboardInitFailed,
   dashboardInitSlow,
   dashboardInitServices,
-  clearDashboardQueriesToUpdate,
+  clearDashboardQueriesToUpdateOnLoad,
-} from './actions';
+} from './reducers';
 
 // Types
 import { DashboardRouteInfo, StoreState, ThunkDispatch, ThunkResult, DashboardDTO } from 'app/types';
@@ -130,7 +130,7 @@ export function initDashboard(args: InitDashboardArgs): ThunkResult<void> {
     // Detect slow loading / initializing and set state flag
     // This is in order to not show loading indication for fast loading dashboards as it creates blinking/flashing
     setTimeout(() => {
-      if (getState().dashboard.model === null) {
+      if (getState().dashboard.getModel() === null) {
         dispatch(dashboardInitSlow());
       }
     }, 500);
@@ -173,8 +173,10 @@ export function initDashboard(args: InitDashboardArgs): ThunkResult<void> {
     timeSrv.init(dashboard);
     annotationsSrv.init(dashboard);
 
-    const { panelId, queries } = storeState.dashboard.modifiedQueries;
+    if (storeState.dashboard.modifiedQueries) {
-    dashboard.meta.fromExplore = !!(panelId && queries);
+      const { panelId, queries } = storeState.dashboard.modifiedQueries;
+      dashboard.meta.fromExplore = !!(panelId && queries);
+    }
 
     // template values service needs to initialize completely before
     // the rest of the dashboard can load
@@ -203,7 +205,8 @@ export function initDashboard(args: InitDashboardArgs): ThunkResult<void> {
       console.log(err);
     }
 
-    if (dashboard.meta.fromExplore) {
+    if (storeState.dashboard.modifiedQueries) {
+      const { panelId, queries } = storeState.dashboard.modifiedQueries;
       updateQueriesWhenComingFromExplore(dispatch, dashboard, panelId, queries);
     }
 
@@ -255,5 +258,5 @@ function updateQueriesWhenComingFromExplore(
   }
 
   // Clear update state now that we're done
-  dispatch(clearDashboardQueriesToUpdate());
+  dispatch(clearDashboardQueriesToUpdateOnLoad());
 }

public/app/features/dashboard/state/reducers.test.ts

@@ -4,7 +4,7 @@ import {
   dashboardInitFetching,
   dashboardInitSlow,
   loadDashboardPermissions,
-} from './actions';
+} from './reducers';
 import { DashboardInitPhase, DashboardState, OrgRole, PermissionLevel } from 'app/types';
 import { dashboardReducer, initialState } from './reducers';
 import { DashboardModel } from './DashboardModel';
@@ -36,7 +36,7 @@ describe('dashboard reducer', () => {
     });
 
     it('should set model', async () => {
-      expect(state.model?.title).toBe('My dashboard');
+      expect(state.getModel()!.title).toBe('My dashboard');
     });
 
     it('should set reset isInitSlow', async () => {
@@ -53,7 +53,7 @@ describe('dashboard reducer', () => {
     });
 
     it('should set model', async () => {
-      expect(state.model?.title).toBe('Dashboard init failed');
+      expect(state.getModel()?.title).toBe('Dashboard init failed');
     });
 
     it('should set reset isInitSlow', async () => {

public/app/features/dashboard/state/reducers.ts

@@ -1,16 +1,12 @@
-import { Action } from 'redux';
+import { createSlice, PayloadAction } from '@reduxjs/toolkit';
-import { DashboardInitPhase, DashboardState } from 'app/types';
 import {
-  cleanUpDashboard,
+  DashboardInitPhase,
-  dashboardInitCompleted,
+  DashboardState,
-  dashboardInitFailed,
+  DashboardAclDTO,
-  dashboardInitFetching,
+  MutableDashboard,
-  dashboardInitServices,
+  DashboardInitError,
-  dashboardInitSlow,
+  QueriesToUpdateOnDashboardLoad,
-  loadDashboardPermissions,
+} from 'app/types';
-  setDashboardQueriesToUpdate,
-  clearDashboardQueriesToUpdate,
-} from './actions';
 import { processAclItems } from 'app/core/utils/acl';
 import { panelEditorReducer } from '../panel_editor/state/reducers';
 import { DashboardModel } from './DashboardModel';
@@ -18,105 +14,74 @@ import { DashboardModel } from './DashboardModel';
 export const initialState: DashboardState = {
   initPhase: DashboardInitPhase.NotStarted,
   isInitSlow: false,
-  model: null,
+  getModel: () => null,
   permissions: [],
-  modifiedQueries: {
+  modifiedQueries: null,
-    panelId: undefined,
+};
-    queries: undefined,
+
+const dashbardSlice = createSlice({
+  name: 'dashboard',
+  initialState,
+  reducers: {
+    loadDashboardPermissions: (state, action: PayloadAction<DashboardAclDTO[]>) => {
+      state.permissions = processAclItems(action.payload);
+    },
+    dashboardInitFetching: state => {
+      state.initPhase = DashboardInitPhase.Fetching;
+    },
+    dashboardInitServices: state => {
+      state.initPhase = DashboardInitPhase.Services;
+    },
+    dashboardInitSlow: state => {
+      state.isInitSlow = true;
+    },
+    dashboardInitCompleted: (state, action: PayloadAction<MutableDashboard>) => {
+      state.getModel = () => action.payload;
+      state.initPhase = DashboardInitPhase.Completed;
+      state.isInitSlow = false;
+    },
+    dashboardInitFailed: (state, action: PayloadAction<DashboardInitError>) => {
+      const failedDashboard = new DashboardModel(
+        { title: 'Dashboard init failed' },
+        { canSave: false, canEdit: false }
+      );
+
+      state.initPhase = DashboardInitPhase.Failed;
+      state.initError = action.payload;
+      state.getModel = () => failedDashboard;
+    },
+    cleanUpDashboard: state => {
+      if (state.getModel()) {
+        state.getModel().destroy();
+        state.getModel = () => null;
+      }
+
+      state.initPhase = DashboardInitPhase.NotStarted;
+      state.isInitSlow = false;
+      state.initError = null;
+    },
+    setDashboardQueriesToUpdateOnLoad: (state, action: PayloadAction<QueriesToUpdateOnDashboardLoad>) => {
+      state.modifiedQueries = action.payload;
+    },
+    clearDashboardQueriesToUpdateOnLoad: state => {
+      state.modifiedQueries = null;
+    },
   },
-};
+});
 
-// Redux Toolkit uses ImmerJs as part of their solution to ensure that state objects are not mutated.
+export const {
-// ImmerJs has an autoFreeze option that freezes objects from change which means this reducer can't be migrated to createSlice
+  loadDashboardPermissions,
-// because the state would become frozen and during run time we would get errors because Angular would try to mutate
+  dashboardInitFetching,
-// the frozen state.
+  dashboardInitFailed,
-// https://github.com/reduxjs/redux-toolkit/issues/242
+  dashboardInitSlow,
-export const dashboardReducer = (state: DashboardState = initialState, action: Action<unknown>): DashboardState => {
+  dashboardInitCompleted,
-  if (loadDashboardPermissions.match(action)) {
+  dashboardInitServices,
-    return {
+  cleanUpDashboard,
-      ...state,
+  setDashboardQueriesToUpdateOnLoad,
-      permissions: processAclItems(action.payload),
+  clearDashboardQueriesToUpdateOnLoad,
-    };
+} = dashbardSlice.actions;
-  }
 
-  if (dashboardInitFetching.match(action)) {
+export const dashboardReducer = dashbardSlice.reducer;
-    return {
-      ...state,
-      initPhase: DashboardInitPhase.Fetching,
-    };
-  }
-
-  if (dashboardInitServices.match(action)) {
-    return {
-      ...state,
-      initPhase: DashboardInitPhase.Services,
-    };
-  }
-
-  if (dashboardInitSlow.match(action)) {
-    return {
-      ...state,
-      isInitSlow: true,
-    };
-  }
-
-  if (dashboardInitFailed.match(action)) {
-    return {
-      ...state,
-      initPhase: DashboardInitPhase.Failed,
-      isInitSlow: false,
-      initError: action.payload,
-      model: new DashboardModel({ title: 'Dashboard init failed' }, { canSave: false, canEdit: false }),
-    };
-  }
-
-  if (dashboardInitCompleted.match(action)) {
-    return {
-      ...state,
-      initPhase: DashboardInitPhase.Completed,
-      model: action.payload,
-      isInitSlow: false,
-    };
-  }
-
-  if (cleanUpDashboard.match(action)) {
-    // Destroy current DashboardModel
-    // Very important as this removes all dashboard event listeners
-    state.model.destroy();
-
-    return {
-      ...state,
-      initPhase: DashboardInitPhase.NotStarted,
-      model: null,
-      isInitSlow: false,
-      initError: null,
-    };
-  }
-
-  if (setDashboardQueriesToUpdate.match(action)) {
-    const { panelId, queries } = action.payload;
-
-    return {
-      ...state,
-      modifiedQueries: {
-        panelId,
-        queries,
-      },
-    };
-  }
-
-  if (clearDashboardQueriesToUpdate.match(action)) {
-    return {
-      ...state,
-      modifiedQueries: {
-        panelId: undefined,
-        queries: undefined,
-      },
-    };
-  }
-
-  return state;
-};
 
 export default {
   dashboard: dashboardReducer,

public/app/features/explore/ExploreToolbar.tsx

@@ -31,7 +31,7 @@ import { ResponsiveButton } from './ResponsiveButton';
 import { RunButton } from './RunButton';
 import { LiveTailControls } from './useLiveTailControls';
 import { getExploreDatasources } from './state/selectors';
-import { setDashboardQueriesToUpdateOnLoad } from '../dashboard/state/actions';
+import { setDashboardQueriesToUpdateOnLoad } from '../dashboard/state/reducers';
 
 const getStyles = memoizeOne(() => {
   return {
@@ -120,7 +120,10 @@ export class UnConnectedExploreToolbar extends PureComponent<Props> {
     const titleSlug = kbn.slugifyForUrl(dash.title);
 
     if (withChanges) {
-      this.props.setDashboardQueriesToUpdateOnLoad(originPanelId, this.cleanQueries(queries));
+      this.props.setDashboardQueriesToUpdateOnLoad({
+        panelId: originPanelId,
+        queries: this.cleanQueries(queries),
+      });
     }
 
     const dashViewOptions = {

public/app/types/dashboard.ts

@@ -65,15 +65,18 @@ export interface DashboardInitError {
 
 export const KIOSK_MODE_TV = 'tv';
 export type KioskUrlValue = 'tv' | '1' | true;
+export type GetMutableDashboardModelFn = () => MutableDashboard | null;
+
+export interface QueriesToUpdateOnDashboardLoad {
+  panelId: number;
+  queries: DataQuery[];
+}
 
 export interface DashboardState {
-  model: MutableDashboard | null;
+  getModel: GetMutableDashboardModelFn;
   initPhase: DashboardInitPhase;
   isInitSlow: boolean;
   initError?: DashboardInitError;
   permissions: DashboardAcl[] | null;
-  modifiedQueries?: {
+  modifiedQueries: QueriesToUpdateOnDashboardLoad | null;
-    panelId: number;
-    queries: DataQuery[];
-  };
 }