diff --git a/e2e/support/helpers/e2e-dashboard-helpers.ts b/e2e/support/helpers/e2e-dashboard-helpers.ts index 582964ffc8325f183a2c82e2f6acc6188e4b8ac5..5fde23073ca81a21fd9469f11847b201aac7e22e 100644 --- a/e2e/support/helpers/e2e-dashboard-helpers.ts +++ b/e2e/support/helpers/e2e-dashboard-helpers.ts @@ -266,6 +266,13 @@ export function openDashboardInfoSidebar() { export function closeDashboardInfoSidebar() { sidesheet().findByLabelText("Close").click(); } +export const openDashboardSettingsSidebar = () => { + dashboardHeader().icon("ellipsis").click(); + popover().findByText("Edit settings").click(); +}; +export const closeDashboardSettingsSidebar = () => { + sidesheet().findByLabelText("Close").click(); +}; export function openDashboardMenu() { dashboardHeader().findByLabelText("Move, trash, and more…").click(); diff --git a/e2e/test/scenarios/admin/performance/helpers/e2e-strategy-form-helpers.ts b/e2e/test/scenarios/admin/performance/helpers/e2e-strategy-form-helpers.ts index 2570c11f6121847555d52feb586a46ec141bebb8..8e16a7b636f1e1a823677f4b457ee469043dcca7 100644 --- a/e2e/test/scenarios/admin/performance/helpers/e2e-strategy-form-helpers.ts +++ b/e2e/test/scenarios/admin/performance/helpers/e2e-strategy-form-helpers.ts @@ -88,14 +88,12 @@ export const getScheduleComponent = (componentType: ScheduleComponentType) => export const openSidebar = (type: "question" | "dashboard") => { // this will change when we move to having a dashboard settings sidesheet if (type === "dashboard") { - cy.icon("info").click(); - return; + cy.findByTestId("dashboard-header").icon("ellipsis").click(); } if (type === "question") { cy.findByTestId("qb-header").icon("ellipsis").click(); } - popover().findByText("Edit settings").click(); }; @@ -111,7 +109,7 @@ export const openSidebarCacheStrategyForm = ( cy.log("Open the cache strategy form in the sidebar"); openSidebar(type); cy.wait("@getCacheConfig"); - cy.findByLabelText("Caching policy").click(); + cy.findByLabelText("When to get new results").click(); }; export const cancelConfirmationModal = () => { diff --git a/e2e/test/scenarios/dashboard-filters/dashboard-filters-auto-apply.cy.spec.js b/e2e/test/scenarios/dashboard-filters/dashboard-filters-auto-apply.cy.spec.js index ad97648a01ac801f5868cdc0d862867426c9743f..3d119f020c08a83f92d486498debb612ea400430 100644 --- a/e2e/test/scenarios/dashboard-filters/dashboard-filters-auto-apply.cy.spec.js +++ b/e2e/test/scenarios/dashboard-filters/dashboard-filters-auto-apply.cy.spec.js @@ -1,6 +1,7 @@ import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database"; import { - closeDashboardInfoSidebar, + closeDashboardSettingsSidebar, + dashboardHeader, dashboardParametersContainer, describeWithSnowplow, editDashboard, @@ -9,7 +10,7 @@ import { expectNoBadSnowplowEvents, filterWidget, getDashboardCard, - openDashboardInfoSidebar, + openDashboardSettingsSidebar, popover, resetSnowplow, restore, @@ -94,13 +95,13 @@ describe( cy.log( "parameter values should be preserved when disabling auto applying filters", ); - openDashboardInfoSidebar(); + openDashboardSettingsSidebar(); sidesheet().within(() => { cy.findByText(filterToggleLabel).click(); cy.wait("@updateDashboard"); cy.findByLabelText(filterToggleLabel).should("not.be.checked"); }); - closeDashboardInfoSidebar(); + closeDashboardSettingsSidebar(); filterWidget().findByText("Gadget").should("be.visible"); getDashboardCard().findByText("Rows 1-4 of 53").should("be.visible"); @@ -129,13 +130,13 @@ describe( filterWidget().findByText("Widget").should("be.visible"); dashboardParametersContainer().button("Apply").should("be.visible"); - openDashboardInfoSidebar(); + openDashboardSettingsSidebar(); sidesheet().within(() => { cy.findByText(filterToggleLabel).click(); cy.wait("@updateDashboard"); cy.findByLabelText(filterToggleLabel).should("be.checked"); }); - closeDashboardInfoSidebar(); + closeDashboardSettingsSidebar(); filterWidget().findByText("Widget").should("be.visible"); getDashboardCard().findByText("Rows 1-4 of 54").should("be.visible"); @@ -150,13 +151,13 @@ describe( cy.button("Update filter").click(); }); - openDashboardInfoSidebar(); + openDashboardSettingsSidebar(); sidesheet().within(() => { cy.findByText(filterToggleLabel).click(); cy.wait("@updateDashboard"); cy.findByLabelText(filterToggleLabel).should("not.be.checked"); }); - closeDashboardInfoSidebar(); + closeDashboardSettingsSidebar(); filterWidget().findByText("2 selections").should("be.visible"); cy.get("@cardQuery.all").should("have.length", 5); @@ -240,20 +241,20 @@ describe( it("should handle toggling auto applying filters on and off", () => { openDashboard(); - openDashboardInfoSidebar(); getDashboardCard().findByText("Rows 1-4 of 53").should("be.visible"); cy.log( "parameter with default value should still be applied after turning auto-apply filter off", ); + openDashboardSettingsSidebar(); sidesheet().within(() => { cy.findByLabelText(filterToggleLabel).should("be.checked"); cy.findByText(filterToggleLabel).click(); cy.wait("@updateDashboard"); cy.findByLabelText(filterToggleLabel).should("not.be.checked"); }); - closeDashboardInfoSidebar(); + closeDashboardSettingsSidebar(); getDashboardCard().findByText("Rows 1-4 of 53").should("be.visible"); @@ -271,7 +272,7 @@ describe( cy.log( "should not use the default parameter after turning auto-apply filter on again since the parameter was manually updated", ); - openDashboardInfoSidebar(); + openDashboardSettingsSidebar(); sidesheet().within(() => { cy.findByLabelText(filterToggleLabel).should("not.be.checked"); cy.findAllByText(filterToggleLabel).click(); @@ -294,7 +295,7 @@ describe( cy.wait("@updateDashboard"); }); - openDashboardInfoSidebar(); + openDashboardSettingsSidebar(); sidesheet().findByLabelText(filterToggleLabel).should("not.be.checked"); // Gadget const filterDefaultValue = FILTER_WITH_DEFAULT_VALUE.default[0]; @@ -327,9 +328,9 @@ describe( cy.wait("@updateDashboard"); }); - openDashboardInfoSidebar(); + openDashboardSettingsSidebar(); sidesheet().findByLabelText(filterToggleLabel).should("not.be.checked"); - closeDashboardInfoSidebar(); + closeDashboardSettingsSidebar(); filterWidget().findByText("Gadget").should("be.visible"); getDashboardCard().findByText("Rows 1-4 of 53").should("be.visible"); }); @@ -405,8 +406,9 @@ describe( openDashboard(); cy.wait("@cardQuery"); - openDashboardInfoSidebar(); - sidesheet().findByLabelText(filterToggleLabel).should("be.disabled"); + // shouldn't even show settings as an option for this user + dashboardHeader().icon("ellipsis").click(); + popover().findByText("Edit settings").should("not.exist"); }); it.skip("should not display a toast even when a dashboard takes longer than 15s to load", () => { @@ -566,7 +568,7 @@ describe( // so to make sure callback in `setTimeout` is called, we need to advance the clock using cy.tick(). cy.tick(); - openDashboardInfoSidebar(); + openDashboardSettingsSidebar(); sidesheet() .findByLabelText(filterToggleLabel) .should("not.be.checked"); @@ -625,7 +627,7 @@ describeWithSnowplow("scenarios > dashboards > filters > auto apply", () => { openDashboard(); cy.wait("@cardQuery"); - openDashboardInfoSidebar(); + openDashboardSettingsSidebar(); sidesheet().within(() => { expectGoodSnowplowEvents( NUMBERS_OF_GOOD_SNOWPLOW_EVENTS_BEFORE_DISABLING_AUTO_APPLY_FILTERS, @@ -644,7 +646,7 @@ describeWithSnowplow("scenarios > dashboards > filters > auto apply", () => { openDashboard(); cy.wait("@cardQuery"); - openDashboardInfoSidebar(); + openDashboardSettingsSidebar(); sidesheet().within(() => { expectGoodSnowplowEvents( NUMBERS_OF_GOOD_SNOWPLOW_EVENTS_BEFORE_DISABLING_AUTO_APPLY_FILTERS, diff --git a/e2e/test/scenarios/dashboard/dashboard-reproductions.cy.spec.js b/e2e/test/scenarios/dashboard/dashboard-reproductions.cy.spec.js index 4dbf8e6506a74ee5d54166748f1324aa341b34b9..e5044a3d8c5d8584b9e721bfaf20b8ed852c31b3 100644 --- a/e2e/test/scenarios/dashboard/dashboard-reproductions.cy.spec.js +++ b/e2e/test/scenarios/dashboard/dashboard-reproductions.cy.spec.js @@ -14,6 +14,7 @@ import { assertQueryBuilderRowCount, cartesianChartCircle, closeDashboardInfoSidebar, + closeDashboardSettingsSidebar, createDashboard, createDashboardWithTabs, createSegment, @@ -32,6 +33,7 @@ import { modal, navigationSidebar, openDashboardInfoSidebar, + openDashboardSettingsSidebar, openQuestionsSidebar, popover, queryBuilderHeader, @@ -396,9 +398,15 @@ describe("issue 16559", () => { .should("be.visible"); cy.log("Toggle auto-apply filters"); + }); + closeDashboardInfoSidebar(); - cy.findByRole("tab", { name: "Overview" }).click(); - cy.findByText("Auto-apply filters").click(); + openDashboardSettingsSidebar(); + sidesheet().findByText("Auto-apply filters").click(); + closeDashboardSettingsSidebar(); + + openDashboardInfoSidebar(); + sidesheet().within(() => { cy.findByRole("tab", { name: "History" }).click(); cy.findByTestId("dashboard-history-list") diff --git a/e2e/test/scenarios/dashboard/dashboard.cy.spec.js b/e2e/test/scenarios/dashboard/dashboard.cy.spec.js index 6107bfc6e352c3b66e72e7138f7d9250ba9e9421..9423142cabd8c7909697c339e69b0bb0893cfc90 100644 --- a/e2e/test/scenarios/dashboard/dashboard.cy.spec.js +++ b/e2e/test/scenarios/dashboard/dashboard.cy.spec.js @@ -1281,13 +1281,19 @@ describeEE("scenarios > dashboard > caching", () => { cy.log( "Check that the newly chosen cache invalidation policy - Duration - is now visible in the sidebar", ); - cy.findByLabelText(/Caching policy/).should("contain", "Duration"); - cy.findByLabelText(/Caching policy/).click(); + cy.findByLabelText(/When to get new results/).should( + "contain", + "Duration", + ); + cy.findByLabelText(/When to get new results/).click(); adaptiveRadioButton().click(); cy.findByLabelText(/Minimum query duration/).type("999"); cy.findByRole("button", { name: /Save/ }).click(); cy.wait("@putCacheConfig"); - cy.findByLabelText(/Caching policy/).should("contain", "Adaptive"); + cy.findByLabelText(/When to get new results/).should( + "contain", + "Adaptive", + ); }); }); diff --git a/e2e/test/scenarios/question/caching.cy.spec.js b/e2e/test/scenarios/question/caching.cy.spec.js index 165f0962a17bfaa70b6cf7effe418367df4469da..a2bceb8f7926438be02b6afe4f44267d288639bb 100644 --- a/e2e/test/scenarios/question/caching.cy.spec.js +++ b/e2e/test/scenarios/question/caching.cy.spec.js @@ -40,8 +40,11 @@ describeEE("scenarios > question > caching", () => { cy.log( "Check that the newly chosen cache invalidation policy - Duration - is now visible in the sidebar", ); - cy.findByLabelText(/Caching policy/).should("contain", "Duration"); - cy.findByLabelText(/Caching policy/).click(); + cy.findByLabelText(/When to get new results/).should( + "contain", + "Duration", + ); + cy.findByLabelText(/When to get new results/).click(); adaptiveRadioButton().click(); cy.findByLabelText(/Minimum query duration/).type("999"); cy.findByRole("button", { name: /Save/ }).click(); @@ -49,7 +52,10 @@ describeEE("scenarios > question > caching", () => { cy.log( "Check that the newly chosen cache invalidation policy - Adaptive - is now visible in the sidebar", ); - cy.findByLabelText(/Caching policy/).should("contain", "Adaptive"); + cy.findByLabelText(/When to get new results/).should( + "contain", + "Adaptive", + ); }); }); diff --git a/enterprise/frontend/src/metabase-enterprise/caching/components/SidebarCacheSection.tsx b/enterprise/frontend/src/metabase-enterprise/caching/components/SidebarCacheSection.tsx index 8620d94ef9d81ece12c8732da300c213a3e6a737..fed06394cadd47515c431f0964fad524f7cf4e73 100644 --- a/enterprise/frontend/src/metabase-enterprise/caching/components/SidebarCacheSection.tsx +++ b/enterprise/frontend/src/metabase-enterprise/caching/components/SidebarCacheSection.tsx @@ -38,7 +38,7 @@ export const SidebarCacheSection = ({ return ( <DelayedLoadingAndErrorWrapper delay={0} loading={loading} error={error}> <Flex align="center" justify="space-between"> - <span id={labelId}>{t`Caching policy`}</span> + <span id={labelId}>{t`When to get new results`}</span> <FormLauncher role="button" onClick={() => setPage("caching")} diff --git a/frontend/src/metabase-types/api/dashboard.ts b/frontend/src/metabase-types/api/dashboard.ts index 70a57a88daf3fff2a76014e852e17d88e781f1db..f76ba09b387001d74ee8b35a3066f7e9be13f103 100644 --- a/frontend/src/metabase-types/api/dashboard.ts +++ b/frontend/src/metabase-types/api/dashboard.ts @@ -12,6 +12,7 @@ import type { ParameterId, ParameterTarget, Table, + UserId, VirtualCardDisplay, } from "metabase-types/api"; @@ -38,6 +39,7 @@ export interface Dashboard { id: DashboardId; entity_id: BaseEntityId; created_at: string; + creator_id: UserId; updated_at: string; collection?: Collection | null; collection_id: CollectionId | null; diff --git a/frontend/src/metabase-types/api/mocks/dashboard.ts b/frontend/src/metabase-types/api/mocks/dashboard.ts index a7ed120e76085d9a8a9b579b7116f96bff9034c6..32d074a6f690e3cf6f02980044495d307ea62ec0 100644 --- a/frontend/src/metabase-types/api/mocks/dashboard.ts +++ b/frontend/src/metabase-types/api/mocks/dashboard.ts @@ -40,6 +40,7 @@ export const createMockDashboard = (opts?: Partial<Dashboard>): Dashboard => ({ embedding_params: null, initially_published_at: null, width: "fixed", + creator_id: 1, ...opts, }); diff --git a/frontend/src/metabase-types/store/dashboard.ts b/frontend/src/metabase-types/store/dashboard.ts index 2d023d804ccdc73849cc4f911e6cc34d6ddaa484..565ea9ac6421c7f31b59c2743f78910366eeeca2 100644 --- a/frontend/src/metabase-types/store/dashboard.ts +++ b/frontend/src/metabase-types/store/dashboard.ts @@ -16,6 +16,7 @@ export type DashboardSidebarName = | "action" | "clickBehavior" | "editParameter" + | "settings" | "sharing" | "info"; diff --git a/frontend/src/metabase/dashboard/components/DashboardHeader/DashboardHeaderButtonRow/DashboardHeaderButtonRow.tsx b/frontend/src/metabase/dashboard/components/DashboardHeader/DashboardHeaderButtonRow/DashboardHeaderButtonRow.tsx index 2b111dafa064aba380e91c2bf30d1b17fc04309b..49df691040c2a38629fd5d98250b416011517440 100644 --- a/frontend/src/metabase/dashboard/components/DashboardHeader/DashboardHeaderButtonRow/DashboardHeaderButtonRow.tsx +++ b/frontend/src/metabase/dashboard/components/DashboardHeader/DashboardHeaderButtonRow/DashboardHeaderButtonRow.tsx @@ -1,16 +1,20 @@ +import { useCallback } from "react"; + +import { setSidebar } from "metabase/dashboard/actions"; import { dashboardActionButtons } from "metabase/dashboard/components/DashboardHeader/DashboardHeaderButtonRow/action-buttons"; import type { DashboardActionKey, DashboardHeaderButtonRowProps, HeaderButtonProps, } from "metabase/dashboard/components/DashboardHeader/DashboardHeaderButtonRow/types"; +import { SIDEBAR_NAME } from "metabase/dashboard/constants"; import { getDashboardComplete, getHasModelActionsEnabled, getIsEditing, } from "metabase/dashboard/selectors"; import { isEmbeddingSdk } from "metabase/env"; -import { useSelector } from "metabase/lib/redux"; +import { useDispatch, useSelector } from "metabase/lib/redux"; import { getPulseFormInput } from "metabase/pulse/selectors"; import { canManageSubscriptions as canManageSubscriptionsSelector, @@ -47,6 +51,12 @@ export const DashboardHeaderButtonRow = ({ ? buttonOptions.filter(key => dashboardActionKeys.includes(key)) : buttonOptions; + const dispatch = useDispatch(); + + const openSettingsSidebar = useCallback(() => { + dispatch(setSidebar({ name: SIDEBAR_NAME.settings })); + }, [dispatch]); + return ( <> {visibleDashboardActionKeys.map(dashboardActionKey => { @@ -63,6 +73,7 @@ export const DashboardHeaderButtonRow = ({ isAdmin, isPublic, isEmbeddingSdk, + openSettingsSidebar, ...props, }; diff --git a/frontend/src/metabase/dashboard/components/DashboardHeader/DashboardHeaderButtonRow/action-buttons.tsx b/frontend/src/metabase/dashboard/components/DashboardHeader/DashboardHeaderButtonRow/action-buttons.tsx index 30364e85170690ab212e555f319c23f328fe8624..ceff9c60d3c9d789b993cf2066c8384b0bd50808 100644 --- a/frontend/src/metabase/dashboard/components/DashboardHeader/DashboardHeaderButtonRow/action-buttons.tsx +++ b/frontend/src/metabase/dashboard/components/DashboardHeader/DashboardHeaderButtonRow/action-buttons.tsx @@ -161,6 +161,7 @@ export const dashboardActionButtons: Record< dashboard, canEdit, location, + openSettingsSidebar, }) => ( <DashboardActionMenu items={getExtraButtons({ @@ -171,6 +172,7 @@ export const dashboardActionButtons: Record< dashboard, canEdit, pathname: location?.pathname, + openSettingsSidebar, })} /> ), diff --git a/frontend/src/metabase/dashboard/components/DashboardHeader/DashboardHeaderButtonRow/types.ts b/frontend/src/metabase/dashboard/components/DashboardHeader/DashboardHeaderButtonRow/types.ts index 0f919d077006557615f320b18a83f80b8cfd151d..ebb8877e042279a009624b770d1ea57c27ead459 100644 --- a/frontend/src/metabase/dashboard/components/DashboardHeader/DashboardHeaderButtonRow/types.ts +++ b/frontend/src/metabase/dashboard/components/DashboardHeader/DashboardHeaderButtonRow/types.ts @@ -31,6 +31,7 @@ export type HeaderButtonProps = { formInput: any; isAdmin: boolean; isEmbeddingSdk: boolean; + openSettingsSidebar: () => void; } & DashboardHeaderButtonRowProps; export type DashboardActionButton = { diff --git a/frontend/src/metabase/dashboard/components/DashboardHeader/DashboardHeaderView.tsx b/frontend/src/metabase/dashboard/components/DashboardHeader/DashboardHeaderView.tsx index c16bf2213af9a04108951ed2e2809c86b8bf3bb9..72319b5a1f54d391c4f432af6396f144007a2194 100644 --- a/frontend/src/metabase/dashboard/components/DashboardHeader/DashboardHeaderView.tsx +++ b/frontend/src/metabase/dashboard/components/DashboardHeader/DashboardHeaderView.tsx @@ -19,6 +19,7 @@ import { getIsEditing, getIsHeaderVisible, getIsShowDashboardInfoSidebar, + getIsShowDashboardSettingsSidebar, getIsSidebarOpen, } from "metabase/dashboard/selectors"; import type { @@ -87,6 +88,7 @@ export function DashboardHeaderView({ const canResetFilters = useSelector(getCanResetFilters); const isSidebarOpen = useSelector(getIsSidebarOpen); const isInfoSidebarOpen = useSelector(getIsShowDashboardInfoSidebar); + const isSettingsSidebarOpen = useSelector(getIsShowDashboardSettingsSidebar); const isDashboardHeaderVisible = useSelector(getIsHeaderVisible); const isAnalyticsDashboard = isInstanceAnalyticsCollection(collection); @@ -166,7 +168,9 @@ export function DashboardHeaderView({ )} <HeaderContainer isFixedWidth={dashboard?.width === "fixed"} - offsetSidebar={isSidebarOpen && !isInfoSidebarOpen} + offsetSidebar={ + isSidebarOpen && !isInfoSidebarOpen && !isSettingsSidebarOpen + } > {isDashboardHeaderVisible && ( <HeaderRow diff --git a/frontend/src/metabase/dashboard/components/DashboardHeader/buttons/DashboardActionMenu.tsx b/frontend/src/metabase/dashboard/components/DashboardHeader/buttons/DashboardActionMenu.tsx index 7ab884042c5f555b0df614d3816045e0600ff943..3ddb3f56e916ff70a672870ac40d739bcaf1abf4 100644 --- a/frontend/src/metabase/dashboard/components/DashboardHeader/buttons/DashboardActionMenu.tsx +++ b/frontend/src/metabase/dashboard/components/DashboardHeader/buttons/DashboardActionMenu.tsx @@ -26,12 +26,14 @@ export const getExtraButtons = ({ dashboard, canEdit, pathname, + openSettingsSidebar, }: DashboardFullscreenControls & { canResetFilters: boolean; onResetFilters: () => void; dashboard: Dashboard; canEdit: boolean; pathname: string; + openSettingsSidebar: () => void; }) => { const extraButtons = []; @@ -50,6 +52,17 @@ export const getExtraButtons = ({ }); if (canEdit) { + extraButtons.push({ + title: t`Edit settings`, + icon: "gear", + action: openSettingsSidebar, + }); + + extraButtons.push({ + separator: true, + key: "separator-after-edit-settings", + }); + extraButtons.push({ title: t`Move`, icon: "move", @@ -64,6 +77,11 @@ export const getExtraButtons = ({ }); if (canEdit) { + extraButtons.push({ + separator: true, + key: "separator-before-ee-buttons-and-trash", + }); + extraButtons.push(...PLUGIN_DASHBOARD_HEADER.extraButtons(dashboard)); extraButtons.push({ diff --git a/frontend/src/metabase/dashboard/components/DashboardInfoSidebar/DashboardInfoSidebar.tsx b/frontend/src/metabase/dashboard/components/DashboardInfoSidebar/DashboardInfoSidebar.tsx index 78fce5da5ae7adca0716a411891bcabebfc76dae..e01c49b85cb8abad29d544eac05291ae7ccae49d 100644 --- a/frontend/src/metabase/dashboard/components/DashboardInfoSidebar/DashboardInfoSidebar.tsx +++ b/frontend/src/metabase/dashboard/components/DashboardInfoSidebar/DashboardInfoSidebar.tsx @@ -1,4 +1,4 @@ -import type { FocusEvent, SetStateAction } from "react"; +import type { FocusEvent } from "react"; import { useCallback, useMemo, useState } from "react"; import { useMount } from "react-use"; import { t } from "ttag"; @@ -15,24 +15,12 @@ import { Timeline } from "metabase/common/components/Timeline"; import { getTimelineEvents } from "metabase/common/components/Timeline/utils"; import { useRevisionListQuery } from "metabase/common/hooks"; import EditableText from "metabase/core/components/EditableText"; -import { - revertToRevision, - toggleAutoApplyFilters, - updateDashboard, -} from "metabase/dashboard/actions"; +import { revertToRevision, updateDashboard } from "metabase/dashboard/actions"; import { DASHBOARD_DESCRIPTION_MAX_LENGTH } from "metabase/dashboard/constants"; -import { isDashboardCacheable } from "metabase/dashboard/utils"; -import { useUniqueId } from "metabase/hooks/use-unique-id"; import { useDispatch, useSelector } from "metabase/lib/redux"; -import { PLUGIN_CACHING } from "metabase/plugins"; import { getUser } from "metabase/selectors/user"; -import { Stack, Switch, Tabs, Text } from "metabase/ui"; -import type { - CacheableDashboard, - Dashboard, - Revision, - User, -} from "metabase-types/api"; +import { Stack, Tabs, Text } from "metabase/ui"; +import type { Dashboard, Revision, User } from "metabase-types/api"; import DashboardInfoSidebarS from "./DashboardInfoSidebar.module.css"; @@ -56,7 +44,6 @@ export function DashboardInfoSidebar({ onClose, }: DashboardInfoSidebarProps) { const [isOpen, setIsOpen] = useState(false); - const [page, setPage] = useState<"default" | "caching">("default"); useMount(() => { // this component is not rendered until it is "open" @@ -103,19 +90,6 @@ export function DashboardInfoSidebar({ ); const canWrite = dashboard.can_write && !dashboard.archived; - const showCaching = canWrite && PLUGIN_CACHING.isGranularCachingEnabled(); - - if (page === "caching") { - return ( - <PLUGIN_CACHING.SidebarCacheForm - item={dashboard as CacheableDashboard} - model="dashboard" - onBack={() => setPage("default")} - onClose={onClose} - pt="md" - /> - ); - } return ( <div data-testid="sidebar-right"> @@ -146,8 +120,6 @@ export function DashboardInfoSidebar({ descriptionError={descriptionError} setDescriptionError={setDescriptionError} canWrite={canWrite} - setPage={setPage} - showCaching={showCaching} /> </Tabs.Panel> <Tabs.Panel value={Tab.History}> @@ -172,8 +144,6 @@ const OverviewTab = ({ descriptionError, setDescriptionError, canWrite, - setPage, - showCaching, }: { dashboard: Dashboard; handleDescriptionChange: (description: string) => void; @@ -181,21 +151,7 @@ const OverviewTab = ({ descriptionError: string | null; setDescriptionError: (error: string | null) => void; canWrite: boolean; - setPage: ( - page: "default" | "caching" | SetStateAction<"default" | "caching">, - ) => void; - showCaching: boolean; }) => { - const isCacheable = isDashboardCacheable(dashboard); - const autoApplyFilterToggleId = useUniqueId(); - const dispatch = useDispatch(); - const handleToggleAutoApplyFilters = useCallback( - (isAutoApplyingFilters: boolean) => { - dispatch(toggleAutoApplyFilters(isAutoApplyingFilters)); - }, - [dispatch], - ); - return ( <Stack spacing="lg"> <SidesheetCard title={t`Description`} pb="md"> @@ -218,31 +174,6 @@ const OverviewTab = ({ </Text> )} </SidesheetCard> - - {!dashboard.archived && ( - <SidesheetCard> - <Switch - disabled={!canWrite} - label={t`Auto-apply filters`} - labelPosition="left" - variant="stretch" - size="sm" - id={autoApplyFilterToggleId} - checked={dashboard.auto_apply_filters} - onChange={e => handleToggleAutoApplyFilters(e.target.checked)} - /> - </SidesheetCard> - )} - - {showCaching && isCacheable && ( - <SidesheetCard title={t`Caching`} pb="md"> - <PLUGIN_CACHING.SidebarCacheSection - model="dashboard" - item={dashboard} - setPage={setPage} - /> - </SidesheetCard> - )} </Stack> ); }; diff --git a/frontend/src/metabase/dashboard/components/DashboardInfoSidebar/tests/common.unit.spec.ts b/frontend/src/metabase/dashboard/components/DashboardInfoSidebar/tests/common.unit.spec.ts index bd7e54d6fa46539bfcfd405c1d75fb906ac53695..2281d3ddb751e8a895b426c35cb6e232d4bd72de 100644 --- a/frontend/src/metabase/dashboard/components/DashboardInfoSidebar/tests/common.unit.spec.ts +++ b/frontend/src/metabase/dashboard/components/DashboardInfoSidebar/tests/common.unit.spec.ts @@ -99,14 +99,4 @@ describe("DashboardInfoSidebar", () => { expect(setDashboardAttribute).toHaveBeenCalledWith("description", ""); }); - - it("should show dashboard auto-apply filter toggle", async () => { - await setup(); - expect(screen.getByText("Auto-apply filters")).toBeInTheDocument(); - }); - - it("should not render caching section in OSS", async () => { - await setup(); - expect(screen.queryByText("Caching")).not.toBeInTheDocument(); - }); }); diff --git a/frontend/src/metabase/dashboard/components/DashboardSettingsSidebar/DashboardSettingsSidebar.tsx b/frontend/src/metabase/dashboard/components/DashboardSettingsSidebar/DashboardSettingsSidebar.tsx new file mode 100644 index 0000000000000000000000000000000000000000..326da337aa68787689685c54d66f772e2abd01b5 --- /dev/null +++ b/frontend/src/metabase/dashboard/components/DashboardSettingsSidebar/DashboardSettingsSidebar.tsx @@ -0,0 +1,124 @@ +import type { Dispatch, SetStateAction } from "react"; +import { useCallback, useState } from "react"; +import { useMount } from "react-use"; +import { t } from "ttag"; + +import ErrorBoundary from "metabase/ErrorBoundary"; +import { Sidesheet, SidesheetCard } from "metabase/common/components/Sidesheet"; +import { toggleAutoApplyFilters } from "metabase/dashboard/actions"; +import { isDashboardCacheable } from "metabase/dashboard/utils"; +import { useUniqueId } from "metabase/hooks/use-unique-id"; +import { useDispatch } from "metabase/lib/redux"; +import { PLUGIN_CACHING } from "metabase/plugins"; +import { Switch } from "metabase/ui"; +import type { CacheableDashboard, Dashboard } from "metabase-types/api"; + +interface DashboardSettingsSidebarProps { + dashboard: Dashboard; + onClose: () => void; +} + +export function DashboardSettingsSidebar({ + dashboard, + onClose, +}: DashboardSettingsSidebarProps) { + const [page, setPage] = useState<"default" | "caching">("default"); + + const [isOpen, setIsOpen] = useState(false); + + useMount(() => { + // this component is not rendered until it is "open" + // but we want to set isOpen after it mounts to get + // pretty animations + setIsOpen(true); + }); + + if (page === "caching") { + return ( + <PLUGIN_CACHING.SidebarCacheForm + item={dashboard as CacheableDashboard} + model="dashboard" + onBack={() => setPage("default")} + onClose={onClose} + pt="md" + /> + ); + } + + return ( + <ErrorBoundary> + <Sidesheet + isOpen={isOpen} + title={t`Dashboard settings`} + onClose={onClose} + data-testid="dashboard-settings-sidebar" + > + <DashboardSidesheetBody + dashboard={dashboard} + page={page} + setPage={setPage} + isOpen={isOpen} + onClose={onClose} + /> + </Sidesheet> + </ErrorBoundary> + ); +} + +export type DashboardSidebarPageProps = { + dashboard: Dashboard; + page: "default" | "caching"; + setPage: Dispatch<SetStateAction<"default" | "caching">>; + isOpen: boolean; + onClose: () => void; +}; + +const DashboardSidesheetBody = ({ + dashboard, + setPage, +}: DashboardSidebarPageProps) => { + const dispatch = useDispatch(); + + const handleToggleAutoApplyFilters = useCallback( + (isAutoApplyingFilters: boolean) => { + dispatch(toggleAutoApplyFilters(isAutoApplyingFilters)); + }, + [dispatch], + ); + + const autoApplyFilterToggleId = useUniqueId(); + const canWrite = dashboard.can_write && !dashboard.archived; + + const isCacheable = isDashboardCacheable(dashboard); + const showCaching = canWrite && PLUGIN_CACHING.isGranularCachingEnabled(); + + if (dashboard.archived) { + return null; + } + + return ( + <> + <SidesheetCard title={t`General`}> + <Switch + disabled={!canWrite} + label={t`Auto-apply filters`} + labelPosition="left" + variant="stretch" + size="sm" + id={autoApplyFilterToggleId} + checked={dashboard.auto_apply_filters} + onChange={e => handleToggleAutoApplyFilters(e.target.checked)} + /> + </SidesheetCard> + {showCaching && isCacheable && ( + <SidesheetCard title={t`Caching`}> + <PLUGIN_CACHING.SidebarCacheSection + model="dashboard" + item={dashboard} + setPage={setPage} + /> + </SidesheetCard> + )} + </> + ); +}; diff --git a/frontend/src/metabase/dashboard/components/DashboardSettingsSidebar/index.ts b/frontend/src/metabase/dashboard/components/DashboardSettingsSidebar/index.ts new file mode 100644 index 0000000000000000000000000000000000000000..8a7b9177d83df44a22d1580d9eeafa44e4e37f82 --- /dev/null +++ b/frontend/src/metabase/dashboard/components/DashboardSettingsSidebar/index.ts @@ -0,0 +1 @@ +export * from "./DashboardSettingsSidebar"; diff --git a/frontend/src/metabase/dashboard/components/DashboardSettingsSidebar/tests/common.unit.spec.ts b/frontend/src/metabase/dashboard/components/DashboardSettingsSidebar/tests/common.unit.spec.ts new file mode 100644 index 0000000000000000000000000000000000000000..0f46259cc494a95fa72b3d32605bc6699d3ee6a1 --- /dev/null +++ b/frontend/src/metabase/dashboard/components/DashboardSettingsSidebar/tests/common.unit.spec.ts @@ -0,0 +1,36 @@ +import userEvent from "@testing-library/user-event"; + +import { screen } from "__support__/ui"; + +import { setup } from "./setup"; + +jest.mock("metabase/dashboard/constants", () => ({ + ...jest.requireActual("metabase/dashboard/constants"), + DASHBOARD_DESCRIPTION_MAX_LENGTH: 20, +})); + +describe("DashboardSettingsSidebar", () => { + it("should render the component", () => { + setup(); + + expect(screen.getByText("Dashboard settings")).toBeInTheDocument(); + expect(screen.getByTestId("sidesheet")).toBeInTheDocument(); + }); + + it("should close when clicking the close button", async () => { + const { onClose } = await setup(); + await userEvent.click(screen.getByRole("button", { name: "Close" })); + + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it("should show dashboard auto-apply filter toggle", async () => { + await setup(); + expect(screen.getByText("Auto-apply filters")).toBeInTheDocument(); + }); + + it("should not render caching section in OSS", async () => { + await setup(); + expect(screen.queryByText("Caching")).not.toBeInTheDocument(); + }); +}); diff --git a/frontend/src/metabase/dashboard/components/DashboardInfoSidebar/tests/enterprise.unit.spec.ts b/frontend/src/metabase/dashboard/components/DashboardSettingsSidebar/tests/enterprise.unit.spec.ts similarity index 78% rename from frontend/src/metabase/dashboard/components/DashboardInfoSidebar/tests/enterprise.unit.spec.ts rename to frontend/src/metabase/dashboard/components/DashboardSettingsSidebar/tests/enterprise.unit.spec.ts index 8f9d5650225176d5afa3c1ccfc5d88317a0b877a..c18a63a3d97657b500ee196abf45d90ff1c144ce 100644 --- a/frontend/src/metabase/dashboard/components/DashboardInfoSidebar/tests/enterprise.unit.spec.ts +++ b/frontend/src/metabase/dashboard/components/DashboardSettingsSidebar/tests/enterprise.unit.spec.ts @@ -2,11 +2,11 @@ import { screen } from "__support__/ui"; import { setupEnterprise } from "./setup"; -describe("DashboardInfoSidebar > enterprise", () => { +describe("DashboardSettingsSidebar > enterprise", () => { it("should render the component", async () => { await setupEnterprise(); - expect(screen.getByText("Info")).toBeInTheDocument(); + expect(screen.getByText("Dashboard settings")).toBeInTheDocument(); expect(screen.getByTestId("sidesheet")).toBeInTheDocument(); }); diff --git a/frontend/src/metabase/dashboard/components/DashboardInfoSidebar/tests/premium.unit.spec.ts b/frontend/src/metabase/dashboard/components/DashboardSettingsSidebar/tests/premium.unit.spec.ts similarity index 84% rename from frontend/src/metabase/dashboard/components/DashboardInfoSidebar/tests/premium.unit.spec.ts rename to frontend/src/metabase/dashboard/components/DashboardSettingsSidebar/tests/premium.unit.spec.ts index 842ab93431ebaf17b0c3311bee4b7ca2123de88b..2df4032a15dc1db51f71405c3d47eac448e15ac7 100644 --- a/frontend/src/metabase/dashboard/components/DashboardInfoSidebar/tests/premium.unit.spec.ts +++ b/frontend/src/metabase/dashboard/components/DashboardSettingsSidebar/tests/premium.unit.spec.ts @@ -13,11 +13,11 @@ const tokenFeatures = { audit_app: true, }; -describe("DashboardInfoSidebar > premium enterprise", () => { +describe("DashboardSettingsSidebar > premium enterprise", () => { it("should render the component", async () => { await setupEnterprise({}, tokenFeatures); - expect(screen.getByText("Info")).toBeInTheDocument(); + expect(screen.getByText("Dashboard settings")).toBeInTheDocument(); expect(screen.getByTestId("sidesheet")).toBeInTheDocument(); }); @@ -25,7 +25,9 @@ describe("DashboardInfoSidebar > premium enterprise", () => { await setupEnterprise({}, tokenFeatures); expect(await screen.findByText("Caching")).toBeInTheDocument(); - expect(await screen.findByText("Caching policy")).toBeInTheDocument(); + expect( + await screen.findByText("When to get new results"), + ).toBeInTheDocument(); }); it("should show cache form when clicking on caching section", async () => { diff --git a/frontend/src/metabase/dashboard/components/DashboardSettingsSidebar/tests/setup.tsx b/frontend/src/metabase/dashboard/components/DashboardSettingsSidebar/tests/setup.tsx new file mode 100644 index 0000000000000000000000000000000000000000..c34fd3990f78a4dac629a1714a964f68d9a4d1d3 --- /dev/null +++ b/frontend/src/metabase/dashboard/components/DashboardSettingsSidebar/tests/setup.tsx @@ -0,0 +1,87 @@ +import { setupEnterprisePlugins } from "__support__/enterprise"; +import { + setupDashboardEndpoints, + setupPerformanceEndpoints, + setupRevisionsEndpoints, + setupUsersEndpoints, +} from "__support__/server-mocks"; +import { mockSettings } from "__support__/settings"; +import { createMockEntitiesState } from "__support__/store"; +import { renderWithProviders, waitForLoaderToBeRemoved } from "__support__/ui"; +import type { Dashboard, Settings, TokenFeatures } from "metabase-types/api"; +import { + createMockDashboard, + createMockSettings, + createMockTokenFeatures, + createMockUser, +} from "metabase-types/api/mocks"; +import { createSampleDatabase } from "metabase-types/api/mocks/presets"; +import { createMockState } from "metabase-types/store/mocks"; + +import { DashboardSettingsSidebar } from "../DashboardSettingsSidebar"; + +export interface SetupOpts { + dashboard?: Dashboard; + settings?: Settings; + hasEnterprisePlugins?: boolean; +} + +export async function setup({ + dashboard = createMockDashboard(), + settings = createMockSettings(), + hasEnterprisePlugins, +}: SetupOpts = {}) { + const setDashboardAttribute = jest.fn(); + const onClose = jest.fn(); + + const currentUser = createMockUser(); + setupDashboardEndpoints(dashboard); + setupUsersEndpoints([currentUser]); + setupRevisionsEndpoints([]); + setupPerformanceEndpoints([]); + + const state = createMockState({ + currentUser, + settings: mockSettings({ + ...settings, + "token-features": createMockTokenFeatures( + settings["token-features"] || {}, + ), + }), + entities: createMockEntitiesState({ + databases: [createSampleDatabase()], + dashboards: [dashboard], + }), + }); + + if (hasEnterprisePlugins) { + setupEnterprisePlugins(); + } + + renderWithProviders( + <DashboardSettingsSidebar dashboard={dashboard} onClose={onClose} />, + { storeInitialState: state }, + ); + await waitForLoaderToBeRemoved(); + + return { + setDashboardAttribute, + onClose, + }; +} + +export const setupEnterprise = ( + opts: SetupOpts = {}, + tokenFeatures: Partial<TokenFeatures> = {}, +) => { + return setup({ + ...opts, + settings: createMockSettings({ + ...opts.settings, + "token-features": createMockTokenFeatures({ + ...tokenFeatures, + }), + }), + hasEnterprisePlugins: true, + }); +}; diff --git a/frontend/src/metabase/dashboard/components/DashboardSidebars.tsx b/frontend/src/metabase/dashboard/components/DashboardSidebars.tsx index 7b114927c512905025d52699a933ad01fde7dd63..4d03879bf526424b2cc696afb1a758f182cfd753 100644 --- a/frontend/src/metabase/dashboard/components/DashboardSidebars.tsx +++ b/frontend/src/metabase/dashboard/components/DashboardSidebars.tsx @@ -32,6 +32,7 @@ import { ActionSidebarConnected } from "./ActionSidebar"; import { AddCardSidebar } from "./AddCardSidebar"; import { ClickBehaviorSidebar } from "./ClickBehaviorSidebar/ClickBehaviorSidebar"; import { DashboardInfoSidebar } from "./DashboardInfoSidebar"; +import { DashboardSettingsSidebar } from "./DashboardSettingsSidebar"; interface DashboardSidebarsProps { dashboard: IDashboard; @@ -207,6 +208,13 @@ export function DashboardSidebars({ /> ); } + case SIDEBAR_NAME.settings: + return ( + <DashboardSettingsSidebar + dashboard={dashboard} + onClose={closeSidebar} + /> + ); case SIDEBAR_NAME.sharing: return <SharingSidebar dashboard={dashboard} onCancel={onCancel} />; case SIDEBAR_NAME.info: diff --git a/frontend/src/metabase/dashboard/components/DashboardTabs/test-utils.ts b/frontend/src/metabase/dashboard/components/DashboardTabs/test-utils.ts index 8292ddadf2780098d616b79a500522a2a51bd7ca..3c608f8a44e065332ae356d634d3dc8573771b42 100644 --- a/frontend/src/metabase/dashboard/components/DashboardTabs/test-utils.ts +++ b/frontend/src/metabase/dashboard/components/DashboardTabs/test-utils.ts @@ -29,6 +29,7 @@ export const TEST_DASHBOARD_STATE: DashboardState = { created_at: "2024-01-01T00:00:00Z", updated_at: "2024-01-01T00:00:00Z", collection_id: 1, + creator_id: 1, name: "", description: "", can_write: true, diff --git a/frontend/src/metabase/dashboard/constants.ts b/frontend/src/metabase/dashboard/constants.ts index 8539ee8cc37c6a2d9d7cd0a8e0810af69a8a68a6..3ab93944c8a8ab7fa0625b36eaadbf9443d6bc90 100644 --- a/frontend/src/metabase/dashboard/constants.ts +++ b/frontend/src/metabase/dashboard/constants.ts @@ -14,6 +14,7 @@ export const SIDEBAR_NAME: Record<DashboardSidebarName, DashboardSidebarName> = clickBehavior: "clickBehavior", editParameter: "editParameter", sharing: "sharing", + settings: "settings", info: "info", }; diff --git a/frontend/src/metabase/dashboard/selectors.ts b/frontend/src/metabase/dashboard/selectors.ts index f6b999c785a2cb5002581fae67bebf98439f0176..666b93f5655066d29da64a23ae88a5f8d768d1c5 100644 --- a/frontend/src/metabase/dashboard/selectors.ts +++ b/frontend/src/metabase/dashboard/selectors.ts @@ -146,6 +146,11 @@ export const getIsShowDashboardInfoSidebar = createSelector( sidebar => sidebar.name === SIDEBAR_NAME.info, ); +export const getIsShowDashboardSettingsSidebar = createSelector( + [getSidebar], + sidebar => sidebar.name === SIDEBAR_NAME.settings, +); + export const getDashboardId = (state: State) => state.dashboard.dashboardId; export const getDashboard = createSelector( diff --git a/frontend/src/metabase/query_builder/components/view/sidebars/QuestionSettingsSidebar/tests/common.unit.spec.tsx b/frontend/src/metabase/query_builder/components/view/sidebars/QuestionSettingsSidebar/tests/common.unit.spec.tsx index 319ce16ac35c49902afb73ee044b1442d1afa5eb..e07572ae5d30eef11f4a609187af2a8d45663d09 100644 --- a/frontend/src/metabase/query_builder/components/view/sidebars/QuestionSettingsSidebar/tests/common.unit.spec.tsx +++ b/frontend/src/metabase/query_builder/components/view/sidebars/QuestionSettingsSidebar/tests/common.unit.spec.tsx @@ -33,7 +33,9 @@ describe("QuestionSettingsSidebar", () => { it("should not allow to configure caching without cache token feature", async () => { await setup({ card }); - expect(screen.queryByText("Caching policy")).not.toBeInTheDocument(); + expect( + screen.queryByText("When to get new results"), + ).not.toBeInTheDocument(); }); it("should not show granular model caching controls on OSS", async () => { diff --git a/frontend/src/metabase/query_builder/components/view/sidebars/QuestionSettingsSidebar/tests/enterprise.unit.spec.tsx b/frontend/src/metabase/query_builder/components/view/sidebars/QuestionSettingsSidebar/tests/enterprise.unit.spec.tsx index e85f9511298257df462b720db982f9b67c830d6c..65af556881f7691aec45a98b064de8a5d46894ef 100644 --- a/frontend/src/metabase/query_builder/components/view/sidebars/QuestionSettingsSidebar/tests/enterprise.unit.spec.tsx +++ b/frontend/src/metabase/query_builder/components/view/sidebars/QuestionSettingsSidebar/tests/enterprise.unit.spec.tsx @@ -35,7 +35,9 @@ describe("QuestionSettingsSidebar", () => { it("should show caching controls with cache token feature", async () => { await setupEnterprise({ card }, { cache_granular_controls: true }); expect(await screen.findByText("Caching")).toBeInTheDocument(); - expect(await screen.findByText("Caching policy")).toBeInTheDocument(); + expect( + await screen.findByText("When to get new results"), + ).toBeInTheDocument(); }); });