diff --git a/e2e/test/scenarios/embedding/embedding-dashboard.cy.spec.js b/e2e/test/scenarios/embedding/embedding-dashboard.cy.spec.js index 63b9b0faf7757c8bcb5db0052d4d4cbcc027aaf6..8621c6bb83ee6565d767c6f2b9a88161302a68cd 100644 --- a/e2e/test/scenarios/embedding/embedding-dashboard.cy.spec.js +++ b/e2e/test/scenarios/embedding/embedding-dashboard.cy.spec.js @@ -6,6 +6,7 @@ import { filterWidget, visitIframe, getDashboardCard, + addOrUpdateDashboardCard, } from "e2e/support/helpers"; import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database"; @@ -249,6 +250,86 @@ describe("scenarios > embedding > dashboard parameters", () => { .should("be.visible"); }); }); + + it("should render error without crashing when embed query returns error (metabase#34954)", () => { + const categoryTemplateTag = { + type: "text", + name: "category", + id: "377a4a4a-179e-4d86-8263-f3b3887df15f", + "display-name": "Category", + }; + const questionDetails = { + native: { + query: "Select * from products Where category = {{category}}", + "template-tags": { + category: categoryTemplateTag, + }, + }, + }; + + const dashboardParameter = { + name: "Category", + slug: "category", + id: "9cd1ee78", + type: "string/=", + sectionId: "string", + values_query_type: "none", + }; + const dashboardDetails = { + name: 'dashboard with "category" parameter', + parameters: [dashboardParameter], + }; + + cy.createNativeQuestionAndDashboard({ + questionDetails, + dashboardDetails, + }).then(({ body: { card_id, dashboard_id } }) => { + cy.wrap(dashboard_id).as("dashboardId2"); + + addOrUpdateDashboardCard({ + card_id, + dashboard_id, + card: { + parameter_mappings: [ + { + parameter_id: dashboardParameter.id, + card_id, + target: ["variable", ["template-tag", categoryTemplateTag.name]], + }, + ], + visualization_settings: { + "card.hide_empty": true, + }, + }, + }); + + cy.request("PUT", `/api/dashboard/${dashboard_id}`, { + embedding_params: { + category: "enabled", + }, + enable_embedding: true, + }); + + const payload = { + resource: { dashboard: dashboard_id }, + params: {}, + }; + + visitEmbeddedPage(payload); + }); + + cy.log("The whole page would have crashed before the fix at this point"); + getDashboardCard() + .findByText("There was a problem displaying this chart.") + .should("be.visible"); + + cy.log("Add a filter to complete the query"); + filterWidget().findByPlaceholderText("Category").type("Widget{enter}"); + + getDashboardCard() + .findByText("Practical Bronze Computer") + .should("be.visible"); + }); }); describe("scenarios > embedding > dashboard parameters with defaults", () => { diff --git a/frontend/src/metabase-types/api/dataset.ts b/frontend/src/metabase-types/api/dataset.ts index 2d02e046f1fc73349d10089c40ccfd8cc652c90a..d613d1d8e1f1e4b863bdc26fac464e50f1531f0b 100644 --- a/frontend/src/metabase-types/api/dataset.ts +++ b/frontend/src/metabase-types/api/dataset.ts @@ -72,7 +72,7 @@ export interface Dataset { status?: string; } -export interface PublicDatasetData { +export interface EmbedDatasetData { rows: RowValues[]; cols: DatasetColumn[]; rows_truncated: number; @@ -82,10 +82,18 @@ export interface PublicDatasetData { results_timezone?: string; } -export interface PublicDataset { - data: PublicDatasetData; - json_query?: JsonQuery; - status?: string; +export type EmbedDataset = SuccessEmbedDataset | ErrorEmbedDataset; + +interface SuccessEmbedDataset { + data: EmbedDatasetData; + json_query: JsonQuery; + status: string; +} + +export interface ErrorEmbedDataset { + error_type: string; + error: string; + status: string; } export interface NativeQueryForm { diff --git a/frontend/src/metabase-types/api/mocks/dataset.ts b/frontend/src/metabase-types/api/mocks/dataset.ts index aa9db7ea0823c31354a012475c9f64fe780f95b5..3a6c7099f88aee4a0f6629621905004173b959b7 100644 --- a/frontend/src/metabase-types/api/mocks/dataset.ts +++ b/frontend/src/metabase-types/api/mocks/dataset.ts @@ -2,8 +2,9 @@ import type { Dataset, DatasetColumn, DatasetData, - PublicDataset, - PublicDatasetData, + EmbedDataset, + EmbedDatasetData, + ErrorEmbedDataset, ResultsMetadata, TemplateTag, } from "metabase-types/api/dataset"; @@ -57,7 +58,7 @@ export const createMockDataset = ({ ...opts, }); -export const createMockPublicDatasetData = ({ +export const createMockEmbedDatasetData = ({ cols = [ createMockColumn({ display_name: "NAME", @@ -66,7 +67,7 @@ export const createMockPublicDatasetData = ({ }), ], ...opts -}: Partial<PublicDatasetData>): PublicDatasetData => ({ +}: Partial<EmbedDatasetData>): EmbedDatasetData => ({ rows: [], cols, rows_truncated: 0, @@ -74,16 +75,29 @@ export const createMockPublicDatasetData = ({ ...opts, }); -export const createMockPublicDataset = ({ - data = {}, - ...opts -}: MockDatasetOpts = {}): PublicDataset => ({ - data: createMockPublicDatasetData(data), - database_id: 1, - row_count: 0, - running_time: 1000, - ...opts, -}); +export type MockEmbedDatasetOpts = Omit<Partial<EmbedDataset>, "data"> & { + data?: Partial<EmbedDatasetData>; +}; + +export const createMockEmbedDataset = ( + opts: MockEmbedDatasetOpts, +): EmbedDataset => { + if ("data" in opts) { + const { data = {}, ...rest } = opts; + return { + data: createMockEmbedDatasetData(data), + json_query: { + database: 1, + type: "native", + native: { query: "SELECT 1" }, + }, + status: "success", + ...rest, + }; + } + + return opts as ErrorEmbedDataset; +}; export const createMockTemplateTag = ( opts?: Partial<TemplateTag>, diff --git a/frontend/src/metabase/dashboard/utils.ts b/frontend/src/metabase/dashboard/utils.ts index 1219757d2c49f6bc76606348680fbd36147c106c..13b66f58c876fe03e0c7b301394ce0d110c91f1e 100644 --- a/frontend/src/metabase/dashboard/utils.ts +++ b/frontend/src/metabase/dashboard/utils.ts @@ -19,6 +19,7 @@ import type { Parameter, StructuredDatasetQuery, ActionDashboardCard, + EmbedDataset, } from "metabase-types/api"; import type { SelectedTabId } from "metabase-types/store"; import Question from "metabase-lib/Question"; @@ -225,14 +226,16 @@ const isDashcardDataLoaded = ( return data != null && Object.values(data).every(result => result != null); }; -const hasRows = (dashcardData: Record<CardId, Dataset>) => { +const hasRows = (dashcardData: Record<CardId, Dataset | EmbedDataset>) => { const queryResults = dashcardData ? Object.values(dashcardData).filter(Boolean) : []; return ( queryResults.length > 0 && - queryResults.every(queryResult => queryResult.data.rows.length > 0) + queryResults.every( + queryResult => "data" in queryResult && queryResult.data.rows.length > 0, + ) ); }; diff --git a/frontend/src/metabase/public/containers/PublicQuestion/PublicQuestion.unit.spec.tsx b/frontend/src/metabase/public/containers/PublicQuestion/PublicQuestion.unit.spec.tsx index 91d1bd45221e0b4bbfe79d2f7139d339e9ed2b02..1dd66aef2a8c074f495fbfca1d7a39c3db4dd6d8 100644 --- a/frontend/src/metabase/public/containers/PublicQuestion/PublicQuestion.unit.spec.tsx +++ b/frontend/src/metabase/public/containers/PublicQuestion/PublicQuestion.unit.spec.tsx @@ -9,7 +9,7 @@ import registerVisualizations from "metabase/visualizations/register"; import { createMockState } from "metabase-types/store/mocks"; import { createMockPublicCard, - createMockPublicDataset, + createMockEmbedDataset, } from "metabase-types/api/mocks"; import { PublicQuestion } from "./PublicQuestion"; @@ -27,7 +27,7 @@ async function setup() { ); setupPublicCardQueryEndpoints( FAKE_UUID, - createMockPublicDataset({ + createMockEmbedDataset({ data: { rows: [["John W."]] }, }), ); diff --git a/frontend/test/__support__/server-mocks/public.ts b/frontend/test/__support__/server-mocks/public.ts index a0af23b86f993f66cb73396efb77f9d0b1221bdf..79a1645a303bf08e55c25c96b28477dfa44cc344 100644 --- a/frontend/test/__support__/server-mocks/public.ts +++ b/frontend/test/__support__/server-mocks/public.ts @@ -1,6 +1,6 @@ import fetchMock from "fetch-mock"; -import type { PublicCard, PublicDataset } from "metabase-types/api"; +import type { PublicCard, EmbedDataset } from "metabase-types/api"; export function setupPublicQuestionEndpoints( uuid: string, @@ -11,7 +11,7 @@ export function setupPublicQuestionEndpoints( export function setupPublicCardQueryEndpoints( uuid: string, - publicDataset: PublicDataset, + publicDataset: EmbedDataset, ) { fetchMock.get(`path:/api/public/card/${uuid}/query`, publicDataset); }