Remove redundant (and wrong) `hasData` check from ViewHeader component (#37566)

- Remove redundant checks
    - `canEditQuery` shouldn't determine whether or not a Save button should be visible; It only marks it as disabled or not.

- Update a related E2E test
    - This commit makes the test more explicit and more understandable.
    - Fix the test assumption
        - In all other cases we show the disabled Save button to the "nodata" user. It was only in this test that we wrongly asserted that the button doesn't exist at all.

* Deprecate `hasData()` methods on StructuredQuery and NativeQuery

Resolves #37516

e2e/test/scenarios/question/reproductions/18978-18977-nested-question-nodata-user.cy.spec.js

@@ -4,7 +4,6 @@ import {
   appBar,
   popover,
   openNavigationSidebar,
-  leftSidebar,
   visitQuestion,
   POPOVER_ELEMENT,
 } from "e2e/support/helpers";
@@ -13,110 +12,93 @@ describe("11914, 18978, 18977", () => {
   beforeEach(() => {
     restore();
     cy.signInAsAdmin();
-  });
-
-  it("should not display query editing controls and 'Browse Data' link", () => {
     cy.createQuestion({
       query: {
         "source-table": `card__${ORDERS_QUESTION_ID}`,
+        limit: 2,
       },
     }).then(({ body: { id: questionId } }) => {
       cy.signIn("nodata");
       visitQuestion(questionId);
-      openNavigationSidebar();
-
-      cy.findByText(/Browse data/i).should("not.exist");
-      cy.icon("add").click();
-
-      popover().within(() => {
-        cy.findByText("Question").should("not.exist");
-        cy.findByText(/SQL query/).should("not.exist");
-        cy.findByText(/Native query/).should("not.exist");
-      });
-
-      // Click outside to close the "new" button popover
-      appBar().click();
-
-      cy.findByTestId("qb-header-action-panel").within(() => {
-        cy.icon("notebook").should("not.exist");
-        cy.findByText("Filter").should("not.exist");
-        cy.findByText("Summarize").should("not.exist");
-        cy.icon("refresh").should("be.visible");
-      });
+    });
+  });
 
-      // Ensure no drills offered when clicking a column header
-      cy.findByText("Subtotal").click();
-      assertNoOpenPopover();
+  it("should not display query editing controls and 'Browse Data' link", () => {
+    cy.log("Make sure we don't prompt user to browse data from the sidebar");
+    openNavigationSidebar();
+    cy.findByTestId("main-navbar-root").should("not.contain", "Browse data");
+
+    cy.log("Make sure we don't prompt user to create a new query");
+    appBar().icon("add").click();
+    popover().within(() => {
+      cy.findByText("Dashboard").should("be.visible");
+      cy.findByText("Question").should("not.exist");
+      cy.findByText(/SQL query/).should("not.exist");
+      cy.findByText("Model").should("not.exist");
+    });
+    // Click anywhere to close the "new" button popover
+    cy.get("body").click("topLeft");
+
+    cy.log(
+      "Make sure we don't prompt user to perform any further query manipulations",
+    );
+    cy.findByTestId("qb-header-action-panel").within(() => {
+      // visualization
+      cy.icon("refresh").should("be.visible");
+      cy.icon("bookmark").should("be.visible");
+      // querying
+      cy.icon("notebook").should("not.exist");
+      cy.findByText("Filter").should("not.exist");
+      cy.findByText("Summarize").should("not.exist");
+      cy.button("Save").should("not.exist");
+    });
 
-      // Ensure no drills offered when clicking a regular cell
-      cy.findByText("6.42").click();
-      assertNoOpenPopover();
+    cy.log("Make sure drill-through menus do not appear");
+    // No drills when clicking a column header
+    cy.findAllByTestId("header-cell").contains("Subtotal").click();
+    assertNoOpenPopover();
 
-      // Ensure no drills offered when clicking FK
-      cy.findByText("184").click();
-      assertNoOpenPopover();
+    // No drills when clicking a regular cell
+    cy.findAllByRole("gridcell").contains("37.65").click();
+    assertNoOpenPopover();
 
-      assertIsNotAdHoc(questionId);
+    // No drills when clicking on a FK
+    cy.get(".Table-FK").contains("123").click();
+    assertNoOpenPopover();
 
-      setVisualizationTo("line");
-      assertIsNotAdHoc(questionId);
+    assertIsNotAdHoc();
 
-      // Rerunning a query with changed viz settings will make it use the `/dataset` endpoint,
-      // so a user will see the "Your don't have permission" error
-      // Need to ensure "refresh" button is hidden now
-      assertNoRefreshButton();
+    cy.log("Make sure user can change visualization but not save the question");
+    cy.findByTestId("viz-type-button").click();
+    cy.findByTestId("Number-button").click();
+    cy.findByTestId("scalar-value").should("exist");
+    assertSaveIsDisabled();
 
-      addGoalLine();
-      assertIsNotAdHoc(questionId);
-      assertNoRefreshButton();
-    });
+    cy.log("Make sure we don't prompt user to refresh the updated query");
+    // Rerunning a query with changed viz settings will make it use the `/dataset` endpoint,
+    // so a user will see the "You don't have permission" error
+    assertNoRefreshButton();
   });
 });
 
