diff --git a/e2e/test/scenarios/dashboard/dashboard.cy.spec.js b/e2e/test/scenarios/dashboard/dashboard.cy.spec.js index 65527ef703e97782f231399abcaa624b0993caad..e76106813daed57a861d06a1c486a6db9d8ebd97 100644 --- a/e2e/test/scenarios/dashboard/dashboard.cy.spec.js +++ b/e2e/test/scenarios/dashboard/dashboard.cy.spec.js @@ -602,6 +602,87 @@ describe("scenarios > dashboard", () => { popover().findByText("Edit question").click(); cy.findByRole("button", { name: "Visualize" }).should("be.visible"); }); + + it("should allow making card hide when it is empty", () => { + const FILTER_ID = "d7988e02"; + + cy.log("Add filter to the dashboard"); + cy.request("PUT", "/api/dashboard/1", { + parameters: [ + { + id: FILTER_ID, + name: "ID", + slug: "id", + type: "id", + }, + ], + }); + + cy.log("Connect filter to the existing card"); + cy.request("PUT", "/api/dashboard/1/cards", { + cards: [ + { + id: 1, + card_id: 1, + row: 0, + col: 0, + size_x: 12, + size_y: 8, + parameter_mappings: [ + { + parameter_id: FILTER_ID, + card_id: 1, + target: ["dimension", ["field", ORDERS.ID]], + }, + ], + visualization_settings: {}, + }, + ], + }); + + visitDashboard(1); + editDashboard(); + + cy.findByTestId("dashboardcard-actions-panel").within(() => { + cy.icon("palette").click({ force: true }); + }); + + cy.findByRole("dialog").within(() => { + cy.findByRole("switch", { + name: "Hide this card if there are no results", + }).click(); + cy.button("Done").click(); + }); + + saveDashboard(); + + // Verify the card is hidden when the value is correct but produces empty results + filterWidget().click(); + popover().within(() => { + cy.findByPlaceholderText("Enter an ID").type("-1{enter}"); + cy.button("Add filter").click(); + }); + + cy.findByTestId("dashcard").should("not.exist"); + + // Verify it becomes visible once the filter is cleared + filterWidget().within(() => { + cy.icon("close").click(); + }); + + cy.findByTestId("dashcard").findByText("Orders"); + + // Verify the card is visible when it returned an error + filterWidget().click(); + popover().within(() => { + cy.findByPlaceholderText("Enter an ID").type("text{enter}"); + cy.button("Add filter").click(); + }); + + cy.findByTestId("dashcard").within(() => { + cy.findByText("There was a problem displaying this chart."); + }); + }); }); function checkOptionsForFilter(filter) { diff --git a/frontend/src/metabase-types/api/dataset.ts b/frontend/src/metabase-types/api/dataset.ts index 07ee9ea44abf2636d71c45f64571d5be4174ac42..08f8487ad5b94b32417ec157a4f195a58a2e03be 100644 --- a/frontend/src/metabase-types/api/dataset.ts +++ b/frontend/src/metabase-types/api/dataset.ts @@ -59,7 +59,11 @@ export interface Dataset { row_count: number; running_time: number; json_query?: JsonQuery; - error?: string; + error_type?: string; + error?: { + status: number; // HTTP status code + data?: string; + }; context?: string; status?: string; } @@ -70,13 +74,7 @@ export interface NativeQueryForm { export type SingleSeries = { card: Card; - data: DatasetData; - error_type?: string; - error?: { - status: number; // HTTP status code - data?: string; - }; -}; +} & Dataset; export type RawSeries = SingleSeries[]; export type TransformedSeries = RawSeries & { _raw: Series }; diff --git a/frontend/src/metabase-types/api/mocks/series.ts b/frontend/src/metabase-types/api/mocks/series.ts index d049fc5424988ba573b1fc37092fa9f007f45a78..52b63c76e93e3f05e57e7970ef074ca868583212 100644 --- a/frontend/src/metabase-types/api/mocks/series.ts +++ b/frontend/src/metabase-types/api/mocks/series.ts @@ -9,7 +9,7 @@ export const createMockSingleSeries = ( ): SingleSeries => { return { card: createMockCard(cardOpts), - data: createMockDataset(dataOpts).data, + ...createMockDataset(dataOpts), }; }; diff --git a/frontend/src/metabase-types/api/query.ts b/frontend/src/metabase-types/api/query.ts index 204969fde4dde9b7b34ffe96969e1f331753e57e..596c0b1b35780f2835919a47029bb663cf41a040 100644 --- a/frontend/src/metabase-types/api/query.ts +++ b/frontend/src/metabase-types/api/query.ts @@ -27,6 +27,7 @@ export interface NativeDatasetQuery { // Database is null when missing data permissions to the database database: DatabaseId | null; + parameters?: unknown[]; } export type DatasetQuery = StructuredDatasetQuery | NativeDatasetQuery; diff --git a/frontend/src/metabase/dashboard/components/DashCard/DashCard.tsx b/frontend/src/metabase/dashboard/components/DashCard/DashCard.tsx index 574e6b55c8cb5b06c1f1f41956019f8a03638562..0656aef0d4444fd681eda87c5b62195d5027f82d 100644 --- a/frontend/src/metabase/dashboard/components/DashCard/DashCard.tsx +++ b/frontend/src/metabase/dashboard/components/DashCard/DashCard.tsx @@ -5,17 +5,15 @@ import type { LocationDescriptor } from "history"; import { useMount } from "react-use"; import { IconProps } from "metabase/components/Icon"; -import { IS_EMBED_PREVIEW } from "metabase/lib/embed"; -import { SERVER_ERROR_TYPES } from "metabase/lib/errors"; import Utils from "metabase/lib/utils"; -import { - getGenericErrorMessage, - getPermissionErrorMessage, -} from "metabase/visualizations/lib/errors"; import { mergeSettings } from "metabase/visualizations/lib/settings"; -import { isVirtualDashCard } from "metabase/dashboard/utils"; +import { + getDashcardResultsError, + isDashcardLoading, + isVirtualDashCard, +} from "metabase/dashboard/utils"; import { isActionCard } from "metabase/actions/utils"; @@ -24,14 +22,13 @@ import ErrorBoundary from "metabase/ErrorBoundary"; import type { Card, CardId, - DatasetData, Dashboard, DashboardOrderedCard, DashCardId, ParameterId, ParameterValueOrArray, - Series, VisualizationSettings, + Dataset, } from "metabase-types/api"; import { DASHBOARD_SLOW_TIMEOUT } from "metabase/dashboard/constants"; @@ -53,41 +50,12 @@ function preventDragging(event: React.SyntheticEvent) { event.stopPropagation(); } -function getSeriesError(series: Series) { - const isAccessRestricted = series.some( - s => - s.error_type === SERVER_ERROR_TYPES.missingPermissions || - s.error?.status === 403, - ); - - if (isAccessRestricted) { - return { - message: getPermissionErrorMessage(), - icon: "key", - }; - } - - const errors = series.map(s => s.error).filter(Boolean); - if (errors.length > 0) { - if (IS_EMBED_PREVIEW) { - const message = errors[0]?.data || getGenericErrorMessage(); - return { message, icon: "warning" }; - } - return { - message: getGenericErrorMessage(), - icon: "warning", - }; - } - - return; -} - interface DashCardProps { dashboard: Dashboard; dashcard: DashboardOrderedCard & { justAdded?: boolean }; gridItemWidth: number; totalNumGridCols: number; - dashcardData: Record<DashCardId, Record<CardId, DatasetData>>; + dashcardData: Record<DashCardId, Record<CardId, Dataset>>; slowCards: Record<CardId, boolean>; parameterValues: Record<ParameterId, ParameterValueOrArray>; metadata: Metadata; @@ -188,13 +156,10 @@ function DashCard({ })); }, [cards, dashcard.id, dashcardData, slowCards]); - const isLoading = useMemo(() => { - if (isVirtualDashCard(dashcard)) { - return false; - } - const hasSeries = series.length > 0 && series.every(s => s.data); - return !hasSeries; - }, [dashcard, series]); + const isLoading = useMemo( + () => isDashcardLoading(dashcard, dashcardData), + [dashcard, dashcardData], + ); const isAction = isActionCard(mainCard); const isEmbed = Utils.isJWT(dashcard.dashboard_id); @@ -211,7 +176,7 @@ function DashCard({ return { expectedDuration, isSlow }; }, [series, isLoading]); - const error = useMemo(() => getSeriesError(series), [series]); + const error = useMemo(() => getDashcardResultsError(series), [series]); const hasError = !!error; const parameterValuesBySlug = useMemo( diff --git a/frontend/src/metabase/dashboard/components/DashCard/DashCardMenu/DashCardMenu.unit.spec.tsx b/frontend/src/metabase/dashboard/components/DashCard/DashCardMenu/DashCardMenu.unit.spec.tsx index 8981d8c3a2f2fb1780356777d61f78299a55dcab..911296879a6e763a2d2025852a665ebf455ab37f 100644 --- a/frontend/src/metabase/dashboard/components/DashCard/DashCardMenu/DashCardMenu.unit.spec.tsx +++ b/frontend/src/metabase/dashboard/components/DashCard/DashCardMenu/DashCardMenu.unit.spec.tsx @@ -50,7 +50,10 @@ const TEST_CARD_UNAUTHORIZED = createMockCard({ const TEST_RESULT = createMockDataset(); const TEST_RESULT_ERROR = createMockDataset({ - error: "An error occurred", + error: { + status: 500, + data: "An error occurred", + }, }); interface SetupOpts { diff --git a/frontend/src/metabase/dashboard/components/DashboardGrid.jsx b/frontend/src/metabase/dashboard/components/DashboardGrid.jsx index b4682e02acce432158201a111c1022882f812c82..9194f14d7b464ba20e4cf3a1f04bb68c0f2650b5 100644 --- a/frontend/src/metabase/dashboard/components/DashboardGrid.jsx +++ b/frontend/src/metabase/dashboard/components/DashboardGrid.jsx @@ -15,6 +15,7 @@ import { PLUGIN_COLLECTIONS } from "metabase/plugins"; import { getVisualizationRaw } from "metabase/visualizations"; import * as MetabaseAnalytics from "metabase/lib/analytics"; import { color } from "metabase/lib/colors"; +import { getVisibleCardIds } from "metabase/dashboard/utils"; import { GRID_WIDTH, @@ -41,9 +42,14 @@ class DashboardGrid extends Component { constructor(props, context) { super(props, context); + const visibleCardIds = getVisibleCardIds( + props.dashboard.ordered_cards, + props.dashcardData, + ); + this.state = { - layouts: this.getLayouts(props), - dashcards: this.getSortedDashcards(props), + visibleCardIds, + layouts: this.getLayouts(props.dashboard.ordered_cards), addSeriesModalDashCard: null, isDragging: false, isAnimationPaused: true, @@ -90,25 +96,42 @@ class DashboardGrid extends Component { } UNSAFE_componentWillReceiveProps(nextProps) { + const { dashboard, dashcardData, isEditing } = nextProps; + + const visibleCardIds = !isEditing + ? getVisibleCardIds( + dashboard.ordered_cards, + dashcardData, + this.state.visibleCardIds, + ) + : new Set(dashboard.ordered_cards.map(card => card.id)); + + const cards = this.getVisibleCards( + dashboard.ordered_cards, + visibleCardIds, + isEditing, + ); + this.setState({ - dashcards: this.getSortedDashcards(nextProps), - layouts: this.getLayouts(nextProps), + visibleCardIds, + layouts: this.getLayouts(cards), }); } onLayoutChange = ({ layout, breakpoint }) => { + const { setMultipleDashCardAttributes, isEditing } = this.props; + // We allow moving and resizing cards only on the desktop // Ensures onLayoutChange triggered by window resize, // won't break the main layout - if (breakpoint !== "desktop") { + if (!isEditing || breakpoint !== "desktop") { return; } - const { dashboard, setMultipleDashCardAttributes } = this.props; const changes = []; layout.forEach(layoutItem => { - const dashboardCard = dashboard.ordered_cards.find( + const dashboardCard = this.getVisibleCards().find( card => String(card.id) === layoutItem.i, ); @@ -137,27 +160,6 @@ class DashboardGrid extends Component { } }; - getSortedDashcards(props) { - return ( - props.dashboard && - props.dashboard.ordered_cards.sort((a, b) => { - if (a.row < b.row) { - return -1; - } - if (a.row > b.row) { - return 1; - } - if (a.col < b.col) { - return -1; - } - if (a.col > b.col) { - return 1; - } - return 0; - }) - ); - } - getLayoutForDashCard(dashcard) { const { visualization } = getVisualizationRaw([{ card: dashcard.card }]); const initialSize = DEFAULT_CARD_SIZE; @@ -174,8 +176,18 @@ class DashboardGrid extends Component { }; } - getLayouts({ dashboard }) { - const desktop = dashboard.ordered_cards.map(this.getLayoutForDashCard); + getVisibleCards = ( + cards = this.props.dashboard.ordered_cards, + visibleCardIds = this.state.visibleCardIds, + isEditing = this.props.isEditing, + ) => { + return isEditing + ? cards + : cards.filter(card => visibleCardIds.has(card.id)); + }; + + getLayouts(cards) { + const desktop = cards.map(this.getLayoutForDashCard); const mobile = generateMobileLayout({ desktopLayout: desktop, defaultCardHeight: 6, @@ -349,7 +361,7 @@ class DashboardGrid extends Component { ); renderGrid() { - const { dashboard, width } = this.props; + const { width } = this.props; const { layouts } = this.state; const rowHeight = this.getRowHeight(); return ( @@ -370,7 +382,7 @@ class DashboardGrid extends Component { onDragStop={this.onDragStop} isEditing={this.isEditingLayout} compactType="vertical" - items={dashboard.ordered_cards} + items={this.getVisibleCards()} itemRenderer={this.renderGridItem} /> ); diff --git a/frontend/src/metabase/dashboard/utils.js b/frontend/src/metabase/dashboard/utils.js deleted file mode 100644 index cceafcf1eefb10a1402b4b0e64cdc551d657501c..0000000000000000000000000000000000000000 --- a/frontend/src/metabase/dashboard/utils.js +++ /dev/null @@ -1,136 +0,0 @@ -import _ from "underscore"; -import { t } from "ttag"; -import Utils from "metabase/lib/utils"; -import { - isDateParameter, - isNumberParameter, - isStringParameter, -} from "metabase-lib/parameters/utils/parameter-type"; -import Question from "metabase-lib/Question"; - -export function syncParametersAndEmbeddingParams(before, after) { - if (after.parameters && before.embedding_params) { - return Object.keys(before.embedding_params).reduce((memo, embedSlug) => { - const slugParam = _.find(before.parameters, param => { - return param.slug === embedSlug; - }); - if (slugParam) { - const slugParamId = slugParam && slugParam.id; - const newParam = _.findWhere(after.parameters, { id: slugParamId }); - if (newParam) { - memo[newParam.slug] = before.embedding_params[embedSlug]; - } - } - return memo; - }, {}); - } else { - return before.embedding_params; - } -} - -// This adds default properties and placeholder IDs for an inline dashboard -export function expandInlineDashboard(dashboard) { - return { - name: "", - parameters: [], - ...dashboard, - ordered_cards: dashboard.ordered_cards.map(dashcard => ({ - visualization_settings: {}, - parameter_mappings: [], - ...dashcard, - id: _.uniqueId("dashcard"), - card: expandInlineCard(dashcard.card), - series: (dashcard.series || []).map(card => expandInlineCard(card)), - })), - }; -} - -export function expandInlineCard(card) { - return { - name: "", - visualization_settings: {}, - ...card, - id: _.uniqueId("card"), - }; -} - -export function isVirtualDashCard(dashcard) { - return _.isObject(dashcard?.visualization_settings?.virtual_card); -} - -export function getVirtualCardType(dashcard) { - return dashcard?.visualization_settings?.virtual_card?.display; -} - -export function isLinkDashCard(dashcard) { - return getVirtualCardType(dashcard) === "link"; -} - -export function isNativeDashCard(dashcard) { - return dashcard.card && new Question(dashcard.card).isNative(); -} - -// For a virtual (text) dashcard without any parameters, returns a boolean indicating whether we should display the -// info text about parameter mapping in the card itself or as a tooltip. -export function showVirtualDashCardInfoText(dashcard, isMobile) { - if (isVirtualDashCard(dashcard)) { - return isMobile || dashcard.size_y > 2 || dashcard.size_x > 5; - } else { - return true; - } -} - -export function getNativeDashCardEmptyMappingText(parameter) { - if (isDateParameter(parameter)) { - return t`Add a date variable to this question to connect it to a dashboard filter.`; - } else if (isNumberParameter(parameter)) { - return t`Add a number variable to this question to connect it to a dashboard filter.`; - } else if (isStringParameter(parameter)) { - return t`Add a string variable to this question to connect it to a dashboard filter.`; - } else { - return t`Add a variable to this question to connect it to a dashboard filter.`; - } -} - -export function getAllDashboardCards(dashboard) { - const results = []; - if (dashboard) { - for (const dashcard of dashboard.ordered_cards) { - const cards = [dashcard.card].concat(dashcard.series || []); - results.push(...cards.map(card => ({ card, dashcard }))); - } - } - return results; -} - -export function hasDatabaseActionsEnabled(database) { - return database.settings?.["database-enable-actions"] ?? false; -} - -export function getDashboardType(id) { - if (id == null || typeof id === "object") { - // HACK: support inline dashboards - return "inline"; - } else if (Utils.isUUID(id)) { - return "public"; - } else if (Utils.isJWT(id)) { - return "embed"; - } else if (/\/auto\/dashboard/.test(id)) { - return "transient"; - } else { - return "normal"; - } -} - -export async function fetchDataOrError(dataPromise) { - try { - return await dataPromise; - } catch (error) { - return { error }; - } -} - -export function getDatasetQueryParams(datasetQuery = {}) { - const { type, query, native, parameters = [] } = datasetQuery; - return { type, query, native, parameters }; -} diff --git a/frontend/src/metabase/dashboard/utils.ts b/frontend/src/metabase/dashboard/utils.ts new file mode 100644 index 0000000000000000000000000000000000000000..594e5940588c43d7aa449666e124684c554743fd --- /dev/null +++ b/frontend/src/metabase/dashboard/utils.ts @@ -0,0 +1,266 @@ +import _ from "underscore"; +import { t } from "ttag"; +import Utils from "metabase/lib/utils"; +import { SERVER_ERROR_TYPES } from "metabase/lib/errors"; +import { + getGenericErrorMessage, + getPermissionErrorMessage, +} from "metabase/visualizations/lib/errors"; +import { IS_EMBED_PREVIEW } from "metabase/lib/embed"; +import { + Card, + CardId, + DashCardId, + Dashboard, + DashboardOrderedCard, + Database, + Dataset, + NativeDatasetQuery, + Parameter, + StructuredDatasetQuery, +} from "metabase-types/api"; +import Question from "metabase-lib/Question"; +import { + isDateParameter, + isNumberParameter, + isStringParameter, +} from "metabase-lib/parameters/utils/parameter-type"; + +export function syncParametersAndEmbeddingParams(before: any, after: any) { + if (after.parameters && before.embedding_params) { + return Object.keys(before.embedding_params).reduce((memo, embedSlug) => { + const slugParam = _.find(before.parameters, param => { + return param.slug === embedSlug; + }); + if (slugParam) { + const slugParamId = slugParam && slugParam.id; + const newParam = _.findWhere(after.parameters, { id: slugParamId }); + if (newParam) { + memo[newParam.slug] = before.embedding_params[embedSlug]; + } + } + return memo; + }, {} as any); + } else { + return before.embedding_params; + } +} + +// This adds default properties and placeholder IDs for an inline dashboard +export function expandInlineDashboard(dashboard: Partial<Dashboard>) { + return { + name: "", + parameters: [], + ...dashboard, + ordered_cards: dashboard.ordered_cards?.map(dashcard => ({ + visualization_settings: {}, + parameter_mappings: [], + ...dashcard, + id: _.uniqueId("dashcard"), + card: expandInlineCard(dashcard?.card), + series: ((dashcard as any).series || []).map((card: Card) => + expandInlineCard(card), + ), + })), + }; +} + +export function expandInlineCard(card?: Card) { + return { + name: "", + visualization_settings: {}, + ...card, + id: _.uniqueId("card"), + }; +} + +export function isVirtualDashCard(dashcard: DashboardOrderedCard) { + return _.isObject(dashcard?.visualization_settings?.virtual_card); +} + +export function getVirtualCardType(dashcard: DashboardOrderedCard) { + return dashcard?.visualization_settings?.virtual_card?.display; +} + +export function isLinkDashCard(dashcard: DashboardOrderedCard) { + return getVirtualCardType(dashcard) === "link"; +} + +export function isNativeDashCard(dashcard: DashboardOrderedCard) { + return dashcard.card && new Question(dashcard.card).isNative(); +} + +// For a virtual (text) dashcard without any parameters, returns a boolean indicating whether we should display the +// info text about parameter mapping in the card itself or as a tooltip. +export function showVirtualDashCardInfoText( + dashcard: DashboardOrderedCard, + isMobile: boolean, +) { + if (isVirtualDashCard(dashcard)) { + return isMobile || dashcard.size_y > 2 || dashcard.size_x > 5; + } else { + return true; + } +} + +export function getNativeDashCardEmptyMappingText(parameter: Parameter) { + if (isDateParameter(parameter)) { + return t`Add a date variable to this question to connect it to a dashboard filter.`; + } else if (isNumberParameter(parameter)) { + return t`Add a number variable to this question to connect it to a dashboard filter.`; + } else if (isStringParameter(parameter)) { + return t`Add a string variable to this question to connect it to a dashboard filter.`; + } else { + return t`Add a variable to this question to connect it to a dashboard filter.`; + } +} + +export function getAllDashboardCards(dashboard: Dashboard) { + const results = []; + if (dashboard) { + for (const dashcard of dashboard.ordered_cards) { + const cards = [dashcard.card].concat((dashcard as any).series || []); + results.push(...cards.map(card => ({ card, dashcard }))); + } + } + return results; +} + +export function hasDatabaseActionsEnabled(database: Database) { + return database.settings?.["database-enable-actions"] ?? false; +} + +export function getDashboardType(id: unknown) { + if (id == null || typeof id === "object") { + // HACK: support inline dashboards + return "inline"; + } else if (Utils.isUUID(id)) { + return "public"; + } else if (Utils.isJWT(id)) { + return "embed"; + } else if (typeof id === "string" && /\/auto\/dashboard/.test(id)) { + return "transient"; + } else { + return "normal"; + } +} + +export async function fetchDataOrError<T>(dataPromise: Promise<T>) { + try { + return await dataPromise; + } catch (error) { + return { error }; + } +} + +export function getDatasetQueryParams( + datasetQuery: Partial<StructuredDatasetQuery> & + Partial<NativeDatasetQuery> = {}, +) { + const { type, query, native, parameters = [] } = datasetQuery; + return { type, query, native, parameters }; +} + +export function isDashcardLoading( + dashcard: DashboardOrderedCard, + dashcardsData: Record<DashCardId, Record<CardId, Dataset | null>>, +) { + if (isVirtualDashCard(dashcard)) { + return false; + } + + if (dashcardsData[dashcard.id] == null) { + return true; + } + + const cardData = Object.values(dashcardsData[dashcard.id]); + return cardData.length === 0 || cardData.some(data => data == null); +} + +export function getDashcardResultsError(datasets: Dataset[]) { + const isAccessRestricted = datasets.some( + s => + s.error_type === SERVER_ERROR_TYPES.missingPermissions || + s.error?.status === 403, + ); + + if (isAccessRestricted) { + return { + message: getPermissionErrorMessage(), + icon: "key", + }; + } + + 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" }; + } + return { + message: getGenericErrorMessage(), + icon: "warning", + }; + } + + return; +} + +const isDashcardDataLoaded = ( + data?: Record<CardId, Dataset | null>, +): data is Record<CardId, Dataset> => { + return data != null && Object.values(data).every(result => result != null); +}; + +const hasRows = (dashcardData: Record<CardId, Dataset>) => { + const queryResults = dashcardData + ? Object.values(dashcardData).filter(Boolean) + : []; + + return ( + queryResults.length > 0 && + queryResults.every(queryResult => queryResult.data.rows.length > 0) + ); +}; + +const shouldHideCard = ( + dashcard: DashboardOrderedCard, + dashcardData: Record<CardId, Dataset | null>, + wasVisible: boolean, +) => { + const shouldHideEmpty = dashcard.visualization_settings?.["card.hide_empty"]; + + if (isVirtualDashCard(dashcard) || !shouldHideEmpty) { + return false; + } + + const isLoading = !isDashcardDataLoaded(dashcardData); + + if (isLoading) { + return !wasVisible; + } + + return ( + !hasRows(dashcardData) && + !getDashcardResultsError(Object.values(dashcardData)) + ); +}; + +export const getVisibleCardIds = ( + cards: DashboardOrderedCard[], + dashcardsData: Record<DashCardId, Record<CardId, Dataset | null>>, + prevVisibleCardIds = new Set<number>(), +) => { + return new Set( + cards + .filter( + card => + !shouldHideCard( + card, + dashcardsData[card.id], + prevVisibleCardIds.has(card.id), + ), + ) + .map(c => c.id), + ); +}; diff --git a/frontend/src/metabase/dashboard/utils.unit.spec.js b/frontend/src/metabase/dashboard/utils.unit.spec.js deleted file mode 100644 index 394b5255193420dd9ac8fd5367c7d63509b0c395..0000000000000000000000000000000000000000 --- a/frontend/src/metabase/dashboard/utils.unit.spec.js +++ /dev/null @@ -1,123 +0,0 @@ -import { - fetchDataOrError, - hasDatabaseActionsEnabled, - syncParametersAndEmbeddingParams, -} from "metabase/dashboard/utils"; -import { createMockDatabase } from "metabase-types/api/mocks"; - -const ENABLED_ACTIONS_DATABASE = createMockDatabase({ - id: 1, - settings: { "database-enable-actions": true }, -}); -const DISABLED_ACTIONS_DATABASE = createMockDatabase({ - id: 2, - settings: { "database-enable-actions": false }, -}); -const NO_ACTIONS_DATABASE = createMockDatabase({ id: 3 }); - -describe("Dashboard utils", () => { - describe("fetchDataOrError()", () => { - it("should return data on successful fetch", async () => { - const data = { - series: [1, 2, 3], - }; - - const successfulFetch = Promise.resolve(data); - - const result = await fetchDataOrError(successfulFetch); - - expect(result.error).toBeUndefined(); - expect(result).toEqual(data); - }); - - it("should return map with error key on failed fetch", async () => { - const error = { - status: 504, - statusText: "GATEWAY_TIMEOUT", - data: { - message: - "Failed to load resource: the server responded with a status of 504 (GATEWAY_TIMEOUT)", - }, - }; - - const failedFetch = Promise.reject(error); - - const result = await fetchDataOrError(failedFetch); - expect(result.error).toEqual(error); - }); - - it("should return true if a database has model actions enabled", () => { - expect(hasDatabaseActionsEnabled(ENABLED_ACTIONS_DATABASE)).toBe(true); - }); - - it("should return false if a database does not have model actions enabled or is undefined", () => { - expect(hasDatabaseActionsEnabled(DISABLED_ACTIONS_DATABASE)).toBe(false); - expect(hasDatabaseActionsEnabled(NO_ACTIONS_DATABASE)).toBe(false); - }); - - it("should return true if any database has actions enabled", () => { - const databases = [ - ENABLED_ACTIONS_DATABASE, - DISABLED_ACTIONS_DATABASE, - NO_ACTIONS_DATABASE, - ]; - - const result = databases.some(hasDatabaseActionsEnabled); - expect(result).toBe(true); - }); - - it("should return false if all databases have actions disabled", () => { - const databases = [DISABLED_ACTIONS_DATABASE, NO_ACTIONS_DATABASE]; - - const result = databases.some(hasDatabaseActionsEnabled); - expect(result).toBe(false); - }); - }); - - describe("syncParametersAndEmbeddingParams", () => { - it("should rename `embedding_parameters` that are renamed in `parameters`", () => { - const before = { - embedding_params: { id: "required" }, - parameters: [{ slug: "id", id: "unique-param-id" }], - }; - const after = { - parameters: [{ slug: "new_id", id: "unique-param-id" }], - }; - - const expectation = { new_id: "required" }; - - const result = syncParametersAndEmbeddingParams(before, after); - expect(result).toEqual(expectation); - }); - - it("should remove `embedding_parameters` that are removed from `parameters`", () => { - const before = { - embedding_params: { id: "required" }, - parameters: [{ slug: "id", id: "unique-param-id" }], - }; - const after = { - parameters: [], - }; - - const expectation = {}; - - const result = syncParametersAndEmbeddingParams(before, after); - expect(result).toEqual(expectation); - }); - - it("should not change `embedding_parameters` when `parameters` hasn't changed", () => { - const before = { - embedding_params: { id: "required" }, - parameters: [{ slug: "id", id: "unique-param-id" }], - }; - const after = { - parameters: [{ slug: "id", id: "unique-param-id" }], - }; - - const expectation = { id: "required" }; - - const result = syncParametersAndEmbeddingParams(before, after); - expect(result).toEqual(expectation); - }); - }); -}); diff --git a/frontend/src/metabase/dashboard/utils.unit.spec.ts b/frontend/src/metabase/dashboard/utils.unit.spec.ts new file mode 100644 index 0000000000000000000000000000000000000000..8f17aa9abc1208aa84ee9b7f664ef644ae2de627 --- /dev/null +++ b/frontend/src/metabase/dashboard/utils.unit.spec.ts @@ -0,0 +1,287 @@ +import { + fetchDataOrError, + getDashcardResultsError, + getVisibleCardIds, + hasDatabaseActionsEnabled, + isDashcardLoading, + syncParametersAndEmbeddingParams, +} from "metabase/dashboard/utils"; +import { + createMockDashboardCardWithVirtualCard, + createMockDashboardOrderedCard, + createMockDatabase, + createMockDataset, + createMockDatasetData, +} from "metabase-types/api/mocks"; +import { SERVER_ERROR_TYPES } from "metabase/lib/errors"; + +const ENABLED_ACTIONS_DATABASE = createMockDatabase({ + id: 1, + settings: { "database-enable-actions": true }, +}); +const DISABLED_ACTIONS_DATABASE = createMockDatabase({ + id: 2, + settings: { "database-enable-actions": false }, +}); +const NO_ACTIONS_DATABASE = createMockDatabase({ id: 3 }); + +describe("Dashboard utils", () => { + describe("fetchDataOrError()", () => { + it("should return data on successful fetch", async () => { + const data = { + series: [1, 2, 3], + }; + + const successfulFetch = Promise.resolve(data); + + const result = (await fetchDataOrError(successfulFetch)) as any; + + expect(result.error).toBeUndefined(); + expect(result).toEqual(data); + }); + + it("should return map with error key on failed fetch", async () => { + const error = { + status: 504, + statusText: "GATEWAY_TIMEOUT", + data: { + message: + "Failed to load resource: the server responded with a status of 504 (GATEWAY_TIMEOUT)", + }, + }; + + const failedFetch = Promise.reject(error); + + const result = await fetchDataOrError(failedFetch); + expect(result.error).toEqual(error); + }); + + it("should return true if a database has model actions enabled", () => { + expect(hasDatabaseActionsEnabled(ENABLED_ACTIONS_DATABASE)).toBe(true); + }); + + it("should return false if a database does not have model actions enabled or is undefined", () => { + expect(hasDatabaseActionsEnabled(DISABLED_ACTIONS_DATABASE)).toBe(false); + expect(hasDatabaseActionsEnabled(NO_ACTIONS_DATABASE)).toBe(false); + }); + + it("should return true if any database has actions enabled", () => { + const databases = [ + ENABLED_ACTIONS_DATABASE, + DISABLED_ACTIONS_DATABASE, + NO_ACTIONS_DATABASE, + ]; + + const result = databases.some(hasDatabaseActionsEnabled); + expect(result).toBe(true); + }); + + it("should return false if all databases have actions disabled", () => { + const databases = [DISABLED_ACTIONS_DATABASE, NO_ACTIONS_DATABASE]; + + const result = databases.some(hasDatabaseActionsEnabled); + expect(result).toBe(false); + }); + }); + + describe("syncParametersAndEmbeddingParams", () => { + it("should rename `embedding_parameters` that are renamed in `parameters`", () => { + const before = { + embedding_params: { id: "required" }, + parameters: [{ slug: "id", id: "unique-param-id" }], + }; + const after = { + parameters: [{ slug: "new_id", id: "unique-param-id" }], + }; + + const expectation = { new_id: "required" }; + + const result = syncParametersAndEmbeddingParams(before, after); + expect(result).toEqual(expectation); + }); + + it("should remove `embedding_parameters` that are removed from `parameters`", () => { + const before = { + embedding_params: { id: "required" }, + parameters: [{ slug: "id", id: "unique-param-id" }], + }; + const after = { + parameters: [], + }; + + const expectation = {}; + + const result = syncParametersAndEmbeddingParams(before, after); + expect(result).toEqual(expectation); + }); + + it("should not change `embedding_parameters` when `parameters` hasn't changed", () => { + const before = { + embedding_params: { id: "required" }, + parameters: [{ slug: "id", id: "unique-param-id" }], + }; + const after = { + parameters: [{ slug: "id", id: "unique-param-id" }], + }; + + const expectation = { id: "required" }; + + const result = syncParametersAndEmbeddingParams(before, after); + expect(result).toEqual(expectation); + }); + }); + + describe("isDashcardLoading", () => { + it("should return false for virtual cards", () => { + expect( + isDashcardLoading(createMockDashboardCardWithVirtualCard(), {}), + ).toBe(false); + }); + + it("should return false for cards with loaded data", () => { + expect( + isDashcardLoading(createMockDashboardOrderedCard({ id: 1 }), { + 1: { 2: createMockDataset() }, + }), + ).toBe(false); + }); + + it("should return true when the dash card data is missing", () => { + expect( + isDashcardLoading(createMockDashboardOrderedCard({ id: 1 }), { + 1: { 2: null, 3: createMockDataset() }, + }), + ).toBe(true); + }); + }); + + describe("getDashcardResultsError", () => { + const expectedPermissionError = { + icon: "key", + message: "Sorry, you don't have permission to see this card.", + }; + + const expectedGenericError = { + icon: "warning", + message: "There was a problem displaying this chart.", + }; + + it("should return the access restricted error when the error type is missing-required-permissions", () => { + const error = getDashcardResultsError([ + createMockDataset({ + error_type: SERVER_ERROR_TYPES.missingPermissions, + }), + ]); + + expect(error).toStrictEqual(expectedPermissionError); + }); + + it("should return the access restricted error when the status code is 403", () => { + const error = getDashcardResultsError([ + createMockDataset({ + error: { + status: 403, + }, + }), + ]); + + expect(error).toStrictEqual(expectedPermissionError); + }); + + it("should return a generic error if a dataset has an error", () => { + const error = getDashcardResultsError([ + createMockDataset({}), + createMockDataset({ + error: { + status: 401, + }, + }), + ]); + + expect(error).toStrictEqual(expectedGenericError); + }); + + it("should not return any errors if there are no any errors", () => { + const error = getDashcardResultsError([createMockDataset({})]); + + expect(error).toBeUndefined(); + }); + }); + + describe("getVisibleCardIds", () => { + const virtualCardId = 1; + const virtualCard = createMockDashboardCardWithVirtualCard({ + id: virtualCardId, + }); + + const normalCardId = 2; + const normalCard = createMockDashboardOrderedCard({ id: normalCardId }); + + const hidingWhenEmptyCardId = 3; + const hidingWhenEmptyCard = createMockDashboardOrderedCard({ + id: hidingWhenEmptyCardId, + visualization_settings: { "card.hide_empty": true }, + }); + + const loadingData = { + [normalCardId]: { + 100: null, + }, + [hidingWhenEmptyCardId]: { + 200: null, + }, + }; + + const loadedEmptyData = { + [normalCardId]: { + 100: createMockDataset(), + }, + [hidingWhenEmptyCardId]: { + 200: createMockDataset(), + }, + }; + + const loadedWithData = { + [normalCardId]: { + 100: createMockDataset({ + data: createMockDatasetData({ rows: [[1]] }), + }), + }, + [hidingWhenEmptyCardId]: { + 200: createMockDataset({ + data: createMockDatasetData({ rows: [[1]] }), + }), + }, + }; + + const cards = [virtualCard, normalCard, hidingWhenEmptyCard]; + + it("when loading and no cards previously were visible it should show only virtual and normal cards", () => { + const visibleIds = getVisibleCardIds(cards, loadingData); + expect(visibleIds).toStrictEqual(new Set([virtualCardId, normalCardId])); + }); + + it("when loading and cards were visible it should show all of cards", () => { + const visibleIds = getVisibleCardIds( + cards, + loadingData, + new Set([virtualCardId, normalCardId, hidingWhenEmptyCardId]), + ); + expect(visibleIds).toStrictEqual( + new Set([virtualCardId, normalCardId, hidingWhenEmptyCardId]), + ); + }); + + it("when loaded empty it should show only virtual and normal cards", () => { + const visibleIds = getVisibleCardIds(cards, loadedEmptyData); + expect(visibleIds).toStrictEqual(new Set([virtualCardId, normalCardId])); + }); + + it("when loaded with data it should show all of cards", () => { + const visibleIds = getVisibleCardIds(cards, loadedWithData); + expect(visibleIds).toStrictEqual( + new Set([virtualCardId, normalCardId, hidingWhenEmptyCardId]), + ); + }); + }); +}); diff --git a/frontend/src/metabase/visualizations/components/ChartCaption.unit.spec.tsx b/frontend/src/metabase/visualizations/components/ChartCaption.unit.spec.tsx index ac6a927567bc5975cd1f80e7810e17261135531b..4e445a1d4bbe423419973270042e2edfba7a48fc 100644 --- a/frontend/src/metabase/visualizations/components/ChartCaption.unit.spec.tsx +++ b/frontend/src/metabase/visualizations/components/ChartCaption.unit.spec.tsx @@ -7,7 +7,7 @@ import { Card, Series } from "metabase-types/api"; import { createMockCard, createMockColumn, - createMockDatasetData, + createMockDataset, } from "metabase-types/api/mocks"; import ChartCaption from "./ChartCaption"; @@ -29,10 +29,12 @@ const getSeries = ({ card }: { card?: Card } = {}): Series => { const series: Series = [ { card: card ?? createMockCard({ name: "" }), - data: createMockDatasetData({ - cols, - rows: [["foo", 1]], - rows_truncated: 0, + ...createMockDataset({ + data: { + rows: [["foo", 1]], + cols, + rows_truncated: 0, + }, }), }, ]; diff --git a/frontend/src/metabase/visualizations/lib/settings/visualization.js b/frontend/src/metabase/visualizations/lib/settings/visualization.js index d936864c14ad0217bf01bc64912c3fe5945d8b1e..6f03738617743dd81f26be7324d64c770776773c 100644 --- a/frontend/src/metabase/visualizations/lib/settings/visualization.js +++ b/frontend/src/metabase/visualizations/lib/settings/visualization.js @@ -1,5 +1,6 @@ import { t } from "ttag"; import { getVisualizationRaw } from "metabase/visualizations"; +import { isVirtualDashCard } from "metabase/dashboard/utils"; import { normalizeFieldRef } from "metabase-lib/queries/utils/dataset"; import { getComputedSettings, @@ -23,6 +24,13 @@ const COMMON_SETTINGS = { dashboard: true, useRawSeries: true, }, + "card.hide_empty": { + title: t`Hide this card if there are no results`, + widget: "toggle", + inline: true, + dashboard: true, + getHidden: ([{ card }]) => isVirtualDashCard(card), + }, click_behavior: {}, };