Migrate isNative to MLv2 (#37805)

nemanjaglumac · kamilmielnik · web-flow · commit 2cd8c5fa41f4 · 2024-01-22T08:44:29.000Z
* Migrate Question `isNative` and `isStructured` methods to MLv2

* Mark methods as deprecated

* Add the guard to the `isNativeDashCard` util

* Add additional guard to the `buildFieldFilterUiParameter` helper

* Update all `isNative` method instances to MLv2

* Fix the negation logic 🤦

* Remove `isNative()` method altogether

* Simplify the `shouldResolveFkField` check

Co-authored-by: Kamil Mielnik &lt;kamil@kamilmielnik.com&gt;

* Remove explicit Boolean conversion

* Simplify conditional logic for `getTargetsForQuestion`

---------

Co-authored-by: Kamil Mielnik &lt;kamil@kamilmielnik.com&gt;
diff --git a/enterprise/frontend/src/metabase-enterprise/audit_app/components/QueryViewer/QueryViewer.tsx b/enterprise/frontend/src/metabase-enterprise/audit_app/components/QueryViewer/QueryViewer.tsx
@@ -1,6 +1,7 @@
 import { useEffect, useMemo } from "react";
 import _ from "underscore";
 
+import * as Lib from "metabase-lib";
 import { useSelector, useDispatch } from "metabase/lib/redux";
 import NativeQueryEditor from "metabase/query_builder/components/NativeQueryEditor";
 import ReadOnlyNotebook from "metabase/query_builder/components/notebook/ReadOnlyNotebook";
@@ -28,7 +29,9 @@ export function QueryViewer({ datasetQuery }: { datasetQuery: DatasetQuery }) {
   }, [card, dispatch]);
 
   const question = new Question(card, metadata);
