do not unlock question display if it was locked in unsensible visualization (#17860)

frontend/src/metabase-lib/lib/Question.js

@@ -304,10 +304,16 @@ export default class Question {
     return this._card && this._card.displayIsLocked;
   }
 
-  // If we're locked to a display that is no longer "sensible", unlock it.
-  maybeUnlockDisplay(sensibleDisplays): Question {
-    const locked =
-      this.displayIsLocked() && sensibleDisplays.includes(this.display());
+  // If we're locked to a display that is no longer "sensible", unlock it
+  // unless it was locked in unsensible
+  maybeUnlockDisplay(sensibleDisplays, previousSensibleDisplays): Question {
+    const wasSensible =
+      previousSensibleDisplays == null ||
+      previousSensibleDisplays.includes(this.display());
+    const isSensible = sensibleDisplays.includes(this.display());
+
+    const shouldUnlock = wasSensible && !isSensible;
+    const locked = this.displayIsLocked() && !shouldUnlock;
     return this.setDisplayIsLocked(locked);
   }
 

frontend/src/metabase/query_builder/actions.js

@@ -49,6 +49,7 @@ import {
   getNativeEditorCursorOffset,
   getNativeEditorSelectedText,
   getSnippetCollectionId,
+  getQueryResults,
 } from "./selectors";
 
 import { MetabaseApi, CardApi, UserApi } from "metabase/services";
@@ -1138,6 +1139,7 @@ export const QUERY_COMPLETED = "metabase/qb/QUERY_COMPLETED";
 export const queryCompleted = (question, queryResults) => {
   return async (dispatch, getState) => {
     const [{ data }] = queryResults;
+    const [{ data: prevData }] = getQueryResults(getState()) || [{}];
     const originalQuestion = getOriginalQuestion(getState());
     const dirty =
       !originalQuestion ||
@@ -1154,7 +1156,10 @@ export const queryCompleted = (question, queryResults) => {
       // Otherwise, trust that the question was saved with the correct display.
       question = question
         // if we are going to trigger autoselection logic, check if the locked display no longer is "sensible".
-        .maybeUnlockDisplay(getSensibleDisplays(data))
+        .maybeUnlockDisplay(
+          getSensibleDisplays(data),
+          prevData && getSensibleDisplays(prevData),
+        )
         .setDefaultDisplay()
         .switchTableScalar(data);
     }

frontend/test/metabase-lib/lib/Question.unit.spec.js

@@ -297,6 +297,30 @@ describe("Question", () => {
         expect(question.display()).toBe("scalar");
       });
     });
+
+    describe("maybeUnlockDisplay", () => {
+      it("should keep display locked when it was locked with unsensible display", () => {
+        const sensibleDisplays = ["table", "scalar"];
+        const previousSensibleDisplays = sensibleDisplays;
+        const question = new Question(orders_count_card, metadata)
+          .setDisplay("funnel")
+          .lockDisplay()
+          .maybeUnlockDisplay(sensibleDisplays, previousSensibleDisplays);
+
+        expect(question.displayIsLocked()).toBe(true);
+      });
+
+      it("should unlock display it was locked with sensible display which has become unsensible", () => {
+        const previousSensibleDisplays = ["funnel"];
+        const sensibleDisplays = ["table", "scalar"];
+        const question = new Question(orders_count_card, metadata)
+          .setDisplay("funnel")
+          .lockDisplay()
+          .maybeUnlockDisplay(sensibleDisplays, previousSensibleDisplays);
+
+        expect(question.displayIsLocked()).toBe(false);
+      });
+    });
   });
 
   // TODO: These are mode-dependent and should probably be tied to modes

frontend/test/metabase/scenarios/visualizations/reproductions/17524.cy.spec.js

@@ -37,7 +37,7 @@ const questionDetails = {
   },
 };
 
-describe.skip("issue 17524", () => {
+describe("issue 17524", () => {
   beforeEach(() => {
     restore();
     cy.signInAsAdmin();