QueryBuilder and SQL questions: hide Explore results with parameters (#36105)

* QueryBuild and SQL questions: hide Explore results with parameters
* Fix AtomicQuery.canNest return type (so that it is always boolean)
* Add Question.canExploreResults tests

frontend/src/metabase-lib/Question.ts

@@ -1249,6 +1249,21 @@ class Question {
     return getIn(this, ["_card", "moderation_reviews"]) || [];
   }
 
+  /**
+   * We can only "explore results" (i.e. create new questions based on this one)
+   * when question is a native query, which is saved, has no parameters
+   * and satisfies other conditionals below.
+   */
+  canExploreResults() {
+    return (
+      this.isNative() &&
+      this.isSaved() &&
+      this.parameters().length === 0 &&
+      this.query().canNest() &&
+      !this.query().readOnly() // originally "canRunAdhocQuery"
+    );
+  }
+
   /**
    * TODO Atte Keinänen 6/13/17: Discussed with Tom that we could use the default Question constructor instead,
    * but it would require changing the constructor signature so that `card` is an optional parameter and has a default value

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

@@ -34,7 +34,7 @@ export default class AtomicQuery extends Query {
     return null;
   }
 
-  canNest() {
-    return this.database()?.hasFeature("nested-queries");
+  canNest(): boolean {
+    return Boolean(this.database()?.hasFeature("nested-queries"));
   }
 }

frontend/src/metabase-lib/question.unit.spec.ts

+import {
+  createAdHocNativeCard,
+  createSampleDatabase,
+  createSavedStructuredCard,
+  createSavedNativeCard,
+} from "metabase-types/api/mocks/presets";
+import { createMockMetadata } from "__support__/metadata";
+import {
+  createMockCard,
+  createMockNativeDatasetQuery,
+  createMockParameter,
+  createMockTemplateTag,
+} from "metabase-types/api/mocks";
+import Question from "./Question";
+
+describe("Question.canExploreResults", () => {
+  const metadata = createMockMetadata({
+    databases: [createSampleDatabase()],
+  });
+
+  it("should be false if not native", () => {
+    const q = new Question(createSavedStructuredCard(), metadata);
+    expect(q.canExploreResults()).toBe(false);
+  });
+
+  it("should be false when not saved", () => {
+    const q = new Question(createAdHocNativeCard(), metadata);
+    expect(q.canExploreResults()).toBe(false);
+  });
+
+  it("should be false with parameters", () => {
+    const card = createMockCard({
+      dataset_query: createMockNativeDatasetQuery({
+        type: "native",
+        native: {
+          query: "select * from order where id > {{min_id}}",
+          "template-tags": {
+            min_id: createMockTemplateTag({
+              type: "text",
+              name: "min_id",
+              "display-name": "Min ID",
+            }),
+          },
+        },
+      }),
+      parameters: [
+        createMockParameter({
+          name: "Min ID",
+          slug: "min_id",
+          type: "category",
+        }),
+      ],
+    });
+    const q = new Question(card, metadata);
+    expect(q.canExploreResults()).toBe(false);
+  });
+
+  it("should be false when not canNest", () => {
+    const metadata = createMockMetadata({
+      databases: [createSampleDatabase({ features: [] })],
+    });
+    const q = new Question(createSavedNativeCard(), metadata);
+    expect(q.canExploreResults()).toBe(false);
+  });
+
+  it("should be false when is readOnly", () => {
+    const metadata = createMockMetadata({
+      databases: [createSampleDatabase({ native_permissions: "none" })],
+    });
+    const q = new Question(createSavedNativeCard(), metadata);
+    expect(q.canExploreResults()).toBe(false);
+  });
+
+  it("should be true", () => {
+    const q = new Question(createSavedNativeCard(), metadata);
+    expect(q.canExploreResults()).toBe(true);
+  });
+});

frontend/src/metabase/query_builder/components/view/ExploreResultsLink.tsx

+import { t } from "ttag";
+import Link from "metabase/core/components/Link";
+import ViewButton from "metabase/query_builder/components/view/ViewButton";
+import type Question from "metabase-lib/Question";
+import { getUrl as ML_getUrl } from "metabase-lib/urls";
+
+interface ExploreResultsLinkProps {
+  question: Question;
+}
+
+export function ExploreResultsLink({ question }: ExploreResultsLinkProps) {
+  const query = question.composeThisQuery();
+  const button = (
+    <ViewButton disabled={!query} medium icon="insight" labelBreakpoint="sm">
+      {t`Explore results`}
+    </ViewButton>
+  );
+
+  if (query) {
+    const url = ML_getUrl(query.setDisplay("table").setSettings({}));
+    return <Link to={url}>{button}</Link>;
+  }
+
+  return button;
+}

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

@@ -11,14 +11,13 @@ import { useToggle } from "metabase/hooks/use-toggle";
 import Link from "metabase/core/components/Link";
 import Tooltip from "metabase/core/components/Tooltip";
 
-import ViewButton from "metabase/query_builder/components/view/ViewButton";
 import SavedQuestionHeaderButton from "metabase/query_builder/components/SavedQuestionHeaderButton/SavedQuestionHeaderButton";
 
 import { navigateBackToDashboard } from "metabase/query_builder/actions";
 import { MODAL_TYPES } from "metabase/query_builder/constants";
 import { getDashboard } from "metabase/query_builder/selectors";
-import * as ML_Urls from "metabase-lib/urls";
 import QuestionActions from "../QuestionActions";
+import { ExploreResultsLink } from "./ExploreResultsLink";
 import { FilterHeaderButton } from "./FilterHeaderButton";
 import { HeadBreadcrumbs } from "./HeaderBreadcrumbs";
 import QuestionDataSource from "./QuestionDataSource";
@@ -390,7 +389,6 @@ function ViewTitleHeaderRightSide(props) {
     toggleBookmark,
     isSaved,
     isDataset,
-    isNative,
     isRunnable,
     isRunning,
     isNativeEditorOpen,
@@ -414,19 +412,12 @@ function ViewTitleHeaderRightSide(props) {
     onModelPersistenceChange,
   } = props;
   const isShowingNotebook = queryBuilderMode === "notebook";
-  const query = question.query();
-  const isReadOnlyQuery = query.readOnly();
-  const canEditQuery = !isReadOnlyQuery;
-  const canRunAdhocQueries = !isReadOnlyQuery;
-  const canNest = query.canNest();
+  const canEditQuery = !question.query().readOnly();
   const hasExploreResultsLink =
-    isNative &&
-    canNest &&
-    isSaved &&
-    canRunAdhocQueries &&
+    question.canExploreResults() &&
     MetabaseSettings.get("enable-nested-queries");
 
-  const isNewQuery = !query.hasData();
+  const isNewQuery = !question.query().hasData();
   const hasSaveButton =
     !isDataset &&
     !!isDirty &&
@@ -511,6 +502,7 @@ function ViewTitleHeaderRightSide(props) {
           />
         </ViewHeaderIconButtonContainer>
       )}
+      {/* TODO(oleggromov) remove divider when nothing else is shown to the left */}
       {isSaved && (
         <QuestionActions
           isShowingQuestionInfoSidebar={isShowingQuestionInfoSidebar}
@@ -547,22 +539,4 @@ function ViewTitleHeaderRightSide(props) {
   );
 }
 
-ExploreResultsLink.propTypes = {
-  question: PropTypes.object.isRequired,
-};
-
-function ExploreResultsLink({ question }) {
-  const url = ML_Urls.getUrl(
-    question.composeThisQuery().setDisplay("table").setSettings({}),
-  );
-
-  return (
-    <Link to={url}>
-      <ViewButton medium p={[2, 1]} icon="insight" labelBreakpoint="sm">
-        {t`Explore results`}
-      </ViewButton>
-    </Link>
-  );
-}
-
 ViewTitleHeader.propTypes = viewTitleHeaderPropTypes;