Migrate updateQuestion to MLv2 (#37499)

e2e/test/scenarios/visualizations-charts/reproductions/21452-xhr-on-every-char-for-rename.cy.spec.js

@@ -36,8 +36,6 @@ describe("issue 21452", () => {
     cy.findByDisplayValue("Cumulative sum of Quantity").clear().type("Foo");
     // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
     cy.findByText("Display type").click();
-    // Blur will result in another POST request which is expected
-    cy.wait("@dataset");
     // Dismiss the popup and close settings
     // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
     cy.findByText("Done").click();
@@ -49,7 +47,7 @@ describe("issue 21452", () => {
       testPairedTooltipValues("Foo", "3,236");
     });
 
-    cy.get("@dataset.all").should("have.length", 2);
+    cy.get("@dataset.all").should("have.length", 1);
   });
 });
 

e2e/test/scenarios/visualizations-tabular/table-column-settings.cy.spec.js

@@ -283,7 +283,6 @@ describe("scenarios > visualizations > table column settings", () => {
       .findByLabelText(column)
       .should("be.checked")
       .click();
-    runQuery();
     cy.wait("@dataset");
     cy.findByText("Doing science...").should("not.exist");
     if (needsScroll) {
@@ -395,7 +394,6 @@ describe("scenarios > visualizations > table column settings", () => {
         .findByLabelText("Remove all")
         .click();
 
-      runQuery();
       cy.wait("@dataset");
       cy.findByTestId("query-builder-main")
         .findByText("Doing science...")
@@ -809,10 +807,6 @@ describe("scenarios > visualizations > table column settings", () => {
   });
 });
 
-const runQuery = () => {
-  cy.findByTestId("query-builder-main").icon("play").click();
-};
-
 const showColumn = column => {
   cy.findByTestId(`${column}-show-button`).click();
 };

frontend/src/metabase/query_builder/actions/core/updateQuestion.ts

@@ -3,15 +3,15 @@ import { assocIn } from "icepick";
 
 import { loadMetadataForCard } from "metabase/questions/actions";
 
-import type { Dataset, Series } from "metabase-types/api";
+import type { Series } from "metabase-types/api";
 import type {
   Dispatch,
   GetState,
   QueryBuilderMode,
 } from "metabase-types/store";
+import * as Lib from "metabase-lib";
 import type Question from "metabase-lib/Question";
 import type NativeQuery from "metabase-lib/queries/NativeQuery";
-import StructuredQuery from "metabase-lib/queries/StructuredQuery";
 import { getTemplateTagParametersFromCard } from "metabase-lib/parameters/utils/template-tags";
 
 import {
@@ -29,17 +29,6 @@ import { onCloseQuestionInfo, setQueryBuilderMode } from "../ui";
 
 import { getQuestionWithDefaultVisualizationSettings } from "./utils";
 
-function hasNewColumns(question: Question, queryResult: Dataset) {
-  // NOTE: this assume column names will change
-  // technically this is wrong because you could add and remove two columns with the same name
-  const query = question.legacyQuery({ useStructuredQuery: true });
-  const previousColumns =
-    (queryResult && queryResult.data.cols.map(col => col.name)) || [];
-  const nextColumns =
-    query instanceof StructuredQuery ? query.columnNames() : [];
-  return _.difference(nextColumns, previousColumns).length > 0;
-}
-
 function checkShouldRerunPivotTableQuestion({
   isPivot,
   wasPivot,
@@ -101,7 +90,7 @@ function shouldTemplateTagEditorBeVisible({
 }
 
 export type UpdateQuestionOpts = {
-  run?: boolean | "auto";
+  run?: boolean;
   shouldUpdateUrl?: boolean;
   shouldStartAdHocQuestion?: boolean;
 };
@@ -152,10 +141,6 @@ export const updateQuestion = (
       queryResult,
     );
 
-    if (run === "auto") {
-      run = hasNewColumns(newQuestion, queryResult);
-    }
-
     if (!newQuestion.canAutoRun()) {
       run = false;
     }
@@ -166,11 +151,7 @@ export const updateQuestion = (
     if (wasPivot || isPivot) {
       const hasBreakouts =
         newQuestion.isStructured() &&
-        (
-          newQuestion.legacyQuery({
-            useStructuredQuery: true,
-          }) as StructuredQuery
-        ).hasBreakouts();
+        Lib.breakouts(newQuestion.query(), -1).length > 0;
 
       // compute the pivot setting now so we can query the appropriate data
       if (isPivot && hasBreakouts) {

frontend/src/metabase/query_builder/actions/visualization-settings.js

@@ -34,22 +34,21 @@ export const updateCardVisualizationSettings =
     const hasWritePermissions = question.isQueryEditable();
     await dispatch(
       updateQuestion(question.updateSettings(settings), {
-        run: hasWritePermissions ? "auto" : false,
         shouldUpdateUrl: hasWritePermissions,
       }),
     );
   };
 
 export const replaceAllCardVisualizationSettings =
-  (settings, question) => async (dispatch, getState) => {
-    question = question ?? getQuestion(getState());
-    question = question.setSettings(settings);
+  (settings, newQuestion) => async (dispatch, getState) => {
+    const oldQuestion = getQuestion(getState());
+    const updatedQuestion = (newQuestion ?? oldQuestion).setSettings(settings);
+    const hasWritePermissions = updatedQuestion.isQueryEditable();
 
-    // The check allows users without data permission to resize/rearrange columns
-    const hasWritePermissions = question.isQueryEditable();
     await dispatch(
-      updateQuestion(question, {
-        run: hasWritePermissions ? "auto" : false,
+      updateQuestion(updatedQuestion, {
+        // rerun the query when it is changed alongside settings
+        run: newQuestion != null && hasWritePermissions,
         shouldUpdateUrl: hasWritePermissions,
       }),
     );