Don't lock display for models (#33047)

* Reproduce #32963

* Don't lock display for models

e2e/test/scenarios/models/reproductions/32963-model-question-display-lock.cy.spec.js

+import {
+  getNotebookStep,
+  leftSidebar,
+  openNotebook,
+  popover,
+  restore,
+  rightSidebar,
+  visualize,
+} from "e2e/support/helpers";
+import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database";
+
+const { ORDERS_ID } = SAMPLE_DATABASE;
+
+describe("issue 32963", () => {
+  beforeEach(() => {
+    restore();
+    cy.signInAsAdmin();
+    cy.createQuestion(
+      {
+        name: "Orders Model",
+        dataset: true,
+        query: { "source-table": ORDERS_ID },
+      },
+      { visitQuestion: true },
+    );
+  });
+
+  it("should pick sensible display for model based questions (metabase#32963)", () => {
+    cy.findByTestId("qb-header").button("Summarize").click();
+
+    rightSidebar().within(() => {
+      cy.findByText("Created At").click();
+      cy.button("Done").click();
+    });
+    cy.wait("@dataset");
+    assertLineChart();
+
+    // Go back to the original model
+    cy.findByTestId("qb-header").findByText("Orders Model").click();
+    openNotebook();
+
+    cy.button("Summarize").click();
+    popover().findByText("Count of rows").click();
+    getNotebookStep("summarize")
+      .findByText("Pick a column to group by")
+      .click();
+    popover().findByText("Created At").click();
+    visualize();
+    assertLineChart();
+  });
+});
+
+function assertLineChart() {
+  cy.findByTestId("viz-type-button").click();
+  leftSidebar().within(() => {
+    cy.findByTestId("Line-container").should(
+      "have.attr",
+      "aria-selected",
+      "true",
+    );
+    cy.findByTestId("Table-container").should(
+      "have.attr",
+      "aria-selected",
+      "false",
+    );
+  });
+}

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

@@ -304,9 +304,11 @@ async function handleQBInit(
   const metadata = getMetadata(getState());
 
   let question = new Question(card, metadata);
+
   if (question.isSaved()) {
-    // Don't set viz automatically for saved questions
-    question = question.lockDisplay();
+    if (!question.isDataset()) {
+      question = question.lockDisplay();
+    }
 
     const currentUser = getUser(getState());
     if (currentUser?.is_qbnewb) {

frontend/src/metabase/query_builder/actions/core/initializeQB.unit.spec.ts

@@ -314,13 +314,6 @@ describe("QB Actions > initializeQB", () => {
       const { card, questionType } = testCase;
 
       describe(questionType, () => {
-        it("locks question display", async () => {
-          const { result } = await setup({
-            card: { ...card, displayIsLocked: true },
-          });
-          expect(result.card.displayIsLocked).toBe(true);
-        });
-
         it("fetches alerts", async () => {
           const fetchAlertsForQuestionSpy = jest.spyOn(
             alert,
@@ -387,6 +380,11 @@ describe("QB Actions > initializeQB", () => {
       const { card, questionType } = testCase;
 
       describe(questionType, () => {
+        it("locks question display", async () => {
+          const { result } = await setup({ card });
+          expect(result.card.displayIsLocked).toBe(true);
+        });
+
         it("throws not found error when opening question with /model URL", async () => {
           const { dispatch } = await setup({
             card: card,
@@ -536,6 +534,11 @@ describe("QB Actions > initializeQB", () => {
       const { card, questionType } = testCase;
 
       describe(questionType, () => {
+        it("doesn't lock display", async () => {
+          const { result } = await setup({ card });
+          expect(result.card.displayIsLocked).toBeFalsy();
+        });
+
         it("runs question query on /query route", async () => {
           const runQuestionQuerySpy = jest.spyOn(querying, "runQuestionQuery");
           const baseUrl = Urls.question(card);
@@ -547,6 +550,7 @@ describe("QB Actions > initializeQB", () => {
 
           expect(runQuestionQuerySpy).toHaveBeenCalledTimes(1);
         });
+
         it("runs question query on /metadata route", async () => {
           const runQuestionQuerySpy = jest.spyOn(querying, "runQuestionQuery");
           const baseUrl = Urls.question(card);