From fff6e35c3ea8c68cd6032a00516337e6dfdc6427 Mon Sep 17 00:00:00 2001
From: Alexander Polyankin <alexander.polyankin@metabase.com>
Date: Tue, 20 Aug 2024 07:49:10 -0400
Subject: [PATCH] Show an error message when the temporal unit is not
applicable to a dashboard card (#46983)
* Fix tests
* Add tests
* Add tests
* Fix types
* Fix types
---
.../temporal-unit-parameters.cy.spec.js | 44 +++++++++++++++++++
frontend/src/metabase-types/api/dataset.ts | 11 +++--
frontend/src/metabase/dashboard/utils.ts | 19 ++++----
.../src/metabase/dashboard/utils.unit.spec.ts | 38 ++++++++++++++++
4 files changed, 99 insertions(+), 13 deletions(-)
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 17faabcf419..ae5d502b25a 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 643c14330c5..9be021ff5bf 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 aaf7497dc13..7867dbc0c86 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 6ecdf1f1044..a3798b93b84 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", () => {
--
GitLab