From 13d8dabd068193a3f5e6c329f07d01e0ed6e9340 Mon Sep 17 00:00:00 2001
From: Alexander Polyankin <alexander.polyankin@metabase.com>
Date: Tue, 24 Sep 2024 11:45:10 -0500
Subject: [PATCH] Fix cancelling of editing of a metric (#48081)

---
 .../metrics/metrics-editing.cy.spec.js        | 33 +++++++++++++++++++
 .../src/metabase/query_builder/actions/ui.ts  |  3 +-
 .../DatasetEditor/DatasetEditor.jsx           |  3 ++
 .../query_builder/components/view/View.jsx    |  4 ++-
 4 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/e2e/test/scenarios/metrics/metrics-editing.cy.spec.js b/e2e/test/scenarios/metrics/metrics-editing.cy.spec.js
index 723a88dfe97..3afb0656655 100644
--- a/e2e/test/scenarios/metrics/metrics-editing.cy.spec.js
+++ b/e2e/test/scenarios/metrics/metrics-editing.cy.spec.js
@@ -164,6 +164,26 @@ describe("scenarios > metrics > editing", () => {
         verifyScalarValue("18,760");
       });
     });
+
+    it("should not crash when cancelling creation of a new metric (metabase#48024)", () => {
+      startNewMetric();
+      entityPickerModal().within(() => {
+        entityPickerModalTab("Tables").click();
+        cy.findByText("Orders").click();
+      });
+      cancelMetricEditing();
+    });
+
+    it("should not crash when cancelling editing of an existing metric (metabase#48024)", () => {
+      createQuestion(ORDERS_SCALAR_METRIC).then(({ body: card }) =>
+        visitMetric(card.id),
+      );
+      openQuestionActions();
+      popover().findByText("Edit metric definition").click();
+      addBreakout({ tableName: "Product", columnName: "Created At" });
+      cancelMetricEditing();
+      verifyScalarValue("18,760");
+    });
   });
 
   describe("data source", () => {
@@ -616,3 +636,16 @@ function verifyLineAreaBarChart({ xAxis, yAxis }) {
     cy.findByText(xAxis).should("be.visible");
   });
 }
+
+function cancelMetricEditing() {
+  cy.log("click cancel but do not confirm");
+  cy.button("Cancel").click();
+  modal().button("Cancel").click();
+  modal().should("not.exist");
+  appBar().should("not.exist");
+
+  cy.log("click cancel and confirm");
+  cy.button("Cancel").click();
+  modal().button("Discard changes").click();
+  appBar().should("be.visible");
+}
diff --git a/frontend/src/metabase/query_builder/actions/ui.ts b/frontend/src/metabase/query_builder/actions/ui.ts
index 70812b6cee7..c955b47fefd 100644
--- a/frontend/src/metabase/query_builder/actions/ui.ts
+++ b/frontend/src/metabase/query_builder/actions/ui.ts
@@ -12,7 +12,7 @@ import type {
 } from "metabase-types/store";
 
 import { updateUrl } from "./navigation";
-import { cancelQuery, runDirtyQuestionQuery } from "./querying";
+import { cancelQuery } from "./querying";
 
 export const SET_UI_CONTROLS = "metabase/qb/SET_UI_CONTROLS";
 export const setUIControls = createAction(SET_UI_CONTROLS);
@@ -121,5 +121,4 @@ export const cancelQuestionChanges =
       type: CANCEL_QUESTION_CHANGES,
       payload: { card: cardBeforeChanges },
     });
-    dispatch(runDirtyQuestionQuery());
   };
diff --git a/frontend/src/metabase/query_builder/components/DatasetEditor/DatasetEditor.jsx b/frontend/src/metabase/query_builder/components/DatasetEditor/DatasetEditor.jsx
index a858480caf7..650fcc225be 100644
--- a/frontend/src/metabase/query_builder/components/DatasetEditor/DatasetEditor.jsx
+++ b/frontend/src/metabase/query_builder/components/DatasetEditor/DatasetEditor.jsx
@@ -71,6 +71,7 @@ const propTypes = {
   isResultDirty: PropTypes.bool.isRequired,
   isRunning: PropTypes.bool.isRequired,
   setQueryBuilderMode: PropTypes.func.isRequired,
+  runDirtyQuestionQuery: PropTypes.func.isRequired,
   setDatasetEditorTab: PropTypes.func.isRequired,
   setMetadataDiff: PropTypes.func.isRequired,
   onSave: PropTypes.func.isRequired,
@@ -201,6 +202,7 @@ function DatasetEditor(props) {
     isDirty: isModelQueryDirty,
     isResultDirty,
     setQueryBuilderMode,
+    runDirtyQuestionQuery,
     runQuestionQuery,
     setDatasetEditorTab,
     setMetadataDiff,
@@ -327,6 +329,7 @@ function DatasetEditor(props) {
   const handleCancelEdit = () => {
     setShowCancelEditWarning(false);
     cancelQuestionChanges();
+    runDirtyQuestionQuery();
     setQueryBuilderMode("view");
   };
 
diff --git a/frontend/src/metabase/query_builder/components/view/View.jsx b/frontend/src/metabase/query_builder/components/view/View.jsx
index 852df46ba2f..1383b0fe6ee 100644
--- a/frontend/src/metabase/query_builder/components/view/View.jsx
+++ b/frontend/src/metabase/query_builder/components/view/View.jsx
@@ -414,6 +414,7 @@ class View extends Component {
       runQuestionQuery,
       cancelQuery,
       setQueryBuilderMode,
+      runDirtyQuestionQuery,
       isShowingQuestionInfoSidebar,
       isShowingQuestionSettingsSidebar,
       cancelQuestionChanges,
@@ -457,8 +458,9 @@ class View extends Component {
                 setQueryBuilderMode("view");
               }}
               onCancel={question => {
-                cancelQuestionChanges();
                 if (question.isSaved()) {
+                  cancelQuestionChanges();
+                  runDirtyQuestionQuery();
                   setQueryBuilderMode("view");
                 } else {
                   onChangeLocation("/");
-- 
GitLab