-  if (question.isNative()) {
+  const { isNative } = Lib.queryDisplayInfo(question.query());
+
+  if (isNative) {
     return (
       <NativeQueryEditor
         question={question}
diff --git a/frontend/src/metabase-lib/Question.ts b/frontend/src/metabase-lib/Question.ts
@@ -184,7 +184,7 @@ class Question {
     }
 
     const isVirtualDashcard = !this._card.id;
-    // `dataset_query` is null for questions on a dashboard the user don't have access to
+    // The `dataset_query` is null for questions on a dashboard the user doesn't have access to
     !isVirtualDashcard &&
       console.warn("Unknown query type: " + datasetQuery?.type);
   });
@@ -203,16 +203,12 @@ class Question {
     return query;
   }
 
-  isNative(): boolean {
-    return (
-      this.legacyQuery({ useStructuredQuery: true }) instanceof NativeQuery
-    );
-  }
-
+  /**
+   * @deprecated use MLv2
+   */
   isStructured(): boolean {
-    return (
-      this.legacyQuery({ useStructuredQuery: true }) instanceof StructuredQuery
-    );
+    const { isNative } = Lib.queryDisplayInfo(this.query());
+    return !isNative;
   }
 
   /**
@@ -1112,9 +1108,10 @@ class Question {
    */
   canExploreResults() {
     const canNest = Boolean(this.database()?.hasFeature("nested-queries"));
+    const { isNative } = Lib.queryDisplayInfo(this.query());
 
     return (
-      this.isNative() &&
+      isNative &&
       this.isSaved() &&
       this.parameters().length === 0 &&
       canNest &&
diff --git a/frontend/src/metabase-lib/metadata/utils/models.ts b/frontend/src/metabase-lib/metadata/utils/models.ts
@@ -8,6 +8,7 @@ import type {
   StructuredDatasetQuery,
   FieldId,
 } from "metabase-types/api";
+import * as Lib from "metabase-lib";
 import { getQuestionVirtualTableId } from "metabase-lib/metadata/utils/saved-questions";
 import type Database from "metabase-lib/metadata/Database";
 import type Question from "metabase-lib/Question";
@@ -101,7 +102,9 @@ export function checkCanBeModel(question: Question) {
     return false;
   }
 
-  if (!question.isNative()) {
+  const { isNative } = Lib.queryDisplayInfo(question.query());
+
+  if (!isNative) {
     return true;
   }
 
diff --git a/frontend/src/metabase-lib/parameters/utils/click-behavior.ts b/frontend/src/metabase-lib/parameters/utils/click-behavior.ts
@@ -132,15 +132,13 @@ function notRelativeDateOrRange({ type }: Parameter) {
 }
 
 export function getTargetsForQuestion(question: Question): Target[] {
-  if (question.isStructured()) {
-    return getTargetsForStructuredQuestion(question);
-  }
+  const { isNative } = Lib.queryDisplayInfo(question.query());
 
-  if (question.isNative()) {
+  if (isNative) {
     return getTargetsForNativeQuestion(question);
   }
 
-  return [];
+  return getTargetsForStructuredQuestion(question);
 }
 
 function getTargetsForStructuredQuestion(question: Question): Target[] {
diff --git a/frontend/src/metabase-lib/queries/drills/native-drill-fallback.ts b/frontend/src/metabase-lib/queries/drills/native-drill-fallback.ts
@@ -1,3 +1,4 @@
+import * as Lib from "metabase-lib";
 import type Question from "metabase-lib/Question";
 
 interface FallbackNativeDrillProps {
@@ -6,7 +7,10 @@ interface FallbackNativeDrillProps {
 
 export function nativeDrillFallback({ question }: FallbackNativeDrillProps) {
   const database = question.database();
-  if (!question.isNative() || !question.isQueryEditable() || !database) {
+  const query = question.query();
+  const { isNative } = Lib.queryDisplayInfo(query);
+
+  if (!isNative || !question.isQueryEditable() || !database) {
     return null;
   }
 
diff --git a/frontend/src/metabase/dashboard/actions/navigation.js b/frontend/src/metabase/dashboard/actions/navigation.js
@@ -2,6 +2,7 @@ import { createThunkAction } from "metabase/lib/redux";
 import * as Urls from "metabase/lib/urls";
 
 import { openUrl } from "metabase/redux/app";
+import * as Lib from "metabase-lib";
 import { getParametersMappedToDashcard } from "metabase/parameters/utils/dashboards";
 import { getMetadata } from "metabase/selectors/metadata";
 import { getCardAfterVisualizationClick } from "metabase/visualizations/lib/utils";
@@ -14,7 +15,8 @@ export const editQuestion = createThunkAction(
   EDIT_QUESTION,
   question => (dispatch, getState) => {
     const dashboardId = getDashboardId(getState());
-    const mode = question.isNative() ? "view" : "notebook";
+    const { isNative } = Lib.queryDisplayInfo(question.query());
+    const mode = isNative ? "view" : "notebook";
     const url = Urls.question(question.card(), { mode });
 
     dispatch(openUrl(url));
@@ -73,8 +75,12 @@ export const navigateToNewCardFromDashboard = createThunkAction(
       // When building a question URL, it'll usually clean the query and
       // strip clauses referencing fields from tables without metadata
       const previousQuestion = new Question(previousCard, metadata);
+      const { isNative: isPreviousNative } = Lib.queryDisplayInfo(
+        previousQuestion.query(),
+      );
+
       const isDrillingFromNativeModel =
-        previousQuestion.isDataset() && previousQuestion.isNative();
+        previousQuestion.isDataset() && isPreviousNative;
 
       const url = ML_Urls.getUrlWithParameters(
         question,
diff --git a/frontend/src/metabase/dashboard/components/ClickMappings.jsx b/frontend/src/metabase/dashboard/components/ClickMappings.jsx
@@ -8,6 +8,7 @@ import { getIn, assocIn, dissocIn } from "icepick";
 import { Icon } from "metabase/ui";
 import Select from "metabase/core/components/Select";
 
+import * as Lib from "metabase-lib";
 import MetabaseSettings from "metabase/lib/settings";
 import { isPivotGroupColumn } from "metabase/lib/data_grid";
 import { GTAPApi } from "metabase/services";
@@ -342,9 +343,10 @@ export function isMappableColumn(column) {
 export function clickTargetObjectType(object) {
   if (!(object instanceof Question)) {
     return "dashboard";
-  } else if (object.isNative()) {
-    return "native";
-  } else {
-    return "gui";
   }
+
+  const query = object.query();
+  const { isNative } = Lib.queryDisplayInfo(query);
+
+  return isNative ? "native" : "gui";
 }
diff --git a/frontend/src/metabase/dashboard/utils.ts b/frontend/src/metabase/dashboard/utils.ts
@@ -23,7 +23,6 @@ import type {
   DashCardDataMap,
 } from "metabase-types/api";
 import type { SelectedTabId } from "metabase-types/store";
-import Question from "metabase-lib/Question";
 import {
   isDateParameter,
   isNumberParameter,
@@ -97,7 +96,8 @@ export function isLinkDashCard(dashcard: DashboardCard) {
 }
 
 export function isNativeDashCard(dashcard: DashboardCard) {
-  return dashcard.card && new Question(dashcard.card).isNative();
+  // The `dataset_query` is null for questions on a dashboard the user doesn't have access to
+  return dashcard.card.dataset_query?.type === "native";
 }
 
 // For a virtual (text) dashcard without any parameters, returns a boolean indicating whether we should display the
diff --git a/frontend/src/metabase/lib/click-behavior.js b/frontend/src/metabase/lib/click-behavior.js
@@ -2,7 +2,6 @@ import { msgid, ngettext, t } from "ttag";
 import { getIn } from "icepick";
 import Questions from "metabase/entities/questions";
 import Dashboards from "metabase/entities/dashboards";
-import Question from "metabase-lib/Question";
 
 export function getClickBehaviorDescription(dashcard) {
   const noBehaviorMessage = hasActionsMenu(dashcard)
@@ -42,7 +41,7 @@ export function getClickBehaviorDescription(dashcard) {
 export function hasActionsMenu(dashcard) {
   // This seems to work, but it isn't the right logic.
   // The right thing to do would be to check for any drills. However, we'd need a "clicked" object for that.
-  return !new Question(dashcard.card).isNative();
+  return dashcard.card.dataset_query?.type === "query";
 }
 
 export function isTableDisplay(dashcard) {
diff --git a/frontend/src/metabase/metabot/selectors.ts b/frontend/src/metabase/metabot/selectors.ts
@@ -1,6 +1,7 @@
 import { createSelector } from "@reduxjs/toolkit";
 import { getMetadata } from "metabase/selectors/metadata";
 import type { State } from "metabase-types/store";
+import * as Lib from "metabase-lib";
 import Question from "metabase-lib/Question";
 import type NativeQuery from "metabase-lib/queries/NativeQuery";
 import { DEFAULT_TABLE_SETTINGS } from "./constants";
@@ -57,7 +58,7 @@ export const getFeedbackType = (state: State) => {
 };
 
 export const getNativeQueryText = createSelector([getQuestion], question => {
-  return question?.isNative()
+  return question && Lib.queryDisplayInfo(question.query()).isNative
     ? (question.legacyQuery() as NativeQuery).queryText()
     : undefined;
 });
diff --git a/frontend/src/metabase/models/components/ModelDetailPage/ModelInfoSidePanel/ModelInfoSidePanel.tsx b/frontend/src/metabase/models/components/ModelDetailPage/ModelInfoSidePanel/ModelInfoSidePanel.tsx
@@ -1,6 +1,7 @@
 import { t } from "ttag";
 
 import type { Card } from "metabase-types/api";
+import * as Lib from "metabase-lib";
 import type Question from "metabase-lib/Question";
 import * as ML_Urls from "metabase-lib/urls";
 import type Table from "metabase-lib/metadata/Table";
@@ -26,6 +27,7 @@ function ModelInfoSidePanel({ model, mainTable, onChangeDescription }: Props) {
 
   const canWrite = model.canWrite();
   const description = model.description();
+  const { isNative } = Lib.queryDisplayInfo(model.query());
 
   return (
     <ModelInfoPanel>
@@ -43,9 +45,7 @@ function ModelInfoSidePanel({ model, mainTable, onChangeDescription }: Props) {
           onChange={onChangeDescription}
         />
       </ModelInfoSection>
-      {!model.isNative() && (
-        <ModelRelationships model={model} mainTable={mainTable} />
-      )}
+      {!isNative && <ModelRelationships model={model} mainTable={mainTable} />}
       {modelCard.creator && (
         <ModelInfoSection>
           <ModelInfoTitle>{t`Created by`}</ModelInfoTitle>
diff --git a/frontend/src/metabase/models/containers/ModelDetailPage/ModelDetailPage.tsx b/frontend/src/metabase/models/containers/ModelDetailPage/ModelDetailPage.tsx
@@ -100,11 +100,13 @@ function ModelDetailPage({
     database != null && database.hasFeature("nested-queries");
 
   const mainTable = useMemo(() => {
-    if (model.isNative()) {
+    const query = model.query();
+    const { isNative } = Lib.queryDisplayInfo(query);
+
+    if (isNative) {
       return null;
     }
 
-    const query = model.query();
     const sourceTableId = Lib.sourceTableOrCardId(query);
     const table = model.metadata().table(sourceTableId);
     return table;
diff --git a/frontend/src/metabase/parameters/utils/dashboards.ts b/frontend/src/metabase/parameters/utils/dashboards.ts
@@ -22,7 +22,6 @@ import {
 } from "metabase-lib/parameters/utils/targets";
 import type Metadata from "metabase-lib/metadata/Metadata";
 import type Field from "metabase-lib/metadata/Field";
-import Question from "metabase-lib/Question";
 
 type ExtendedMapping = DashboardParameterMapping & {
   dashcard_id: number;
@@ -139,12 +138,15 @@ function buildFieldFilterUiParameter(
   );
   const mappedFields = mappingsForParameter.map(mapping => {
     const { target, card } = mapping;
-    const question = new Question(card, metadata);
 
     try {
       const field = getTargetFieldFromCard(target, card, metadata);
 
-      return { field, shouldResolveFkField: !question.isNative() };
+      return {
+        field,
+        // The `dataset_query` is null for questions on a dashboard the user doesn't have access to
+        shouldResolveFkField: card.dataset_query?.type === "query",
+      };
     } catch (e) {
       console.error("Error getting a field from a card", { card });
       throw e;
diff --git a/frontend/src/metabase/query_builder/actions/core/initializeQB.ts b/frontend/src/metabase/query_builder/actions/core/initializeQB.ts
@@ -329,6 +329,8 @@ async function handleQBInit(
   const metadata = getMetadata(getState());
 
   let question = new Question(card, metadata);
+  const query = question.query();
+  const { isNative } = Lib.queryDisplayInfo(query);
 
   if (question.isSaved()) {
     if (!question.isDataset()) {
@@ -342,12 +344,12 @@ async function handleQBInit(
     }
   }
 
-  if (question.isNative()) {
+  if (isNative) {
     const isEditing = getIsEditingInDashboard(getState());
     uiControls.isNativeEditorOpen = isEditing || !question.isSaved();
   }
 
-  if (question.isNative() && question.isQueryEditable()) {
+  if (isNative && question.isQueryEditable()) {
     const query = question.legacyQuery() as NativeQuery;
     const newQuery = await updateTemplateTagNames(query, getState, dispatch);
     question = question.setLegacyQuery(newQuery);
diff --git a/frontend/src/metabase/query_builder/actions/core/updateQuestion.ts b/frontend/src/metabase/query_builder/actions/core/updateQuestion.ts
@@ -74,10 +74,16 @@ function shouldTemplateTagEditorBeVisible({
   if (queryBuilderMode === "dataset") {
     return isVisible;
   }
-  const previousTags = currentQuestion?.isNative()
+  const isCurrentQuestionNative =
+    currentQuestion && Lib.queryDisplayInfo(currentQuestion.query()).isNative;
+  const isNewQuestionNative = Lib.queryDisplayInfo(
+    newQuestion.query(),
+  ).isNative;
+
+  const previousTags = isCurrentQuestionNative
     ? (currentQuestion.legacyQuery() as NativeQuery).variableTemplateTags()
     : [];
-  const nextTags = newQuestion.isNative()
+  const nextTags = isNewQuestionNative
     ? (newQuestion.legacyQuery() as NativeQuery).variableTemplateTags()
     : [];
   if (nextTags.length > previousTags.length) {
@@ -148,9 +154,15 @@ export const updateQuestion = (
     const isPivot = newQuestion.display() === "pivot";
     const wasPivot = currentQuestion?.display() === "pivot";
 
+    const isCurrentQuestionNative =
+      currentQuestion && Lib.queryDisplayInfo(currentQuestion.query()).isNative;
+    const isNewQuestionNative = Lib.queryDisplayInfo(
+      newQuestion.query(),
+    ).isNative;
+
     if (wasPivot || isPivot) {
       const hasBreakouts =
-        newQuestion.isStructured() &&
+        !isNewQuestionNative &&
         Lib.breakouts(newQuestion.query(), -1).length > 0;
 
       // compute the pivot setting now so we can query the appropriate data
@@ -178,7 +190,7 @@ export const updateQuestion = (
     }
 
     // Native query should never be in notebook mode (metabase#12651)
-    if (queryBuilderMode === "notebook" && newQuestion.isNative()) {
+    if (queryBuilderMode === "notebook" && isNewQuestionNative) {
       await dispatch(
         setQueryBuilderMode("view", {
           shouldUpdateUrl: false,
@@ -187,7 +199,7 @@ export const updateQuestion = (
     }
 
     // Sync card's parameters with the template tags;
-    if (newQuestion.isNative()) {
+    if (isNewQuestionNative) {
       const parameters = getTemplateTagParametersFromCard(newQuestion.card());
       newQuestion = newQuestion.setParameters(parameters);
     }
@@ -201,7 +213,7 @@ export const updateQuestion = (
       dispatch(updateUrl(null, { dirty: true }));
     }
 
-    if (currentQuestion?.isNative?.() || newQuestion.isNative()) {
+    if (isCurrentQuestionNative || isNewQuestionNative) {
       const isVisible = getIsShowingTemplateTagsEditor(getState());
       const shouldBeVisible = shouldTemplateTagEditorBeVisible({
         currentQuestion,
diff --git a/frontend/src/metabase/query_builder/actions/querying.js b/frontend/src/metabase/query_builder/actions/querying.js
@@ -1,6 +1,7 @@
 import { t } from "ttag";
 import { createAction } from "redux-actions";
 
+import * as Lib from "metabase-lib";
 import * as MetabaseAnalytics from "metabase/lib/analytics";
 import { startTimer } from "metabase/lib/performance";
 import { defer } from "metabase/lib/promise";
@@ -183,7 +184,9 @@ export const queryCompleted = (question, queryResults) => {
       question.isDirtyComparedTo(originalQuestion);
 
     if (isDirty) {
-      if (question.isNative()) {
+      const { isNative } = Lib.queryDisplayInfo(question.query());
+
+      if (isNative) {
         question = question.syncColumnsAndSettings(
           originalQuestion,
           queryResults[0],
diff --git a/frontend/src/metabase/query_builder/components/DatasetEditor/DatasetEditor.jsx b/frontend/src/metabase/query_builder/components/DatasetEditor/DatasetEditor.jsx
diff --git a/frontend/src/metabase/query_builder/components/DatasetEditor/DatasetFieldMetadataSidebar/DatasetFieldMetadataSidebar.jsx b/frontend/src/metabase/query_builder/components/DatasetEditor/DatasetFieldMetadataSidebar/DatasetFieldMetadataSidebar.jsx
diff --git a/frontend/src/metabase/query_builder/components/DatasetEditor/DatasetQueryEditor.jsx b/frontend/src/metabase/query_builder/components/DatasetEditor/DatasetQueryEditor.jsx
diff --git a/frontend/src/metabase/query_builder/components/VisualizationError/VisualizationError.tsx b/frontend/src/metabase/query_builder/components/VisualizationError/VisualizationError.tsx
diff --git a/frontend/src/metabase/query_builder/components/view/PreviewQueryButton/PreviewQueryButton.tsx b/frontend/src/metabase/query_builder/components/view/PreviewQueryButton/PreviewQueryButton.tsx
diff --git a/frontend/src/metabase/query_builder/components/view/ViewHeader.jsx b/frontend/src/metabase/query_builder/components/view/ViewHeader.jsx
diff --git a/frontend/src/metabase/query_builder/selectors.js b/frontend/src/metabase/query_builder/selectors.js
diff --git a/frontend/src/metabase/query_builder/utils.ts b/frontend/src/metabase/query_builder/utils.ts

Original file line number	Diff line number	Diff line change
`@@ -132,15 +132,13 @@ function notRelativeDateOrRange({ type }: Parameter) {`
`132`	`132`	`}`
`133`	`133`
`134`	`134`	`export function getTargetsForQuestion(question: Question): Target[] {`
`135`		`- if (question.isStructured()) {`
`136`		`- return getTargetsForStructuredQuestion(question);`
`137`		`- }`
	`135`	`+ const { isNative } = Lib.queryDisplayInfo(question.query());`
`138`	`136`
`139`		`- if (question.isNative()) {`
	`137`	`+ if (isNative) {`
`140`	`138`	`return getTargetsForNativeQuestion(question);`
`141`	`139`	`}`
`142`	`140`
`143`		`- return [];`
	`141`	`+ return getTargetsForStructuredQuestion(question);`
`144`	`142`	`}`
`145`	`143`
`146`	`144`	`function getTargetsForStructuredQuestion(question: Question): Target[] {`