Replace `dashboard.isEditing` state with `dashboard.editingDashboard` (#39472) (#39507)

* Use `null` instead of `false` for `setEditingDashboard`

* Replace `isEditing` with `editingDashboard`

* Extract `EditableActionButtonAttrs`

Co-authored-by: Anton Kulyk <kuliks.anton@gmail.com>

frontend/src/metabase-types/store/dashboard.ts

@@ -97,7 +97,7 @@ export interface DashboardState {
     showLoadCompleteFavicon?: boolean;
   };
 
-  isEditing: Dashboard | null;
+  editingDashboard: Dashboard | null;
   isAddParameterPopoverOpen: boolean;
   isNavigatingBackToDashboard: boolean;
 

frontend/src/metabase-types/store/mocks/dashboard.ts

@@ -16,7 +16,7 @@ export const createMockDashboardState = (
     endTime: null,
   },
   loadingControls: {},
-  isEditing: null,
+  editingDashboard: null,
   isAddParameterPopoverOpen: false,
   isNavigatingBackToDashboard: false,
   slowCards: {},

frontend/src/metabase/dashboard/actions/actions.ts

@@ -19,12 +19,14 @@ import { getDashboardType } from "../utils";
 import { setDashCardAttributes } from "./core";
 import { closeSidebar, setSidebar } from "./ui";
 
+type EditableActionButtonAttrs = Pick<
+  ActionDashboardCard,
+  "card_id" | "action" | "parameter_mappings" | "visualization_settings"
+>;
+
 export function updateButtonActionMapping(
   dashCardId: number,
-  attributes: Pick<
-    ActionDashboardCard,
-    "card_id" | "action" | "parameter_mappings" | "visualization_settings"
-  >,
+  attributes: EditableActionButtonAttrs,
 ) {
   return (dispatch: Dispatch) => {
     dispatch(

frontend/src/metabase/dashboard/actions/actions.unit.spec.js

@@ -168,7 +168,7 @@ describe("dashboard actions", () => {
 
       const getState = () => ({
         dashboard: {
-          isEditing: dashboard,
+          editingDashboard: dashboard,
           dashboardId: 1,
           dashboards: {
             1: {

frontend/src/metabase/dashboard/actions/cards.unit.spec.ts

@@ -158,7 +158,7 @@ function setup({
     dashboards: {
       [dashboard.id]: { ...dashboard, dashcards: dashcards.map(dc => dc.id) },
     },
-    isEditing: DASHBOARD,
+    editingDashboard: DASHBOARD,
     dashcards: _.indexBy(dashcards, "id"),
   });
 

frontend/src/metabase/dashboard/actions/core.ts

@@ -14,7 +14,7 @@ export const RESET = "metabase/dashboard/RESET";
 export const reset = createAction(RESET);
 
 export const SET_EDITING_DASHBOARD = "metabase/dashboard/SET_EDITING_DASHBOARD";
-export const setEditingDashboard = createAction<Dashboard | false>(
+export const setEditingDashboard = createAction<Dashboard | null>(
   SET_EDITING_DASHBOARD,
 );
 

frontend/src/metabase/dashboard/actions/save.js

@@ -129,7 +129,7 @@ export const updateDashboardAndCards = createThunkAction(
         duration_milliseconds,
       });
 
-      dispatch(setEditingDashboard(false));
+      dispatch(setEditingDashboard(null));
 
       // make sure that we've fully cleared out any dirty state from editing (this is overkill, but simple)
       await dispatch(

frontend/src/metabase/dashboard/components/Dashboard/Dashboard.tsx

@@ -139,7 +139,7 @@ interface DashboardProps {
   archiveDashboard: (id: DashboardId) => Promise<void>;
 
   onRefreshPeriodChange: (period: number | null) => void;
-  setEditingDashboard: (dashboard: IDashboard | boolean) => void;
+  setEditingDashboard: (dashboard: IDashboard | null) => void;
   setDashboardAttributes: (opts: SetDashboardAttributesOpts) => void;
   setSharing: (isSharing: boolean) => void;
   toggleSidebar: (sidebarName: DashboardSidebarName) => void;
@@ -297,7 +297,7 @@ function DashboardInner(props: DashboardProps) {
   );
 
   const handleSetEditing = useCallback(
-    (dashboard: IDashboard | boolean) => {
+    (dashboard: IDashboard | null) => {
       onRefreshPeriodChange(null);
       setEditingDashboard(dashboard);
     },

frontend/src/metabase/dashboard/components/DashboardHeader/DashboardHeader.tsx

@@ -118,7 +118,7 @@ interface DashboardHeaderProps {
   showAddParameterPopover: () => void;
   hideAddParameterPopover: () => void;
 
-  onEditingChange: (arg: Dashboard | boolean) => void;
+  onEditingChange: (arg: Dashboard | null) => void;
   onRefreshPeriodChange: (period: number | null) => void;
   onFullscreenChange: (
     isFullscreen: boolean,
@@ -249,7 +249,7 @@ export const DashboardHeader = (props: DashboardHeaderProps) => {
   };
 
   const onDoneEditing = () => {
-    onEditingChange(false);
+    onEditingChange(null);
   };
 
   const onRevert = () => {

frontend/src/metabase/dashboard/constants.ts

@@ -16,7 +16,7 @@ export const SIDEBAR_NAME: Record<DashboardSidebarName, DashboardSidebarName> =
 export const INITIAL_DASHBOARD_STATE: DashboardState = {
   dashboardId: null,
   selectedTabId: null,
-  isEditing: null,
+  editingDashboard: null,
   dashboards: {},
   dashcards: {},
   dashcardData: {},

frontend/src/metabase/dashboard/reducers.js

@@ -66,15 +66,15 @@ const dashboardId = handleActions(
   INITIAL_DASHBOARD_STATE.dashboardId,
 );
 
-const isEditing = handleActions(
+const editingDashboard = handleActions(
   {
-    [INITIALIZE]: { next: state => null },
+    [INITIALIZE]: { next: () => INITIAL_DASHBOARD_STATE.editingDashboard },
     [SET_EDITING_DASHBOARD]: {
-      next: (state, { payload }) => (payload ? payload : null),
+      next: (state, { payload }) => payload ?? null,
     },
-    [RESET]: { next: state => null },
+    [RESET]: { next: () => INITIAL_DASHBOARD_STATE.editingDashboard },
   },
-  INITIAL_DASHBOARD_STATE.isEditing,
+  INITIAL_DASHBOARD_STATE.editingDashboard,
 );
 
 const loadingControls = handleActions(
@@ -519,7 +519,7 @@ export const dashboardReducers = reduceReducers(
   INITIAL_DASHBOARD_STATE,
   combineReducers({
     dashboardId,
-    isEditing,
+    editingDashboard,
     loadingControls,
     dashboards,
     dashcards,

frontend/src/metabase/dashboard/reducers.unit.spec.js

+import { createMockDashboard } from "metabase-types/api/mocks";
+
 import {
   INITIALIZE,
   SET_EDITING_DASHBOARD,
@@ -11,6 +13,8 @@ import {
 } from "./actions";
 import { dashboardReducers as reducer } from "./reducers";
 
+const TEST_DASHBOARD = createMockDashboard();
+
 describe("dashboard reducers", () => {
   let initState;
   beforeEach(() => {
@@ -26,7 +30,7 @@ describe("dashboard reducers", () => {
       dashcards: {},
       isAddParameterPopoverOpen: false,
       isNavigatingBackToDashboard: false,
-      isEditing: null,
+      editingDashboard: null,
       loadingDashCards: {
         loadingIds: [],
         startTime: null,
@@ -101,7 +105,7 @@ describe("dashboard reducers", () => {
             type: INITIALIZE,
           },
         ),
-      ).toEqual({ ...initState, isEditing: null });
+      ).toEqual({ ...initState, editingDashboard: null });
     });
 
     it("should return unchanged state if `clearCache: false` passed", () => {
@@ -154,9 +158,13 @@ describe("dashboard reducers", () => {
       expect(
         reducer(state, {
           type: SET_EDITING_DASHBOARD,
-          payload: true,
+          payload: TEST_DASHBOARD,
         }),
-      ).toEqual({ ...state, isEditing: true, sidebar: { props: {} } });
+      ).toEqual({
+        ...state,
+        editingDashboard: TEST_DASHBOARD,
+        sidebar: { props: {} },
+      });
     });
 
     it("should clear sidebar state when leaving edit mode", () => {
@@ -167,9 +175,9 @@ describe("dashboard reducers", () => {
       expect(
         reducer(state, {
           type: SET_EDITING_DASHBOARD,
-          payload: false,
+          payload: null,
         }),
-      ).toEqual({ ...initState, isEditing: null });
+      ).toEqual({ ...initState, editingDashboard: null });
     });
   });
 

frontend/src/metabase/dashboard/selectors.ts

@@ -46,7 +46,7 @@ function isEditParameterSidebar(
 const createDeepEqualSelector = createSelectorCreator(lruMemoize, _.isEqual);
 
 export const getDashboardBeforeEditing = (state: State) =>
-  state.dashboard.isEditing;
+  state.dashboard.editingDashboard;
 
 export const getIsEditing = (state: State) =>
   Boolean(getDashboardBeforeEditing(state));