diff --git a/e2e/test/scenarios/dashboard-filters/temporal-unit-parameters.cy.spec.js b/e2e/test/scenarios/dashboard-filters/temporal-unit-parameters.cy.spec.js index 17faabcf41923a9e3f126bca2b00d4fba90775ce..ae5d502b25ac6fc4e71e2989f948fc40e5bb7810 100644 --- a/e2e/test/scenarios/dashboard-filters/temporal-unit-parameters.cy.spec.js +++ b/e2e/test/scenarios/dashboard-filters/temporal-unit-parameters.cy.spec.js @@ -187,6 +187,22 @@ const nativeUnitQuestionDetails = { }, }; +const nativeTimeQuestionDetails = { + name: "SQL time", + display: "table", + native: { + query: "SELECT CAST('10:00' AS TIME) AS TIME", + }, +}; + +const getNativeTimeQuestionBasedQuestionDetails = card => ({ + query: { + "source-table": `card__${card.id}`, + aggregation: [["count"]], + breakout: [["field", "TIME", { "base-type": "type/Time" }]], + }, +}); + const parameterDetails = { id: "1", name: "Unit of Time", @@ -857,6 +873,34 @@ describe("scenarios > dashboard > temporal unit parameters", () => { resetFilterWidgetToDefault(); getDashboardCard().findByText("Created At: Year").should("be.visible"); }); + + it("should show an error message when an incompatible temporal unit is used", () => { + cy.log("setup dashboard with a time column"); + createNativeQuestion(nativeTimeQuestionDetails).then(({ body: card }) => { + cy.createDashboardWithQuestions({ + questions: [getNativeTimeQuestionBasedQuestionDetails(card)], + }).then(({ dashboard }) => { + visitDashboard(dashboard.id); + }); + }); + editDashboard(); + addTemporalUnitParameter(); + selectDashboardFilter(getDashboardCard(), "TIME"); + saveDashboard(); + + cy.log("use an invalid temporal unit"); + filterWidget().click(); + popover().findByText("Year").click(); + getDashboardCard().should( + "contain.text", + "This chart can not be broken out by the selected unit of time: year.", + ); + + cy.log("use an valid temporal unit"); + filterWidget().click(); + popover().findByText("Minute").click(); + getDashboardCard().findByText("TIME: Minute").should("be.visible"); + }); }); describe("query string parameters", () => { diff --git a/frontend/src/metabase-types/api/dataset.ts b/frontend/src/metabase-types/api/dataset.ts index 643c14330c550b6f9fe5d761a2abaf29271022d4..9be021ff5bf8e989f08a8ef58cfe502160ccabe8 100644 --- a/frontend/src/metabase-types/api/dataset.ts +++ b/frontend/src/metabase-types/api/dataset.ts @@ -74,11 +74,14 @@ export interface Dataset { row_count: number; running_time: number; json_query?: JsonQuery; + error?: + | string + | { + status: number; // HTTP status code + data?: string; + }; error_type?: string; - error?: { - status: number; // HTTP status code - data?: string; - }; + error_is_curated?: boolean; context?: string; status?: string; } diff --git a/frontend/src/metabase/dashboard/utils.ts b/frontend/src/metabase/dashboard/utils.ts index aaf7497dc1364ef7de7b231da2617b47a4d247be..7867dbc0c86497e55bb1adf9fdef4e2b09c67eb0 100644 --- a/frontend/src/metabase/dashboard/utils.ts +++ b/frontend/src/metabase/dashboard/utils.ts @@ -1,7 +1,6 @@ import type { Location } from "history"; import _ from "underscore"; -import { IS_EMBED_PREVIEW } from "metabase/lib/embed"; import { SERVER_ERROR_TYPES } from "metabase/lib/errors"; import { isJWT } from "metabase/lib/utils"; import { isUuid } from "metabase/lib/uuid"; @@ -207,7 +206,7 @@ export function getDashcardResultsError(datasets: Dataset[]) { const isAccessRestricted = datasets.some( s => s.error_type === SERVER_ERROR_TYPES.missingPermissions || - s.error?.status === 403, + (typeof s.error === "object" && s.error?.status === 403), ); if (isAccessRestricted) { @@ -217,14 +216,16 @@ export function getDashcardResultsError(datasets: Dataset[]) { }; } - const errors = datasets.map(s => s.error).filter(Boolean); - if (errors.length > 0) { - if (IS_EMBED_PREVIEW) { - const message = errors[0]?.data || getGenericErrorMessage(); - return { message, icon: "warning" as const }; - } + if (datasets.some(dataset => dataset.error)) { + const curatedErrorDataset = datasets.find( + dataset => dataset.error && dataset.error_is_curated, + ); + return { - message: getGenericErrorMessage(), + message: + typeof curatedErrorDataset?.error === "string" + ? curatedErrorDataset.error + : getGenericErrorMessage(), icon: "warning" as const, }; } diff --git a/frontend/src/metabase/dashboard/utils.unit.spec.ts b/frontend/src/metabase/dashboard/utils.unit.spec.ts index 6ecdf1f104499616942ea9a77d337831f79d55db..a3798b93b84c123afcc87d7003729c237d275f79 100644 --- a/frontend/src/metabase/dashboard/utils.unit.spec.ts +++ b/frontend/src/metabase/dashboard/utils.unit.spec.ts @@ -213,11 +213,49 @@ describe("Dashboard utils", () => { expect(error).toStrictEqual(expectedGenericError); }); + it("should return a curated error in case it is set in the response", () => { + const error = getDashcardResultsError([ + createMockDataset({}), + createMockDataset({ + error: "Wrong query", + error_is_curated: true, + }), + ]); + + expect(error).toEqual({ + icon: "warning", + message: "Wrong query", + }); + }); + + it("should return a generic error in case the error is curated but is not a string", () => { + const error = getDashcardResultsError([ + createMockDataset({}), + createMockDataset({ + error: { status: 500 }, + error_is_curated: true, + }), + ]); + + expect(error).toEqual(expectedGenericError); + }); + it("should not return any errors if there are no any errors", () => { const error = getDashcardResultsError([createMockDataset({})]); expect(error).toBeUndefined(); }); + + it("should not return any errors if the error is curated but there is no error message or object set", () => { + const error = getDashcardResultsError([ + createMockDataset({ + error: undefined, + error_is_curated: true, + }), + ]); + + expect(error).toBeUndefined(); + }); }); describe("getVisibleCardIds", () => {