From d2aeef85f117aeeaf551767977eb8e24422ed2f0 Mon Sep 17 00:00:00 2001 From: Emmad Usmani <emmadusmani@berkeley.edu> Date: Tue, 18 Jul 2023 12:38:01 -0700 Subject: [PATCH] fix error when archiving custom homepage dashboard (#32398) * fix error when archiving custom homepage dashboard * add e2e test case * fix type error * update unit test --- .../onboarding/home/homepage.cy.spec.js | 24 +++++++++++++++++ frontend/src/metabase-types/api/dashboard.ts | 1 + .../src/metabase-types/api/mocks/dashboard.ts | 1 + .../components/DashboardTabs/test-utils.ts | 1 + .../home/components/HomePage/HomePage.tsx | 26 ++++++++++++++++--- .../HomePage/HomePage.unit.spec.tsx | 12 ++++++--- 6 files changed, 57 insertions(+), 8 deletions(-) diff --git a/e2e/test/scenarios/onboarding/home/homepage.cy.spec.js b/e2e/test/scenarios/onboarding/home/homepage.cy.spec.js index 174f4da07b1..b7711700dfe 100644 --- a/e2e/test/scenarios/onboarding/home/homepage.cy.spec.js +++ b/e2e/test/scenarios/onboarding/home/homepage.cy.spec.js @@ -10,6 +10,7 @@ import { expectNoBadSnowplowEvents, resetSnowplow, enableTracking, + main, } from "e2e/support/helpers"; import { USERS } from "e2e/support/cypress_data"; @@ -294,6 +295,29 @@ describe("scenarios > home > custom homepage", () => { ).should("have.length", 1); }); }); + + it("should show the default homepage if the dashboard was archived (#31599)", () => { + // Archive dashboard + visitDashboard(1); + dashboardHeader().within(() => { + cy.findByLabelText("dashboard-menu-button").click(); + }); + popover().within(() => { + cy.findByText("Archive").click(); + }); + modal().within(() => { + cy.findByText("Archive").click(); + }); + + // Navigate to home + navigationSidebar().within(() => { + cy.findByText("Home").click(); + }); + main().within(() => { + cy.findByText("We're a little lost...").should("not.exist"); + cy.findByText("Customize").should("be.visible"); + }); + }); }); }); diff --git a/frontend/src/metabase-types/api/dashboard.ts b/frontend/src/metabase-types/api/dashboard.ts index 25237daa976..9742c4a0d6e 100644 --- a/frontend/src/metabase-types/api/dashboard.ts +++ b/frontend/src/metabase-types/api/dashboard.ts @@ -31,6 +31,7 @@ export interface Dashboard { timestamp: string; }; auto_apply_filters: boolean; + archived: boolean; } export type DashCardId = number; diff --git a/frontend/src/metabase-types/api/mocks/dashboard.ts b/frontend/src/metabase-types/api/mocks/dashboard.ts index bbe8d173e02..a6cd7665e0c 100644 --- a/frontend/src/metabase-types/api/mocks/dashboard.ts +++ b/frontend/src/metabase-types/api/mocks/dashboard.ts @@ -23,6 +23,7 @@ export const createMockDashboard = (opts?: Partial<Dashboard>): Dashboard => ({ timestamp: "2018-01-01", }, auto_apply_filters: true, + archived: false, ...opts, }); diff --git a/frontend/src/metabase/dashboard/components/DashboardTabs/test-utils.ts b/frontend/src/metabase/dashboard/components/DashboardTabs/test-utils.ts index e65f919008a..2b42bfa4984 100644 --- a/frontend/src/metabase/dashboard/components/DashboardTabs/test-utils.ts +++ b/frontend/src/metabase/dashboard/components/DashboardTabs/test-utils.ts @@ -50,6 +50,7 @@ export const TEST_DASHBOARD_STATE: DashboardState = { can_write: true, cache_ttl: null, auto_apply_filters: true, + archived: false, "last-edit-info": { id: 1, email: "", diff --git a/frontend/src/metabase/home/components/HomePage/HomePage.tsx b/frontend/src/metabase/home/components/HomePage/HomePage.tsx index c57b9d4a493..605037cb298 100644 --- a/frontend/src/metabase/home/components/HomePage/HomePage.tsx +++ b/frontend/src/metabase/home/components/HomePage/HomePage.tsx @@ -7,12 +7,13 @@ import { updateSetting } from "metabase/admin/settings/settings"; import { addUndo } from "metabase/redux/undo"; import { useDispatch, useSelector } from "metabase/lib/redux"; import { + useDashboardQuery, useDatabaseListQuery, useSearchListQuery, } from "metabase/common/hooks"; import LoadingAndErrorWrapper from "metabase/components/LoadingAndErrorWrapper"; import { canUseMetabotOnDatabase } from "metabase/metabot/utils"; -import { CollectionItem } from "metabase-types/api"; +import { CollectionItem, DashboardId } from "metabase-types/api"; import { getSettingsLoading } from "metabase/selectors/settings"; import Database from "metabase-lib/metadata/Database"; import { @@ -83,12 +84,22 @@ const getHasMetabot = ( const useDashboardPage = () => { const dashboardId = useSelector(getCustomHomePageDashboardId); - const isLoading = useSelector(getSettingsLoading); + const isLoadingSettings = useSelector(getSettingsLoading); const hasDismissedToast = useSelector(getHasDismissedCustomHomePageToast); const dispatch = useDispatch(); + const { data: dashboard, isLoading: isLoadingDash } = useDashboardQuery({ + enabled: dashboardId !== null, + id: dashboardId as DashboardId, + }); + useEffect(() => { - if (dashboardId && !isLoading) { + if ( + dashboardId && + !isLoadingSettings && + !isLoadingDash && + !dashboard?.archived + ) { dispatch(replace(`/dashboard/${dashboardId}`)); if (!hasDismissedToast) { @@ -109,5 +120,12 @@ const useDashboardPage = () => { ); } } - }, [dashboardId, isLoading, hasDismissedToast, dispatch]); + }, [ + dashboardId, + isLoadingSettings, + hasDismissedToast, + dispatch, + isLoadingDash, + dashboard?.archived, + ]); }; diff --git a/frontend/src/metabase/home/components/HomePage/HomePage.unit.spec.tsx b/frontend/src/metabase/home/components/HomePage/HomePage.unit.spec.tsx index 74bdc0ad2fc..17c7d7df23c 100644 --- a/frontend/src/metabase/home/components/HomePage/HomePage.unit.spec.tsx +++ b/frontend/src/metabase/home/components/HomePage/HomePage.unit.spec.tsx @@ -4,9 +4,10 @@ import { createMockSettingsState, createMockState, } from "metabase-types/store/mocks"; -import { createMockUser } from "metabase-types/api/mocks"; +import { createMockDashboard, createMockUser } from "metabase-types/api/mocks"; import { renderWithProviders, screen } from "__support__/ui"; import { + setupDashboardEndpoints, setupDatabasesEndpoints, setupPopularItemsEndpoints, setupRecentViewsEndpoints, @@ -23,7 +24,7 @@ interface SetupOpts { dashboardId?: DashboardId; } -const setup = ({ dashboardId }: SetupOpts = {}) => { +const setup = async ({ dashboardId }: SetupOpts = {}) => { const state = createMockState({ currentUser: createMockUser({ first_name: TEST_USER_NAME, @@ -38,6 +39,9 @@ const setup = ({ dashboardId }: SetupOpts = {}) => { setupSearchEndpoints([]); setupRecentViewsEndpoints([]); setupPopularItemsEndpoints([]); + if (dashboardId !== undefined) { + setupDashboardEndpoints(createMockDashboard({ id: dashboardId })); + } renderWithProviders( <> @@ -57,9 +61,9 @@ describe("HomePage", () => { expect(screen.getByText(new RegExp(TEST_USER_NAME))).toBeInTheDocument(); }); - it("should redirect you to a dashboard when one has been defined to be used as a homepage", () => { + it("should redirect you to a dashboard when one has been defined to be used as a homepage", async () => { setup({ dashboardId: 1 }); - expect(screen.getByText(TEST_DASHBOARD_NAME)).toBeInTheDocument(); + expect(await screen.findByText(TEST_DASHBOARD_NAME)).toBeInTheDocument(); }); it("should render the homepage when a custom dashboard is not set", () => { -- GitLab