-function setVisualizationTo(vizName) {
-  cy.findByTestId("viz-type-button").click();
-
-  leftSidebar().within(() => {
-    cy.icon(vizName).click();
-    cy.icon(vizName).realHover();
-    cy.icon("gear").click();
-    cy.findByText("X-axis").parent().findByText("Select a field").click();
-  });
-  selectFromDropdown("Created At");
-
-  leftSidebar().within(() => {
-    cy.findByText("Y-axis").parent().findByText("Select a field").click();
-  });
-  selectFromDropdown("Quantity");
-
-  leftSidebar().findByText("Done").click();
+function assertSaveIsDisabled() {
+  saveButton().should("have.attr", "aria-disabled", "true");
 }
 
-function addGoalLine() {
-  cy.findByTestId("viz-settings-button").click();
-  leftSidebar().within(() => {
-    cy.findByText("Display").click();
-    cy.findByText("Goal line").parent().find("input").click();
-    cy.findByText("Done").click();
-  });
-  cy.get(".Visualization").get(".goal").should("be.visible");
-}
-
-function assertIsNotAdHoc(questionId) {
-  cy.url().should("include", `/question/${questionId}`);
-  cy.findByTestId("qb-header").findByText("Save").should("not.exist");
+function assertIsNotAdHoc() {
+  // Ad-hoc questions have a base64 encoded hash in the URL
+  cy.location("hash").should("eq", "");
+  saveButton().should("not.exist");
 }
 
 function assertNoRefreshButton() {
-  cy.findByTestId("qb-header-action-panel").within(() => {
-    cy.icon("refresh").should("not.exist");
-  });
+  cy.findByTestId("qb-header-action-panel").icon("refresh").should("not.exist");
 }
 
 function assertNoOpenPopover() {
   cy.get(POPOVER_ELEMENT).should("not.exist");
 }
 
-function selectFromDropdown(option) {
-  popover().findByText(option).click();
+function saveButton() {
+  return cy.findByTestId("qb-header").button("Save");
 }

frontend/src/metabase-lib/queries/NativeQuery.ts

@@ -102,6 +102,10 @@ export default class NativeQuery extends AtomicQuery {
   }
 
   /* Query superclass methods */
+
+  /**
+   * @deprecated use MLv2
+   */
   hasData() {
     return (
       this._databaseId() != null && (!this.requiresTable() || this.collection())

frontend/src/metabase-lib/queries/StructuredQuery.ts

@@ -402,6 +402,9 @@ class StructuredQuery extends AtomicQuery {
     }
   }
 
+  /**
+   * @deprecated use MLv2
+   */
   hasData() {
     return !!this.table();
   }

frontend/src/metabase/query_builder/components/view/ViewHeader.jsx

@@ -417,14 +417,10 @@ function ViewTitleHeaderRightSide(props) {
     question.canExploreResults() &&
     MetabaseSettings.get("enable-nested-queries");
 
-  const isNewQuery = !question
-    .legacyQuery({ useStructuredQuery: true })
-    .hasData();
-  const hasSaveButton =
-    !isDataset &&
-    !!isDirty &&
-    (isNewQuery || canEditQuery) &&
-    isActionListVisible;
+  // Models can't be saved. But changing anything about the model will prompt the user
+  // to save it as a new question (based on that model). In other words, at this point
+  // the `dataset` field is set to false.
+  const hasSaveButton = !isDataset && !!isDirty && isActionListVisible;
   const isMissingPermissions =
     result?.error_type === SERVER_ERROR_TYPES.missingPermissions;
   const hasRunButton =