Revert "Remove unused code from QuestionDataSource (#37691)" (#37918)

This reverts commit a5dd4444.

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

@@ -12,10 +12,11 @@ import TableInfoPopover from "metabase/components/MetadataInfo/TableInfoPopover"
 
 import * as Lib from "metabase-lib";
 import {
+  isVirtualCardId,
   getQuestionIdFromVirtualTableId,
   getQuestionVirtualTableId,
-  isVirtualCardId,
 } from "metabase-lib/metadata/utils/saved-questions";
+import * as ML_Urls from "metabase-lib/urls";
 
 import { HeadBreadcrumbs } from "./HeaderBreadcrumbs";
 import { TablesDivider } from "./QuestionDataSource.styled";
@@ -23,6 +24,8 @@ import { TablesDivider } from "./QuestionDataSource.styled";
 QuestionDataSource.propTypes = {
   question: PropTypes.object,
   originalQuestion: PropTypes.object,
+  subHead: PropTypes.bool,
+  isObjectDetail: PropTypes.bool,
 };
 
 function isMaybeBasedOnDataset(question) {
@@ -31,13 +34,17 @@ function isMaybeBasedOnDataset(question) {
   return isVirtualCardId(sourceTableId);
 }
 
-function QuestionDataSource({ question, originalQuestion, ...props }) {
+function QuestionDataSource({ question, originalQuestion, subHead, ...props }) {
   if (!question) {
     return null;
   }
 
+  const variant = subHead ? "subhead" : "head";
+
   if (!question.isStructured() || !isMaybeBasedOnDataset(question)) {
-    return <DataSourceCrumbs question={question} {...props} />;
+    return (
+      <DataSourceCrumbs question={question} variant={variant} {...props} />
+    );
   }
 
   const query = question.query();
@@ -45,7 +52,13 @@ function QuestionDataSource({ question, originalQuestion, ...props }) {
   const sourceQuestionId = getQuestionIdFromVirtualTableId(sourceTableId);
 
   if (originalQuestion?.id() === sourceQuestionId) {
-    return <SourceDatasetBreadcrumbs model={originalQuestion} {...props} />;
+    return (
+      <SourceDatasetBreadcrumbs
+        model={originalQuestion}
+        variant={variant}
+        {...props}
+      />
+    );
   }
 
   return (
@@ -64,11 +77,18 @@ function QuestionDataSource({ question, originalQuestion, ...props }) {
                 <SourceDatasetBreadcrumbs
                   model={sourceQuestion}
                   collection={collection}
+                  variant={variant}
                   {...props}
                 />
               );
             }
-            return <DataSourceCrumbs question={question} {...props} />;
+            return (
+              <DataSourceCrumbs
+                question={question}
+                variant={variant}
+                {...props}
+              />
+            );
           }}
         </Collections.Loader>
       )}
@@ -78,13 +98,17 @@ function QuestionDataSource({ question, originalQuestion, ...props }) {
 
 DataSourceCrumbs.propTypes = {
   question: PropTypes.object,
+  variant: PropTypes.oneOf(["head", "subhead"]),
+  isObjectDetail: PropTypes.bool,
 };
 
-function DataSourceCrumbs({ question, ...props }) {
+function DataSourceCrumbs({ question, variant, isObjectDetail, ...props }) {
   const parts = getDataSourceParts({
     question,
+    subHead: variant === "subhead",
+    isObjectDetail,
   });
-  return <HeadBreadcrumbs parts={parts} {...props} />;
+  return <HeadBreadcrumbs parts={parts} variant={variant} {...props} />;
 }
 
 SourceDatasetBreadcrumbs.propTypes = {
@@ -132,10 +156,10 @@ function SourceDatasetBreadcrumbs({ model, collection, ...props }) {
   );
 }
 
-QuestionDataSource.shouldRender = ({ question }) =>
-  getDataSourceParts({ question }).length > 0;
+QuestionDataSource.shouldRender = ({ question, isObjectDetail }) =>
+  getDataSourceParts({ question, isObjectDetail }).length > 0;
 
-function getDataSourceParts({ question }) {
+function getDataSourceParts({ question, subHead, isObjectDetail }) {
   if (!question) {
     return [];
   }
@@ -153,7 +177,7 @@ function getDataSourceParts({ question }) {
   const database = metadata.database(Lib.databaseID(query));
   if (database) {
     parts.push({
-      icon: "database",
+      icon: !subHead ? "database" : undefined,
       name: database.displayName(),
       href: database.id >= 0 && Urls.browseDatabase(database),
     });
@@ -173,9 +197,11 @@ function getDataSourceParts({ question }) {
   }
 
   if (table) {
+    const hasTableLink = subHead || isObjectDetail;
     if (!isStructured) {
       return {
         name: table.displayName(),
+        link: hasTableLink ? getTableURL() : "",
       };
     }
 
@@ -196,7 +222,14 @@ function getDataSourceParts({ question }) {
         }),
     ].filter(isNotNull);
 
-    parts.push(<QuestionTableBadges tables={allTables} />);
+    parts.push(
+      <QuestionTableBadges
+        tables={allTables}
+        subHead={subHead}
+        hasLink={hasTableLink}
+        isLast={!isObjectDetail}
+      />,
+    );
   }
 
   return parts.filter(part => isValidElement(part) || part.name || part.icon);
@@ -204,11 +237,20 @@ function getDataSourceParts({ question }) {
 
 QuestionTableBadges.propTypes = {
   tables: PropTypes.arrayOf(PropTypes.object).isRequired,
+  hasLink: PropTypes.bool,
+  subHead: PropTypes.bool,
+  isLast: PropTypes.bool,
 };
 
-function QuestionTableBadges({ tables }) {
+function QuestionTableBadges({ tables, subHead, hasLink, isLast }) {
+  const badgeInactiveColor = isLast && !subHead ? "text-dark" : "text-light";
+
   const parts = tables.map(table => (
-    <HeadBreadcrumbs.Badge key={table.id} to={""} inactiveColor="text-dark">
+    <HeadBreadcrumbs.Badge
+      key={table.id}
+      to={hasLink ? getTableURL(table) : ""}
+      inactiveColor={badgeInactiveColor}
+    >
       <TableInfoPopover table={table} placement="bottom-start">
         <span>{table.displayName()}</span>
       </TableInfoPopover>
@@ -218,11 +260,19 @@ function QuestionTableBadges({ tables }) {
   return (
     <HeadBreadcrumbs
       parts={parts}
-      variant="head"
+      variant={subHead ? "subhead" : "head"}
       divider={<TablesDivider>+</TablesDivider>}
       data-testid="question-table-badges"
     />
   );
 }
 
+function getTableURL(table) {
+  if (isVirtualCardId(table.id)) {
+    const cardId = getQuestionIdFromVirtualTableId(table.id);
+    return Urls.question({ id: cardId, name: table.displayName() });
+  }
+  return ML_Urls.getUrl(table.newQuestion());
+}
+
 export default QuestionDataSource;

frontend/src/metabase/query_builder/components/view/QuestionDataSource.unit.spec.js

@@ -7,20 +7,21 @@ import {
 } from "metabase-types/api/mocks";
 
 import {
-  createSampleDatabase,
-  ORDERS,
   ORDERS_ID,
-  PEOPLE,
-  PEOPLE_ID,
+  ORDERS,
+  SAMPLE_DB_ID,
   PRODUCTS,
+  PEOPLE,
   PRODUCTS_ID,
-  SAMPLE_DB_ID,
+  PEOPLE_ID,
+  createSampleDatabase,
 } from "metabase-types/api/mocks/presets";
 import { createMockMetadata } from "__support__/metadata";
 import { setupCardEndpoints } from "__support__/server-mocks/card";
 import { renderWithProviders, screen } from "__support__/ui";
 import * as Urls from "metabase/lib/urls";
 import Question from "metabase-lib/Question";
+import * as ML_Urls from "metabase-lib/urls";
 import QuestionDataSource from "./QuestionDataSource";
 
 const MULTI_SCHEMA_DB_ID = 2;
@@ -58,6 +59,24 @@ const SAVED_QUESTION = {
   collection_id: null,
 };
 
+const ORDERS_QUERY = {
+  type: "query",
+  database: SAMPLE_DB_ID,
+  query: { "source-table": ORDERS_ID },
+};
+
+const PRODUCTS_QUERY = {
+  type: "query",
+  database: SAMPLE_DB_ID,
+  query: { "source-table": PRODUCTS_ID },
+};
+
+const PEOPLE_QUERY = {
+  type: "query",
+  database: SAMPLE_DB_ID,
+  query: { "source-table": PEOPLE_ID },
+};
+
 const QUERY_IN_MULTI_SCHEMA_DB = {
   type: "query",
   database: MULTI_SCHEMA_DB_ID,
@@ -180,6 +199,18 @@ function getSavedNativeQuestion(overrides) {
   });
 }
 
+function getAdHocOrdersQuestion() {
+  return getAdHocQuestion({ dataset_query: ORDERS_QUERY });
+}
+
+function getAdHocProductsQuestion() {
+  return getAdHocQuestion({ dataset_query: PRODUCTS_QUERY });
+}
+
+function getAdHocPeopleQuestion() {
+  return getAdHocQuestion({ dataset_query: PEOPLE_QUERY });
+}
+
 function getNestedQuestionTableMock(isMultiSchemaDB) {
   const dbId = isMultiSchemaDB ? MULTI_SCHEMA_DB_ID : SAMPLE_DB_ID;
   const metadata = getMetadata();
@@ -368,7 +399,7 @@ describe("QuestionDataSource", () => {
       describe(questionType, () => {
         it("displays database name", () => {
           setup({ question });
-          const node = screen.getByText(question.database().displayName());
+          const node = screen.queryByText(question.database().displayName());
           expect(node).toBeInTheDocument();
           expect(node.closest("a")).toHaveAttribute(
             "href",
@@ -402,9 +433,33 @@ describe("QuestionDataSource", () => {
       describe(questionType, () => {
         it("displays table name", () => {
           setup({ question });
-          expect(
-            screen.getByText(new RegExp(question.table().displayName())),
-          ).toBeInTheDocument();
+          const node = screen.queryByText(
+            new RegExp(question.table().displayName()),
+          );
+          expect(node).toBeInTheDocument();
+          expect(node.closest("a")).not.toBeInTheDocument();
+        });
+
+        it("displays table link in subhead variant", () => {
+          setup({ question, subHead: true });
+          const node = screen.queryByText(
+            new RegExp(question.table().displayName()),
+          );
+          expect(node.closest("a")).toHaveAttribute(
+            "href",
+            ML_Urls.getUrl(question.table().newQuestion()),
+          );
+        });
+
+        it("displays table link in object detail view", () => {
+          setup({ question, isObjectDetail: true });
+          const node = screen.queryByText(
+            new RegExp(question.table().displayName()),
+          );
+          expect(node.closest("a")).toHaveAttribute(
+            "href",
+            ML_Urls.getUrl(question.table().newQuestion()),
+          );
         });
       });
     });
@@ -442,11 +497,19 @@ describe("QuestionDataSource", () => {
         it("displays 2 joined tables (metabase#17961)", () => {
           setup({ question, subHead: true });
 
-          const orders = screen.getByText(/Orders/);
-          const products = screen.getByText(/Products/);
+          const orders = screen.queryByText(/Orders/);
+          const products = screen.queryByText(/Products/);
 
           expect(orders).toBeInTheDocument();
+          expect(orders.closest("a")).toHaveAttribute(
+            "href",
+            ML_Urls.getUrl(getAdHocOrdersQuestion()),
+          );
           expect(products).toBeInTheDocument();
+          expect(products.closest("a")).toHaveAttribute(
+            "href",
+            ML_Urls.getUrl(getAdHocProductsQuestion()),
+          );
         });
       });
     });
@@ -460,9 +523,26 @@ describe("QuestionDataSource", () => {
       describe(questionType, () => {
         it("displays > 2 joined tables (metabase#17961)", () => {
           setup({ question, subHead: true });
-          expect(screen.getByText(/Orders/)).toBeInTheDocument();
-          expect(screen.getByText(/Products/)).toBeInTheDocument();
-          expect(screen.getByText(/People/)).toBeInTheDocument();
+
+          const orders = screen.queryByText(/Orders/);
+          const products = screen.queryByText(/Products/);
+          const people = screen.queryByText(/People/);
+
+          expect(orders).toBeInTheDocument();
+          expect(orders.closest("a")).toHaveAttribute(
+            "href",
+            ML_Urls.getUrl(getAdHocOrdersQuestion()),
+          );
+          expect(products).toBeInTheDocument();
+          expect(products.closest("a")).toHaveAttribute(
+            "href",
+            ML_Urls.getUrl(getAdHocProductsQuestion()),
+          );
+          expect(people).toBeInTheDocument();
+          expect(people.closest("a")).toHaveAttribute(
+            "href",
+            ML_Urls.getUrl(getAdHocPeopleQuestion()),
+          );
         });
       });
     });