From 49db9a9b779c3292fe172379124f0be7a9588648 Mon Sep 17 00:00:00 2001
From: Nemanja Glumac <31325167+nemanjaglumac@users.noreply.github.com>
Date: Tue, 20 Aug 2024 14:15:13 +0200
Subject: [PATCH] Fix 46221: Wrong collection shown in ad-hoc model-based
 questions (#47008)

* Remove `Collections` entity loader

* Load collections directly within the component

* Add E2E repro for #46221
---
 .../scenarios/models/reproductions.cy.spec.ts | 38 +++++++++++
 .../QuestionDataSource/QuestionDataSource.jsx | 68 +++++++++----------
 2 files changed, 71 insertions(+), 35 deletions(-)

diff --git a/e2e/test/scenarios/models/reproductions.cy.spec.ts b/e2e/test/scenarios/models/reproductions.cy.spec.ts
index fbe45ccee65..f20b3770c2d 100644
--- a/e2e/test/scenarios/models/reproductions.cy.spec.ts
+++ b/e2e/test/scenarios/models/reproductions.cy.spec.ts
@@ -1,5 +1,6 @@
 import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database";
 import {
+  FIRST_COLLECTION_ID,
   ORDERS_DASHBOARD_ID,
   ORDERS_QUESTION_ID,
 } from "e2e/support/cypress_sample_instance_data";
@@ -44,6 +45,7 @@ import {
   navigationSidebar,
   newButton,
   tableInteractive,
+  type NativeQuestionDetails,
 } from "e2e/support/helpers";
 import type { CardId, FieldReference } from "metabase-types/api";
 
@@ -1260,3 +1262,39 @@ describe.skip("issues 28270, 33708", () => {
     });
   }
 });
+
+describe("issue 46221", () => {
+  const modelDetails: NativeQuestionDetails = {
+    name: "46221",
+    native: { query: "select 42" },
+    type: "model",
+    collection_id: FIRST_COLLECTION_ID as number,
+  };
+
+  beforeEach(() => {
+    restore();
+    cy.signInAsAdmin();
+
+    createNativeQuestion(modelDetails, { visitQuestion: true });
+  });
+
+  it("should retain the same collection name between ad-hoc question based on a model and a model itself (metabase#46221)", () => {
+    cy.location("pathname").should("match", /^\/model\/\d+/);
+    cy.findByTestId("head-crumbs-container")
+      .should("contain", "First collection")
+      .and("contain", modelDetails.name);
+
+    cy.log("Change the viz type");
+    cy.findByTestId("viz-type-button").click();
+    cy.findByTestId("sidebar-left").within(() => {
+      cy.findByTestId("Table-button").click();
+    });
+
+    cy.log("Make sure we're now in an ad-hoc question mode");
+    cy.location("pathname").should("eq", "/question");
+
+    cy.findByTestId("head-crumbs-container")
+      .should("contain", "First collection")
+      .and("contain", modelDetails.name);
+  });
+});
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 402158c58b7..938055801cd 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
@@ -2,9 +2,9 @@ import PropTypes from "prop-types";
 import { isValidElement } from "react";
 import { t } from "ttag";
 
+import { skipToken, useGetCollectionQuery } from "metabase/api";
 import { TableInfoIcon } from "metabase/components/MetadataInfo/TableInfoIcon/TableInfoIcon";
 import Tooltip from "metabase/core/components/Tooltip";
-import Collections from "metabase/entities/collections";
 import Questions from "metabase/entities/questions";
 import { color } from "metabase/lib/colors";
 import { isNotNull } from "metabase/lib/types";
@@ -70,38 +70,27 @@ export function QuestionDataSource({
 
   return (
     <Questions.Loader id={sourceQuestionId} loadingAndErrorWrapper={false}>
-      {({ question: sourceQuestion }) => (
-        <Collections.Loader
-          id={sourceQuestion?.collectionId()}
-          loadingAndErrorWrapper={false}
-        >
-          {({ collection, loading }) => {
-            if (!sourceQuestion || loading) {
-              return null;
-            }
-            if (
-              sourceQuestion.type() === "model" ||
-              sourceQuestion.type() === "metric"
-            ) {
-              return (
-                <SourceDatasetBreadcrumbs
-                  question={sourceQuestion}
-                  collection={collection}
-                  variant={variant}
-                  {...props}
-                />
-              );
-            }
-            return (
-              <DataSourceCrumbs
-                question={question}
-                variant={variant}
-                {...props}
-              />
-            );
-          }}
-        </Collections.Loader>
-      )}
+      {({ question: sourceQuestion }) => {
+        if (!sourceQuestion) {
+          return null;
+        }
+
+        if (
+          sourceQuestion.type() === "model" ||
+          sourceQuestion.type() === "metric"
+        ) {
+          return (
+            <SourceDatasetBreadcrumbs
+              question={sourceQuestion}
+              variant={variant}
+              {...props}
+            />
+          );
+        }
+        return (
+          <DataSourceCrumbs question={question} variant={variant} {...props} />
+        );
+      }}
     </Questions.Loader>
   );
 }
@@ -123,10 +112,19 @@ function DataSourceCrumbs({ question, variant, isObjectDetail, ...props }) {
 
 SourceDatasetBreadcrumbs.propTypes = {
   question: PropTypes.object.isRequired,
-  collection: PropTypes.object.isRequired,
 };
 
-function SourceDatasetBreadcrumbs({ question, collection, ...props }) {
+function SourceDatasetBreadcrumbs({ question, ...props }) {
+  const collectionId = question?.collectionId();
+
+  const { data: collection, isFetching } = useGetCollectionQuery(
+    collectionId ? { id: collectionId } : skipToken,
+  );
+
+  if (isFetching) {
+    return null;
+  }
+
   return (
     <HeadBreadcrumbs
       {...props}
-- 
GitLab