From c114973eb2139fe7d8d2334ff929b91d86773465 Mon Sep 17 00:00:00 2001 From: github-automation-metabase <166700802+github-automation-metabase@users.noreply.github.com> Date: Tue, 12 Nov 2024 18:46:21 -0500 Subject: [PATCH] =?UTF-8?q?=F0=9F=A4=96=20backported=20"In=20sidesheets,?= =?UTF-8?q?=20show=20correct=20sources=20for=20a=20model"=20(#49290)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Raphael Krut-Landau <raphael.kl@gmail.com> Co-authored-by: Uladzimir Havenchyk <125459446+uladzimirdev@users.noreply.github.com> --- .../components/QuestionDataSource/utils.js | 3 ++ .../QuestionInfoSidebar/QuestionDetails.tsx | 5 +- .../components/QuestionSources.tsx | 44 ++++++++++------ .../components/QuestionSources.unit.spec.tsx | 50 +++++++++++++++---- .../QuestionInfoSidebar/tests/setup.tsx | 6 ++- 5 files changed, 81 insertions(+), 27 deletions(-) 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.js index 583d67926a1..3f94837c4ce 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.js @@ -153,6 +153,9 @@ function QuestionTableBadges({ tables, subHead, hasLink, isLast }) { } function getTableURL(table) { + if (!table) { + return ""; + } if (isVirtualCardId(table.id)) { const cardId = getQuestionIdFromVirtualTableId(table.id); return Urls.question({ id: cardId, name: table.displayName() }); diff --git a/frontend/src/metabase/query_builder/components/view/sidebars/QuestionInfoSidebar/QuestionDetails.tsx b/frontend/src/metabase/query_builder/components/view/sidebars/QuestionInfoSidebar/QuestionDetails.tsx index be1534814f9..5491c56c768 100644 --- a/frontend/src/metabase/query_builder/components/view/sidebars/QuestionInfoSidebar/QuestionDetails.tsx +++ b/frontend/src/metabase/query_builder/components/view/sidebars/QuestionInfoSidebar/QuestionDetails.tsx @@ -2,6 +2,7 @@ import cx from "classnames"; import { useState } from "react"; import { c, t } from "ttag"; +import ErrorBoundary from "metabase/ErrorBoundary"; import { getCollectionName } from "metabase/collections/utils"; import { SidesheetCardSection } from "metabase/common/components/Sidesheet"; import DateTime from "metabase/components/DateTime"; @@ -71,7 +72,9 @@ export const QuestionDetails = ({ question }: { question: Question }) => { </Flex> </SidesheetCardSection> <SharingDisplay question={question} /> - <QuestionSources question={question} /> + <ErrorBoundary> + <QuestionSources /> + </ErrorBoundary> </> ); }; diff --git a/frontend/src/metabase/query_builder/components/view/sidebars/QuestionInfoSidebar/components/QuestionSources.tsx b/frontend/src/metabase/query_builder/components/view/sidebars/QuestionInfoSidebar/components/QuestionSources.tsx index 7fa2bba3cc6..97ffc961c07 100644 --- a/frontend/src/metabase/query_builder/components/view/sidebars/QuestionInfoSidebar/components/QuestionSources.tsx +++ b/frontend/src/metabase/query_builder/components/view/sidebars/QuestionInfoSidebar/components/QuestionSources.tsx @@ -3,30 +3,42 @@ import { c } from "ttag"; import { SidesheetCardSection } from "metabase/common/components/Sidesheet"; import Link from "metabase/core/components/Link"; +import { useSelector } from "metabase/lib/redux"; +import { getQuestionWithParameters } from "metabase/query_builder/selectors"; import { Flex, FixedSizeIcon as Icon } from "metabase/ui"; -import type Question from "metabase-lib/v1/Question"; import { getDataSourceParts } from "../../../ViewHeader/components/QuestionDataSource/utils"; import type { QuestionSource } from "./types"; import { getIconPropsForSource } from "./utils"; -export const QuestionSources = ({ question }: { question: Question }) => { - const sources = getDataSourceParts({ - question, - subHead: false, - isObjectDetail: true, - formatTableAsComponent: false, - }) as unknown as QuestionSource[]; +export const QuestionSources = () => { + const questionWithParameters = useSelector(getQuestionWithParameters); const sourcesWithIcons: QuestionSource[] = useMemo(() => { - return sources.map(source => ({ - ...source, - iconProps: getIconPropsForSource(source), - })); - }, [sources]); + const sources = questionWithParameters + ? (getDataSourceParts({ + question: questionWithParameters, + subHead: false, + isObjectDetail: true, + formatTableAsComponent: false, + }) as QuestionSource[] | QuestionSource) + : []; - if (!sources.length) { + const sourceArray = Array.isArray(sources) ? sources : [sources]; + + return ( + sourceArray + // we can't get a table for native questions in v51 + .filter(source => "href" in source) + .map(source => ({ + ...source, + iconProps: getIconPropsForSource(source as QuestionSource), + })) + ); + }, [questionWithParameters]); + + if (!questionWithParameters || !sourcesWithIcons.length) { return null; } @@ -47,7 +59,9 @@ export const QuestionSources = ({ question }: { question: Question }) => { {name} </Flex> </Link> - {index < sources.length - 1 && <Flex lh="1.25rem">{"/"}</Flex>} + {index < sourcesWithIcons.length - 1 && ( + <Flex lh="1.25rem">{"/"}</Flex> + )} </Fragment> ))} </Flex> diff --git a/frontend/src/metabase/query_builder/components/view/sidebars/QuestionInfoSidebar/components/QuestionSources.unit.spec.tsx b/frontend/src/metabase/query_builder/components/view/sidebars/QuestionInfoSidebar/components/QuestionSources.unit.spec.tsx index d12e56ed2bf..1a51a18852f 100644 --- a/frontend/src/metabase/query_builder/components/view/sidebars/QuestionInfoSidebar/components/QuestionSources.unit.spec.tsx +++ b/frontend/src/metabase/query_builder/components/view/sidebars/QuestionInfoSidebar/components/QuestionSources.unit.spec.tsx @@ -5,13 +5,14 @@ import { mockSettings } from "__support__/settings"; import { createMockEntitiesState } from "__support__/store"; import { renderWithProviders, screen, within } from "__support__/ui"; import { modelIconMap } from "metabase/lib/icon"; -import { checkNotNull } from "metabase/lib/types"; -import { getMetadata } from "metabase/selectors/metadata"; import { convertSavedQuestionToVirtualTable } from "metabase-lib/v1/metadata/utils/saved-questions"; import type { Card, NormalizedTable } from "metabase-types/api"; import { createMockCard, createMockSettings } from "metabase-types/api/mocks"; import { createSampleDatabase } from "metabase-types/api/mocks/presets"; -import { createMockState } from "metabase-types/store/mocks"; +import { + createMockQueryBuilderState, + createMockState, +} from "metabase-types/store/mocks"; import { QuestionSources } from "./QuestionSources"; @@ -25,6 +26,7 @@ const setup = async ({ sourceCard, }: SetupOpts = {}) => { const state = createMockState({ + qb: createMockQueryBuilderState({ card }), settings: mockSettings(createMockSettings()), entities: createMockEntitiesState({ databases: [createSampleDatabase()], @@ -54,16 +56,11 @@ const setup = async ({ }; } - const metadata = getMetadata(state); - const question = checkNotNull(metadata.question(card.id)); - return renderWithProviders( - <Route - path="/" - component={() => <QuestionSources question={question} />} - />, + <Route path="/" component={() => <QuestionSources />} />, { withRouter: true, + storeInitialState: state, }, ); }; @@ -142,4 +139,37 @@ describe("QuestionSources", () => { "/question/2-my-source-question", ); }); + + it("shows source information for a model (its database and table)", async () => { + const model = createMockCard({ + name: "My Model", + type: "model", + }); + + await setup({ card: model }); + + const databaseLink = await screen.findByRole("link", { + name: /Sample Database/i, + }); + + expect( + await within(databaseLink).findByLabelText("database icon"), + ).toBeInTheDocument(); + expect(databaseLink).toHaveAttribute( + "href", + "/browse/databases/1-sample-database", + ); + + expect(screen.getByText("/")).toBeInTheDocument(); + + const tableLink = await screen.findByRole("link", { name: /Products/i }); + expect(tableLink).toBeInTheDocument(); + expect( + await within(tableLink).findByLabelText(`table icon`), + ).toBeInTheDocument(); + expect(tableLink).toHaveAttribute( + "href", + expect.stringMatching(/^\/question#[a-zA-Z0-9]{20}/), + ); + }); }); diff --git a/frontend/src/metabase/query_builder/components/view/sidebars/QuestionInfoSidebar/tests/setup.tsx b/frontend/src/metabase/query_builder/components/view/sidebars/QuestionInfoSidebar/tests/setup.tsx index 07316afa137..3db1c0ac6ec 100644 --- a/frontend/src/metabase/query_builder/components/view/sidebars/QuestionInfoSidebar/tests/setup.tsx +++ b/frontend/src/metabase/query_builder/components/view/sidebars/QuestionInfoSidebar/tests/setup.tsx @@ -20,7 +20,10 @@ import { createMockUser, } from "metabase-types/api/mocks"; import { createSampleDatabase } from "metabase-types/api/mocks/presets"; -import { createMockState } from "metabase-types/store/mocks"; +import { + createMockQueryBuilderState, + createMockState, +} from "metabase-types/store/mocks"; import { QuestionInfoSidebar } from "../QuestionInfoSidebar"; @@ -45,6 +48,7 @@ export const setup = async ({ const state = createMockState({ currentUser, settings: mockSettings(settings), + qb: createMockQueryBuilderState({ card }), entities: createMockEntitiesState({ databases: [createSampleDatabase()], questions: [card], -- GitLab