diff --git a/.clj-kondo/config.edn b/.clj-kondo/config.edn index 29f439e79fdec7afefe07f9b74dfae736a696fc1..829179ff5d8932fc7f397b82b57f2ce5630b8232 100644 --- a/.clj-kondo/config.edn +++ b/.clj-kondo/config.edn @@ -530,6 +530,7 @@ metabase.api.common/defendpoint-async-schema hooks.metabase.api.common/defendpoint metabase.api.common/defendpoint-schema hooks.metabase.api.common/defendpoint metabase.api.dashboard-test/with-chain-filter-fixtures hooks.common/let-one-with-optional-value + metabase.api.dashboard-test/with-simple-dashboard-with-tabs hooks.common/with-one-binding metabase.api.card-test/with-card-param-values-fixtures hooks.common/let-one-with-optional-value metabase.api.embed-test/do-response-formats hooks.common/with-two-bindings metabase.api.embed-test/with-chain-filter-fixtures hooks.common/let-one-with-optional-value @@ -551,6 +552,7 @@ metabase.models.collection-test/with-collection-hierarchy hooks.common/let-one-with-optional-value metabase.models.collection-test/with-personal-and-impersonal-collections hooks.common/with-two-bindings metabase.models.dashboard-test/with-dash-in-collection hooks.common/with-three-bindings + metabase.models.dashboard-tab-test/with-dashtab-in-personal-collection hooks.common/with-one-top-level-binding metabase.models.interface/define-simple-hydration-method hooks.metabase.models.interface/define-hydration-method metabase.models.interface/define-batched-hydration-method hooks.metabase.models.interface/define-hydration-method metabase.models.pulse-test/with-dashboard-subscription-in-collection hooks.common/with-four-bindings diff --git a/e2e/snapshot-creators/default.cy.snap.js b/e2e/snapshot-creators/default.cy.snap.js index 628bd99578b741a0cf200a75c3cd644a80e8ac9c..d107d5bdffd60e8252a6bdfc5fc3f63c5243d68e 100644 --- a/e2e/snapshot-creators/default.cy.snap.js +++ b/e2e/snapshot-creators/default.cy.snap.js @@ -173,22 +173,11 @@ describe("snapshots", () => { // dashboard 1: Orders in a dashboard const dashboardDetails = { name: "Orders in a dashboard" }; - cy.createQuestionAndDashboard({ questionDetails, dashboardDetails }).then( - ({ body: { id, card_id, dashboard_id } }) => { - cy.request("PUT", `/api/dashboard/${dashboard_id}/cards`, { - cards: [ - { - id, - card_id, - row: 0, - col: 0, - size_x: 12, - size_y: 8, - }, - ], - }); - }, - ); + cy.createQuestionAndDashboard({ + questionDetails, + dashboardDetails, + cardDetails: { size_x: 12, size_y: 8 }, + }); // question 2: Orders, Count cy.createQuestion({ diff --git a/e2e/support/commands/api/composite/createNativeQuestionAndDashboard.js b/e2e/support/commands/api/composite/createNativeQuestionAndDashboard.js index 4a68190c1de8508555add3892b9cba3cbcf284ea..f3de7a2607061fdafb680a2f90828ec23d52e807 100644 --- a/e2e/support/commands/api/composite/createNativeQuestionAndDashboard.js +++ b/e2e/support/commands/api/composite/createNativeQuestionAndDashboard.js @@ -19,7 +19,7 @@ Cypress.Commands.add( ], }).then(response => ({ ...response, - body: response.body[0], + body: response.body.cards[0], })); }, ); diff --git a/e2e/support/commands/api/composite/createQuestionAndAddToDashboard.js b/e2e/support/commands/api/composite/createQuestionAndAddToDashboard.js index 50ff47c18d62ede50c51ed97df9ad0f3355019a1..8df03af937d28f8f122816595fe9cf78f351b3f6 100644 --- a/e2e/support/commands/api/composite/createQuestionAndAddToDashboard.js +++ b/e2e/support/commands/api/composite/createQuestionAndAddToDashboard.js @@ -26,7 +26,7 @@ Cypress.Commands.add( }) .then(response => ({ ...response, - body: response.body[0], + body: response.body.cards[0], })), ), ), diff --git a/e2e/support/commands/api/composite/createQuestionAndDashboard.js b/e2e/support/commands/api/composite/createQuestionAndDashboard.js index a21509d586f6d3a0457565068478e3241749a942..951dc6019bbb773bb9e811ce431c81abce93a10f 100644 --- a/e2e/support/commands/api/composite/createQuestionAndDashboard.js +++ b/e2e/support/commands/api/composite/createQuestionAndDashboard.js @@ -19,7 +19,7 @@ Cypress.Commands.add( ], }).then(response => ({ ...response, - body: response.body[0], + body: response.body.cards[0], })); }, ); diff --git a/e2e/support/helpers/e2e-dashboard-helpers.js b/e2e/support/helpers/e2e-dashboard-helpers.js index fbbab652d1879968d33e6899519d13f49682f3b2..8317c93c88e7218dbb6288d421d72367aa6a1089 100644 --- a/e2e/support/helpers/e2e-dashboard-helpers.js +++ b/e2e/support/helpers/e2e-dashboard-helpers.js @@ -37,7 +37,7 @@ export function addOrUpdateDashboardCard({ card_id, dashboard_id, card }) { }) .then(response => ({ ...response, - body: response.body[0], + body: response.body.cards[0], })); } /** @@ -45,8 +45,9 @@ export function addOrUpdateDashboardCard({ card_id, dashboard_id, card }) { * Can be used to remove cards (exclude from array), or add/update them. */ export function updateDashboardCards({ dashboard_id, cards }) { + let id = -1; return cy.request("PUT", getDashCardApiUrl(dashboard_id), { - cards: cards.map(card => ({ ...DEFAULT_CARD, ...card })), + cards: cards.map(card => ({ ...DEFAULT_CARD, id: id--, ...card })), }); } @@ -99,3 +100,7 @@ export function addTextBox(string, options = {}) { "You can use Markdown here, and include variables {{like_this}}", ).type(string, options); } + +export function openQuestionsSidebar() { + cy.findByLabelText("add questions").click(); +} diff --git a/e2e/test/scenarios/collections/revision-history.cy.spec.js b/e2e/test/scenarios/collections/revision-history.cy.spec.js index e36e4161550c56b7c46d60b061d7584c08c4a312..8a7ba9ff31a9efb1e7c39e64157eb2068915e0b5 100644 --- a/e2e/test/scenarios/collections/revision-history.cy.spec.js +++ b/e2e/test/scenarios/collections/revision-history.cy.spec.js @@ -6,6 +6,7 @@ import { visitQuestion, questionInfoButton, rightSidebar, + openQuestionsSidebar, } from "e2e/support/helpers"; const PERMISSIONS = { @@ -68,7 +69,7 @@ describe("revision history", () => { cy.createDashboard().then(({ body }) => { visitAndEditDashboard(body.id); }); - cy.icon("add").last().click(); + openQuestionsSidebar(); // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage cy.findByText("Orders, Count").click(); saveDashboard(); @@ -175,7 +176,7 @@ describe("revision history", () => { }); function clickRevert(event_name, index = 0) { - cy.findAllByText(event_name).eq(index).siblings("button").first().click(); + cy.findAllByLabelText(event_name).eq(index).click(); } function visitAndEditDashboard(id) { diff --git a/e2e/test/scenarios/dashboard/dashboard.cy.spec.js b/e2e/test/scenarios/dashboard/dashboard.cy.spec.js index 49352fa7084a353d8ff55acfd9dd4fd650063e4a..65527ef703e97782f231399abcaa624b0993caad 100644 --- a/e2e/test/scenarios/dashboard/dashboard.cy.spec.js +++ b/e2e/test/scenarios/dashboard/dashboard.cy.spec.js @@ -13,6 +13,7 @@ import { rightSidebar, getDashboardCardMenu, addOrUpdateDashboardCard, + openQuestionsSidebar, } from "e2e/support/helpers"; import { SAMPLE_DB_ID } from "e2e/support/cypress_data"; @@ -191,7 +192,7 @@ describe("scenarios > dashboard", () => { it("should add a question", () => { visitDashboard(1); cy.icon("pencil").click(); - cy.get(".QueryBuilder-section .Icon-add").click(); + openQuestionsSidebar(); // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage cy.findByText("Orders, Count").click(); saveDashboard(); @@ -233,7 +234,7 @@ describe("scenarios > dashboard", () => { cy.findByText("This dashboard is looking empty."); // add previously created question to it cy.icon("pencil").click(); - cy.icon("add").last().click(); + openQuestionsSidebar(); // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage cy.findByText("11007").click(); @@ -535,7 +536,7 @@ describe("scenarios > dashboard", () => { cy.intercept("GET", "/api/search*").as("search"); visitDashboard(1); cy.icon("pencil").click(); - cy.icon("add").last().click(); + openQuestionsSidebar(); sidebar().within(() => { // From the list diff --git a/e2e/test/scenarios/dashboard/tabs.cy.spec.js b/e2e/test/scenarios/dashboard/tabs.cy.spec.js new file mode 100644 index 0000000000000000000000000000000000000000..4986c53865d8c00bcb794511d3f27d6210f1bc66 --- /dev/null +++ b/e2e/test/scenarios/dashboard/tabs.cy.spec.js @@ -0,0 +1,35 @@ +import { + editDashboard, + restore, + visitDashboard, + saveDashboard, + openQuestionsSidebar, +} from "e2e/support/helpers"; + +describe("scenarios > dashboard tabs", () => { + beforeEach(() => { + restore(); + cy.signInAsAdmin(); + }); + + it("should only display cards on the selected tab", () => { + visitDashboard(1); + + editDashboard(); + cy.findByLabelText("Create new tab").click(); + // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage + cy.findByText("Orders").should("not.exist"); + + cy.icon("pencil").click(); + openQuestionsSidebar(); + // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage + cy.findByText("Orders, Count").click(); + saveDashboard(); + + cy.findByRole("tab", { name: "Page 1" }).click(); + // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage + cy.findByText("Orders, count").should("not.exist"); + // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage + cy.findByText("Orders").should("be.visible"); + }); +}); diff --git a/e2e/test/scenarios/models/models.cy.spec.js b/e2e/test/scenarios/models/models.cy.spec.js index 2f5c645ef7e8872186911fd491c10e2c39e19ea7..adc62428789cd20afbcd93eceb24f9d10ef0f0f4 100644 --- a/e2e/test/scenarios/models/models.cy.spec.js +++ b/e2e/test/scenarios/models/models.cy.spec.js @@ -16,6 +16,7 @@ import { closeQuestionActions, visitCollection, undo, + openQuestionsSidebar, } from "e2e/support/helpers"; import { SAMPLE_DB_ID } from "e2e/support/cypress_data"; @@ -522,7 +523,7 @@ describe("scenarios > models", () => { cy.createDashboard().then(({ body: { id: dashboardId } }) => { visitDashboard(dashboardId); cy.icon("pencil").click(); - cy.get(".QueryBuilder-section .Icon-add").click(); + openQuestionsSidebar(); sidebar().findByText("Orders Model").click(); cy.button("Save").click(); // The first fetch happened when visiting dashboard, and the second one upon saving it. diff --git a/e2e/test/scenarios/onboarding/home/activity-page.cy.spec.js b/e2e/test/scenarios/onboarding/home/activity-page.cy.spec.js index 740e41d4a6723422dd83e5913cee98b9948d3ab2..0be3fcd000525ac6863fb84caf619f2c5b3a0503 100644 --- a/e2e/test/scenarios/onboarding/home/activity-page.cy.spec.js +++ b/e2e/test/scenarios/onboarding/home/activity-page.cy.spec.js @@ -7,6 +7,7 @@ import { saveDashboard, visitDashboard, getFullName, + openQuestionsSidebar, } from "e2e/support/helpers"; import { USERS } from "e2e/support/cypress_data"; @@ -75,8 +76,7 @@ describe("metabase > scenarios > home > activity-page", () => { cy.wait("@dashboard"); editDashboard(); - - cy.icon("add").last().click(); + openQuestionsSidebar(); sidebar().within(() => { cy.findByTestId("loading-spinner").should("not.exist"); diff --git a/enterprise/backend/src/metabase_enterprise/serialization/v2/seed_entity_ids.clj b/enterprise/backend/src/metabase_enterprise/serialization/v2/seed_entity_ids.clj index 8680f9bad20b3a67fc3f74d835effb5a85288739..6b08dcc8e3478e1544b9c26cfc15f831c1a91463 100644 --- a/enterprise/backend/src/metabase_enterprise/serialization/v2/seed_entity_ids.clj +++ b/enterprise/backend/src/metabase_enterprise/serialization/v2/seed_entity_ids.clj @@ -40,7 +40,6 @@ [] (into {} (for [model (toucan-models) - :when (mdb.u/toucan-model? model) :let [table-name (some-> model t2/table-name name)] :when table-name ;; ignore any models defined in test namespaces. diff --git a/enterprise/backend/test/metabase_enterprise/sandbox/api/dashboard_test.clj b/enterprise/backend/test/metabase_enterprise/sandbox/api/dashboard_test.clj index 0d455683a1d3e48fd355a0a6d09c91f61def1760..49b0a0f9d095e4c6a2c08f17159f0f8f480c9567 100644 --- a/enterprise/backend/test/metabase_enterprise/sandbox/api/dashboard_test.clj +++ b/enterprise/backend/test/metabase_enterprise/sandbox/api/dashboard_test.clj @@ -89,7 +89,7 @@ s/Keyword s/Any} "mapping")] s/Keyword s/Any}] - (add-card! 200))) + (:cards (add-card! 200)))) (is (schema= [(s/one {:card_id (s/eq card-id) :parameter_mappings (s/eq mappings) s/Keyword s/Any} diff --git a/frontend/src/metabase-types/api/dashboard.ts b/frontend/src/metabase-types/api/dashboard.ts index 6e1d745fe2f4e2a0e14e2bbe234c69eb5470c158..8e560ae6a85808d35f8a3ebc98e45c2296f4c3ad 100644 --- a/frontend/src/metabase-types/api/dashboard.ts +++ b/frontend/src/metabase-types/api/dashboard.ts @@ -18,6 +18,7 @@ export interface Dashboard { description: string | null; model?: string; ordered_cards: (DashboardOrderedCard | ActionDashboardCard)[]; + ordered_tabs?: DashboardOrderedTab[]; parameters?: Parameter[] | null; can_write: boolean; cache_ttl: number | null; @@ -36,6 +37,7 @@ export type DashCardId = number; export type BaseDashboardOrderedCard = { id: DashCardId; dashboard_id: DashboardId; + dashboard_tab_id?: DashboardTabId; size_x: number; size_y: number; col: number; @@ -64,6 +66,18 @@ export type DashboardOrderedCard = BaseDashboardOrderedCard & { series?: Card[]; }; +export type DashboardTabId = number; + +export type DashboardOrderedTab = { + id: DashboardTabId; + dashboard_id: DashboardId; + entity_id: string; + name: string; + position?: number; + created_at: string; + updated_at: string; +}; + export type DashboardParameterMapping = { card_id: CardId; parameter_id: ParameterId; diff --git a/frontend/src/metabase-types/store/dashboard.ts b/frontend/src/metabase-types/store/dashboard.ts index d978f103eedf32c1329b11e9682a5beb6ab0129d..d1c2c8be5129066701177b98e911e403ebbd5bdd 100644 --- a/frontend/src/metabase-types/store/dashboard.ts +++ b/frontend/src/metabase-types/store/dashboard.ts @@ -10,16 +10,29 @@ import type { export type DashboardSidebarName = | "addQuestion" + | "action" | "clickBehavior" | "editParameter" | "sharing" | "info"; +export type StoreDashboard = Omit<Dashboard, "ordered_cards"> & { + ordered_cards: DashCardId[]; +}; + +export type StoreDashcard = DashboardOrderedCard & { + isDirty?: boolean; + isRemoved?: boolean; +}; + +export type SelectedTabId = DashboardId | null; + export interface DashboardState { dashboardId: DashboardId | null; - dashboards: Record<DashboardId, Dashboard>; + selectedTabId: SelectedTabId; + dashboards: Record<DashboardId, StoreDashboard>; - dashcards: Record<DashCardId, DashboardOrderedCard>; + dashcards: Record<DashCardId, StoreDashcard>; dashcardData: DashCardDataMap; parameterValues: Record<ParameterId, ParameterValueOrArray>; @@ -29,6 +42,7 @@ export interface DashboardState { loadingIds: DashCardId[]; loadingStatus: "idle" | "running" | "complete"; startTime: number | null; + endTime: number | null; }; loadingControls: { documentTitle?: string; @@ -44,4 +58,11 @@ export interface DashboardState { name?: DashboardSidebarName; props: Record<string, unknown>; }; + + missingActionParameters: unknown; + + autoApplyFilters: { + toastId: number | null; + toastDashboardId: number | null; + }; } diff --git a/frontend/src/metabase-types/store/mocks/dashboard.ts b/frontend/src/metabase-types/store/mocks/dashboard.ts index d598de9f727b95e571da7b095f57c9d921ceaaba..31af2648155ab7f0ff31e0a2059d81e906a99ad6 100644 --- a/frontend/src/metabase-types/store/mocks/dashboard.ts +++ b/frontend/src/metabase-types/store/mocks/dashboard.ts @@ -13,6 +13,7 @@ export const createMockDashboardState = ( loadingIds: [], loadingStatus: "idle", startTime: null, + endTime: null, }, loadingControls: {}, isEditing: null, @@ -21,5 +22,11 @@ export const createMockDashboardState = ( sidebar: { props: {}, }, + selectedTabId: null, + missingActionParameters: null, + autoApplyFilters: { + toastId: null, + toastDashboardId: null, + }, ...opts, }); diff --git a/frontend/src/metabase/actions/containers/ActionParametersInputForm/ActionParametersInputForm.tsx b/frontend/src/metabase/actions/containers/ActionParametersInputForm/ActionParametersInputForm.tsx index 1f369ec507d4040094dc5e6c80cea7d07d213dc7..86d3f459d303333deb12a3fecbc005e1059bf048 100644 --- a/frontend/src/metabase/actions/containers/ActionParametersInputForm/ActionParametersInputForm.tsx +++ b/frontend/src/metabase/actions/containers/ActionParametersInputForm/ActionParametersInputForm.tsx @@ -85,12 +85,13 @@ function ActionParametersInputForm({ const hasValueFromDashboard = Object.keys(dashcardParamValues).length > 0; const canPrefetch = hasValueFromDashboard && dashboard && dashcard; - if (shouldPrefetch) { + if (shouldPrefetch && !hasPrefetchedValues) { setPrefetchedValues({}); canPrefetch && fetchInitialValues(); } }, [ shouldPrefetch, + hasPrefetchedValues, dashboard, dashcard, dashcardParamValues, diff --git a/frontend/src/metabase/components/Timeline/Timeline.jsx b/frontend/src/metabase/components/Timeline/Timeline.jsx index b0360b390e84d28906c0ced09b28ba2238e69764..72405cf8ddc5d3e3ae9d1940851bf3f284c12ee3 100644 --- a/frontend/src/metabase/components/Timeline/Timeline.jsx +++ b/frontend/src/metabase/components/Timeline/Timeline.jsx @@ -67,6 +67,7 @@ function Timeline({ const { icon, title, + titleText, description, timestamp, formattedTimestamp, @@ -96,6 +97,7 @@ function Timeline({ borderless onClick={() => revertFn(revision)} data-testid="question-revert-button" + aria-label={t`revert to ${titleText}`} /> </Tooltip> )} diff --git a/frontend/src/metabase/core/components/TabButton/TabButton.tsx b/frontend/src/metabase/core/components/TabButton/TabButton.tsx index 27327cbb9071698c3a71dd045aba8fb87e9468f4..53dff7a850f0997f4873791df80fc67402d97a9c 100644 --- a/frontend/src/metabase/core/components/TabButton/TabButton.tsx +++ b/frontend/src/metabase/core/components/TabButton/TabButton.tsx @@ -25,30 +25,28 @@ import { import { TabButtonInput, TabButtonRoot, MenuButton } from "./TabButton.styled"; import TabButtonMenu from "./TabButtonMenu"; -export type TabButtonValue = string | number; - -export type TabButtonMenuAction = ( +export type TabButtonMenuAction<T> = ( context: TabContextType, - value?: TabButtonValue, + value: T, ) => void; -export interface TabButtonMenuItem { +export interface TabButtonMenuItem<T> { label: string; - action: TabButtonMenuAction; + action: TabButtonMenuAction<T>; } -export interface TabButtonProps extends HTMLAttributes<HTMLDivElement> { +export interface TabButtonProps<T> extends HTMLAttributes<HTMLDivElement> { label: string; - value?: TabButtonValue; + value: T; showMenu?: boolean; - menuItems?: TabButtonMenuItem[]; + menuItems?: TabButtonMenuItem<T>[]; onEdit?: ChangeEventHandler<HTMLInputElement>; onFinishEditing?: () => void; isEditing?: boolean; disabled?: boolean; } -const TabButton = forwardRef(function TabButton( +const TabButton = forwardRef(function TabButton<T>( { value, menuItems, @@ -60,7 +58,7 @@ const TabButton = forwardRef(function TabButton( isEditing = false, showMenu: showMenuProp = true, ...props - }: TabButtonProps, + }: TabButtonProps<T>, inputRef: Ref<HTMLInputElement>, ) { const { value: selectedValue, idPrefix, onChange } = useContext(TabContext); @@ -90,15 +88,11 @@ const TabButton = forwardRef(function TabButton( const handleInputKeyPress: KeyboardEventHandler<HTMLInputElement> = useCallback( event => { - if (event.key !== "Enter") { - return; - } - if (typeof inputRef === "object") { + if (event.key === "Enter" && typeof inputRef === "object") { inputRef?.current?.blur(); } - onFinishEditing?.(); }, - [onFinishEditing, inputRef], + [inputRef], ); return ( @@ -111,6 +105,7 @@ const TabButton = forwardRef(function TabButton( aria-selected={isSelected} aria-controls={getTabPanelId(idPrefix, value)} aria-disabled={disabled} + aria-label={label} id={getTabId(idPrefix, value)} > <TabButtonInput @@ -122,6 +117,7 @@ const TabButton = forwardRef(function TabButton( onKeyPress={handleInputKeyPress} onFocus={e => e.currentTarget.select()} onBlur={onFinishEditing} + aria-labelledby={getTabId(idPrefix, value)} id={getTabButtonInputId(idPrefix, value)} ref={inputRef} /> @@ -143,7 +139,7 @@ const TabButton = forwardRef(function TabButton( /> )} popoverContent={({ closePopover }) => ( - <TabButtonMenu + <TabButtonMenu<T> menuItems={menuItems} value={value} closePopover={closePopover} @@ -155,26 +151,30 @@ const TabButton = forwardRef(function TabButton( ); }); -export interface RenameableTabButtonProps - extends Omit<TabButtonProps, "onEdit" | "onFinishEditing" | "isEditing"> { +export interface RenameableTabButtonProps<T> + extends Omit<TabButtonProps<T>, "onEdit" | "onFinishEditing" | "isEditing"> { onRename: (newLabel: string) => void; renameMenuLabel?: string; renameMenuIndex?: number; } -export function RenameableTabButton({ - label: originalLabel, +export function RenameableTabButton<T>({ + label: labelProp, menuItems: originalMenuItems = [], onRename, renameMenuLabel = t`Rename`, renameMenuIndex = 0, ...props -}: RenameableTabButtonProps) { - const [label, setLabel] = useState(originalLabel); +}: RenameableTabButtonProps<T>) { + const [label, setLabel] = useState(labelProp); const [prevLabel, setPrevLabel] = useState(label); const [isEditing, setIsEditing] = useState(false); const inputRef = useRef<HTMLInputElement>(null); + useEffect(() => { + setLabel(labelProp); + }, [labelProp]); + useEffect(() => { if (isEditing) { inputRef.current?.focus(); diff --git a/frontend/src/metabase/core/components/TabButton/TabButton.unit.spec.tsx b/frontend/src/metabase/core/components/TabButton/TabButton.unit.spec.tsx index 1439631a16acc571f758167a7841a2f8a06ca02e..f1bc74a49c31826e77ed087b906c93daac11aa42 100644 --- a/frontend/src/metabase/core/components/TabButton/TabButton.unit.spec.tsx +++ b/frontend/src/metabase/core/components/TabButton/TabButton.unit.spec.tsx @@ -4,9 +4,9 @@ import userEvent from "@testing-library/user-event"; import { getIcon } from "__support__/ui"; import TabRow from "../TabRow"; -import TabButton, { TabButtonProps } from "./TabButton"; +import TabButton, { RenameableTabButtonProps } from "./TabButton"; -function setup(props?: Omit<TabButtonProps, "label">) { +function setup(props?: Partial<RenameableTabButtonProps<string>>) { const action = jest.fn(); const onRename = jest.fn(); const value = "some_value"; diff --git a/frontend/src/metabase/core/components/TabButton/TabButtonMenu.tsx b/frontend/src/metabase/core/components/TabButton/TabButtonMenu.tsx index 7ecc704806b43c46936916868648a6b84231e60d..b43e97975717a68a4e407b586348ca301f6e18b6 100644 --- a/frontend/src/metabase/core/components/TabButton/TabButtonMenu.tsx +++ b/frontend/src/metabase/core/components/TabButton/TabButtonMenu.tsx @@ -1,27 +1,23 @@ import React, { useContext } from "react"; import { TabContext } from "../Tab/TabContext"; import { getTabButtonInputId } from "../Tab/utils"; -import { - TabButtonMenuAction, - TabButtonMenuItem, - TabButtonValue, -} from "./TabButton"; +import { TabButtonMenuAction, TabButtonMenuItem } from "./TabButton"; import { MenuContent, MenuItem } from "./TabButton.styled"; -interface TabButtonMenuProps { - menuItems: TabButtonMenuItem[]; - value?: TabButtonValue; +interface TabButtonMenuProps<T> { + menuItems: TabButtonMenuItem<T>[]; + value: T; closePopover: () => void; } -export default function TabButtonMenu({ +export default function TabButtonMenu<T>({ menuItems, value, closePopover, -}: TabButtonMenuProps) { +}: TabButtonMenuProps<T>) { const context = useContext(TabContext); - const clickHandler = (action: TabButtonMenuAction) => () => { + const clickHandler = (action: TabButtonMenuAction<T>) => () => { action(context, value); closePopover(); }; diff --git a/frontend/src/metabase/core/components/TabList/TabList.tsx b/frontend/src/metabase/core/components/TabList/TabList.tsx index 6ed471cc28854b159e15bf3b17ed33a2ba635f65..51abcf77549d732dc0f1bafd1cc03b2830c18391 100644 --- a/frontend/src/metabase/core/components/TabList/TabList.tsx +++ b/frontend/src/metabase/core/components/TabList/TabList.tsx @@ -5,7 +5,6 @@ import React, { Ref, useContext, useMemo, - useRef, } from "react"; import { useUniqueId } from "metabase/hooks/use-unique-id"; import { TabContext, TabContextType } from "../Tab"; @@ -15,18 +14,17 @@ export interface TabListProps<T> extends Omit<HTMLAttributes<HTMLDivElement>, "onChange"> { value?: T; onChange?: (value: T) => void; + onScroll?: React.UIEventHandler<HTMLDivElement>; children?: ReactNode; } const TabList = forwardRef(function TabGroup<T>( - { value, onChange, children, ...props }: TabListProps<T>, + { value, onChange, onScroll, children, ...props }: TabListProps<T>, ref: Ref<HTMLDivElement>, ) { const idPrefix = useUniqueId(); const outerContext = useContext(TabContext); - const tabListContentRef = useRef(null); - const innerContext = useMemo(() => { return { value, idPrefix, onChange }; }, [value, idPrefix, onChange]); @@ -34,8 +32,8 @@ const TabList = forwardRef(function TabGroup<T>( const activeContext = outerContext.isDefault ? innerContext : outerContext; return ( - <TabListRoot {...props} ref={ref} role="tablist"> - <TabListContent ref={tabListContentRef}> + <TabListRoot {...props} role="tablist"> + <TabListContent ref={ref} onScroll={onScroll}> <TabContext.Provider value={activeContext as TabContextType}> {children} </TabContext.Provider> diff --git a/frontend/src/metabase/core/components/TabRow/TabRow.stories.tsx b/frontend/src/metabase/core/components/TabRow/TabRow.stories.tsx index 20aae3215ca4fa62c6bebcfebf9346705d84aff9..6bf64f574d455a291c8b7c85993a574482d18a42 100644 --- a/frontend/src/metabase/core/components/TabRow/TabRow.stories.tsx +++ b/frontend/src/metabase/core/components/TabRow/TabRow.stories.tsx @@ -21,6 +21,7 @@ const sampleStyle = { width: "100%", padding: "10px", border: "1px solid #ccc", + backgroundColor: "white", }; const Template: ComponentStory<typeof TabRow> = args => { @@ -28,12 +29,15 @@ const Template: ComponentStory<typeof TabRow> = args => { const handleChange = (value: unknown) => updateArgs({ value }); const [message, setMessage] = useState(""); - const action: TabButtonMenuAction = ({ value: selectedValue }, value) => + const action: TabButtonMenuAction<unknown> = ( + { value: selectedValue }, + value, + ) => setMessage( `Clicked ${value}, currently selected value is ${selectedValue}`, ); - const menuItems: TabButtonMenuItem[] = [ + const menuItems: TabButtonMenuItem<unknown>[] = [ { label: "Click me!", action, @@ -64,6 +68,11 @@ const Template: ComponentStory<typeof TabRow> = args => { <TabButton label="Tab 5" value={5} menuItems={menuItems} /> <TabButton label="Tab 6" value={6} disabled /> <TabButton label="Tab 7" value={7} menuItems={menuItems} disabled /> + <TabButton + label="Tab 8 with a very long name" + value={8} + menuItems={menuItems} + /> </TabRow> {message} </div> diff --git a/frontend/src/metabase/core/components/TabRow/TabRow.styled.tsx b/frontend/src/metabase/core/components/TabRow/TabRow.styled.tsx index 9c3e8112403952470a3db579bc47f747ab534ffb..c8e137608df8bfd894f35b58e1ffe395ab24f454 100644 --- a/frontend/src/metabase/core/components/TabRow/TabRow.styled.tsx +++ b/frontend/src/metabase/core/components/TabRow/TabRow.styled.tsx @@ -1,16 +1,24 @@ import styled from "@emotion/styled"; -import { color } from "metabase/lib/colors"; +import { alpha, color } from "metabase/lib/colors"; import BaseTabList from "metabase/core/components/TabList"; import TabLink from "metabase/core/components/TabLink"; import TabButton from "metabase/core/components/TabButton"; +import { space } from "metabase/styled-components/theme"; export const TabList = styled(BaseTabList)` + width: 100%; border-bottom: 1px solid ${color("border")}; ${BaseTabList.Content} { display: flex; - overflow: hidden; + overflow-x: scroll; + /* Chrome */ + ::-webkit-scrollbar { + display: none; + } + -ms-overflow-style: none; /* IE and Edge */ + scrollbar-width: none; /* Firefox */ } ${TabLink.Root}:not(:last-child) { @@ -21,3 +29,29 @@ export const TabList = styled(BaseTabList)` margin-right: 2rem; } `; + +interface ScrollButtonProps { + direction: "left" | "right"; +} + +export const ScrollButton = styled.button<ScrollButtonProps>` + position: absolute; + cursor: pointer; + height: 100%; + height: 100%; + width: 3rem; + padding-bottom: ${space(2)}; + text-align: ${props => props.direction}; + color: ${color("text-light")}; + &:hover { + color: ${color("brand")}; + } + ${props => props.direction}: 0; + background: linear-gradient( + to ${props => props.direction}, + ${alpha("white", 0.1)}, + ${alpha("white", 0.5)}, + 30%, + ${color("white")} + ); +`; diff --git a/frontend/src/metabase/core/components/TabRow/TabRow.tsx b/frontend/src/metabase/core/components/TabRow/TabRow.tsx index 9203198469ea9ccb7f750d3be8dd3ef4f9b4c0c4..d08efaa6f03593256dee034c6c4bb06bf2b95425 100644 --- a/frontend/src/metabase/core/components/TabRow/TabRow.tsx +++ b/frontend/src/metabase/core/components/TabRow/TabRow.tsx @@ -1,8 +1,80 @@ -import React from "react"; +import React, { useState, useRef, useEffect } from "react"; +import Icon from "metabase/components/Icon/Icon"; +import ExplicitSize from "metabase/components/ExplicitSize"; import { TabListProps } from "../TabList/TabList"; -import { TabList } from "./TabRow.styled"; +import { ScrollButton, TabList } from "./TabRow.styled"; -export default function TabRow<T>({ onChange, ...props }: TabListProps<T>) { - return <TabList onChange={onChange as (value: unknown) => void} {...props} />; +interface TabRowInnerProps<T> extends TabListProps<T> { + width: number | null; +} +function TabRowInner<T>({ + width, + onChange, + children, + ...props +}: TabRowInnerProps<T>) { + const tabListRef = useRef<HTMLDivElement>(null); + const [scrollPosition, setScrollPosition] = useState(0); + const [showScrollRight, setShowScrollRight] = useState(false); + const showScrollLeft = scrollPosition > 0; + + useEffect(() => { + if (!width || !tabListRef.current) { + return; + } + + setShowScrollRight( + scrollPosition + width < tabListRef.current?.scrollWidth, + ); + }, [children, scrollPosition, width]); + + const scroll = (direction: "left" | "right") => { + if (!tabListRef.current || !width) { + return; + } + + const scrollDistance = width * (direction === "left" ? -1 : 1); + tabListRef.current.scrollBy(scrollDistance, 0); + }; + + return ( + <TabList + onChange={onChange as (value: unknown) => void} + onScroll={event => setScrollPosition(event.currentTarget.scrollLeft)} + ref={tabListRef} + {...props} + > + {children} + {showScrollLeft && ( + <ScrollArrow direction="left" onClick={() => scroll("left")} /> + )} + {showScrollRight && ( + <ScrollArrow direction="right" onClick={() => scroll("right")} /> + )} + </TabList> + ); +} + +const TabRowInnerWithSize = ExplicitSize()(TabRowInner); + +export default function TabRow<T>(props: TabListProps<T>) { + return <TabRowInnerWithSize {...props} />; +} + +interface ScrollArrowProps { + direction: "left" | "right"; + onClick: () => void; +} + +export function ScrollArrow({ direction, onClick }: ScrollArrowProps) { + return ( + <ScrollButton + onClick={onClick} + direction={direction} + aria-label={`scroll tabs ${direction}`} + > + <Icon name={`chevron${direction}`} color="brand" /> + </ScrollButton> + ); } diff --git a/frontend/src/metabase/core/components/TabRow/index.ts b/frontend/src/metabase/core/components/TabRow/index.ts index b4856f361a4857855fe5b8cea58240cba833a96b..911fd8dbf2e6098d9e121b6cad1a0ab00b452397 100644 --- a/frontend/src/metabase/core/components/TabRow/index.ts +++ b/frontend/src/metabase/core/components/TabRow/index.ts @@ -1 +1,2 @@ export { default } from "./TabRow"; +export * from "./TabRow"; diff --git a/frontend/src/metabase/dashboard/actions/cards.js b/frontend/src/metabase/dashboard/actions/cards.js index afb92b5295c1975151f74682a5f298d6fc95c5a8..fe78bd94b9598f69e28fd5f235fa0ff6f897aa6b 100644 --- a/frontend/src/metabase/dashboard/actions/cards.js +++ b/frontend/src/metabase/dashboard/actions/cards.js @@ -24,7 +24,7 @@ function generateTemporaryDashcardId() { } export const addCardToDashboard = - ({ dashId, cardId }) => + ({ dashId, cardId, tabId }) => async (dispatch, getState) => { await dispatch(Questions.actions.fetch({ id: cardId })); const card = Questions.selectors.getObject(getState(), { @@ -42,6 +42,7 @@ export const addCardToDashboard = const dashcard = { id: generateTemporaryDashcardId(), dashboard_id: dashId, + dashboard_tab_id: tabId ?? null, card_id: card.id, card: card, series: [], @@ -59,7 +60,11 @@ export const addCardToDashboard = dispatch(loadMetadataForDashboard([dashcard])); }; -export const addDashCardToDashboard = function ({ dashId, dashcardOverrides }) { +export const addDashCardToDashboard = function ({ + dashId, + dashcardOverrides, + tabId, +}) { return function (dispatch, getState) { const { dashboards, dashcards } = getState().dashboard; const dashboard = dashboards[dashId]; @@ -75,6 +80,7 @@ export const addDashCardToDashboard = function ({ dashId, dashcardOverrides }) { card_id: null, card: null, dashboard_id: dashId, + dashboard_tab_id: tabId ?? null, series: [], ...getPositionForNewDashCard( existingCards, @@ -89,7 +95,7 @@ export const addDashCardToDashboard = function ({ dashId, dashcardOverrides }) { }; }; -export const addTextDashCardToDashboard = function ({ dashId }) { +export const addTextDashCardToDashboard = function ({ dashId, tabId }) { const virtualTextCard = createCard(); virtualTextCard.display = "text"; virtualTextCard.archived = false; @@ -103,10 +109,11 @@ export const addTextDashCardToDashboard = function ({ dashId }) { return addDashCardToDashboard({ dashId: dashId, dashcardOverrides: dashcardOverrides, + tabId, }); }; -export const addLinkDashCardToDashboard = function ({ dashId }) { +export const addLinkDashCardToDashboard = function ({ dashId, tabId }) { const virtualLinkCard = { ...createCard(), display: "link", @@ -122,11 +129,12 @@ export const addLinkDashCardToDashboard = function ({ dashId }) { return addDashCardToDashboard({ dashId: dashId, dashcardOverrides: dashcardOverrides, + tabId, }); }; export const addActionToDashboard = - async ({ dashId, action, displayType }) => + async ({ dashId, tabId, action, displayType }) => dispatch => { const virtualActionsCard = { ...createCard(), @@ -153,6 +161,7 @@ export const addActionToDashboard = addDashCardToDashboard({ dashId: dashId, dashcardOverrides: dashcardOverrides, + tabId, }), ); }; diff --git a/frontend/src/metabase/dashboard/actions/core.js b/frontend/src/metabase/dashboard/actions/core.js index a588803e6a3e1082609dbb2e12ce544f1c639790..d73a594a7b0cc3d5eacfea51820be852c4145a6b 100644 --- a/frontend/src/metabase/dashboard/actions/core.js +++ b/frontend/src/metabase/dashboard/actions/core.js @@ -54,9 +54,3 @@ export const onReplaceAllDashCardVisualizationSettings = createAction( REPLACE_ALL_DASHCARD_VISUALIZATION_SETTINGS, (id, settings) => ({ id, settings }), ); - -export const UPDATE_DASHCARD_IDS = "metabase/dashboard/UPDATE_DASHCARD_IDS"; -export const updateDashcardIds = createAction( - UPDATE_DASHCARD_IDS, - (oldDashcardIds, newDashcardIds) => ({ oldDashcardIds, newDashcardIds }), -); diff --git a/frontend/src/metabase/dashboard/actions/index.ts b/frontend/src/metabase/dashboard/actions/index.ts index fd79eb7ee6b79a07764cf436f7ec95367d7d4a8e..eede58b76b18c29a0bdda66124e5c2b44ecb6daf 100644 --- a/frontend/src/metabase/dashboard/actions/index.ts +++ b/frontend/src/metabase/dashboard/actions/index.ts @@ -8,3 +8,4 @@ export * from "./save"; export * from "./sharing"; export * from "./ui"; export * from "./actions"; +export * from "./tabs"; diff --git a/frontend/src/metabase/dashboard/actions/save.js b/frontend/src/metabase/dashboard/actions/save.js index e1ba63805c6b12abcba38d9b28dbf920955f8d6d..ccff08622ff497d2dcac15ab0f59d36512465a5d 100644 --- a/frontend/src/metabase/dashboard/actions/save.js +++ b/frontend/src/metabase/dashboard/actions/save.js @@ -10,9 +10,9 @@ import { clickBehaviorIsValid } from "metabase-lib/parameters/utils/click-behavi import { getDashboardBeforeEditing } from "../selectors"; -import { updateDashcardIds } from "./core"; import { fetchDashboard } from "./data-fetching"; import { hasDashboardChanged, haveDashboardCardsChanged } from "./utils"; +import { saveCardsAndTabs } from "./tabs"; export const SAVE_DASHBOARD_AND_CARDS = "metabase/dashboard/SAVE_DASHBOARD_AND_CARDS"; @@ -99,15 +99,16 @@ export const saveDashboardAndCards = createThunkAction( ); } - // update the dashboard cards + // update the dashboard cards and tabs const dashcardsToUpdate = dashboard.ordered_cards.filter( dc => !dc.isRemoved, ); - const updatedDashCards = await DashboardApi.updateCards({ + const updatedCardsAndTabs = await DashboardApi.updateCardsAndTabs({ dashId: dashboard.id, cards: dashcardsToUpdate.map(dc => ({ id: dc.id, card_id: dc.card_id, + dashboard_tab_id: dc.dashboard_tab_id, action_id: dc.action_id, row: dc.row, col: dc.col, @@ -117,13 +118,12 @@ export const saveDashboardAndCards = createThunkAction( visualization_settings: dc.visualization_settings, parameter_mappings: dc.parameter_mappings, })), + ordered_tabs: (dashboard.ordered_tabs ?? []).map(({ id, name }) => ({ + id, + name, + })), }); - dispatch( - updateDashcardIds( - dashcardsToUpdate.map(dc => dc.id), - updatedDashCards.map(dc => dc.id), - ), - ); + dispatch(saveCardsAndTabs(updatedCardsAndTabs)); await dispatch(Dashboards.actions.update(dashboard)); diff --git a/frontend/src/metabase/dashboard/actions/tabs.ts b/frontend/src/metabase/dashboard/actions/tabs.ts new file mode 100644 index 0000000000000000000000000000000000000000..13ba5c288e3242fc62f541cf63265362b89784a3 --- /dev/null +++ b/frontend/src/metabase/dashboard/actions/tabs.ts @@ -0,0 +1,280 @@ +import type { Action } from "redux-actions"; +import { t } from "ttag"; + +import { + DashboardId, + DashboardOrderedCard, + DashboardOrderedTab, + DashboardTabId, +} from "metabase-types/api"; +import { DashboardState } from "metabase-types/store"; +import { createAction, handleActions } from "metabase/lib/redux"; + +import { INITIAL_DASHBOARD_STATE } from "../constants"; + +type CreateNewTabPayload = { tabId: DashboardTabId }; +type DeleteTabPayload = { tabId: DashboardTabId | null }; +type SelectTabPayload = { tabId: DashboardTabId | null }; +type RenameTabPayload = { tabId: DashboardTabId | null; name: string }; +type SaveCardsAndTabsPayload = { + cards: DashboardOrderedCard[]; + ordered_tabs: DashboardOrderedTab[]; +}; +type TabsReducerPayload = CreateNewTabPayload & + DeleteTabPayload & + SelectTabPayload & + RenameTabPayload & + SaveCardsAndTabsPayload; + +const CREATE_NEW_TAB = "metabase/dashboard/CREATE_NEW_TAB"; +const DELETE_TAB = "metabase/dashboard/DELETE_TAB"; +const RENAME_TAB = "metabase/dashboard/RENAME_TAB"; +const SELECT_TAB = "metabase/dashboard/SELECT_TAB"; +const SAVE_CARDS_AND_TABS = "metabase/dashboard/SAVE_CARDS_AND_TABS"; +const INIT_TABS = "metabase/dashboard/INIT_TABS"; + +let tempNewTabId = -2; +export function createNewTab() { + // Decrement by 2 to leave space for two new tabs if dash doesn't have tabs already + const tabId = tempNewTabId; + tempNewTabId -= 2; + + const action = createAction<CreateNewTabPayload>(CREATE_NEW_TAB); + return action({ tabId }); +} + +export const deleteTab = createAction<DeleteTabPayload>(DELETE_TAB); + +export const selectTab = createAction<SelectTabPayload>(SELECT_TAB); + +export const renameTab = createAction<RenameTabPayload>(RENAME_TAB); + +export const saveCardsAndTabs = + createAction<SaveCardsAndTabsPayload>(SAVE_CARDS_AND_TABS); + +export const initTabs = createAction(INIT_TABS); + +function getPrevDashAndTabs(state: DashboardState) { + const dashId = state.dashboardId; + const prevDash = dashId ? state.dashboards[dashId] : null; + const prevTabs = prevDash?.ordered_tabs ?? []; + + return { dashId, prevDash, prevTabs }; +} + +export function getDefaultTab({ + tabId, + dashId, + name, +}: { + tabId: DashboardTabId; + dashId: DashboardId; + name: string; +}) { + return { + id: tabId, + dashboard_id: dashId, + name, + entity_id: "", + created_at: "", + updated_at: "", + }; +} + +export const tabsReducer = handleActions<DashboardState, TabsReducerPayload>( + { + [CREATE_NEW_TAB]: ( + state, + { payload: { tabId } }: Action<CreateNewTabPayload>, + ) => { + const { dashId, prevDash, prevTabs } = getPrevDashAndTabs(state); + if (!dashId || !prevDash) { + throw Error( + `CREATE_NEW_TAB was dispatched but either dashId (${dashId}) or prevDash (${prevDash}) are null`, + ); + } + + // Case 1: Dashboard already has tabs + if (prevTabs.length !== 0) { + // 1. Create new tab, add to dashboard + const newTab = getDefaultTab({ + tabId, + dashId, + name: t`Page ${prevTabs.length + 1}`, + }); + const dashboards: DashboardState["dashboards"] = { + ...state.dashboards, + [dashId]: { + ...prevDash, + ordered_tabs: [...prevTabs, newTab], + }, + }; + + // 2. Select new tab + const selectedTabId = tabId; + + return { ...state, dashboards, selectedTabId }; + } + + // Case 2: Dashboard doesn't have tabs + + // 1. Create two new tabs, add to dashboard + const firstTabId = tabId + 1; + const secondTabId = tabId; + const newTabs = [ + getDefaultTab({ tabId: firstTabId, dashId, name: t`Page 1` }), + getDefaultTab({ tabId: secondTabId, dashId, name: t`Page 2` }), + ]; + const dashboards: DashboardState["dashboards"] = { + ...state.dashboards, + [dashId]: { + ...prevDash, + ordered_tabs: [...prevTabs, ...newTabs], + }, + }; + + // 2. Select second tab + const selectedTabId = secondTabId; + + // 3. Assign existing dashcards to first tab + const dashcards: DashboardState["dashcards"] = { ...state.dashcards }; + if (prevTabs.length === 0) { + prevDash.ordered_cards.forEach(id => { + dashcards[id] = { + ...state.dashcards[id], + isDirty: true, + dashboard_tab_id: firstTabId, + }; + }); + } + + return { + ...state, + dashboards, + selectedTabId, + dashcards, + }; + }, + [DELETE_TAB]: (state, { payload: { tabId } }: Action<DeleteTabPayload>) => { + const { dashId, prevDash, prevTabs } = getPrevDashAndTabs(state); + const tabToRemove = prevTabs.find(({ id }) => id === tabId); + if (!dashId || !prevDash || !tabToRemove) { + throw Error( + `DELETE_TAB was dispatched but either dashId (${dashId}), prevDash (${prevDash}), or tabToRemove (${tabToRemove}) is null/undefined`, + ); + } + + // 1. Select a different tab if needed + let selectedTabId = state.selectedTabId; + + const noTabsRemaining = prevTabs.length === 1; + const deletingSelectedTab = selectedTabId === tabToRemove.id; + if (noTabsRemaining) { + selectedTabId = null; + } else if (deletingSelectedTab) { + const tabToRemoveIndex = prevTabs.findIndex( + ({ id }) => id === tabToRemove.id, + ); + const targetIndex = tabToRemoveIndex === 0 ? 1 : tabToRemoveIndex - 1; + selectedTabId = prevTabs[targetIndex].id; + } + + // 2. Remove the tab + const newTabs = prevTabs.filter(({ id }) => id !== tabId); + const dashboards: DashboardState["dashboards"] = { + ...state.dashboards, + [dashId]: { + ...prevDash, + ordered_tabs: newTabs, + }, + }; + + // 3. Remove dashcards that were on the deleted tab + const removedCardIds = prevDash.ordered_cards.filter( + id => state.dashcards[id].dashboard_tab_id === tabId, + ); + const removedDashcards: DashboardState["dashcards"] = {}; + removedCardIds.forEach(id => { + removedDashcards[id] = { + ...state.dashcards[id], + isRemoved: true, + }; + }); + const dashcards = { ...state.dashcards, ...removedDashcards }; + + return { ...state, selectedTabId, dashboards, dashcards }; + }, + [RENAME_TAB]: ( + state, + { payload: { tabId, name } }: Action<RenameTabPayload>, + ) => { + const { dashId, prevDash, prevTabs } = getPrevDashAndTabs(state); + const tabToRename = prevTabs.find(({ id }) => id === tabId); + if (!dashId || !prevDash || !tabToRename) { + throw Error( + `RENAME_TAB was dispatched but either dashId (${dashId}), prevDash (${prevDash}), or tabToRename (${tabToRename}) is null/undefined`, + ); + } + + const tabToRenameIndex = prevTabs.findIndex( + ({ id }) => id === tabToRename.id, + ); + const newTabs = [...prevTabs]; + newTabs[tabToRenameIndex] = { ...tabToRename, name }; + + const dashboards: DashboardState["dashboards"] = { + ...state.dashboards, + [dashId]: { + ...prevDash, + ordered_tabs: newTabs, + }, + }; + + return { ...state, dashboards }; + }, + [SELECT_TAB]: ( + state, + { payload: { tabId } }: Action<SelectTabPayload>, + ): DashboardState => ({ + ...state, + selectedTabId: tabId, + }), + [SAVE_CARDS_AND_TABS]: ( + state, + { + payload: { cards: newCards, ordered_tabs: newTabs }, + }: Action<SaveCardsAndTabsPayload>, + ) => { + const { prevDash, prevTabs } = getPrevDashAndTabs(state); + if (!prevDash) { + throw Error( + `SAVE_CARDS_AND_TABS was dispatched but prevDash (${prevDash}) is null`, + ); + } + + // 1. Replace temporary with real dashcard ids + const dashcardData: DashboardState["dashcardData"] = {}; + const prevCards = prevDash.ordered_cards.filter( + id => !state.dashcards[id].isRemoved, + ); + prevCards.forEach((oldId, index) => { + dashcardData[newCards[index].id] = state.dashcardData[oldId]; + }); + + // 2. Re-select the currently selected tab with its real id + const selectedTabIndex = prevTabs.findIndex( + tab => tab.id === state.selectedTabId, + ); + const selectedTabId = newTabs[selectedTabIndex]?.id ?? null; + + return { ...state, dashcardData, selectedTabId }; + }, + [INIT_TABS]: state => { + const { prevTabs } = getPrevDashAndTabs(state); + const selectedTabId = prevTabs[0]?.id ?? null; + + return { ...state, selectedTabId }; + }, + }, + INITIAL_DASHBOARD_STATE, +); diff --git a/frontend/src/metabase/dashboard/components/Dashboard/Dashboard.jsx b/frontend/src/metabase/dashboard/components/Dashboard/Dashboard.jsx index ea8aa4b4a72353d9e40d0203c8f520940a1a573f..39693814bae7a34e7b4012db3189e4fda88cf1f5 100644 --- a/frontend/src/metabase/dashboard/components/Dashboard/Dashboard.jsx +++ b/frontend/src/metabase/dashboard/components/Dashboard/Dashboard.jsx @@ -56,6 +56,7 @@ class Dashboard extends Component { dashboard: PropTypes.object, dashboardId: PropTypes.number, + selectedTabId: PropTypes.number, parameters: PropTypes.array, parameterValues: PropTypes.object, draftParameterValues: PropTypes.object, @@ -175,7 +176,11 @@ class Dashboard extends Component { this.setEditing(this.props.dashboard); } if (addCardOnLoad != null) { - addCardToDashboard({ dashId: dashboardId, cardId: addCardOnLoad }); + addCardToDashboard({ + dashId: dashboardId, + cardId: addCardOnLoad, + tabId: this.props.dashboard.ordered_tabs[0]?.id ?? null, + }); } } catch (error) { if (error.status === 404) { @@ -187,6 +192,22 @@ class Dashboard extends Component { } } + getDashboardWithFilteredCards = () => { + if (!this.props.dashboard) { + return; + } + + return { + ...this.props.dashboard, + ordered_cards: this.props.dashboard.ordered_cards.filter( + dc => + !this.props.selectedTabId || + dc.dashboard_tab_id === this.props.selectedTabId || + dc.dashboard_tab_id === null, + ), + }; + }; + setEditing = isEditing => { this.props.onRefreshPeriodChange(null); this.props.setEditingDashboard(isEditing); @@ -238,7 +259,8 @@ class Dashboard extends Component { const { error, isParametersWidgetSticky } = this.state; const shouldRenderAsNightMode = isNightMode && isFullscreen; - const dashboardHasCards = dashboard => dashboard.ordered_cards.length > 0; + const dashboardHasCards = + this.getDashboardWithFilteredCards()?.ordered_cards.length > 0 ?? false; const visibleParameters = getVisibleParameters(parameters); const parametersWidget = ( @@ -325,9 +347,10 @@ class Dashboard extends Component { addMarginTop={cardsContainerShouldHaveMarginTop} id="Dashboard-Cards-Container" > - {dashboardHasCards(dashboard) ? ( + {dashboardHasCards ? ( <DashboardGrid {...this.props} + dashboard={this.getDashboardWithFilteredCards()} isNightMode={shouldRenderAsNightMode} onEditingChange={this.setEditing} /> diff --git a/frontend/src/metabase/dashboard/components/DashboardGrid.jsx b/frontend/src/metabase/dashboard/components/DashboardGrid.jsx index 9e8100a9b1a733e731549e1faa19f17e39213401..b4682e02acce432158201a111c1022882f812c82 100644 --- a/frontend/src/metabase/dashboard/components/DashboardGrid.jsx +++ b/frontend/src/metabase/dashboard/components/DashboardGrid.jsx @@ -132,7 +132,7 @@ class DashboardGrid extends Component { }); if (changes.length > 0) { - setMultipleDashCardAttributes(changes); + setMultipleDashCardAttributes({ dashcards: changes }); MetabaseAnalytics.trackStructEvent("Dashboard", "Layout Changed"); } }; diff --git a/frontend/src/metabase/dashboard/components/DashboardHeader.styled.tsx b/frontend/src/metabase/dashboard/components/DashboardHeaderView.styled.tsx similarity index 99% rename from frontend/src/metabase/dashboard/components/DashboardHeader.styled.tsx rename to frontend/src/metabase/dashboard/components/DashboardHeaderView.styled.tsx index c4203babca8bfecad6bc5c4356f6af51456b4c32..b61d16f7ee85c5b13238afe08346691b60df59e9 100644 --- a/frontend/src/metabase/dashboard/components/DashboardHeader.styled.tsx +++ b/frontend/src/metabase/dashboard/components/DashboardHeaderView.styled.tsx @@ -16,7 +16,7 @@ interface TypeForItemsThatRespondToNavBarOpen { isNavBarOpen: boolean; } -export const HeaderRoot = styled( +export const HeaderRow = styled( FullWidthContainer, )<TypeForItemsThatRespondToNavBarOpen>` display: flex; diff --git a/frontend/src/metabase/dashboard/components/DashboardHeader.tsx b/frontend/src/metabase/dashboard/components/DashboardHeaderView.tsx similarity index 67% rename from frontend/src/metabase/dashboard/components/DashboardHeader.tsx rename to frontend/src/metabase/dashboard/components/DashboardHeaderView.tsx index bfadf8141990bb92dfa0d868f7557102e38f37f3..6b6a70cbe8a3cc561923f419f40c290590c93a6f 100644 --- a/frontend/src/metabase/dashboard/components/DashboardHeader.tsx +++ b/frontend/src/metabase/dashboard/components/DashboardHeaderView.tsx @@ -17,7 +17,7 @@ import EditBar from "metabase/components/EditBar"; import HeaderModal from "metabase/components/HeaderModal"; import { EditWarning, - HeaderRoot, + HeaderRow, HeaderBadges, HeaderContent, HeaderButtonsContainer, @@ -25,9 +25,10 @@ import { HeaderLastEditInfoLabel, HeaderCaption, HeaderCaptionContainer, -} from "./DashboardHeader.styled"; +} from "./DashboardHeaderView.styled"; +import { DashboardTabs } from "./DashboardTabs/DashboardTabs"; -interface DashboardHeaderProps { +interface DashboardHeaderViewProps { editingTitle: string; editingSubtitle: string; editingButtons: JSX.Element[]; @@ -41,7 +42,6 @@ interface DashboardHeaderProps { dashboard: Dashboard; isBadgeVisible: boolean; isLastEditInfoVisible: boolean; - children: React.ReactNode; onHeaderModalDone: () => null; onHeaderModalCancel: () => null; onLastEditInfoClick: () => null; @@ -49,7 +49,7 @@ interface DashboardHeaderProps { setDashboardAttribute: (prop: string, value: string) => null; } -const DashboardHeader = ({ +function DashboardHeaderView({ editingTitle = "", editingSubtitle = "", editingButtons = [], @@ -61,13 +61,12 @@ const DashboardHeader = ({ isNavBarOpen, dashboard, isLastEditInfoVisible, - children, onHeaderModalDone, onHeaderModalCancel, onLastEditInfoClick, onSave, setDashboardAttribute, -}: DashboardHeaderProps) => { +}: DashboardHeaderViewProps) { const [headerHeight, setHeaderHeight] = useState(0); const [showSubHeader, setShowSubHeader] = useState(true); const header = useRef<HTMLDivElement>(null); @@ -134,41 +133,45 @@ const DashboardHeader = ({ onDone={onHeaderModalDone} onCancel={onHeaderModalCancel} /> - <HeaderRoot - isNavBarOpen={isNavBarOpen} - className={cx("QueryBuilder-section", headerClassName)} - data-testid="dashboard-header" - ref={header} - > - <HeaderContent hasSubHeader showSubHeader={showSubHeader}> - <HeaderCaptionContainer> - <HeaderCaption - key={dashboard.name} - initialValue={dashboard.name} - placeholder={t`Add title`} - isDisabled={!dashboard.can_write} - data-testid="dashboard-name-heading" - onChange={handleUpdateCaption} - /> - </HeaderCaptionContainer> - <HeaderBadges> - {isLastEditInfoVisible && ( - <HeaderLastEditInfoLabel - item={dashboard} - onClick={onLastEditInfoClick} - className="" + <div> + <HeaderRow + isNavBarOpen={isNavBarOpen} + className={cx("QueryBuilder-section", headerClassName)} + data-testid="dashboard-header" + ref={header} + > + <HeaderContent hasSubHeader showSubHeader={showSubHeader}> + <HeaderCaptionContainer> + <HeaderCaption + key={dashboard.name} + initialValue={dashboard.name} + placeholder={t`Add title`} + isDisabled={!dashboard.can_write} + data-testid="dashboard-name-heading" + onChange={handleUpdateCaption} /> - )} - </HeaderBadges> - </HeaderContent> + </HeaderCaptionContainer> + <HeaderBadges> + {isLastEditInfoVisible && ( + <HeaderLastEditInfoLabel + item={dashboard} + onClick={onLastEditInfoClick} + className="" + /> + )} + </HeaderBadges> + </HeaderContent> - <HeaderButtonsContainer isNavBarOpen={isNavBarOpen}> - {_headerButtons} - </HeaderButtonsContainer> - </HeaderRoot> - {children} + <HeaderButtonsContainer isNavBarOpen={isNavBarOpen}> + {_headerButtons} + </HeaderButtonsContainer> + </HeaderRow> + <HeaderRow isNavBarOpen={isNavBarOpen}> + <DashboardTabs isEditing={isEditing} /> + </HeaderRow> + </div> </div> ); -}; +} -export default DashboardHeader; +export default DashboardHeaderView; diff --git a/frontend/src/metabase/dashboard/components/DashboardSidebars.jsx b/frontend/src/metabase/dashboard/components/DashboardSidebars.jsx index 408245b0ab9f4015f3d9e7e0ec1ff501a17281d2..bebf2884a74d66645ba8ed11be57012775e0cff6 100644 --- a/frontend/src/metabase/dashboard/components/DashboardSidebars.jsx +++ b/frontend/src/metabase/dashboard/components/DashboardSidebars.jsx @@ -2,6 +2,7 @@ import React, { useCallback } from "react"; import PropTypes from "prop-types"; import _ from "underscore"; +import { useSelector } from "metabase/lib/redux"; import { SIDEBAR_NAME } from "metabase/dashboard/constants"; import ParameterSidebar from "metabase/parameters/components/ParameterSidebar"; @@ -11,6 +12,7 @@ import ClickBehaviorSidebar from "./ClickBehaviorSidebar"; import DashboardInfoSidebar from "./DashboardInfoSidebar"; import { AddCardSidebar } from "./add-card-sidebar/AddCardSidebar"; import { ActionSidebar } from "./ActionSidebar"; +import { getSelectedTabId } from "./DashboardTabs"; DashboardSidebars.propTypes = { dashboard: PropTypes.object, @@ -74,15 +76,17 @@ export function DashboardSidebars({ setDashboardAttribute, saveDashboardAndCards, }) { + const tabId = useSelector(getSelectedTabId); const handleAddCard = useCallback( cardId => { addCardToDashboard({ dashId: dashboard.id, cardId: cardId, + tabId, }); MetabaseAnalytics.trackStructEvent("Dashboard", "Add Card"); }, - [addCardToDashboard, dashboard.id], + [addCardToDashboard, dashboard.id, tabId], ); if (isFullscreen) { diff --git a/frontend/src/metabase/dashboard/components/DashboardTabs/DashboardTabs.styled.tsx b/frontend/src/metabase/dashboard/components/DashboardTabs/DashboardTabs.styled.tsx new file mode 100644 index 0000000000000000000000000000000000000000..77e2843a6867d3f13d909be0aa7489b448977d2c --- /dev/null +++ b/frontend/src/metabase/dashboard/components/DashboardTabs/DashboardTabs.styled.tsx @@ -0,0 +1,34 @@ +import React from "react"; +import styled from "@emotion/styled"; + +import BaseTabButton, { + RenameableTabButtonProps, +} from "metabase/core/components/TabButton"; +import BaseButton from "metabase/core/components/Button"; + +export const Container = styled.div` + display: flex; + align-items: start; + gap: 1.5rem; + width: 100%; +`; + +const _PlaceholderTab = styled(BaseTabButton)` + padding-top: 0; + padding-bottom: 0.5rem; +`; +export const PlaceholderTab = ({ label }: { label: string }) => ( + <_PlaceholderTab label={label} value={null} disabled /> +); + +// Manually styling this component because `styled` doesn't work with generics +export const Tab = <T,>(props: RenameableTabButtonProps<T>) => ( + <BaseTabButton.Renameable<T> + style={{ paddingTop: 0, paddingBottom: "0.5rem" }} + {...props} + /> +); + +export const CreateTabButton = styled(BaseButton)` + border: none; +`; diff --git a/frontend/src/metabase/dashboard/components/DashboardTabs/DashboardTabs.tsx b/frontend/src/metabase/dashboard/components/DashboardTabs/DashboardTabs.tsx new file mode 100644 index 0000000000000000000000000000000000000000..9588dc43031d3d4807ef29d2fffaebedf57bfe76 --- /dev/null +++ b/frontend/src/metabase/dashboard/components/DashboardTabs/DashboardTabs.tsx @@ -0,0 +1,64 @@ +import React from "react"; +import { t } from "ttag"; + +import TabRow from "metabase/core/components/TabRow"; +import { SelectedTabId } from "metabase-types/store"; + +import { + Container, + Tab, + CreateTabButton, + PlaceholderTab, +} from "./DashboardTabs.styled"; +import { useDashboardTabs } from "./useDashboardTabs"; + +interface DashboardTabsProps { + isEditing: boolean; +} + +export function DashboardTabs({ isEditing }: DashboardTabsProps) { + const { tabs, createNewTab, deleteTab, renameTab, selectTab, selectedTabId } = + useDashboardTabs(); + const showTabs = tabs.length > 1 || isEditing; + const showPlaceholder = tabs.length <= 1 && isEditing; + + if (!showTabs) { + return null; + } + + return ( + <Container> + <TabRow<SelectedTabId> value={selectedTabId} onChange={selectTab}> + {showPlaceholder ? ( + <PlaceholderTab + label={tabs.length === 1 ? tabs[0].name : t`Page 1`} + /> + ) : ( + tabs.map(tab => ( + <Tab<SelectedTabId> + key={tab.id} + value={tab.id} + label={tab.name} + onRename={name => renameTab(tab.id, name)} + showMenu={isEditing} + menuItems={[ + { + label: t`Delete`, + action: (_, value) => deleteTab(value), + }, + ]} + /> + )) + )} + {isEditing && ( + <CreateTabButton + icon="add" + iconSize={12} + onClick={createNewTab} + aria-label={t`Create new tab`} + /> + )} + </TabRow> + </Container> + ); +} diff --git a/frontend/src/metabase/dashboard/components/DashboardTabs/DashboardTabs.unit.spec.tsx b/frontend/src/metabase/dashboard/components/DashboardTabs/DashboardTabs.unit.spec.tsx new file mode 100644 index 0000000000000000000000000000000000000000..953574c0bab36a3cc3059b4a729f2bb8b2741b74 --- /dev/null +++ b/frontend/src/metabase/dashboard/components/DashboardTabs/DashboardTabs.unit.spec.tsx @@ -0,0 +1,291 @@ +import React from "react"; +import userEvent from "@testing-library/user-event"; + +import { renderWithProviders, screen, fireEvent } from "__support__/ui"; +import { DashboardState, State, StoreDashcard } from "metabase-types/store"; +import { DashboardOrderedTab } from "metabase-types/api"; +import { createMockCard } from "metabase-types/api/mocks"; +import { ORDERS_ID, SAMPLE_DB_ID } from "metabase-types/api/mocks/presets"; +import { INITIAL_DASHBOARD_STATE } from "metabase/dashboard/constants"; +import { getDefaultTab } from "metabase/dashboard/actions"; + +import { DashboardTabs } from "./DashboardTabs"; + +const TEST_CARD = createMockCard({ + dataset_query: { + database: SAMPLE_DB_ID, + type: "query", + query: { + "source-table": ORDERS_ID, + aggregation: [["count"]], + }, + }, +}); + +function createMockDashCard({ + dashCardId, + tabId, +}: { + dashCardId: number; + tabId: number | undefined; +}) { + return { + id: dashCardId, + dashboard_id: 1, + dashboard_tab_id: tabId, + card_id: 1, + size_x: 4, + size_y: 4, + col: 0, + row: 0, + entity_id: "", + created_at: "", + updated_at: "", + card: TEST_CARD, + }; +} + +const TEST_DASHBOARD_STATE: DashboardState = { + ...INITIAL_DASHBOARD_STATE, + dashboardId: 1, + dashboards: { + 1: { + id: 1, + collection_id: 1, + name: "", + description: "", + can_write: true, + cache_ttl: null, + auto_apply_filters: true, + "last-edit-info": { + id: 1, + email: "", + first_name: "", + last_name: "", + timestamp: "", + }, + ordered_cards: [1, 2], + ordered_tabs: [ + getDefaultTab({ tabId: 1, dashId: 1, name: "Page 1" }), + getDefaultTab({ tabId: 2, dashId: 1, name: "Page 2" }), + getDefaultTab({ tabId: 3, dashId: 1, name: "Page 3" }), + ], + }, + }, + dashcards: { + 1: createMockDashCard({ dashCardId: 1, tabId: 1 }), + 2: createMockDashCard({ dashCardId: 2, tabId: 2 }), + }, +}; + +function setup({ + isEditing = true, + tabs, +}: { + isEditing?: boolean; + tabs?: DashboardOrderedTab[]; + cards?: StoreDashcard[]; +} = {}) { + const dashboard: DashboardState = { + ...TEST_DASHBOARD_STATE, + dashboards: { + 1: { + ...TEST_DASHBOARD_STATE.dashboards[1], + ordered_tabs: tabs ?? TEST_DASHBOARD_STATE.dashboards[1].ordered_tabs, + }, + }, + }; + + const { store } = renderWithProviders( + <DashboardTabs isEditing={isEditing} />, + { + withSampleDatabase: true, + storeInitialState: { dashboard }, + }, + ); + return { + getDashcards: () => + Object.values((store.getState() as unknown as State).dashboard.dashcards), + }; +} + +function queryTab(numOrName: number | string) { + const name = typeof numOrName === "string" ? numOrName : `Page ${numOrName}`; + return screen.queryByRole("tab", { name }); +} + +function selectTab(num: number) { + const selectedTab = queryTab(num) as HTMLElement; + userEvent.click(selectedTab); + return selectedTab; +} + +function createNewTab() { + userEvent.click(screen.getByLabelText("Create new tab")); +} + +async function selectTabMenuItem(num: number, name: "Delete" | "Rename") { + const dropdownIcons = screen.getAllByRole("img", { + name: "chevrondown icon", + }); + userEvent.click(dropdownIcons[num - 1]); + (await screen.findByRole("option", { name })).click(); +} + +async function deleteTab(num: number) { + return selectTabMenuItem(num, "Delete"); +} + +async function renameTab(num: number, name: string) { + await selectTabMenuItem(num, "Rename"); + + const inputEl = screen.getByRole("textbox", { name: `Page ${num}` }); + userEvent.type(inputEl, name); + fireEvent.keyPress(inputEl, { key: "Enter", charCode: 13 }); +} + +describe("DashboardTabs", () => { + describe("when not editing", () => { + it("should display tabs without menus when there are two or more", () => { + setup({ isEditing: false }); + + expect(queryTab(1)).toBeVisible(); + expect(queryTab(2)).toBeVisible(); + }); + + it("should not display tabs when there is one", () => { + setup({ + isEditing: false, + tabs: [getDefaultTab({ tabId: 1, dashId: 1, name: "Page 1" })], + }); + + expect(queryTab(1)).not.toBeInTheDocument(); + }); + + it("should not display tabs when there are none", () => { + setup({ + isEditing: false, + tabs: [], + }); + + expect(queryTab(1)).not.toBeInTheDocument(); + }); + + it("should automatically select the first tab on render", () => { + setup({ isEditing: false }); + + expect(queryTab(1)).toHaveAttribute("aria-selected", "true"); + expect(queryTab(2)).toHaveAttribute("aria-selected", "false"); + expect(queryTab(3)).toHaveAttribute("aria-selected", "false"); + }); + + it("should allow you to click to select tabs", () => { + setup({ isEditing: false }); + + expect(selectTab(2)).toHaveAttribute("aria-selected", "true"); + expect(queryTab(1)).toHaveAttribute("aria-selected", "false"); + expect(queryTab(3)).toHaveAttribute("aria-selected", "false"); + }); + }); + + describe("when editing", () => { + it("should display a placeholder tab when there are none", () => { + setup({ tabs: [] }); + + const placeholderTab = queryTab("Page 1"); + expect(placeholderTab).toHaveAttribute("aria-disabled", "true"); + }); + + it("should display a placeholder tab when there is only one", () => { + setup({ + tabs: [getDefaultTab({ tabId: 1, dashId: 1, name: "Lonely tab" })], + }); + + const placeholderTab = queryTab("Lonely tab"); + expect(placeholderTab).toHaveAttribute("aria-disabled", "true"); + }); + + it("should allow you to click to select tabs", () => { + setup(); + + expect(selectTab(2)).toHaveAttribute("aria-selected", "true"); + expect(queryTab(1)).toHaveAttribute("aria-selected", "false"); + expect(queryTab(3)).toHaveAttribute("aria-selected", "false"); + }); + + describe("when adding tabs", () => { + it("should add two tabs, assign cards to the first, and select the second when adding a tab for the first time", () => { + const { getDashcards } = setup({ tabs: [] }); + createNewTab(); + const firstNewTab = queryTab(1); + const secondNewTab = queryTab(2); + + expect(firstNewTab).toBeVisible(); + expect(secondNewTab).toBeVisible(); + + expect(firstNewTab).toHaveAttribute("aria-selected", "false"); + expect(secondNewTab).toHaveAttribute("aria-selected", "true"); + + const dashcards = getDashcards(); + expect(dashcards[0].dashboard_tab_id).toEqual(-1); + expect(dashcards[1].dashboard_tab_id).toEqual(-1); + }); + + it("should add add one tab, not reassign cards, and select the tab when adding an additional tab", () => { + const { getDashcards } = setup(); + createNewTab(); + const newTab = queryTab(4); + + expect(newTab).toBeVisible(); + + expect(queryTab(1)).toHaveAttribute("aria-selected", "false"); + expect(queryTab(2)).toHaveAttribute("aria-selected", "false"); + expect(queryTab(3)).toHaveAttribute("aria-selected", "false"); + expect(newTab).toHaveAttribute("aria-selected", "true"); + + const dashcards = getDashcards(); + expect(dashcards[0].dashboard_tab_id).toEqual(1); + expect(dashcards[1].dashboard_tab_id).toEqual(2); + }); + }); + + describe("when deleting tabs", () => { + it("should delete the tab and its cards after clicking `Delete` in the menu", async () => { + const { getDashcards } = setup(); + await deleteTab(2); + + expect(queryTab(2)).not.toBeInTheDocument(); + + const dashcards = getDashcards(); + expect(dashcards[0].isRemoved).toEqual(undefined); + expect(dashcards[1].isRemoved).toEqual(true); + }); + + it("should select the tab to the left if the selected tab was deleted", async () => { + setup(); + selectTab(2); + await deleteTab(2); + + expect(queryTab(1)).toHaveAttribute("aria-selected", "true"); + }); + + it("should select the tab to the right if the selected tab was deleted and was the first tab", async () => { + setup(); + selectTab(1); + await deleteTab(1); + + expect(queryTab(2)).toHaveAttribute("aria-selected", "true"); + }); + }); + + describe("when renaming tabs", () => { + it("should allow the user to rename the tab after clicking `Rename` in the menu", async () => { + setup(); + const newName = "A cool new name"; + await renameTab(1, newName); + + expect(queryTab(newName)).toBeInTheDocument(); + }); + }); + }); +}); diff --git a/frontend/src/metabase/dashboard/components/DashboardTabs/index.ts b/frontend/src/metabase/dashboard/components/DashboardTabs/index.ts new file mode 100644 index 0000000000000000000000000000000000000000..813861b63dce01d93a6a489e786d2d445572790f --- /dev/null +++ b/frontend/src/metabase/dashboard/components/DashboardTabs/index.ts @@ -0,0 +1,2 @@ +export * from "./DashboardTabs"; +export * from "./useDashboardTabs"; diff --git a/frontend/src/metabase/dashboard/components/DashboardTabs/useDashboardTabs.ts b/frontend/src/metabase/dashboard/components/DashboardTabs/useDashboardTabs.ts new file mode 100644 index 0000000000000000000000000000000000000000..cd553b05e0442cce1cbceca3bdbd3a2d263f2924 --- /dev/null +++ b/frontend/src/metabase/dashboard/components/DashboardTabs/useDashboardTabs.ts @@ -0,0 +1,39 @@ +import { useMount } from "react-use"; + +import { useDispatch, useSelector } from "metabase/lib/redux"; +import { + createNewTab as createNewTabAction, + renameTab as renameTabAction, + deleteTab as deleteTabAction, + initTabs, + selectTab as selectTabAction, +} from "metabase/dashboard/actions"; +import { SelectedTabId, State } from "metabase-types/store"; +import { getDashboardId } from "metabase/dashboard/selectors"; + +export function getSelectedTabId(state: State) { + return state.dashboard.selectedTabId; +} + +export function useDashboardTabs() { + const dispatch = useDispatch(); + const dashboardId = useSelector(getDashboardId); + const tabs = useSelector(state => + dashboardId + ? state.dashboard.dashboards[dashboardId].ordered_tabs ?? [] + : [], + ); + const selectedTabId = useSelector(getSelectedTabId); + + useMount(() => dispatch(initTabs())); + + return { + tabs, + selectedTabId, + createNewTab: () => dispatch(createNewTabAction()), + deleteTab: (tabId: SelectedTabId) => dispatch(deleteTabAction({ tabId })), + renameTab: (tabId: SelectedTabId, name: string) => + dispatch(renameTabAction({ tabId, name })), + selectTab: (tabId: SelectedTabId) => dispatch(selectTabAction({ tabId })), + }; +} diff --git a/frontend/src/metabase/dashboard/constants.js b/frontend/src/metabase/dashboard/constants.js deleted file mode 100644 index a432307da25a957a14d786f0dbfed633a13b25ca..0000000000000000000000000000000000000000 --- a/frontend/src/metabase/dashboard/constants.js +++ /dev/null @@ -1,10 +0,0 @@ -export const SIDEBAR_NAME = { - addQuestion: "addQuestion", - action: "action", - clickBehavior: "clickBehavior", - editParameter: "editParameter", - sharing: "sharing", - info: "info", -}; - -export const DASHBOARD_SLOW_TIMEOUT = 15 * 1000; diff --git a/frontend/src/metabase/dashboard/constants.ts b/frontend/src/metabase/dashboard/constants.ts new file mode 100644 index 0000000000000000000000000000000000000000..7bd53fdfc6a2600ddc49959c8c394918a80f3084 --- /dev/null +++ b/frontend/src/metabase/dashboard/constants.ts @@ -0,0 +1,39 @@ +import { DashboardSidebarName, DashboardState } from "metabase-types/store"; + +export const SIDEBAR_NAME: Record<DashboardSidebarName, DashboardSidebarName> = + { + addQuestion: "addQuestion", + action: "action", + clickBehavior: "clickBehavior", + editParameter: "editParameter", + sharing: "sharing", + info: "info", + }; + +export const INITIAL_DASHBOARD_STATE: DashboardState = { + dashboardId: null, + selectedTabId: null, + isEditing: null, + dashboards: {}, + dashcards: {}, + dashcardData: {}, + parameterValues: {}, + loadingDashCards: { + dashcardIds: [], + loadingIds: [], + loadingStatus: "idle" as const, + startTime: null, + endTime: null, + }, + loadingControls: {}, + isAddParameterPopoverOpen: false, + slowCards: {}, + sidebar: { props: {} }, + missingActionParameters: null, + autoApplyFilters: { + toastId: null, + toastDashboardId: null, + }, +}; + +export const DASHBOARD_SLOW_TIMEOUT = 15 * 1000; diff --git a/frontend/src/metabase/dashboard/containers/DashboardApp/DashboardApp.jsx b/frontend/src/metabase/dashboard/containers/DashboardApp/DashboardApp.jsx index d3aa66d2d229cae8f76237c74a29ad82f06482d3..830cc25db8fcacd876f9594bb2d7800dcd5dacb9 100644 --- a/frontend/src/metabase/dashboard/containers/DashboardApp/DashboardApp.jsx +++ b/frontend/src/metabase/dashboard/containers/DashboardApp/DashboardApp.jsx @@ -35,6 +35,7 @@ import Dashboards from "metabase/entities/dashboards"; import { useDispatch } from "metabase/lib/redux"; import { addUndo, dismissUndo } from "metabase/redux/undo"; import { useUniqueId } from "metabase/hooks/use-unique-id"; +import { getSelectedTabId } from "metabase/dashboard/components/DashboardTabs"; import * as dashboardActions from "../../actions"; import { getIsEditing, @@ -102,6 +103,7 @@ const mapStateToProps = (state, props) => { isHeaderVisible: getIsHeaderVisible(state), isAdditionalInfoVisible: getIsAdditionalInfoVisible(state), embedOptions: getEmbedOptions(state), + selectedTabId: getSelectedTabId(state), isAutoApplyFilters: getIsAutoApplyFilters(state), }; }; diff --git a/frontend/src/metabase/dashboard/containers/DashboardHeader.jsx b/frontend/src/metabase/dashboard/containers/DashboardHeader.jsx index ac253040a558535649f7cb2784bf853b07005370..e77defb44d21b869b36a3cd1f9601190da7679d1 100644 --- a/frontend/src/metabase/dashboard/containers/DashboardHeader.jsx +++ b/frontend/src/metabase/dashboard/containers/DashboardHeader.jsx @@ -33,7 +33,7 @@ import { import { hasDatabaseActionsEnabled } from "metabase/dashboard/utils"; import { saveDashboardPdf } from "metabase/visualizations/lib/save-dashboard-pdf"; -import Header from "../components/DashboardHeader"; +import DashboardHeaderView from "../components/DashboardHeaderView"; import { SIDEBAR_NAME } from "../constants"; import { DashboardHeaderButton, @@ -45,6 +45,7 @@ const mapStateToProps = (state, props) => { isBookmarked: getIsBookmarked(state, props), isNavBarOpen: getIsNavbarOpen(state), isShowingDashboardInfoSidebar: getIsShowDashboardInfoSidebar(state), + selectedTabId: state.dashboard.selectedTabId, }; }; @@ -123,16 +124,23 @@ class DashboardHeader extends Component { } onAddTextBox() { - this.props.addTextDashCardToDashboard({ dashId: this.props.dashboard.id }); + this.props.addTextDashCardToDashboard({ + dashId: this.props.dashboard.id, + tabId: this.props.selectedTabId, + }); } onAddLinkCard() { - this.props.addLinkDashCardToDashboard({ dashId: this.props.dashboard.id }); + this.props.addLinkDashCardToDashboard({ + dashId: this.props.dashboard.id, + tabId: this.props.selectedTabId, + }); } onAddAction() { this.props.addActionToDashboard({ dashId: this.props.dashboard.id, + tabId: this.props.selectedTabId, displayType: "button", action: {}, }); @@ -246,6 +254,7 @@ class DashboardHeader extends Component { isActive={activeSidebarName === SIDEBAR_NAME.addQuestion} onClick={() => toggleSidebar(SIDEBAR_NAME.addQuestion)} data-metabase-event="Dashboard;Add Card Sidebar" + aria-label="add questions" /> </Tooltip>, ); @@ -445,7 +454,7 @@ class DashboardHeader extends Component { const hasLastEditInfo = dashboard["last-edit-info"] != null; return ( - <Header + <DashboardHeaderView headerClassName="wrapper" objectType="dashboard" analyticsContext="Dashboard" diff --git a/frontend/src/metabase/dashboard/reducers.js b/frontend/src/metabase/dashboard/reducers.js index 1f44c4f154240fe5b092d3a03747568b11414cdd..1deeafc3ba85df8273f0244ebc86ae2b345a586c 100644 --- a/frontend/src/metabase/dashboard/reducers.js +++ b/frontend/src/metabase/dashboard/reducers.js @@ -1,4 +1,6 @@ import { assoc, dissoc, assocIn, updateIn, chain, merge } from "icepick"; +import reduceReducers from "reduce-reducers"; + import { handleActions, combineReducers } from "metabase/lib/redux"; import Dashboards from "metabase/entities/dashboards"; @@ -22,7 +24,6 @@ import { REMOVE_PARAMETER, FETCH_CARD_DATA, CLEAR_CARD_DATA, - UPDATE_DASHCARD_IDS, MARK_CARD_AS_SLOW, SET_PARAMETER_VALUE, FETCH_DASHBOARD_CARD_DATA, @@ -37,9 +38,11 @@ import { SET_PARAMETER_VALUES, UNDO_REMOVE_CARD_FROM_DASH, SHOW_AUTO_APPLY_FILTERS_TOAST, + tabsReducer, } from "./actions"; import { isVirtualDashCard, syncParametersAndEmbeddingParams } from "./utils"; +import { INITIAL_DASHBOARD_STATE } from "./constants"; const dashboardId = handleActions( { @@ -49,7 +52,7 @@ const dashboardId = handleActions( }, [RESET]: { next: state => null }, }, - null, + INITIAL_DASHBOARD_STATE.dashboardId, ); const isEditing = handleActions( @@ -60,7 +63,7 @@ const isEditing = handleActions( }, [RESET]: { next: state => null }, }, - null, + INITIAL_DASHBOARD_STATE.isEditing, ); const loadingControls = handleActions( @@ -75,7 +78,7 @@ const loadingControls = handleActions( }), [RESET]: { next: state => ({}) }, }, - {}, + INITIAL_DASHBOARD_STATE.loadingControls, ); function newDashboard(before, after, isDirty) { @@ -146,7 +149,7 @@ const dashboards = handleActions( ), }, }, - {}, + INITIAL_DASHBOARD_STATE.dashboards, ); const dashcards = handleActions( @@ -164,7 +167,7 @@ const dashcards = handleActions( }), }, [SET_MULTIPLE_DASHCARD_ATTRIBUTES]: { - next: (state, { payload: dashcards }) => { + next: (state, { payload: { dashcards } }) => { const nextState = { ...state }; dashcards.forEach(({ id, attributes }) => { nextState[id] = { @@ -226,7 +229,7 @@ const dashcards = handleActions( [dashcardId]: { ...state[dashcardId], justAdded: false }, }), }, - {}, + INITIAL_DASHBOARD_STATE.dashcards, ); const isAddParameterPopoverOpen = handleActions( @@ -236,7 +239,7 @@ const isAddParameterPopoverOpen = handleActions( [INITIALIZE]: () => false, [RESET]: () => false, }, - false, + INITIAL_DASHBOARD_STATE.isAddParameterPopoverOpen, ); const dashcardData = handleActions( @@ -251,21 +254,9 @@ const dashcardData = handleActions( next: (state, { payload: { cardId, dashcardId } }) => assocIn(state, [dashcardId, cardId]), }, - [UPDATE_DASHCARD_IDS]: { - next: (state, { payload: { oldDashcardIds, newDashcardIds } }) => - oldDashcardIds - .reduce( - (wrappedState, oldDcId, index) => - wrappedState - .dissoc(oldDcId) - .assoc(newDashcardIds[index], state[oldDcId]), - chain(state), - ) - .value(), - }, [RESET]: { next: state => ({}) }, }, - {}, + INITIAL_DASHBOARD_STATE.dashcardData, ); const slowCards = handleActions( @@ -277,7 +268,7 @@ const slowCards = handleActions( }), }, }, - {}, + INITIAL_DASHBOARD_STATE.slowCards, ); const parameterValues = handleActions( @@ -303,7 +294,7 @@ const parameterValues = handleActions( }, [RESET]: { next: state => ({}) }, }, - {}, + INITIAL_DASHBOARD_STATE.parameterValues, ); const draftParameterValues = handleActions( @@ -390,13 +381,7 @@ const loadingDashCards = handleActions( }), }, }, - { - dashcardIds: [], - loadingIds: [], - startTime: null, - endTime: null, - loadingStatus: "idle", - }, + INITIAL_DASHBOARD_STATE.loadingDashCards, ); const DEFAULT_SIDEBAR = { props: {} }; @@ -424,7 +409,7 @@ const sidebar = handleActions( next: () => DEFAULT_SIDEBAR, }, }, - DEFAULT_SIDEBAR, + INITIAL_DASHBOARD_STATE.sidebar, ); const missingActionParameters = handleActions( @@ -436,7 +421,7 @@ const missingActionParameters = handleActions( next: (state, payload) => null, }, }, - null, + INITIAL_DASHBOARD_STATE.missingActionParameters, ); export const autoApplyFilters = handleActions( @@ -449,25 +434,28 @@ export const autoApplyFilters = handleActions( }), }, }, - { - toastId: null, - toastDashboardId: null, - }, + INITIAL_DASHBOARD_STATE.autoApplyFilters, ); -export default combineReducers({ - dashboardId, - isEditing, - loadingControls, - dashboards, - dashcards, - dashcardData, - slowCards, - parameterValues, - draftParameterValues, - loadingDashCards, - isAddParameterPopoverOpen, - sidebar, - autoApplyFilters, - missingActionParameters, -}); +export default reduceReducers( + INITIAL_DASHBOARD_STATE, + combineReducers({ + dashboardId, + isEditing, + loadingControls, + dashboards, + dashcards, + dashcardData, + slowCards, + parameterValues, + draftParameterValues, + loadingDashCards, + isAddParameterPopoverOpen, + sidebar, + missingActionParameters, + autoApplyFilters, + // Combined reducer needs to init state for every slice + selectedTabId: (state = INITIAL_DASHBOARD_STATE.selectedTabId) => state, + }), + tabsReducer, +); diff --git a/frontend/src/metabase/dashboard/reducers.unit.spec.js b/frontend/src/metabase/dashboard/reducers.unit.spec.js index a81b41bd82bc8a1aac3c807352430ff1e18b9c41..ec2e2be28030c0a4747dda2a04196f687d3e6c29 100644 --- a/frontend/src/metabase/dashboard/reducers.unit.spec.js +++ b/frontend/src/metabase/dashboard/reducers.unit.spec.js @@ -19,6 +19,7 @@ describe("dashboard reducers", () => { it("should return the initial state", () => { expect(initState).toEqual({ dashboardId: null, + selectedTabId: null, dashboards: {}, dashcardData: {}, dashcards: {}, diff --git a/frontend/src/metabase/lib/revisions/components.jsx b/frontend/src/metabase/lib/revisions/components.jsx index 28aeb769210d7df1cf2943bc73531dcd5971d8eb..42c5e18eb96ceb2e3a47596338bb414aafc47f4c 100644 --- a/frontend/src/metabase/lib/revisions/components.jsx +++ b/frontend/src/metabase/lib/revisions/components.jsx @@ -5,6 +5,7 @@ import { t } from "ttag"; import { color } from "metabase/lib/colors"; import { capitalize } from "metabase/lib/formatting"; import RawEntityLink from "metabase/entities/containers/EntityLink"; +import { getRevisionTitleText } from "./revisions"; export const EntityLink = styled(RawEntityLink)` color: ${color("brand")}; @@ -28,7 +29,7 @@ const revisionTitlePropTypes = { }; export function RevisionTitle({ username, message }) { - return <span>{[username, " ", message]}</span>; + return <span>{getRevisionTitleText(username, message)}</span>; } RevisionTitle.propTypes = revisionTitlePropTypes; diff --git a/frontend/src/metabase/lib/revisions/revisions.js b/frontend/src/metabase/lib/revisions/revisions.js index 5fa473fa7e0a438d19cd6aa7582e920a0b93e660..ac86182f2eb140ca472a54c8e514330be690877e 100644 --- a/frontend/src/metabase/lib/revisions/revisions.js +++ b/frontend/src/metabase/lib/revisions/revisions.js @@ -184,6 +184,10 @@ export function getChangedFields(revision) { return fields.filter(field => registeredFields.includes(field)); } +export function getRevisionTitleText(username, message) { + return `${username} ${message}`; +} + export function getRevisionDescription(revision) { const { diff, is_creation, is_reversion } = revision; if (is_creation) { @@ -267,10 +271,10 @@ export function getRevisionEventsForTimeline( // we want to show the changelog in a description and set a title to just "User edited this" // If only one field is changed, we just show everything in the title // like "John added a description" + let message; if (isChangeEvent && isMultipleFieldsChange) { - event.title = ( - <RevisionTitle username={username} message={t`edited this`} /> - ); + message = t`edited this`; + event.title = <RevisionTitle username={username} message={message} />; event.description = ( <RevisionBatchedDescription changes={changes} @@ -278,8 +282,10 @@ export function getRevisionEventsForTimeline( /> ); } else { - event.title = <RevisionTitle username={username} message={changes} />; + message = changes; + event.title = <RevisionTitle username={username} message={message} />; } + event.titleText = getRevisionTitleText(username, message); return event; }) diff --git a/frontend/src/metabase/lib/revisions/revisions.unit.spec.js b/frontend/src/metabase/lib/revisions/revisions.unit.spec.js index 33d0106de2d8401ee99c285efca6f818361da48c..55693505fea2eccfa9ea3d6593904165263854ac 100644 --- a/frontend/src/metabase/lib/revisions/revisions.unit.spec.js +++ b/frontend/src/metabase/lib/revisions/revisions.unit.spec.js @@ -449,16 +449,19 @@ describe("getRevisionEvents", () => { message="reverted to an earlier revision" /> ), + titleText: "Bar reverted to an earlier revision", isRevertable: false, revision: latestRevisionEvent, }), getExpectedEvent({ title: <RevisionTitle username="Foo" message="added a description" />, + titleText: "Foo added a description", isRevertable: false, revision: changeEvent, }), getExpectedEvent({ title: <RevisionTitle username="Foo" message="created this" />, + titleText: "Foo created this", isRevertable: false, revision: creationEvent, }), @@ -496,6 +499,7 @@ describe("getRevisionEvents", () => { expect(timelineEvents).toEqual([ getExpectedEvent({ title: <RevisionTitle username="Foo" message="added a description" />, + titleText: "Foo added a description", isRevertable: false, revision: changeEvent, }), @@ -518,6 +522,7 @@ describe("getRevisionEvents", () => { expect(timelineEvents).toEqual([ getExpectedEvent({ title: <RevisionTitle username="Foo" message="added a description" />, + titleText: "Foo added a description", isRevertable: false, revision: changeEvent, }), @@ -537,6 +542,7 @@ describe("getRevisionEvents", () => { expect(timelineEvents).toEqual([ getExpectedEvent({ title: <RevisionTitle username="You" message="created this" />, + titleText: "You created this", isRevertable: false, revision: event, }), diff --git a/frontend/src/metabase/nav/containers/MainNavbar/MainNavbar.unit.spec.tsx b/frontend/src/metabase/nav/containers/MainNavbar/MainNavbar.unit.spec.tsx index 13ace74bfe1524d413eaf7a0ad3080c5b57658e2..f94e52fb4eeacd5ef54c1bba31439e98425321aa 100644 --- a/frontend/src/metabase/nav/containers/MainNavbar/MainNavbar.unit.spec.tsx +++ b/frontend/src/metabase/nav/containers/MainNavbar/MainNavbar.unit.spec.tsx @@ -17,7 +17,8 @@ import { createMockEntitiesState } from "__support__/store"; import * as Urls from "metabase/lib/urls"; import { ROOT_COLLECTION } from "metabase/entities/collections"; -import type { Card, Dashboard, User } from "metabase-types/api"; + +import type { Card, Dashboard, DashboardId, User } from "metabase-types/api"; import { createMockCard, createMockCollection, @@ -31,6 +32,8 @@ import { createMockDashboardState, createMockQueryBuilderState, } from "metabase-types/store/mocks"; + +import { DashboardState } from "metabase-types/store"; import MainNavbar from "./MainNavbar"; type SetupOpts = { @@ -106,15 +109,26 @@ async function setup({ setupCardsEndpoints([openQuestionCard]); } - const dashboards = openDashboard ? { [openDashboard.id]: openDashboard } : {}; - const dashboardId = openDashboard ? openDashboard.id : null; + let dashboardId: DashboardId | null = null; + const dashboardsForState: DashboardState["dashboards"] = {}; + const dashboardsForEntities: Dashboard[] = []; + if (openDashboard) { + dashboardId = openDashboard.id; + dashboardsForState[openDashboard.id] = { + ...openDashboard, + ordered_cards: openDashboard.ordered_cards.map(c => c.id), + }; + dashboardsForEntities.push(openDashboard); + } + const storeInitialState = createMockState({ currentUser: user, - dashboard: createMockDashboardState({ dashboardId, dashboards }), - qb: createMockQueryBuilderState({ card: openQuestionCard }), - entities: createMockEntitiesState({ - dashboards: Object.values(dashboards), + dashboard: createMockDashboardState({ + dashboardId, + dashboards: dashboardsForState, }), + qb: createMockQueryBuilderState({ card: openQuestionCard }), + entities: createMockEntitiesState({ dashboards: dashboardsForEntities }), }); renderWithProviders( diff --git a/frontend/src/metabase/services.js b/frontend/src/metabase/services.js index de055db8b605f0ba3061677704d1e47834323950..06f76a08e1bc16dbe436eed6a3d8970078e7c675 100644 --- a/frontend/src/metabase/services.js +++ b/frontend/src/metabase/services.js @@ -185,7 +185,7 @@ export const DashboardApi = { get: GET("/api/dashboard/:dashId"), update: PUT("/api/dashboard/:id"), delete: DELETE("/api/dashboard/:dashId"), - updateCards: PUT("/api/dashboard/:dashId/cards"), + updateCardsAndTabs: PUT("/api/dashboard/:dashId/cards"), favorite: POST("/api/dashboard/:dashId/favorite"), unfavorite: DELETE("/api/dashboard/:dashId/favorite"), parameterValues: GET("/api/dashboard/:dashId/params/:paramId/values"), diff --git a/package.json b/package.json index 130dea4056463c4c86f547d2680596ecc258753f..3a6b11c3e962f546b25e07d40d78532387d1a45a 100644 --- a/package.json +++ b/package.json @@ -91,6 +91,7 @@ "react-transition-group": "^4.4.5", "react-use": "^17.4.0", "react-virtualized": "^9.22.3", + "reduce-reducers": "^1.0.4", "redux-actions": "^2.0.1", "redux-auth-wrapper": "^1.0.0", "redux-promise": "^0.5.0", diff --git a/resources/migrations/000_migrations.yaml b/resources/migrations/000_migrations.yaml index fb49f176c27504fc6128b58fb0e718b864cac5a9..c43b56e9f56c276d8117f2f4a0090ae7cfe8679b 100644 --- a/resources/migrations/000_migrations.yaml +++ b/resources/migrations/000_migrations.yaml @@ -14421,6 +14421,94 @@ databaseChangeLog: constraints: nullable: false + - changeSet: + id: v47.00-006 + author: qnkhuat + comment: Added 0.47.0 - Add dashboard_tab table + changes: + - createTable: + tableName: dashboard_tab + remarks: "Join table connecting dashboard to dashboardcards" + columns: + - column: + name: id + type: int + autoIncrement: true + constraints: + primaryKey: true + nullable: false + - column: + name: dashboard_id + type: int + remarks: The dashboard that a tab is on + constraints: + nullable: false + referencedTableName: report_dashboard + referencedColumnNames: id + foreignKeyName: fk_dashboard_tab_ref_dashboard_id + deleteCascade: true + - column: + name: name + remarks: Displayed name of the tab + type: ${text.type} + constraints: + nullable: false + - column: + name: position + remarks: Position of the tab with respect to others tabs in dashboard + type: int + constraints: + nullable: false + - column: + name: entity_id + type: char(21) + remarks: Random NanoID tag for unique identity. + constraints: + nullable: false + unique: true + - column: + name: created_at + remarks: The timestamp at which the tab was created + type: ${timestamp_type} + defaultValueComputed: current_timestamp + constraints: + nullable: false + - column: + name: updated_at + remarks: The timestamp at which the tab was last updated + type: ${timestamp_type} + defaultValueComputed: current_timestamp + constraints: + nullable: false + + - changeSet: + id: v47.00-007 + author: qnkhuat + comment: Added 0.47.0 -- add report_dashboardcard.dashboard_tab_id + changes: + - addColumn: + tableName: report_dashboardcard + columns: + - column: + name: dashboard_tab_id + type: int + remarks: The referenced tab id that dashcard is on, it's nullable for dashboard with no tab + constraints: + nullable: true + + - changeSet: + id: v47.00-008 + author: qnkhuat + comment: Added 0.47.0 -- add report_dashboardcard.dashboard_tab_id fk constraint + changes: + - addForeignKeyConstraint: + baseTableName: report_dashboardcard + baseColumnNames: dashboard_tab_id + referencedTableName: dashboard_tab + referencedColumnNames: id + constraintName: fk_report_dashboardcard_ref_dashboard_tab_id + deleteCascade: true + - changeSet: id: v47.00-009 author: qwef diff --git a/src/metabase/api/dashboard.clj b/src/metabase/api/dashboard.clj index 02048ca6820d2159d9f21280730eb30ab3889f72..6cded709ef9c157cb06cce48fcab6f4f7c35edfb 100644 --- a/src/metabase/api/dashboard.clj +++ b/src/metabase/api/dashboard.clj @@ -218,7 +218,7 @@ ;; have a hydration key and an id. moderation_reviews currently aren't batch hydrated but i'm worried they ;; cannot be in this situation (hydrate [:ordered_cards [:card [:moderation_reviews :moderator_details]] :series :dashcard/action :dashcard/linkcard-info] - :collection_authority_level :can_write :param_fields :param_values) + :ordered_tabs :collection_authority_level :can_write :param_fields :param_values) api/read-check api/check-not-archived hide-unreadable-cards @@ -517,25 +517,81 @@ [:to-create [:sequential [:map [:id ms/NegativeInt]]]] [:to-update [:sequential [:map [:id ms/PositiveInt]]]] [:to-delete [:sequential [:map [:id ms/PositiveInt]]]]] - "Given 2 lists of seq maps changes, where each map an `id` keys, - return a map of 3 keys, `:to-create`, `:to-update`, `:to-delete`. + "Given 2 lists of seq maps of changes, where each map an has an `id` key, + return a map of 3 keys: `:to-create`, `:to-update`, `:to-delete`. Where: :to-create is a list of maps that has negative ids in `new-items` :to-update is a list of maps that has ids in both `current-items` and `new-items` :to delete is a list of maps that has ids only in `current-items`" [current-items :- [:sequential [:map [:id ms/PositiveInt]]] - new-items :- [:sequential [:map [:id int?]]]] + new-items :- [:sequential [:map [:id ms/Int]]]] (let [new-change-ids (set (map :id new-items)) - to-create (remove (comp pos? :id) new-items) + to-create (filter (comp neg? :id) new-items) ;; to-update items are new items with id in the current items - to-update (remove (comp neg? :id) new-items) + to-update (filter (comp pos? :id) new-items) ;; to delete items in current but not new items to-delete (remove (comp new-change-ids :id) current-items)] {:to-update to-update :to-delete to-delete :to-create to-create})) +(mu/defn ^:private create-tabs! :- [:map-of neg-int? pos-int?] + "Create the new tabs and returned a mapping from temporary tab ID to the new tab ID." + [dashboard :- map? + new-tabs :- [:sequential [:map [:id neg-int?]]]] + (let [new-tab-ids (t2/insert-returning-pks! :model/DashboardTab (->> new-tabs + (map #(dissoc % :id)) + (map #(assoc % :dashboard_id (:id dashboard)))))] + + (zipmap (map :id new-tabs) new-tab-ids))) + +(mu/defn ^:private update-tabs! :- nil? + [dashboard :- [:map [:id pos-int?] + [:ordered_tabs [:maybe [:sequential map?]]]] + new-tabs :- [:sequential [:map [:id ms/PositiveInt]]]] + (let [current-tabs (:ordered_tabs dashboard) + update-ks [:name :position] + id->current-tab (group-by :id current-tabs) + to-update-tabs (filter + ;; filter out tabs that haven't changed + (fn [new-tab] + (let [current-tab (get id->current-tab (:id new-tab))] + (not= (select-keys current-tab update-ks) + (select-keys new-tab update-ks)))) + + new-tabs)] + (doseq [tab to-update-tabs] + (t2/update! :model/DashboardTab (:id tab) (select-keys tab update-ks))) + nil)) + +(mu/defn ^:private delete-tabs! :- nil? + [tab-ids :- [:sequential {:min 1} ms/PositiveInt]] + (when (seq tab-ids) + (t2/delete! :model/DashboardTab :id [:in tab-ids])) + nil) + +(defn ^:private do-update-tabs! + "Given current tabs and new tabs, do the necessary create/update/delete to apply new tab changes. + Returns: + - a map of temporary tab ID to the new real tab ID + - a list of deleted tab ids" + [dashboard current-tabs new-tabs] + (when-not (= (range (count new-tabs)) + (sort (map :position new-tabs))) + (throw (ex-info (tru "Tab positions must be sequential and start at 0") + {:status-code 400}))) + (let [{:keys [to-create to-update to-delete]} (classify-changes current-tabs new-tabs) + to-delete-ids (set (map :id to-delete)) + _ (when-let [to-delete-ids (seq to-delete-ids)] + (delete-tabs! to-delete-ids)) + temp-tab-id->tab-id (when (seq to-create) + (create-tabs! dashboard to-create)) + _ (when (seq to-update) + (update-tabs! dashboard to-update))] + {:temp->real-tab-ids temp-tab-id->tab-id + :deleted-tab-ids to-delete-ids})) + (defn- create-dashcards! [dashboard dashcards] (doseq [{:keys [card_id]} dashcards @@ -567,6 +623,16 @@ dashboard-id api/*current-user-id*))) +(defn- do-update-dashcards! + [dashboard current-cards new-cards] + (let [{:keys [to-create to-update to-delete]} (classify-changes current-cards new-cards)] + (when (seq to-delete) + (delete-dashcards! dashboard (map :id to-delete))) + (when (seq to-create) + (create-dashcards! dashboard to-create)) + (when (seq to-update) + (update-dashcards! dashboard to-update)))) + (def ^:private UpdatedDashboardCard [:map ;; id can be negative, it indicates a new card and BE should create them @@ -580,37 +646,59 @@ [:target :any]]]]] [:series {:optional true} [:maybe [:sequential map?]]]]) +(def ^:private UpdatedDashboardTab + [:map + ;; id can be negative, it indicates a new card and BE should create them + [:id ms/Int] + [:name ms/NonBlankString]]) + (api/defendpoint PUT "/:id/cards" - "Update `Cards` on a Dashboard. Request body should have the form: - - {:cards [{:id ... ; DashboardCard ID - :size_x ... - :size_y ... - :row ... - :col ... - :parameter_mappings ... - :series [{:id 123 - ...}]} - ...]}" - [id :as {{:keys [cards]} :body}] - {id ms/PositiveInt - cards [:sequential UpdatedDashboardCard]} - (let [dashboard (-> (api/write-check Dashboard id) - api/check-not-archived - (t2/hydrate [:ordered_cards :series :card])) - current-cards (:ordered_cards dashboard) - - {:keys [to-update to-delete to-create]} (classify-changes current-cards cards)] + "Update `Cards` and `Tabs` on a Dashboard. Request body should have the form: + + {:cards [{:id ... ; DashboardCard ID + :size_x ... + :size_y ... + :row ... + :col ... + :parameter_mappings ... + :series [{:id 123 + ...}]} + ...] + :ordered_tabs [{:id ... ; DashboardTab ID + :name ...}]}" + [id :as {{:keys [cards ordered_tabs]} :body}] + {id ms/PositiveInt + cards (ms/maps-with-unique-key [:sequential UpdatedDashboardCard] :id) + ;; ordered_tabs should be required in production, making it optional because lots of + ;; e2e tests curerntly doesn't include it + ordered_tabs [:maybe (ms/maps-with-unique-key [:sequential UpdatedDashboardTab] :id)]} + (let [dashboard (-> (api/write-check Dashboard id) + api/check-not-archived + (t2/hydrate [:ordered_cards :series :card] :ordered_tabs)) + new-tabs (map-indexed (fn [idx tab] (assoc tab :position idx)) ordered_tabs)] + (when (and (seq (:ordered_tabs dashboard)) + (not (every? #(some? (:dashboard_tab_id %)) cards))) + (throw (ex-info (tru "This dashboard has tab, makes sure every card has a tab") + {:status-code 400}))) (api/check-500 (t2/with-transaction [_conn] - (when (seq to-delete) - (delete-dashcards! dashboard (map :id to-delete))) - (when (seq to-create) - (create-dashcards! dashboard (map #(dissoc % :id) to-create))) - (when (seq to-update) - (update-dashcards! dashboard to-update)) + (let [{:keys [temp->real-tab-ids + deleted-tab-ids]} (do-update-tabs! dashboard (:ordered_tabs dashboard) new-tabs) + current-cards (cond->> (:ordered_cards dashboard) + (seq deleted-tab-ids) + (remove (fn [card] + (contains? deleted-tab-ids (:dashboard_tab_id card))))) + new-cards (cond->> cards + ;; fixup the temporary tab ids with the real ones + (seq temp->real-tab-ids) + (map (fn [card] + (if-let [real-tab-id (get temp->real-tab-ids (:dashboard_tab_id card))] + (assoc card :dashboard_tab_id real-tab-id) + card))))] + (do-update-dashcards! dashboard current-cards new-cards)) true)) - (t2/hydrate (dashboard/ordered-cards id) :series))) + {:cards (t2/hydrate (dashboard/ordered-cards id) :series) + :ordered_tabs (dashboard/ordered-tabs id)})) #_{:clj-kondo/ignore [:deprecated-var]} (api/defendpoint-schema GET "/:id/revisions" diff --git a/src/metabase/models.clj b/src/metabase/models.clj index 99d1762df2621ea9f8d6e5adc8baced6102d068d..98ff92fd1dbad9a19f475ce7bafa93471b3bfbb6 100644 --- a/src/metabase/models.clj +++ b/src/metabase/models.clj @@ -11,6 +11,7 @@ [metabase.models.dashboard :as dashboard] [metabase.models.dashboard-card :as dashboard-card] [metabase.models.dashboard-card-series :as dashboard-card-series] + [metabase.models.dashboard-tab :as dashboard-tab] [metabase.models.database :as database] [metabase.models.dimension :as dimension] [metabase.models.field :as field] @@ -56,6 +57,7 @@ dashboard/keep-me dashboard-card/keep-me dashboard-card-series/keep-me + dashboard-tab/keep-me database/keep-me dimension/keep-me field/keep-me diff --git a/src/metabase/models/dashboard.clj b/src/metabase/models/dashboard.clj index aa9e9e85c314b7c09e9f432fbf682d1c06cb3201..76f33a4b7e14353fdbca15c87cfff7f094109dd1 100644 --- a/src/metabase/models/dashboard.clj +++ b/src/metabase/models/dashboard.clj @@ -45,10 +45,10 @@ (methodical/defmethod t2/table-name :model/Dashboard [_model] :report_dashboard) (doto :model/Dashboard - (derive ::perms/use-parent-collection-perms) - (derive :metabase/model) - (derive :hook/timestamped?) - (derive :hook/entity-id)) + (derive :metabase/model) + (derive ::perms/use-parent-collection-perms) + (derive :hook/timestamped?) + (derive :hook/entity-id)) (t2/deftransforms :model/Dashboard {:parameters mi/transform-parameters-list @@ -137,6 +137,12 @@ ;;; --------------------------------------------------- Hydration ---------------------------------------------------- +(mi/define-simple-hydration-method ordered-tabs + :ordered_tabs + "Return the ordered DashboardTabs associated with `dashboard-or-id`, sorted by tab position." + [dashboard-or-id] + (t2/select :model/DashboardTab :dashboard_id (u/the-id dashboard-or-id) {:order-by [[:position :asc]]})) + (mi/define-simple-hydration-method ordered-cards :ordered_cards "Return the DashboardCards associated with `dashboard`, in the order they were created." diff --git a/src/metabase/models/dashboard_card.clj b/src/metabase/models/dashboard_card.clj index 48a4799207de16af4af456343bfcca15fd9b841b..f4628884d8b599188899346c59df3490cee9e725 100644 --- a/src/metabase/models/dashboard_card.clj +++ b/src/metabase/models/dashboard_card.clj @@ -178,7 +178,7 @@ old-dashboard-card :- DashboardCardUpdates] (t2/with-transaction [_conn] (let [update-ks (cond-> [:action_id :row :col :size_x :size_y - :parameter_mappings :visualization_settings] + :parameter_mappings :visualization_settings :dashboard_tab_id] ;; Allow changing card_id for action dashcards, but not for card dashcards. ;; This is to preserve the existing behavior of questions and card_id ;; I don't know why card_id couldn't be changed for cards though. @@ -222,8 +222,7 @@ :visualization_settings {}} (select-keys dashcard [:dashboard_id :card_id :action_id :size_x :size_y :row :col - :parameter_mappings - :visualization_settings]))))] + :parameter_mappings :visualization_settings :dashboard_tab_id]))))] ;; add series to the DashboardCard (update-dashboard-cards-series! (zipmap dashboard-card-ids (map #(get % :series []) dashboard-cards))) ;; return the full DashboardCard diff --git a/src/metabase/models/dashboard_tab.clj b/src/metabase/models/dashboard_tab.clj new file mode 100644 index 0000000000000000000000000000000000000000..0d9502fb76ca958dab311581f22176877e65e308 --- /dev/null +++ b/src/metabase/models/dashboard_tab.clj @@ -0,0 +1,31 @@ +(ns metabase.models.dashboard-tab + (:require + [metabase.models.dashboard :refer [Dashboard]] + [metabase.models.interface :as mi] + [metabase.models.serialization :as serdes] + [methodical.core :as methodical] + [toucan2.core :as t2])) + +(methodical/defmethod t2/table-name :model/DashboardTab [_model] :dashboard_tab) + +(doto :model/DashboardTab + (derive :metabase/model) + (derive ::mi/read-policy.full-perms-for-perms-set) + (derive ::mi/write-policy.full-perms-for-perms-set) + (derive :hook/timestamped?) + (derive :hook/entity-id)) + +(defmethod mi/perms-objects-set :model/DashboardTab + [dashtab read-or-write] + (let [dashboard (or (:dashboard dashtab) + (t2/select-one Dashboard :id (:dashboard_id dashtab)))] + (mi/perms-objects-set dashboard read-or-write))) + +(defmethod serdes/hash-fields :model/DashboardTab + [_dashboard-tab] + [:name + (comp serdes/identity-hash + #(t2/select-one Dashboard :id %) + :dashboard_id) + :position + :created_at]) diff --git a/src/metabase/models/interface.clj b/src/metabase/models/interface.clj index 3d89980e03b4258db5c897bf113bc37a7dd511e7..2b56f9e01ded498603e530d62ae96bd2195f04a6 100644 --- a/src/metabase/models/interface.clj +++ b/src/metabase/models/interface.clj @@ -414,7 +414,6 @@ {:in (comp json-in normalize-parameters-list) :out (comp (catch-normalization-exceptions normalize-parameters-list) json-out-with-keywordization)}) - ;; --- predefined hooks (t2/define-before-insert :hook/timestamped? diff --git a/src/metabase/util/malli/schema.clj b/src/metabase/util/malli/schema.clj index af5d525647a63275982072f02a2d025d51418410..7255c8c51cbf15fcd967f7890d3f08e99f3f017d 100644 --- a/src/metabase/util/malli/schema.clj +++ b/src/metabase/util/malli/schema.clj @@ -23,9 +23,22 @@ [user :- (ms/InstanceOf User)] ...)" [model] - (mc/schema - [:fn {:error/fn (fn [_ _] (deferred-tru "value must be an instance of {0}" (name model)))} - #(models.dispatch/instance-of? model %)])) + (mu/with-api-error-message + [:fn #(models.dispatch/instance-of? model %)] + (deferred-tru "value must be an instance of {0}" (name model)))) + +(defn maps-with-unique-key + "Given a schema of a sequence of maps, returns as chema that do an additional unique check on key `k`." + [maps-schema k] + (mu/with-api-error-message + [:and + [:fn (fn [maps] + (= (count maps) + (-> (map #(get % k) maps) + distinct + count)))] + maps-schema] + (deferred-tru "value must be seq of maps in which {0}s are unique" (name k)))) ;;; -------------------------------------------------- Schemas -------------------------------------------------- @@ -43,6 +56,12 @@ ;; FIXME: greater than _or equal to_ zero. (deferred-tru "value must be an integer greater than zero."))) +(def Int + "Schema representing an integer." + (mu/with-api-error-message + int? + (deferred-tru "value must be an integer."))) + (def PositiveInt "Schema representing an integer than must also be greater than zero." (mu/with-api-error-message diff --git a/test/metabase/api/dashboard_test.clj b/test/metabase/api/dashboard_test.clj index 22e2759551ddb943d4decca0463ef63ce0a5204a..44dbb298a187355b61487c7be80c6522851bd4d3 100644 --- a/test/metabase/api/dashboard_test.clj +++ b/test/metabase/api/dashboard_test.clj @@ -28,6 +28,7 @@ Revision Table User]] + [metabase.models.dashboard :as dashboard] [metabase.models.dashboard-card :as dashboard-card] [metabase.models.dashboard-test :as dashboard-test] [metabase.models.field-values :as field-values] @@ -280,50 +281,53 @@ (deftest fetch-dashboard-test (testing "GET /api/dashboard/:id" (testing "fetch a dashboard WITH a dashboard card on it" - (mt/with-temp* [Dashboard [{dashboard-id :id - :as dashboard} {:name "Test Dashboard"}] - Card [{card-id :id - :as card} {:name "Dashboard Test Card"}] - DashboardCard [dashcard {:dashboard_id dashboard-id, :card_id card-id}] - User [{user-id :id} {:first_name "Test" :last_name "User" - :email "test@example.com"}] - Revision [_ {:user_id user-id - :model "Dashboard" - :model_id dashboard-id - :object (revision/serialize-instance dashboard - dashboard-id - dashboard)}]] + (mt/with-temp* [Dashboard [{dashboard-id :id + :as dashboard} {:name "Test Dashboard"}] + Card [{card-id :id + :as card} {:name "Dashboard Test Card"}] + :model/DashboardTab [{dashtab-id :id} {:name "Test Dashboard Tab" :position 0 :dashboard_id dashboard-id}] + DashboardCard [dashcard {:dashboard_id dashboard-id :card_id card-id :dashboard_tab_id dashtab-id}] + User [{user-id :id} {:first_name "Test" :last_name "User" + :email "test@example.com"}] + Revision [_ {:user_id user-id + :model "Dashboard" + :model_id dashboard-id + :object (revision/serialize-instance dashboard + dashboard-id + dashboard)}]] (with-dashboards-in-readable-collection [dashboard-id] (api.card-test/with-cards-in-readable-collection [card-id] - (is (= (merge - dashboard-defaults - {:name "Test Dashboard" - :creator_id (mt/user->id :rasta) - :collection_id true - :collection_authority_level nil - :can_write false - :param_fields nil - :param_values nil - :last-edit-info {:timestamp true :id true :first_name "Test" :last_name "User" :email "test@example.com"} - :ordered_cards [{:size_x 4 - :size_y 4 - :col 0 - :row 0 - :collection_authority_level nil - :updated_at true - :created_at true - :entity_id (:entity_id dashcard) - :parameter_mappings [] - :visualization_settings {} - :card (merge api.card-test/card-defaults-no-hydrate - {:name "Dashboard Test Card" - :creator_id (mt/user->id :rasta) - :collection_id true - :display "table" - :entity_id (:entity_id card) - :visualization_settings {} - :result_metadata nil}) - :series []}]}) + (is (=? (merge + dashboard-defaults + {:name "Test Dashboard" + :creator_id (mt/user->id :rasta) + :collection_id true + :collection_authority_level nil + :can_write false + :param_fields nil + :param_values nil + :last-edit-info {:timestamp true :id true :first_name "Test" :last_name "User" :email "test@example.com"} + :ordered_tabs [{:name "Test Dashboard Tab" :position 0 :id dashtab-id :dashboard_id dashboard-id}] + :ordered_cards [{:size_x 4 + :size_y 4 + :col 0 + :row 0 + :collection_authority_level nil + :updated_at true + :created_at true + :entity_id (:entity_id dashcard) + :parameter_mappings [] + :visualization_settings {} + :dashboard_tab_id dashtab-id + :card (merge api.card-test/card-defaults-no-hydrate + {:name "Dashboard Test Card" + :creator_id (mt/user->id :rasta) + :collection_id true + :display "table" + :entity_id (:entity_id card) + :visualization_settings {} + :result_metadata nil}) + :series []}]}) (dashboard-response (mt/user-http-request :rasta :get 200 (format "dashboard/%d" dashboard-id))))))))) (testing "a dashboard that has link cards on it" @@ -393,6 +397,7 @@ :has_field_values "search" :name_field nil :dimensions []}} + :ordered_tabs [] :ordered_cards [{:size_x 4 :size_y 4 :col 0 @@ -405,6 +410,7 @@ :parameter_id "foo" :target ["dimension" ["field" field-id nil]]}] :visualization_settings {} + :dashboard_tab_id nil :card (merge api.card-test/card-defaults-no-hydrate {:name "Dashboard Test Card" :creator_id (mt/user->id :rasta) @@ -1254,13 +1260,21 @@ :col 0 :size_x 4 :size_y 4 - :parameter_mappings mappings}]})) + :parameter_mappings mappings}] + :ordered_tabs []})) :dashcards (fn [] (t2/select DashboardCard :dashboard_id dashboard-id))}))))) (defn- dashcard-like-response [id] (t2/hydrate (t2/select-one DashboardCard :id id) :series)) +(defn- current-cards + "Returns the current ordered cards of a dashboard." + [dashboard-id] + (-> dashboard-id + dashboard/ordered-cards + (t2/hydrate :series))) + (defn do-with-update-cards-parameter-mapping-permissions-fixtures [f] (do-with-add-card-parameter-mapping-permissions-fixtures (fn [{:keys [dashboard-id card-id mappings]}] @@ -1280,13 +1294,133 @@ :update-mappings! (fn [expected-status-code] (mt/user-http-request :rasta :put expected-status-code (format "dashboard/%d/cards" dashboard-id) - {:cards [(assoc dashcard-info :parameter_mappings new-mappings)]})) + {:cards [(assoc dashcard-info :parameter_mappings new-mappings)] + :ordered_tabs []})) :update-size! (fn [] (mt/user-http-request :rasta :put 200 (format "dashboard/%d/cards" dashboard-id) - {:cards [new-dashcard-info]}))})))))) + {:cards [new-dashcard-info] + :ordered_tabs []}))})))))) -(deftest e2e-update-cards-test +(defn- do-with-simple-dashboard-with-tabs + [f] + (t2.with-temp/with-temp + [Dashboard {dashboard-id :id} {} + + Card {card-id-1 :id} {} + + Card {card-id-2 :id} {} + :model/DashboardTab {dashtab-id-1 :id} {:name "Tab 1" + :dashboard_id dashboard-id + :position 0} + :model/DashboardTab {dashtab-id-2 :id} {:name "Tab 2" + :dashboard_id dashboard-id + :position 1} + DashboardCard {dashcard-id-1 :id} {:dashboard_id dashboard-id + :card_id card-id-1 + :dashboard_tab_id dashtab-id-1} + DashboardCard {dashcard-id-2 :id} {:dashboard_id dashboard-id + :card_id card-id-2 + :dashboard_tab_id dashtab-id-2}] + (f {:dashboard-id dashboard-id + :card-id-1 card-id-1 + :card-id-2 card-id-1 + :dashtab-id-1 dashtab-id-1 + :dashtab-id-2 dashtab-id-2 + :dashcard-id-1 dashcard-id-1 + :dashcard-id-2 dashcard-id-2}))) + +(defmacro ^:private with-simple-dashboard-with-tabs + "Create a simple dashboard with 2 tabs and 2 cards in each tab and run `body` with the dashboard and cards ids bound to" + [[bindings] & body] + `(do-with-simple-dashboard-with-tabs (fn [~bindings] ~@body))) + +(deftest e2e-update-cards-and-tabs-test + (testing "PUT /api/dashboard/:id/cards with create/update/delete in a single req" + (t2.with-temp/with-temp + [Dashboard {dashboard-id :id} {} + Card {card-id-1 :id} {} + Card {card-id-2 :id} {} + :model/DashboardTab {dashtab-id-1 :id} {:name "Tab 1" :dashboard_id dashboard-id :position 0} + :model/DashboardTab {dashtab-id-2 :id} {:name "Tab 2" :dashboard_id dashboard-id :position 1} + :model/DashboardTab {dashtab-id-3 :id} {:name "Tab 3" :dashboard_id dashboard-id :position 2} + DashboardCard {dashcard-id-1 :id} {:dashboard_id dashboard-id, :card_id card-id-1, :dashboard_tab_id dashtab-id-1} + DashboardCard {dashcard-id-2 :id} {:dashboard_id dashboard-id, :card_id card-id-1, :dashboard_tab_id dashtab-id-2} + DashboardCard {dashcard-id-3 :id} {:dashboard_id dashboard-id, :card_id card-id-1, :dashboard_tab_id dashtab-id-2}] + (let [resp (mt/user-http-request :rasta :put 200 (format "dashboard/%d/cards" dashboard-id) + {:ordered_tabs [{:id dashtab-id-1 + :name "Tab 1 edited"} + {:id dashtab-id-2 + :name "Tab 2"} + {:id -1 + :name "New tab"}] + :cards [{:id dashcard-id-1 + :size_x 4 + :size_y 4 + :col 1 + :row 1 + ;; initialy was in tab1, now in tab 2 + :dashboard_tab_id dashtab-id-2 + :card_id card-id-1} + {:id dashcard-id-2 + :dashboard_tab_id dashtab-id-2 + :size_x 2 + :size_y 2 + :col 2 + :row 2} + ;; remove the dashcard3 and create a new card using negative numbers + ;; and assign into the newly created dashcard + {:id -1 + :size_x 1 + :size_y 1 + :col 3 + :row 3 + :dashboard_tab_id -1 + :card_id card-id-2}]})] + (testing "tabs got updated correctly " + (is (=? [{:id dashtab-id-1 + :dashboard_id dashboard-id + :name "Tab 1 edited" + :position 0} + {:id dashtab-id-2 + :dashboard_id dashboard-id + :name "Tab 2" + :position 1} + {:id #hawk/schema (s/pred pos-int?) + :dashboard_id dashboard-id + :name "New tab" + :position 2}] + (:ordered_tabs resp))) + (testing "dashtab 3 got deleted" + (is (nil? (t2/select-one :model/DashboardTab :id dashtab-id-3))))) + + (testing "dashcards got updated correctly" + (let [new-tab-id (t2/select-one-pk :model/DashboardTab :name "New tab" :dashboard_id dashboard-id)] + (is (=? [{:id dashcard-id-1 + :card_id card-id-1 + :dashboard_tab_id dashtab-id-2 + :size_x 4 + :size_y 4 + :col 1 + :row 1} + {:id dashcard-id-2 + :dashboard_tab_id dashtab-id-2 + :size_x 2 + :size_y 2 + :col 2 + :row 2} + {:id #hawk/schema (s/pred pos-int?) + :size_x 1 + :size_y 1 + :col 3 + :row 3 + :dashboard_tab_id new-tab-id + :card_id card-id-2}] + (:cards resp)))) + (testing "dashcard 3 got deleted" + (is (nil? (t2/select-one DashboardCard :id dashcard-id-3))))))))) + +(deftest e2e-update-cards-only-test (testing "PUT /api/dashboard/:id/cards with create/update/delete in a single req" (t2.with-temp/with-temp [Dashboard {dashboard-id :id} {} @@ -1299,28 +1433,30 @@ Card {series-id-2 :id} {:name "Series Card 2"} DashboardCardSeries _ {:dashboardcard_id dashcard-id-1, :card_id series-id-1 :position 0}] - (let [resp (mt/user-http-request :crowberto :put 200 (format "dashboard/%d/cards" dashboard-id) - {:cards [{:id dashcard-id-1 - :size_x 4 - :size_y 4 - :col 1 - :row 1 - ;; update series for card 1 - :series [{:id series-id-2}] - :card_id card-id-1} - {:id dashcard-id-2 - :size_x 2 - :size_y 2 - :col 2 - :row 2} - ;; remove the dashcard3 and create a new card using negative numbers - {:id -1 - :size_x 1 - :size_y 1 - :col 3 - :row 3 - :card_id card-id-2 - :series [{:id series-id-1}]}]}) + ;; send a request that update and create and delete some cards at the same time + (let [cards (:cards (mt/user-http-request :crowberto :put 200 (format "dashboard/%d/cards" dashboard-id) + {:cards [{:id dashcard-id-1 + :size_x 4 + :size_y 4 + :col 1 + :row 1 + ;; update series for card 1 + :series [{:id series-id-2}] + :card_id card-id-1} + {:id dashcard-id-2 + :size_x 2 + :size_y 2 + :col 2 + :row 2} + ;; remove the dashcard3 and create a new card using negative numbers + {:id -1 + :size_x 1 + :size_y 1 + :col 3 + :row 3 + :card_id card-id-2 + :series [{:id series-id-1}]}] + :ordered_tabs []})) updated-card-1 {:id dashcard-id-1 :card_id card-id-1 :dashboard_id dashboard-id @@ -1350,29 +1486,130 @@ (is (=? [updated-card-1 updated-card-2 new-card] - resp)) + cards)) ;; dashcard 3 is deleted (is (nil? (t2/select-one DashboardCard :id dashcard-id-3))))))) -;;; -------------------------------------- Create dashcards only tests --------------------------------------- +(deftest e2e-update-tabs-only-test + (testing "PUT /api/dashboard/:id/cards with create/update/delete tabs in a single req" + (with-simple-dashboard-with-tabs [{:keys [dashboard-id dashtab-id-1 dashtab-id-2]}] + (with-dashboards-in-writeable-collection [dashboard-id] + ;; send a request that update and create and delete some cards at the same time + (is (some? (t2/select-one :model/DashboardTab :id dashtab-id-2))) + (let [tabs (:ordered_tabs (mt/user-http-request :rasta :put 200 (format "dashboard/%d/cards" dashboard-id) + {:ordered_tabs [{:name "Tab new" + :id -1} + {:name "Tab 1 moved to second position" + :id dashtab-id-1}] + :cards []}))] + + (is (=? [{:dashboard_id dashboard-id + :name "Tab new" + :position 0} + {:id dashtab-id-1 + :dashboard_id dashboard-id + :name "Tab 1 moved to second position" + :position 1}] + tabs)) + ;; dashtab 2 is deleted + (is (nil? (t2/select-one :model/DashboardTab :id dashtab-id-2)))))))) + +(deftest upgrade-from-non-tab-dashboard-to-has-tabs + (testing "we introduced tabs in 47 but there are dashboards without tabs before this + this test check the flow to upgrade a dashboard pre-47 to have tabs" + (t2.with-temp/with-temp + [Dashboard {dashboard-id :id} {} + Card {card-id-1 :id} {} + Card {card-id-2 :id} {} + DashboardCard {dashcard-1-id :id} {:card_id card-id-1 + :dashboard_id dashboard-id}] + ;; create 2 tabs, assign the existing dashcard to the 1st tab + ;; create 2 new dashcards, 1 for each tab + (let [resp (mt/user-http-request :rasta :put 200 (format "dashboard/%d/cards" dashboard-id) + {:ordered_tabs [{:id -1 + :name "New Tab 1"} + {:id -2 + :name "New Tab 2"}] + :cards [{:id dashcard-1-id + :size_x 4 + :size_y 4 + :col 1 + :row 1 + :dashboard_tab_id -1 + :card_id card-id-1} + {:id -2 + :size_x 4 + :size_y 4 + :col 1 + :row 1 + :dashboard_tab_id -1 + :card_id card-id-1} + {:id -1 + :size_x 4 + :size_y 4 + :col 1 + :row 1 + :dashboard_tab_id -2 + :card_id card-id-2}]}) + tab-1-id (t2/select-one-pk :model/DashboardTab :name "New Tab 1" :dashboard_id dashboard-id) + tab-2-id (t2/select-one-pk :model/DashboardTab :name "New Tab 2" :dashboard_id dashboard-id)] + (is (=? {:cards [{:id dashcard-1-id + :card_id card-id-1 + :dashboard_id dashboard-id + :dashboard_tab_id tab-1-id} + {:id #hawk/schema (s/pred pos-int?) + :card_id card-id-1 + :dashboard_id dashboard-id + :dashboard_tab_id tab-1-id} + {:id #hawk/schema (s/pred pos-int?) + :card_id card-id-2 + :dashboard_id dashboard-id + :dashboard_tab_id tab-2-id}] + :ordered_tabs [{:id tab-1-id + :dashboard_id dashboard-id + :name "New Tab 1" + :position 0} + {:id tab-2-id + :dashboard_id dashboard-id + :name "New Tab 2" + :position 1}]} + resp)))))) + +(deftest update-cards-error-handling-test + (testing "PUT /api/dashboard/:id/cards" + (with-simple-dashboard-with-tabs [{:keys [dashboard-id]}] + (testing "if a dashboard has tabs, check if all cards from the request has a tab_id" + (is (= "This dashboard has tab, makes sure every card has a tab" + (mt/user-http-request :crowberto :put 400 (format "dashboard/%d/cards" dashboard-id) + {:cards (conj + (current-cards dashboard-id) + {:id -1 + :size_x 4 + :size_y 4 + :col 1 + :row 1}) + :ordered_tabs (dashboard/ordered-tabs dashboard-id)}))))))) + +;;; -------------------------------------- Create dashcards tests --------------------------------------- (deftest simple-creation-with-no-additional-series-test (mt/with-temp* [Dashboard [{dashboard-id :id}] Card [{card-id :id}]] (with-dashboards-in-writeable-collection [dashboard-id] (api.card-test/with-cards-in-readable-collection [card-id] - (let [resp (mt/user-http-request :rasta :put 200 (format "dashboard/%d/cards" dashboard-id) - {:cards [{:id -1 - :card_id card-id - :row 4 - :col 4 - :size_x 4 - :size_y 4 - :parameter_mappings [{:parameter_id "abc" - :card_id 123 - :hash "abc" - :target "foo"}] - :visualization_settings {}}]})] + (let [resp (:cards (mt/user-http-request :rasta :put 200 (format "dashboard/%d/cards" dashboard-id) + {:cards [{:id -1 + :card_id card-id + :row 4 + :col 4 + :size_x 4 + :size_y 4 + :parameter_mappings [{:parameter_id "abc" + :card_id 123 + :hash "abc" + :target "foo"}] + :visualization_settings {}}] + :ordered_tabs []}))] ;; extra sure here because the dashcard we given has a negative id (testing "the inserted dashcards has ids auto-generated" (is (pos? (:id (first resp))))) @@ -1381,6 +1618,7 @@ :col 4 :row 4 :series [] + :dashboard_tab_id nil :parameter_mappings [{:parameter_id "abc" :card_id 123, :hash "abc", :target "foo"}] :visualization_settings {} :created_at true @@ -1407,14 +1645,15 @@ Card [{series-id-1 :id} {:name "Series Card"}]] (with-dashboards-in-writeable-collection [dashboard-id] (api.card-test/with-cards-in-readable-collection [card-id series-id-1] - (let [dashboard-cards (mt/user-http-request :rasta :put 200 (format "dashboard/%d/cards" dashboard-id) - {:cards [{:id -1 - :card_id card-id - :row 4 - :col 4 - :size_x 4 - :size_y 4 - :series [{:id series-id-1}]}]})] + (let [dashboard-cards (:cards (mt/user-http-request :rasta :put 200 (format "dashboard/%d/cards" dashboard-id) + {:cards [{:id -1 + :card_id card-id + :row 4 + :col 4 + :size_x 4 + :size_y 4 + :series [{:id series-id-1}]}] + :ordered_tabs []}))] (is (=? [{:row 4 :col 4 :size_x 4 @@ -1448,15 +1687,16 @@ (is (partial= [{:visualization_settings {:label "Update"} :action_id action-id :card_id nil}] - (mt/user-http-request :crowberto :put 200 (format "dashboard/%s/cards" dashboard-id) - {:cards [{:id -1 - :size_x 1 - :size_y 1 - :row 1 - :col 1 - :card_id nil - :action_id action-id - :visualization_settings {:label "Update"}}]}))) + (:cards (mt/user-http-request :crowberto :put 200 (format "dashboard/%s/cards" dashboard-id) + {:cards [{:id -1 + :size_x 1 + :size_y 1 + :row 1 + :col 1 + :card_id nil + :action_id action-id + :visualization_settings {:label "Update"}}] + :ordered_tabs []})))) (is (partial= {:ordered_cards [{:action (cond-> {:visualization_settings {:hello true} :type (name action-type) :parameters [{:id "id"}]} @@ -1484,13 +1724,13 @@ (testing "If they have data permissions, it should be ok" (perms/grant-permissions! (perms-group/all-users) (perms/table-query-path (mt/id :venues))) (is (schema= [{:card_id (s/eq card-id) - :parameter_mappings [(s/one - {:parameter_id (s/eq "_CATEGORY_ID_") - :target (s/eq ["dimension" ["field" (mt/id :venues :category_id) nil]]) - s/Keyword s/Any} - "mapping")] - s/Keyword s/Any}] - (add-card! 200))) + :parameter_mappings [(s/one + {:parameter_id (s/eq "_CATEGORY_ID_") + :target (s/eq ["dimension" ["field" (mt/id :venues :category_id) nil]]) + s/Keyword s/Any} + "mapping")] + s/Keyword s/Any}] + (:cards (add-card! 200)))) (is (schema= [(s/one {:card_id (s/eq card-id) :parameter_mappings (s/eq mappings) s/Keyword s/Any} @@ -1503,17 +1743,18 @@ Card {card-id :id} {:archived true}] (is (= "The object has been archived." (:message (mt/user-http-request :rasta :put 404 (format "dashboard/%d/cards" dashboard-id) - {:cards [{:id -1 - :card_id card-id - :row 4 - :col 4 - :size_x 4 - :size_y 4 - :parameter_mappings [{:parameter_id "abc" - :card_id 123 - :hash "abc" - :target "foo"}] - :visualization_settings {}}]})))))) + {:cards [{:id -1 + :card_id card-id + :row 4 + :col 4 + :size_x 4 + :size_y 4 + :parameter_mappings [{:parameter_id "abc" + :card_id 123 + :hash "abc" + :target "foo"}] + :visualization_settings {}}] + :ordered_tabs []})))))) ;;; -------------------------------------- Update dashcards only tests --------------------------------------- @@ -1548,17 +1789,18 @@ (remove-ids-and-booleanize-timestamps (dashboard-card/retrieve-dashboard-card dashcard-id-2)))) ;; TODO adds tests for return (mt/user-http-request :rasta :put 200 (format "dashboard/%d/cards" dashboard-id) - {:cards [{:id dashcard-id-1 - :size_x 4 - :size_y 2 - :col 0 - :row 0 - :series [{:id series-id-1}]} - {:id dashcard-id-2 - :size_x 1 - :size_y 1 - :col 1 - :row 3}]}) + {:cards [{:id dashcard-id-1 + :size_x 4 + :size_y 2 + :col 0 + :row 0 + :series [{:id series-id-1}]} + {:id dashcard-id-2 + :size_x 1 + :size_y 1 + :col 1 + :row 3}] + :ordered_tabs []}) (is (= {:size_x 4 :size_y 2 :col 0 @@ -1621,13 +1863,28 @@ ;; TODO adds test for return (mt/user-http-request :rasta :put 200 (format "dashboard/%d/cards" dashboard-id) {:cards [(assoc action-card :card_id model-id-2) - (assoc question-card :card_id model-id-2)]}) + (assoc question-card :card_id model-id-2)] + :ordered_tabs []}) ;; Only action card should change (is (partial= [{:card_id model-id-2} {:card_id model-id}] (t2/select DashboardCard :dashboard_id dashboard-id {:order-by [:id]})))))))) -;;; -------------------------------------- Delete dashcards only tests --------------------------------------- +(deftest update-tabs-test + (with-simple-dashboard-with-tabs [{:keys [dashboard-id dashtab-id-1 dashtab-id-2]}] + (testing "change tab name and change the position" + (is (=? [{:id dashtab-id-2 + :name "Tab 2"} + {:id dashtab-id-1 + :name "Tab 1 edited"}] + (:ordered_tabs (mt/user-http-request :crowberto :put 200 (format "dashboard/%d/cards" dashboard-id) + {:ordered_tabs [{:id dashtab-id-2 + :name "Tab 2"} + {:id dashtab-id-1 + :name "Tab 1 edited"}] + :cards (current-cards dashboard-id)}))))))) + +;;; -------------------------------------- Delete dashcards tests --------------------------------------- (deftest delete-cards-test (testing "PUT /api/dashboard/id/:cards to delete" @@ -1646,10 +1903,12 @@ (with-dashboards-in-writeable-collection [dashboard-id] (is (= 3 (count (t2/select-pks-set DashboardCard, :dashboard_id dashboard-id)))) - (is (=? [{:id dashcard-id-3 - :series [{:id series-id-1}]}] + (is (=? {:cards [{:id dashcard-id-3 + :series [{:id series-id-1}]}] + :ordered_tabs []} (mt/user-http-request :rasta :put 200 - (format "dashboard/%d/cards" dashboard-id) {:cards [(dashcard-like-response dashcard-id-3)]}))) + (format "dashboard/%d/cards" dashboard-id) {:cards [(dashcard-like-response dashcard-id-3)] + :ordered_tabs []}))) (is (= 1 (count (t2/select-pks-set DashboardCard, :dashboard_id dashboard-id))))))) (testing "prune" @@ -1660,12 +1919,55 @@ (with-dashboards-in-writeable-collection [dashboard-id] (is (= 2 (count (t2/select-pks-set DashboardCard, :dashboard_id dashboard-id)))) - (is (=? [] + (is (=? {:ordered_tabs [] + :cards []} (mt/user-http-request :rasta :put 200 - (format "dashboard/%d/cards" dashboard-id) {:cards []}))) + (format "dashboard/%d/cards" dashboard-id) {:cards [] :ordered_tabs []}))) (is (= 0 (count (t2/select-pks-set DashboardCard, :dashboard_id dashboard-id))))))))) +(deftest delete-tabs-test + (testing "PUT /api/dashboard/id/:cards to delete" + (testing "partial delete" + (with-simple-dashboard-with-tabs [{:keys [dashboard-id dashtab-id-1 dashtab-id-2]}] + (testing "we have 2 tabs, each has 1 card to begin with" + (is (= 2 + (t2/count DashboardCard, :dashboard_id dashboard-id))) + (is (= 2 + (t2/count :model/DashboardTab :dashboard_id dashboard-id)))) + (is (=? {:ordered_tabs [{:id dashtab-id-1}]} + (mt/user-http-request :rasta :put 200 + (format "dashboard/%d/cards" dashboard-id) + {:ordered_tabs [(t2/select-one :model/DashboardTab :id dashtab-id-1)] + :cards (remove #(= (:dashboard_tab_id %) dashtab-id-2) (current-cards dashboard-id))}))) + (testing "deteted 1 tab, we should have" + (testing "1 card left" + (is (= 1 + (t2/count DashboardCard :dashboard_id dashboard-id)))) + (testing "1 tab left" + (is (= 1 + (t2/count :model/DashboardTab :dashboard_id dashboard-id))))))) + (testing "prune" + (with-simple-dashboard-with-tabs [{:keys [dashboard-id dashtab-id-2]}] + (testing "we have 2 tabs, each has 1 card to begin with" + (is (= 2 + (t2/count DashboardCard, :dashboard_id dashboard-id))) + (is (= 2 + (t2/count :model/DashboardTab :dashboard_id dashboard-id)))) + (is (=? {:ordered_tabs [] + :cards []} + (mt/user-http-request :rasta :put 200 + (format "dashboard/%d/cards" dashboard-id) + {:ordered_tabs [] + :cards (remove #(= (:dashboard_tab_id %) dashtab-id-2) (current-cards dashboard-id))}))) + (testing "dashboard should be empty" + (testing "0 card left" + (is (= 0 + (t2/count DashboardCard :dashboard_id dashboard-id)))) + (testing "0 tab left" + (is (= 0 + (t2/count :model/DashboardTab :dashboard_id dashboard-id))))))))) + (deftest classify-changes-test (testing "classify correctly" (is (= {:to-update [{:id 2 :name "c3"} {:id 4 :name "c4"}] diff --git a/test/metabase/models/dashboard_tab_test.clj b/test/metabase/models/dashboard_tab_test.clj new file mode 100644 index 0000000000000000000000000000000000000000..34fa2aaffef0092c2b1bc91d704d26b8e60f9468 --- /dev/null +++ b/test/metabase/models/dashboard_tab_test.clj @@ -0,0 +1,89 @@ +(ns metabase.models.dashboard-tab-test + (:require + [clojure.test :refer :all] + [metabase.api.common :as api] + [metabase.models :refer [Card Collection Dashboard DashboardCard]] + [metabase.models.interface :as mi] + [metabase.models.permissions :as perms] + [metabase.test :as mt] + [metabase.test.fixtures :as fixtures] + [toucan2.core :as t2] + [toucan2.tools.with-temp :as t2.with-temp])) + +(use-fixtures + :once + (fixtures/initialize :test-users-personal-collections)) + +(defn do-with-dashtab-in-personal-collection [f] + (let [owner-id (mt/user->id :rasta) + coll (t2/select-one Collection :personal_owner_id owner-id)] + (t2.with-temp/with-temp + [Card card {} + Dashboard dash {:collection_id (:id coll)} + :model/DashboardTab dashtab {:dashboard_id (:id dash)} + DashboardCard dashcard {:dashboard_id (:id dash) :card_id (:id card) :dashboard_tab_id (:id dashtab)}] + (f {:owner-id owner-id + :collection coll + :card card + :dashboard dash + :dashcard dashcard + :dashtab dashtab})))) + +(defmacro with-dashtab-in-personal-collection + [binding & body] + `(do-with-dashtab-in-personal-collection (fn [~binding] ~@body))) + +(deftest perms-test + (with-dashtab-in-personal-collection {:keys [collection dashboard dashtab] :as _dashtab} + (testing "dashtab's permission is the permission of dashboard they're on" + (is (= (mi/perms-objects-set dashtab :read) + (mi/perms-objects-set dashboard :read))) + (is (= (mi/perms-objects-set dashtab :write) + (mi/perms-objects-set dashboard :write)))) + + (testing (str "Check that if a Dashtab of a Dashboard is in a Collection, someone who would not be able to see it under the old " + "artifact-permissions regime will be able to see it if they have permissions for that Collection") + (binding [api/*current-user-permissions-set* (delay #{(perms/collection-read-path collection)})] + (mi/perms-objects-set dashtab :read) + (is (= true (mi/can-read? dashtab))) + (is (= false (mi/can-write? dashtab))))) + + (testing "Do we have *write* Permissions for a dashtab if we have *write* Permissions for the Collection it's in?" + (binding [api/*current-user-permissions-set* (delay #{(perms/collection-readwrite-path collection)})] + (is (= true (mi/can-read? dashtab))) + (is (= true (mi/can-write? dashtab))))) + + (testing "A user that can't see the Collection that the Dashboard is in can't read and write the dashtab" + (mt/with-current-user (mt/user->id :lucky) + (is (= false (mi/can-read? dashboard))) + (is (= false (mi/can-write? dashboard))))) + + (testing "A user that can see the Collection that the Dashboard is in can read and write the dashtab" + (mt/with-current-user (mt/user->id :rasta) + (is (= true (mi/can-read? dashboard))) + (is (= true (mi/can-write? dashboard))))))) + +(deftest dependency-test + (testing "Deleting a dashtab should delete the associated dashboardcards" + (with-dashtab-in-personal-collection {:keys [dashtab dashcard]} + (t2/delete! dashtab) + (is (= nil (t2/select-one DashboardCard :id (:id dashcard)))))) + + (testing "Deleting a dashboard will delete all its dashcards" + (with-dashtab-in-personal-collection {:keys [dashboard dashtab dashcard]} + (t2/delete! dashboard) + (is (= nil (t2/select-one :model/DashboardTab :id (:id dashtab)))) + (is (= nil (t2/select-one DashboardCard :id (:id dashcard))))))) + +(deftest hydration-test + (testing "hydrate a dashboard will return all of its tabs" + (t2.with-temp/with-temp + [Card card {} + Dashboard dashboard {} + :model/DashboardTab dashtab-1 {:dashboard_id (:id dashboard) :position 0} + :model/DashboardTab dashtab-2 {:dashboard_id (:id dashboard) :position 1} + DashboardCard _ {:dashboard_id (:id dashboard) :card_id (:id card) :dashboard_tab_id (:id dashtab-1)} + DashboardCard _ {:dashboard_id (:id dashboard) :card_id (:id card) :dashboard_tab_id (:id dashtab-2)}] + (is (=? {:ordered_tabs [{:id (:id dashtab-1) :position 0 :dashboard_id (:id dashboard)} + {:id (:id dashtab-2) :position 1 :dashboard_id (:id dashboard)}]} + (t2/hydrate dashboard :ordered_tabs)))))) diff --git a/test/metabase/test/util.clj b/test/metabase/test/util.clj index 014b9b5298ff98a60caa1ee4065db6077f37a3c0..64cd2db467982b59c737a631a114848c100d065a 100644 --- a/test/metabase/test/util.clj +++ b/test/metabase/test/util.clj @@ -133,6 +133,11 @@ :model/DashboardCardSeries (constantly {:position 0}) + :model/DashboardTab + (fn [_] + {:name (tu.random/random-name) + :position 0}) + Database (fn [_] {:details {} :engine :h2 diff --git a/yarn.lock b/yarn.lock index 966e42b4c148f8c7980acb5f0d4e8a3a49af1f44..854634bae00003b6bd15e0b77374c5cc8d32a9b9 100644 --- a/yarn.lock +++ b/yarn.lock @@ -19201,6 +19201,11 @@ reduce-reducers@^0.4.3: resolved "https://registry.yarnpkg.com/reduce-reducers/-/reduce-reducers-0.4.3.tgz#8e052618801cd8fc2714b4915adaa8937eb6d66c" integrity sha512-+CNMnI8QhgVMtAt54uQs3kUxC3Sybpa7Y63HR14uGLgI9/QR5ggHvpxwhGGe3wmx5V91YwqQIblN9k5lspAmGw== +reduce-reducers@^1.0.4: + version "1.0.4" + resolved "https://registry.yarnpkg.com/reduce-reducers/-/reduce-reducers-1.0.4.tgz#fb77e751a9eb0201760ac5a605ca8c9c2d0537f8" + integrity sha512-Mb2WZ2bJF597exiqX7owBzrqJ74DHLK3yOQjCyPAaNifRncE8OD0wFIuoMhXxTnHK07+8zZ2SJEKy/qtiyR7vw== + redux-actions@^2.0.1: version "2.6.5" resolved "https://registry.yarnpkg.com/redux-actions/-/redux-actions-2.6.5.tgz#bdca548768ee99832a63910c276def85e821a27e"