From fe8642dad54956271df1c2d1ffd89f09ee8df64b Mon Sep 17 00:00:00 2001
From: Alexander Polyankin <alexander.polyankin@metabase.com>
Date: Thu, 17 Oct 2024 11:58:30 -0400
Subject: [PATCH] Fix question header when the original question is a native
query (#48815)
---
frontend/src/metabase-types/api/table.ts | 4 +-
frontend/src/metabase/lib/urls/browse.ts | 2 +-
.../QuestionDataSource/QuestionDataSource.jsx | 4 +-
.../QuestionDataSource.unit.spec.js | 40 ++++++++-
.../{utils.js => utils.tsx} | 85 ++++++++++++++-----
5 files changed, 107 insertions(+), 28 deletions(-)
rename frontend/src/metabase/query_builder/components/view/ViewHeader/components/QuestionDataSource/{utils.js => utils.tsx} (63%)
diff --git a/frontend/src/metabase-types/api/table.ts b/frontend/src/metabase-types/api/table.ts
index 25b45a77232..906a0449c7b 100644
--- a/frontend/src/metabase-types/api/table.ts
+++ b/frontend/src/metabase-types/api/table.ts
@@ -1,4 +1,4 @@
-import type { Card } from "./card";
+import type { Card, CardType } from "./card";
import type { Database, DatabaseId, InitialSyncStatus } from "./database";
import type { Field, FieldDimensionOption, FieldId } from "./field";
import type { Segment } from "./segment";
@@ -22,7 +22,7 @@ export type TableFieldOrder = "database" | "alphabetical" | "custom" | "smart";
export type Table = {
id: TableId;
-
+ type?: CardType;
name: string;
display_name: string;
description: string | null;
diff --git a/frontend/src/metabase/lib/urls/browse.ts b/frontend/src/metabase/lib/urls/browse.ts
index dd76b2c570f..351385cdac5 100644
--- a/frontend/src/metabase/lib/urls/browse.ts
+++ b/frontend/src/metabase/lib/urls/browse.ts
@@ -18,7 +18,7 @@ export function browseDatabase(database: DatabaseV1 | Database) {
export function browseSchema(table: {
db_id?: Table["db_id"];
- schema_name: Table["schema_name"] | null;
+ schema_name?: Table["schema_name"] | null;
db?: Pick<DatabaseV1, "id">;
}) {
const databaseId = table.db?.id || table.db_id;
diff --git a/frontend/src/metabase/query_builder/components/view/ViewHeader/components/QuestionDataSource/QuestionDataSource.jsx b/frontend/src/metabase/query_builder/components/view/ViewHeader/components/QuestionDataSource/QuestionDataSource.jsx
index f6b2ec9453a..b4a93d60695 100644
--- a/frontend/src/metabase/query_builder/components/view/ViewHeader/components/QuestionDataSource/QuestionDataSource.jsx
+++ b/frontend/src/metabase/query_builder/components/view/ViewHeader/components/QuestionDataSource/QuestionDataSource.jsx
@@ -14,7 +14,7 @@ import {
import { HeadBreadcrumbs } from "../HeaderBreadcrumbs";
-import { getDataSourceParts } from "./utils";
+import { getDataSourceParts, getQuestionIcon } from "./utils";
QuestionDataSource.propTypes = {
question: PropTypes.object,
@@ -121,7 +121,7 @@ function SourceDatasetBreadcrumbs({ question, ...props }) {
<HeadBreadcrumbs.Badge
key="dataset-collection"
to={Urls.collection(collection)}
- icon={question.type() === "metric" ? "metric" : "model"}
+ icon={getQuestionIcon(question)}
inactiveColor="text-light"
>
{collection?.name || t`Our analytics`}
diff --git a/frontend/src/metabase/query_builder/components/view/ViewHeader/components/QuestionDataSource/QuestionDataSource.unit.spec.js b/frontend/src/metabase/query_builder/components/view/ViewHeader/components/QuestionDataSource/QuestionDataSource.unit.spec.js
index 9bf5b2e803a..7d1e93e5d32 100644
--- a/frontend/src/metabase/query_builder/components/view/ViewHeader/components/QuestionDataSource/QuestionDataSource.unit.spec.js
+++ b/frontend/src/metabase/query_builder/components/view/ViewHeader/components/QuestionDataSource/QuestionDataSource.unit.spec.js
@@ -3,9 +3,12 @@ import { Component } from "react";
import { createMockMetadata } from "__support__/metadata";
import { setupCardEndpoints } from "__support__/server-mocks/card";
-import { renderWithProviders, screen } from "__support__/ui";
+import { getIcon, renderWithProviders, screen } from "__support__/ui";
import * as Urls from "metabase/lib/urls";
+import * as Lib from "metabase-lib";
+import { SAMPLE_METADATA } from "metabase-lib/test-helpers";
import Question from "metabase-lib/v1/Question";
+import { getQuestionVirtualTableId } from "metabase-lib/v1/metadata/utils/saved-questions";
import * as ML_Urls from "metabase-lib/v1/urls";
import {
createMockCard,
@@ -233,12 +236,14 @@ const SOURCE_CARD = createMockCard({ id: SOURCE_QUESTION_ID });
function setup({
card,
+ originalCard,
subHead = false,
isObjectDetail = false,
hasPermissions = true,
} = {}) {
const metadata = hasPermissions ? getMetadata() : createMockMetadata({});
const question = card && new Question(card, metadata);
+ const originalQuestion = originalCard && new Question(originalCard, metadata);
setupCardEndpoints(SOURCE_CARD);
@@ -247,6 +252,7 @@ function setup({
<ErrorBoundary onError={onError}>
<QuestionDataSource
question={question}
+ originalQuestion={originalQuestion}
subHead={subHead}
isObjectDetail={isObjectDetail}
/>
@@ -571,4 +577,36 @@ describe("QuestionDataSource", () => {
setup({ card: SOURCE_CARD, subHead: true });
expect(screen.queryByLabelText("More info")).not.toBeInTheDocument();
});
+
+ it("should show the correct icon when the original question is a native query", () => {
+ const metadataProvider = Lib.metadataProvider(
+ SAMPLE_DB_ID,
+ SAMPLE_METADATA,
+ );
+ const originalQuery = Lib.nativeQuery(
+ SAMPLE_DB_ID,
+ metadataProvider,
+ "SELECT * FROM ORDERS",
+ );
+ const originalQuestion = Question.create()
+ .setId(1)
+ .setDisplayName("SQL query")
+ .setQuery(originalQuery);
+ const newMetadata = createMockMetadata({
+ databases: [createSampleDatabase()],
+ questions: [originalQuestion.card()],
+ });
+ const newMetadataProvider = Lib.metadataProvider(SAMPLE_DB_ID, newMetadata);
+ const newQuery = Lib.queryFromTableOrCardMetadata(
+ newMetadataProvider,
+ Lib.tableOrCardMetadata(
+ newMetadataProvider,
+ getQuestionVirtualTableId(originalQuestion.id()),
+ ),
+ );
+ const newQuestion = Question.create().setQuery(newQuery);
+ setup({ card: newQuestion.card(), originalCard: originalQuestion.card() });
+ expect(screen.getByText("SQL query")).toBeInTheDocument();
+ expect(getIcon("table2")).toBeInTheDocument();
+ });
});
diff --git a/frontend/src/metabase/query_builder/components/view/ViewHeader/components/QuestionDataSource/utils.js b/frontend/src/metabase/query_builder/components/view/ViewHeader/components/QuestionDataSource/utils.tsx
similarity index 63%
rename from frontend/src/metabase/query_builder/components/view/ViewHeader/components/QuestionDataSource/utils.js
rename to frontend/src/metabase/query_builder/components/view/ViewHeader/components/QuestionDataSource/utils.tsx
index 227175fb8f8..796804d0602 100644
--- a/frontend/src/metabase/query_builder/components/view/ViewHeader/components/QuestionDataSource/utils.js
+++ b/frontend/src/metabase/query_builder/components/view/ViewHeader/components/QuestionDataSource/utils.tsx
@@ -1,27 +1,45 @@
-import PropTypes from "prop-types";
-import { isValidElement } from "react";
+import { type ReactElement, isValidElement } from "react";
+import { match } from "ts-pattern";
import { TableInfoIcon } from "metabase/components/MetadataInfo/TableInfoIcon/TableInfoIcon";
import { isNotNull } from "metabase/lib/types";
import * as Urls from "metabase/lib/urls";
+import type { IconName } from "metabase/ui";
import * as Lib from "metabase-lib";
+import type Question from "metabase-lib/v1/Question";
+import type Table from "metabase-lib/v1/metadata/Table";
import {
getQuestionIdFromVirtualTableId,
getQuestionVirtualTableId,
isVirtualCardId,
} from "metabase-lib/v1/metadata/utils/saved-questions";
+import type NativeQuery from "metabase-lib/v1/queries/NativeQuery";
import * as ML_Urls from "metabase-lib/v1/urls";
import { HeadBreadcrumbs } from "../HeaderBreadcrumbs";
import { IconWrapper, TablesDivider } from "./QuestionDataSource.styled";
+type DataSourcePart = ReactElement | DataSourceBadgePart;
+
+type DataSourceBadgePart = {
+ name?: string;
+ href?: string;
+ icon?: IconName;
+ model?: "database" | "schema" | "table" | "question" | "model" | "metric";
+};
+
export function getDataSourceParts({
question,
subHead,
isObjectDetail,
formatTableAsComponent = true,
-}) {
+}: {
+ question: Question;
+ subHead?: boolean;
+ isObjectDetail?: boolean;
+ formatTableAsComponent?: boolean;
+}): DataSourcePart[] {
if (!question) {
return [];
}
@@ -34,7 +52,7 @@ export function getDataSourceParts({
return [];
}
- const parts = [];
+ const parts: DataSourcePart[] = [];
const metadata = question.metadata();
const database = metadata.database(Lib.databaseID(query));
@@ -43,21 +61,21 @@ export function getDataSourceParts({
parts.push({
icon: !subHead ? "database" : undefined,
name: database.displayName(),
- href: database.id >= 0 && Urls.browseDatabase(database),
+ href: database.id >= 0 ? Urls.browseDatabase(database) : undefined,
model: "database",
});
}
const table = !isNative
? metadata.table(Lib.sourceTableOrCardId(query))
- : question.legacyQuery().table();
+ : (question.legacyQuery() as NativeQuery).table();
if (table && table.hasSchema()) {
const isBasedOnSavedQuestion = isVirtualCardId(table.id);
- if (!isBasedOnSavedQuestion) {
+ if (database != null && !isBasedOnSavedQuestion) {
parts.push({
model: "schema",
name: table.schema_name,
- href: database.id >= 0 && Urls.browseSchema(table),
+ href: database.id >= 0 ? Urls.browseSchema(table) : undefined,
});
}
}
@@ -65,10 +83,12 @@ export function getDataSourceParts({
if (table) {
const hasTableLink = subHead || isObjectDetail;
if (isNative) {
- return {
- name: table.displayName(),
- link: hasTableLink ? getTableURL() : "",
- };
+ return [
+ {
+ name: table.displayName(),
+ href: hasTableLink ? getTableURL(table) : "",
+ },
+ ];
}
const allTables = [
@@ -88,7 +108,7 @@ export function getDataSourceParts({
}),
].filter(isNotNull);
- const part = formatTableAsComponent ? (
+ const part: DataSourcePart = formatTableAsComponent ? (
<QuestionTableBadges
tables={allTables}
subHead={subHead}
@@ -106,17 +126,27 @@ export function getDataSourceParts({
parts.push(part);
}
- return parts.filter(part => isValidElement(part) || part.name || part.icon);
+ return parts.filter(
+ part =>
+ isValidElement(part) ||
+ ("name" in part && part.name) ||
+ ("icon" in part && part.icon),
+ );
}
-QuestionTableBadges.propTypes = {
- tables: PropTypes.arrayOf(PropTypes.object).isRequired,
- hasLink: PropTypes.bool,
- subHead: PropTypes.bool,
- isLast: PropTypes.bool,
+type QuestionTableBadgesProps = {
+ tables: Table[];
+ subHead?: boolean;
+ hasLink?: boolean;
+ isLast?: boolean;
};
-function QuestionTableBadges({ tables, subHead, hasLink, isLast }) {
+function QuestionTableBadges({
+ tables,
+ subHead,
+ hasLink,
+ isLast,
+}: QuestionTableBadgesProps) {
const badgeInactiveColor = isLast && !subHead ? "text-dark" : "text-light";
const parts = tables.map(table => (
@@ -151,10 +181,21 @@ function QuestionTableBadges({ tables, subHead, hasLink, isLast }) {
);
}
-function getTableURL(table) {
+function getTableURL(table: Table) {
if (isVirtualCardId(table.id)) {
const cardId = getQuestionIdFromVirtualTableId(table.id);
- return Urls.question({ id: cardId, name: table.displayName() });
+ if (cardId != null) {
+ return Urls.question({ id: cardId, name: table.displayName() });
+ }
}
return ML_Urls.getUrl(table.newQuestion());
}
+
+export function getQuestionIcon(question: Question): IconName {
+ return match(question.type())
+ .returnType<IconName>()
+ .with("question", () => "table2")
+ .with("model", () => "model")
+ .with("metric", () => "metric")
+ .exhaustive();
+}
--
GitLab