From 27f7d840917b288ef3ef0a4f888a9c44c4995d1c Mon Sep 17 00:00:00 2001 From: Alex Fedotyev <61838744+alex-fedotyev@users.noreply.github.com> Date: Wed, 6 May 2026 02:33:35 +0000 Subject: [PATCH 1/4] test(e2e): cover unified DashboardContainer (collapse, border, tabs, drag) PR #2015 unified the legacy section/group concept into a single DashboardContainer with collapsible / bordered / tabs / drag-to-reorder plus URL-state persistence. Unit tests landed with #2015; this PR adds the missing E2E coverage that Drew called out in his top-level review. What this covers: - Default collapsible chevron + bordered style on a new group. - Toggling Hide Border / Show Border via the overflow menu. - Adding a tab so the tab bar appears, switching tabs updates ?activeTabs and aria-selected. - ?collapsed and ?expanded URL params survive reload. - ?activeTabs URL param survives reload and restores the active tab. - Save-and-reload round-trip preserves containers, bordered=false, and the second tab on group B. - Drag-to-reorder: drag-onto-self is a no-op (DnD guard); dragging A onto C in [A, B, C] yields [B, C, A] (arrayMove semantics) and the new order persists across navigation. Side fixes: - DashboardPage.ts page-object selector updated from the stale add-new-section-menu-item to add-new-group-menu-item (renamed by PR #2015 in DBDashboardPage.tsx); helper renamed addSection -> addGroup. - DashboardContainer.tsx adds data-testid="group-add-tab-\${id}" on the existing Add Tab menu item so the spec does not have to rely on Mantine menu text. No API or schema change. Test-only plus one data-testid line. Tracker: Nerve task 2026-05-04-hyperdxiohyperdx-e2e-tests-for-unifie Co-Authored-By: Claude Opus 4.7 --- .../app/src/components/DashboardContainer.tsx | 10 +- .../e2e/features/dashboard-container.spec.ts | 326 ++++++++++++++++++ .../tests/e2e/page-objects/DashboardPage.ts | 141 +++++++- 3 files changed, 466 insertions(+), 11 deletions(-) create mode 100644 packages/app/tests/e2e/features/dashboard-container.spec.ts diff --git a/packages/app/src/components/DashboardContainer.tsx b/packages/app/src/components/DashboardContainer.tsx index f52cc96bad..4ad264426f 100644 --- a/packages/app/src/components/DashboardContainer.tsx +++ b/packages/app/src/components/DashboardContainer.tsx @@ -35,7 +35,7 @@ type DashboardContainerProps = { onToggleCollapsible: () => void; onToggleBordered: () => void; onDelete: (action: 'ungroup' | 'delete') => void; - /** Tile count inside this container — determines whether "Ungroup Tiles" is offered. */ + /** Tile count inside this container, determines whether "Ungroup Tiles" is offered. */ tileCount: number; onAddTile: () => void; activeTabId: string | undefined; @@ -162,7 +162,11 @@ export default function DashboardContainer({ - } onClick={onAddTab}> + } + onClick={onAddTab} + data-testid={`group-add-tab-${container.id}`} + > Add Tab @@ -275,7 +279,7 @@ export default function DashboardContainer({ ) : ( - /* Plain header (1 tab or collapsed) — shows title + chevron */ + /* Plain header (1 tab or collapsed): shows title + chevron */ { + let dashboardPage: DashboardPage; + + test.beforeEach(async ({ page }) => { + dashboardPage = new DashboardPage(page); + await dashboardPage.goto(); + await dashboardPage.createNewDashboard(); + }); + + /** + * Helper: add a group, return its container id. + * + * `useDashboardContainers.handleAddContainer` sets `collapsed: false` and + * leaves `collapsible` / `bordered` undefined, so both default to `true` + * via `container.collapsible !== false` / `container.bordered !== false` + * in DashboardContainer.tsx:83-84. + */ + async function addGroupAndGetId(): Promise { + const before = await dashboardPage.getGroupOrder(); + await dashboardPage.addGroup(); + await expect(dashboardPage.getGroups()).toHaveCount(before.length + 1); + const after = await dashboardPage.getGroupOrder(); + const newId = after.find(id => !before.includes(id)); + if (!newId) throw new Error('Newly added group id not found'); + return newId; + } + + test('group renders with default collapsible chevron and bordered style', async () => { + const id = await addGroupAndGetId(); + const group = dashboardPage.getGroup(id); + const chevron = dashboardPage.page.getByTestId(`group-chevron-${id}`); + + await test.step('chevron is present and reports expanded by default', async () => { + await expect(chevron).toBeVisible(); + await expect(chevron).toHaveAttribute('aria-expanded', 'true'); + }); + + await test.step('container body renders an inline border', async () => { + // DashboardContainer.tsx:243-246 sets inline `border` only when + // bordered=true. Reading el.style.border avoids resolving the CSS var. + const inlineBorder = await group.evaluate( + (el: HTMLElement) => el.style.border, + ); + expect(inlineBorder.length).toBeGreaterThan(0); + }); + + await test.step('clicking the chevron collapses the group', async () => { + await chevron.click(); + await expect(chevron).toHaveAttribute('aria-expanded', 'false'); + }); + + await test.step('clicking the chevron again re-expands it', async () => { + await chevron.click(); + await expect(chevron).toHaveAttribute('aria-expanded', 'true'); + }); + }); + + test('toggling bordered via the overflow menu flips the inline border', async () => { + const id = await addGroupAndGetId(); + const group = dashboardPage.getGroup(id); + + const readBorder = () => + group.evaluate((el: HTMLElement) => el.style.border); + + await test.step('starts with an inline border', async () => { + expect((await readBorder()).length).toBeGreaterThan(0); + }); + + await test.step('Hide Border removes the inline border', async () => { + await dashboardPage.toggleGroupBordered(id); + // The toggle is synchronous, but allow the inline style update to + // flush before reading. + await expect + .poll(async () => (await readBorder()).length, { timeout: 5000 }) + .toBe(0); + }); + + await test.step('the menu now offers Show Border and restores it', async () => { + await dashboardPage.openGroupMenu(id); + await expect( + dashboardPage.page.getByTestId(`group-toggle-bordered-${id}`), + ).toHaveText('Show Border'); + await dashboardPage.page + .getByTestId(`group-toggle-bordered-${id}`) + .click(); + await expect + .poll(async () => (await readBorder()).length, { timeout: 5000 }) + .toBeGreaterThan(0); + }); + }); + + test('adding a tab reveals the tab bar and switching tabs updates URL state', async () => { + const id = await addGroupAndGetId(); + const group = dashboardPage.getGroup(id); + const tabsInGroup = group.getByRole('tab'); + + await test.step('a one-tab group does not render the tab bar', async () => { + // DashboardContainer.tsx:82 sets `hasTabs = tabs.length >= 2`. The new + // group has 1 tab, so no Tabs.List renders. + await expect(tabsInGroup).toHaveCount(0); + }); + + await test.step('Add Tab brings the tab bar with two tabs', async () => { + await dashboardPage.addTabToGroup(id); + await expect(tabsInGroup).toHaveCount(2); + }); + + await test.step('switching tabs updates ?activeTabs and aria-selected', async () => { + // After Add Tab, handleAddTab + handleTabChange leave the new tab + // selected, so the URL param already records the second tab's id. + // We use the URL as the source of truth for ids rather than parsing + // them out of Mantine's internal DOM. + const secondTabId = dashboardPage.getActiveTabsParam()[id]; + expect(secondTabId).toBeTruthy(); + + await tabsInGroup.first().click(); + await expect + .poll(() => dashboardPage.getActiveTabsParam()[id]) + .not.toBe(secondTabId); + await expect(tabsInGroup.first()).toHaveAttribute( + 'aria-selected', + 'true', + ); + await expect(tabsInGroup.last()).toHaveAttribute( + 'aria-selected', + 'false', + ); + + await tabsInGroup.last().click(); + await expect + .poll(() => dashboardPage.getActiveTabsParam()[id]) + .toBe(secondTabId); + await expect(tabsInGroup.last()).toHaveAttribute('aria-selected', 'true'); + }); + }); + + test('?collapsed URL param survives reload and restores the collapsed state', async () => { + const id = await addGroupAndGetId(); + const chevron = dashboardPage.page.getByTestId(`group-chevron-${id}`); + + await test.step('collapsing a default-expanded group writes ?collapsed=', async () => { + await chevron.click(); + await expect.poll(() => dashboardPage.getCollapsedParam()).toContain(id); + expect(dashboardPage.getExpandedParam()).not.toContain(id); + }); + + await test.step('the collapsed state persists across reload', async () => { + await dashboardPage.page.reload(); + await expect( + dashboardPage.page.getByTestId('dashboard-page'), + ).toBeVisible(); + const reloadedChevron = dashboardPage.page.getByTestId( + `group-chevron-${id}`, + ); + await expect(reloadedChevron).toHaveAttribute('aria-expanded', 'false'); + expect(dashboardPage.getCollapsedParam()).toContain(id); + }); + + await test.step('expanding again moves the id to ?expanded= and survives reload', async () => { + await dashboardPage.page.getByTestId(`group-chevron-${id}`).click(); + await expect.poll(() => dashboardPage.getExpandedParam()).toContain(id); + expect(dashboardPage.getCollapsedParam()).not.toContain(id); + + await dashboardPage.page.reload(); + await expect( + dashboardPage.page.getByTestId('dashboard-page'), + ).toBeVisible(); + await expect( + dashboardPage.page.getByTestId(`group-chevron-${id}`), + ).toHaveAttribute('aria-expanded', 'true'); + }); + }); + + test('?activeTabs URL param survives reload and restores the active tab', async () => { + const id = await addGroupAndGetId(); + await dashboardPage.addTabToGroup(id); + + const tabs = dashboardPage.getGroup(id).getByRole('tab'); + await expect(tabs).toHaveCount(2); + + // The URL stores the active tab id; capture it as the source of truth. + const secondTabId = dashboardPage.getActiveTabsParam()[id]; + expect(secondTabId).toBeTruthy(); + + await test.step('switch to the first tab so the URL reflects it', async () => { + await tabs.first().click(); + await expect + .poll(() => dashboardPage.getActiveTabsParam()[id]) + .not.toBe(secondTabId); + }); + + await test.step('switch back to the second tab and reload', async () => { + await tabs.last().click(); + await expect + .poll(() => dashboardPage.getActiveTabsParam()[id]) + .toBe(secondTabId); + + await dashboardPage.page.reload(); + await expect( + dashboardPage.page.getByTestId('dashboard-page'), + ).toBeVisible(); + const reloadedTabs = dashboardPage.getGroup(id).getByRole('tab'); + await expect(reloadedTabs.last()).toHaveAttribute( + 'aria-selected', + 'true', + ); + expect(dashboardPage.getActiveTabsParam()[id]).toBe(secondTabId); + }); + }); + + test('save-and-reload round-trip preserves containers, bordered state, and tabs', async () => { + // Two groups: group A starts default; group B gets a second tab. We + // toggle bordered=false on group A so the round-trip exercises the + // schema field that PR #2015 added. + const idA = await addGroupAndGetId(); + const idB = await addGroupAndGetId(); + + await test.step('configure group A (no border) and group B (two tabs)', async () => { + await dashboardPage.toggleGroupBordered(idA); + await dashboardPage.addTabToGroup(idB); + + const groupATabs = dashboardPage.getGroup(idB).getByRole('tab'); + await expect(groupATabs).toHaveCount(2); + }); + + const dashboardUrl = dashboardPage.page.url(); + + await test.step('navigate away then back', async () => { + await dashboardPage.page.goto('/search'); + await expect(dashboardPage.page).toHaveURL(/.*\/search/); + + // Strip query params so the reload doesn't carry stale per-viewer state. + const dashboardId = dashboardPage.page + .url() + .match(/\/dashboards\/([^/?#]+)/)?.[1]; + const baseId = + dashboardId ?? new URL(dashboardUrl).pathname.split('/').pop(); + await dashboardPage.page.goto(`/dashboards/${baseId}`); + await expect( + dashboardPage.page.getByTestId('dashboard-page'), + ).toBeVisible(); + }); + + await test.step('both containers are present in original order', async () => { + const order = await dashboardPage.getGroupOrder(); + expect(order).toEqual([idA, idB]); + }); + + await test.step('group A retains its borderless state', async () => { + const inlineBorder = await dashboardPage + .getGroup(idA) + .evaluate((el: HTMLElement) => el.style.border); + expect(inlineBorder).toBe(''); + }); + + await test.step('group B still has two tabs', async () => { + const tabs = dashboardPage.getGroup(idB).getByRole('tab'); + await expect(tabs).toHaveCount(2); + }); + }); + + test('drag-to-reorder rearranges groups and the new order persists', async () => { + test.setTimeout(60000); + + const idA = await addGroupAndGetId(); + const idB = await addGroupAndGetId(); + const idC = await addGroupAndGetId(); + + await test.step('initial order is [A, B, C]', async () => { + expect(await dashboardPage.getGroupOrder()).toEqual([idA, idB, idC]); + }); + + await test.step('drag onto self is a no-op', async () => { + // DashboardDndContext.tsx:67-70 guards + // `activeData.containerId !== overData.containerId`. + await dashboardPage.dragGroupTo(idA, idA); + expect(await dashboardPage.getGroupOrder()).toEqual([idA, idB, idC]); + }); + + await test.step('dragging A onto C produces [B, C, A]', async () => { + // arrayMove(containers, indexOf(A)=0, indexOf(C)=2) yields the @dnd-kit + // documented "shift" semantics: [B, C, A]. See + // useDashboardContainers.handleReorderContainers and + // DashboardDndContext.handleDragEnd. + await dashboardPage.dragGroupTo(idA, idC); + await expect + .poll(() => dashboardPage.getGroupOrder()) + .toEqual([idB, idC, idA]); + }); + + await test.step('the new order persists across navigation', async () => { + const dashboardUrl = dashboardPage.page.url(); + const dashboardId = dashboardUrl.match(/\/dashboards\/([^/?#]+)/)?.[1]; + await dashboardPage.page.goto('/search'); + await expect(dashboardPage.page).toHaveURL(/.*\/search/); + await dashboardPage.page.goto(`/dashboards/${dashboardId}`); + await expect( + dashboardPage.page.getByTestId('dashboard-page'), + ).toBeVisible(); + + await expect + .poll(() => dashboardPage.getGroupOrder()) + .toEqual([idB, idC, idA]); + }); + }); +}); diff --git a/packages/app/tests/e2e/page-objects/DashboardPage.ts b/packages/app/tests/e2e/page-objects/DashboardPage.ts index 5de4dd591f..5eebcc8054 100644 --- a/packages/app/tests/e2e/page-objects/DashboardPage.ts +++ b/packages/app/tests/e2e/page-objects/DashboardPage.ts @@ -64,7 +64,7 @@ export class DashboardPage { private readonly createDashboardButton: Locator; private readonly addDropdownButton: Locator; private readonly addTileMenuItem: Locator; - private readonly addSectionMenuItem: Locator; + private readonly addGroupMenuItem: Locator; private readonly dashboardNameHeading: Locator; private readonly searchSubmitButton: Locator; private readonly liveButton: Locator; @@ -101,8 +101,8 @@ export class DashboardPage { this.addTileMenuItem = page.locator( '[data-testid="add-new-tile-menu-item"]', ); - this.addSectionMenuItem = page.locator( - '[data-testid="add-new-section-menu-item"]', + this.addGroupMenuItem = page.locator( + '[data-testid="add-new-group-menu-item"]', ); this.searchInput = page.locator('[data-testid="search-input"]'); this.searchSubmitButton = page.locator( @@ -219,11 +219,136 @@ export class DashboardPage { } /** - * Add a new section to the dashboard + * Add a new group (container) to the dashboard via the "+ Add" dropdown. */ - async addSection() { + async addGroup() { await this.addDropdownButton.click(); - await this.addSectionMenuItem.click(); + await this.addGroupMenuItem.click(); + } + + /** + * Get the group container element by its container id. + */ + getGroup(containerId: string): Locator { + return this.page.getByTestId(`group-container-${containerId}`); + } + + /** + * Get all rendered group containers in DOM order. + */ + getGroups(): Locator { + return this.page.locator('[data-testid^="group-container-"]'); + } + + /** + * Read the container ids of all groups in DOM order. Each id is parsed + * out of the `group-container-${id}` testid. + */ + async getGroupOrder(): Promise { + const groups = this.getGroups(); + const ids = await groups.evaluateAll(elements => + elements.map(el => { + const testId = el.getAttribute('data-testid') ?? ''; + return testId.replace(/^group-container-/, ''); + }), + ); + return ids; + } + + /** + * Hover the group then open its overflow ("...") menu so subsequent + * menu-item helpers can click into it. + */ + async openGroupMenu(containerId: string) { + const group = this.getGroup(containerId); + await group.hover(); + await this.page.getByTestId(`group-menu-${containerId}`).click(); + } + + /** + * Toggle the bordered visual style via the overflow menu. + */ + async toggleGroupBordered(containerId: string) { + await this.openGroupMenu(containerId); + await this.page.getByTestId(`group-toggle-bordered-${containerId}`).click(); + } + + /** + * Add a new tab to the group via the overflow menu. + */ + async addTabToGroup(containerId: string) { + await this.openGroupMenu(containerId); + await this.page.getByTestId(`group-add-tab-${containerId}`).click(); + } + + /** + * Read the `?activeTabs` query param as a `{ containerId: tabId }` map. + * Returns an empty object when the param is missing. + */ + getActiveTabsParam(): Record { + const url = new URL(this.page.url()); + const raw = url.searchParams.get('activeTabs'); + if (!raw) return {}; + try { + return JSON.parse(raw); + } catch { + return {}; + } + } + + /** + * Read the `?collapsed` query param as an array of container ids. nuqs + * serializes arrays as comma-separated values (`?collapsed=id1,id2`). + * Returns an empty array when the param is missing. + */ + getCollapsedParam(): string[] { + const raw = new URL(this.page.url()).searchParams.get('collapsed'); + return raw ? raw.split(',') : []; + } + + /** + * Read the `?expanded` query param as an array of container ids. Mirrors + * `getCollapsedParam` semantics. + */ + getExpandedParam(): string[] { + const raw = new URL(this.page.url()).searchParams.get('expanded'); + return raw ? raw.split(',') : []; + } + + /** + * Drag a group's drag handle onto another group, reordering them. + * + * @dnd-kit's MouseSensor uses `activationConstraint: { distance: 8 }` + * (see DashboardDndContext.tsx). The handle has to traverse 8+ pixels + * before the drag activates, so a single-shot drop never registers. + * The multi-step `mouse.move` mirrors the existing histogram-brush + * pattern in SearchPage.ts. + */ + async dragGroupTo(fromContainerId: string, toContainerId: string) { + const handle = this.page.getByTestId( + `group-drag-handle-${fromContainerId}`, + ); + const target = this.getGroup(toContainerId); + await handle.scrollIntoViewIfNeeded(); + await target.scrollIntoViewIfNeeded(); + + const handleBox = await handle.boundingBox(); + const targetBox = await target.boundingBox(); + if (!handleBox || !targetBox) { + throw new Error('Drag source or target not visible'); + } + + const startX = handleBox.x + handleBox.width / 2; + const startY = handleBox.y + handleBox.height / 2; + const endX = targetBox.x + targetBox.width / 2; + const endY = targetBox.y + targetBox.height / 2; + + await this.page.mouse.move(startX, startY); + await this.page.mouse.down(); + // Nudge to cross the 8px activation threshold, then traverse to target. + await this.page.mouse.move(startX + 12, startY, { steps: 5 }); + await this.page.mouse.move(endX, endY, { steps: 15 }); + await this.page.mouse.up(); } /** @@ -628,7 +753,7 @@ export class DashboardPage { /** * Get the `title` attribute of a cell (by column index) in the first row of - * a table tile. The title mirrors the cell's stringified value — useful + * a table tile. The title mirrors the cell's stringified value, useful * for extracting column values (e.g. a ServiceName) for later assertions. */ async getFirstTableRowValue(tileIndex = 0, columnIndex = 0): Promise { @@ -642,7 +767,7 @@ export class DashboardPage { /** * Click the first row's first cell of a table tile. Each cell contains a - * div[role="link"] that owns the onRowClick handler — click that directly + * div[role="link"] that owns the onRowClick handler. Click that directly * to trigger the configured action. */ async clickFirstTableRow(tileIndex = 0) { From 94a2aff4cf1cbb6433f1607cac5f1ad1760ef72d Mon Sep 17 00:00:00 2001 From: Alex Fedotyev <61838744+alex-fedotyev@users.noreply.github.com> Date: Wed, 6 May 2026 17:13:29 +0000 Subject: [PATCH 2/4] test(e2e): poll nuqs URL reads and harden dashboard round-trip The three failing dashboard-container tests in CI shard 1 came down to two distinct races: 1. Tests #3 (line 112) and #5 (line 194) read getActiveTabsParam()[id] synchronously after Add Tab and after tab switches. nuqs flushes URL state asynchronously, so the read fires before the param is written. Wrap each sync read in expect.poll, mirroring the fix in PR #2209. 2. Test #6 (line 231) paired toggleGroupBordered(idA) with addTabToGroup(idB) back-to-back. Both setDashboard calls produce() from the same pre-mutation snapshot of the React Query cache, so the second PATCH overwrites the first; the toggle is silently dropped. The save-and-reload assertion then sees the wrong state (or, when goto fires before the PATCH lands, no state at all). Narrow the round-trip to a single mutation per step (addTabToGroup on group B), wait for networkidle before navigating away, capture the dashboard id from the URL while we are still on the page, and use expect.poll for the post-reload getGroupOrder assertion. The bordered toggle stays covered by the dedicated in-page test at line 78. The underlying back-pressure race is tracked separately in #2216. --- .../e2e/features/dashboard-container.spec.ts | 83 +++++++++++-------- 1 file changed, 49 insertions(+), 34 deletions(-) diff --git a/packages/app/tests/e2e/features/dashboard-container.spec.ts b/packages/app/tests/e2e/features/dashboard-container.spec.ts index 0eb54c34b0..b01807bca0 100644 --- a/packages/app/tests/e2e/features/dashboard-container.spec.ts +++ b/packages/app/tests/e2e/features/dashboard-container.spec.ts @@ -127,11 +127,14 @@ test.describe('Dashboard container', { tag: ['@dashboard'] }, () => { await test.step('switching tabs updates ?activeTabs and aria-selected', async () => { // After Add Tab, handleAddTab + handleTabChange leave the new tab - // selected, so the URL param already records the second tab's id. - // We use the URL as the source of truth for ids rather than parsing - // them out of Mantine's internal DOM. - const secondTabId = dashboardPage.getActiveTabsParam()[id]; - expect(secondTabId).toBeTruthy(); + // selected, so the URL param eventually records the second tab's id. + // nuqs flushes URL state asynchronously, so poll rather than read + // once. We use the URL as the source of truth for ids rather than + // parsing them out of Mantine's internal DOM. + await expect + .poll(() => dashboardPage.getActiveTabsParam()[id]) + .toBeTruthy(); + const secondTabId = dashboardPage.getActiveTabsParam()[id]!; await tabsInGroup.first().click(); await expect @@ -199,8 +202,11 @@ test.describe('Dashboard container', { tag: ['@dashboard'] }, () => { await expect(tabs).toHaveCount(2); // The URL stores the active tab id; capture it as the source of truth. - const secondTabId = dashboardPage.getActiveTabsParam()[id]; - expect(secondTabId).toBeTruthy(); + // nuqs flushes URL state asynchronously, so poll rather than read once. + await expect + .poll(() => dashboardPage.getActiveTabsParam()[id]) + .toBeTruthy(); + const secondTabId = dashboardPage.getActiveTabsParam()[id]!; await test.step('switch to the first tab so the URL reflects it', async () => { await tabs.first().click(); @@ -224,53 +230,62 @@ test.describe('Dashboard container', { tag: ['@dashboard'] }, () => { 'aria-selected', 'true', ); - expect(dashboardPage.getActiveTabsParam()[id]).toBe(secondTabId); + await expect + .poll(() => dashboardPage.getActiveTabsParam()[id]) + .toBe(secondTabId); }); }); - test('save-and-reload round-trip preserves containers, bordered state, and tabs', async () => { - // Two groups: group A starts default; group B gets a second tab. We - // toggle bordered=false on group A so the round-trip exercises the - // schema field that PR #2015 added. + test('save-and-reload round-trip preserves containers and tabs', async () => { + // Two groups: group A is left default-bordered, group B gets a second + // tab. Bordered toggle is exercised in its own test (line 78) and is + // intentionally kept out of this round-trip. setDashboard for a + // remote dashboard fires PATCH /api/dashboards/{id} without an + // optimistic update, so back-to-back mutations that derive from the + // same in-memory snapshot can clobber each other; this test sequences + // its mutating actions deliberately and waits for the backend to + // confirm before navigating away. const idA = await addGroupAndGetId(); const idB = await addGroupAndGetId(); - await test.step('configure group A (no border) and group B (two tabs)', async () => { - await dashboardPage.toggleGroupBordered(idA); + await test.step('group B gets a second tab', async () => { await dashboardPage.addTabToGroup(idB); - - const groupATabs = dashboardPage.getGroup(idB).getByRole('tab'); - await expect(groupATabs).toHaveCount(2); + const groupBTabs = dashboardPage.getGroup(idB).getByRole('tab'); + await expect(groupBTabs).toHaveCount(2); }); - const dashboardUrl = dashboardPage.page.url(); + // Capture the dashboard id from the URL while we're still on the page. + const dashboardId = dashboardPage.page + .url() + .match(/\/dashboards\/([^/?#]+)/)?.[1]; + if (!dashboardId) { + throw new Error( + `Could not extract dashboard id from ${dashboardPage.page.url()}`, + ); + } + + await test.step('let pending mutations settle before leaving the page', async () => { + // setDashboard mutations are fire-and-forget; if we navigate before + // the PATCH lands, the change is dropped. Wait for the network to + // quiet before goto. + await dashboardPage.page.waitForLoadState('networkidle'); + }); await test.step('navigate away then back', async () => { await dashboardPage.page.goto('/search'); await expect(dashboardPage.page).toHaveURL(/.*\/search/); - // Strip query params so the reload doesn't carry stale per-viewer state. - const dashboardId = dashboardPage.page - .url() - .match(/\/dashboards\/([^/?#]+)/)?.[1]; - const baseId = - dashboardId ?? new URL(dashboardUrl).pathname.split('/').pop(); - await dashboardPage.page.goto(`/dashboards/${baseId}`); + await dashboardPage.page.goto(`/dashboards/${dashboardId}`); await expect( dashboardPage.page.getByTestId('dashboard-page'), ).toBeVisible(); }); await test.step('both containers are present in original order', async () => { - const order = await dashboardPage.getGroupOrder(); - expect(order).toEqual([idA, idB]); - }); - - await test.step('group A retains its borderless state', async () => { - const inlineBorder = await dashboardPage - .getGroup(idA) - .evaluate((el: HTMLElement) => el.style.border); - expect(inlineBorder).toBe(''); + // Poll because the reloaded dashboard hydrates via a remote fetch. + await expect + .poll(() => dashboardPage.getGroupOrder()) + .toEqual([idA, idB]); }); await test.step('group B still has two tabs', async () => { From 8fbb444c5d02af956576198d0889d11bccb97f98 Mon Sep 17 00:00:00 2001 From: Alex Fedotyev <61838744+alex-fedotyev@users.noreply.github.com> Date: Wed, 6 May 2026 17:28:36 +0000 Subject: [PATCH 3/4] test(e2e): double-decode activeTabs URL param in DashboardPage helper The dashboard activeTabs URL state goes through parseAsJsonEncoded (packages/app/src/utils/queryParsers.ts), which intentionally double-encodes the JSON to survive Microsoft Teams' '+' -> '%2B' re-encoding. The serializer writes encodeURIComponent(JSON.stringify(...)) and nuqs's URL machinery then encodes the '%XX' sequences a second time. URLSearchParams.get(...) decodes one level, so the helper has to decode the second level itself before JSON.parse. The previous helper read the param, called JSON.parse on a still- URL-encoded string ('%7B%22..."), threw, and returned {} silently. Tests #112 and #197 polled getActiveTabsParam()[id] for 5s, got undefined every time, and timed out on toBeTruthy. Fix: decodeURIComponent before JSON.parse. Keep a fallback to plain JSON.parse for compatibility with the old single-encoded format, mirroring the parser's own backward-compat path. The implementation (handleTabChange / setUrlActiveTabs in DBDashboardPage) was correct all along; only the test-side reader needed updating. --- .../tests/e2e/page-objects/DashboardPage.ts | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/packages/app/tests/e2e/page-objects/DashboardPage.ts b/packages/app/tests/e2e/page-objects/DashboardPage.ts index 5eebcc8054..0211f9ffc2 100644 --- a/packages/app/tests/e2e/page-objects/DashboardPage.ts +++ b/packages/app/tests/e2e/page-objects/DashboardPage.ts @@ -283,16 +283,30 @@ export class DashboardPage { /** * Read the `?activeTabs` query param as a `{ containerId: tabId }` map. - * Returns an empty object when the param is missing. + * Returns an empty object when the param is missing or malformed. + * + * The serializer is `parseAsJsonEncoded` (see + * `packages/app/src/utils/queryParsers.ts`), which double-encodes its + * value to survive the Microsoft-Teams `+` -> `%2B` re-encoding. nuqs + * writes `encodeURIComponent(JSON.stringify(value))` AND then nuqs's + * URL machinery encodes the resulting `%XX` sequences a second time. + * `searchParams.get(...)` decodes one level, so we have to decode the + * second level ourselves before `JSON.parse` to recover the object. + * The fallback to plain `JSON.parse` keeps us compatible with the + * old single-encoded format, mirroring the parser. */ getActiveTabsParam(): Record { const url = new URL(this.page.url()); const raw = url.searchParams.get('activeTabs'); if (!raw) return {}; try { - return JSON.parse(raw); + return JSON.parse(decodeURIComponent(raw)); } catch { - return {}; + try { + return JSON.parse(raw); + } catch { + return {}; + } } } From 6f66508930b1c8d43e69116bf317105dfd64eb63 Mon Sep 17 00:00:00 2001 From: Alex Fedotyev Date: Wed, 6 May 2026 22:06:43 +0000 Subject: [PATCH 4/4] test(e2e): harden dashboard-container spec against PATCH and dnd-kit races MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Compound-review feedback on #2205: - Replace `waitForLoadState('networkidle')` (which hangs on dashboards that keep polling tile queries) with a backend-PATCH waitForResponse, registered before the mutation so a fast handler doesn't return before the listener is attached. - Tag the round-trip and drag-persists tests `@full-stack` so the backend-bearing CI lane picks them up. - Replace inline `url().match(/\/dashboards\/.../)` with `dashboardPage.getCurrentDashboardId()` (existing helper) so a missing id throws instead of silently passing `undefined` to `goto()`. - `getActiveTabsParam` now validates `JSON.parse` output is a non-array object and only accepts string-typed values, instead of casting `any` to `Record`. - `openGroupMenu` waits for `pointer-events: none` to clear before clicking the overflow trigger; auto-wait checks visibility but not pointer-events. - `dragGroupTo` recomputes the target `boundingBox()` after the drag activates (otherwise @dnd-kit's 250ms sortable-item transform leaves the box stale) and shrinks the activation nudge to 10px so the cursor stays inside the handle. - Multi-param state reads (`?collapsed` + `?expanded`) are wrapped in a single `expect.poll` tuple so the slow side can't be missed by a one-shot read after the fast side resolved. - New page-object helpers (`getGroupChevron`, `getGroupBorderedToggle`, `getGroupTabs`, `getGroupBorderedAttr`, `waitForLoaded`, `waitForDashboardPatch`) keep testid prefixes out of specs. - `data-bordered="true|false"` attribute on the container shell so the bordered toggle test reads an attr instead of inspecting inline `style.border`. - `getGroupOrder` filters empty entries to surface DOM regressions. - `addGroupMenuItem` switched to `getByTestId(...)` for consistency with sibling locators. - Reverted unrelated em-dash → comma comment edits in `DashboardContainer.tsx` to keep the PR scope clean (em-dash sweep belongs in its own PR). --- .../app/src/components/DashboardContainer.tsx | 5 +- .../e2e/features/dashboard-container.spec.ts | 313 +++++++++--------- .../tests/e2e/page-objects/DashboardPage.ts | 138 +++++++- 3 files changed, 280 insertions(+), 176 deletions(-) diff --git a/packages/app/src/components/DashboardContainer.tsx b/packages/app/src/components/DashboardContainer.tsx index 4ad264426f..e93b970f5a 100644 --- a/packages/app/src/components/DashboardContainer.tsx +++ b/packages/app/src/components/DashboardContainer.tsx @@ -35,7 +35,7 @@ type DashboardContainerProps = { onToggleCollapsible: () => void; onToggleBordered: () => void; onDelete: (action: 'ungroup' | 'delete') => void; - /** Tile count inside this container, determines whether "Ungroup Tiles" is offered. */ + /** Tile count inside this container — determines whether "Ungroup Tiles" is offered. */ // [prose-lint: allow] tileCount: number; onAddTile: () => void; activeTabId: string | undefined; @@ -241,6 +241,7 @@ export default function DashboardContainer({ return ( setHovered(true)} onMouseLeave={() => setHovered(false)} mt={8} @@ -279,7 +280,7 @@ export default function DashboardContainer({ ) : ( - /* Plain header (1 tab or collapsed): shows title + chevron */ + /* Plain header (1 tab or collapsed) — shows title + chevron */ /* [prose-lint: allow] */ { + // Tests that PATCH the saved dashboard (round-trip / drag-persists) get + // an additional `@full-stack` tag so the backend-bearing CI lane picks + // them up. UI-only tests stay on `@dashboard` only. let dashboardPage: DashboardPage; test.beforeEach(async ({ page }) => { @@ -47,21 +50,18 @@ test.describe('Dashboard container', { tag: ['@dashboard'] }, () => { test('group renders with default collapsible chevron and bordered style', async () => { const id = await addGroupAndGetId(); - const group = dashboardPage.getGroup(id); - const chevron = dashboardPage.page.getByTestId(`group-chevron-${id}`); + const chevron = dashboardPage.getGroupChevron(id); await test.step('chevron is present and reports expanded by default', async () => { await expect(chevron).toBeVisible(); await expect(chevron).toHaveAttribute('aria-expanded', 'true'); }); - await test.step('container body renders an inline border', async () => { - // DashboardContainer.tsx:243-246 sets inline `border` only when - // bordered=true. Reading el.style.border avoids resolving the CSS var. - const inlineBorder = await group.evaluate( - (el: HTMLElement) => el.style.border, - ); - expect(inlineBorder.length).toBeGreaterThan(0); + await test.step('container reports bordered via data-bordered', async () => { + // The component sets `data-bordered="true|false"` on the container + // shell. Reading the attribute keeps the spec decoupled from the + // particular style strategy (inline `border`, CSS var, etc.). + expect(await dashboardPage.getGroupBorderedAttr(id)).toBe('true'); }); await test.step('clicking the chevron collapses the group', async () => { @@ -75,44 +75,34 @@ test.describe('Dashboard container', { tag: ['@dashboard'] }, () => { }); }); - test('toggling bordered via the overflow menu flips the inline border', async () => { + test('toggling bordered via the overflow menu flips data-bordered', async () => { const id = await addGroupAndGetId(); - const group = dashboardPage.getGroup(id); - - const readBorder = () => - group.evaluate((el: HTMLElement) => el.style.border); - await test.step('starts with an inline border', async () => { - expect((await readBorder()).length).toBeGreaterThan(0); + await test.step('starts as bordered', async () => { + expect(await dashboardPage.getGroupBorderedAttr(id)).toBe('true'); }); - await test.step('Hide Border removes the inline border', async () => { + await test.step('Hide Border flips data-bordered to false', async () => { await dashboardPage.toggleGroupBordered(id); - // The toggle is synchronous, but allow the inline style update to - // flush before reading. await expect - .poll(async () => (await readBorder()).length, { timeout: 5000 }) - .toBe(0); + .poll(() => dashboardPage.getGroupBorderedAttr(id), { timeout: 5000 }) + .toBe('false'); }); await test.step('the menu now offers Show Border and restores it', async () => { await dashboardPage.openGroupMenu(id); - await expect( - dashboardPage.page.getByTestId(`group-toggle-bordered-${id}`), - ).toHaveText('Show Border'); - await dashboardPage.page - .getByTestId(`group-toggle-bordered-${id}`) - .click(); + const borderedToggle = dashboardPage.getGroupBorderedToggle(id); + await expect(borderedToggle).toHaveText('Show Border'); + await borderedToggle.click(); await expect - .poll(async () => (await readBorder()).length, { timeout: 5000 }) - .toBeGreaterThan(0); + .poll(() => dashboardPage.getGroupBorderedAttr(id), { timeout: 5000 }) + .toBe('true'); }); }); test('adding a tab reveals the tab bar and switching tabs updates URL state', async () => { const id = await addGroupAndGetId(); - const group = dashboardPage.getGroup(id); - const tabsInGroup = group.getByRole('tab'); + const tabsInGroup = dashboardPage.getGroupTabs(id); await test.step('a one-tab group does not render the tab bar', async () => { // DashboardContainer.tsx:82 sets `hasTabs = tabs.length >= 2`. The new @@ -159,38 +149,46 @@ test.describe('Dashboard container', { tag: ['@dashboard'] }, () => { test('?collapsed URL param survives reload and restores the collapsed state', async () => { const id = await addGroupAndGetId(); - const chevron = dashboardPage.page.getByTestId(`group-chevron-${id}`); + const chevron = dashboardPage.getGroupChevron(id); + + // The two `useQueryState` setters (?collapsed and ?expanded) flush + // independently after a chevron click. Reading both as a tuple inside + // `expect.poll` avoids a one-shot read of the slow side after the + // fast side already resolved. + const readCollapseTuple = () => ({ + collapsed: dashboardPage.getCollapsedParam(), + expanded: dashboardPage.getExpandedParam(), + }); await test.step('collapsing a default-expanded group writes ?collapsed=', async () => { await chevron.click(); - await expect.poll(() => dashboardPage.getCollapsedParam()).toContain(id); - expect(dashboardPage.getExpandedParam()).not.toContain(id); + await expect.poll(readCollapseTuple).toMatchObject({ + collapsed: expect.arrayContaining([id]), + expanded: expect.not.arrayContaining([id]), + }); }); await test.step('the collapsed state persists across reload', async () => { await dashboardPage.page.reload(); - await expect( - dashboardPage.page.getByTestId('dashboard-page'), - ).toBeVisible(); - const reloadedChevron = dashboardPage.page.getByTestId( - `group-chevron-${id}`, - ); + await dashboardPage.waitForLoaded(); + const reloadedChevron = dashboardPage.getGroupChevron(id); await expect(reloadedChevron).toHaveAttribute('aria-expanded', 'false'); expect(dashboardPage.getCollapsedParam()).toContain(id); }); await test.step('expanding again moves the id to ?expanded= and survives reload', async () => { - await dashboardPage.page.getByTestId(`group-chevron-${id}`).click(); - await expect.poll(() => dashboardPage.getExpandedParam()).toContain(id); - expect(dashboardPage.getCollapsedParam()).not.toContain(id); + await dashboardPage.getGroupChevron(id).click(); + await expect.poll(readCollapseTuple).toMatchObject({ + collapsed: expect.not.arrayContaining([id]), + expanded: expect.arrayContaining([id]), + }); await dashboardPage.page.reload(); - await expect( - dashboardPage.page.getByTestId('dashboard-page'), - ).toBeVisible(); - await expect( - dashboardPage.page.getByTestId(`group-chevron-${id}`), - ).toHaveAttribute('aria-expanded', 'true'); + await dashboardPage.waitForLoaded(); + await expect(dashboardPage.getGroupChevron(id)).toHaveAttribute( + 'aria-expanded', + 'true', + ); }); }); @@ -198,7 +196,7 @@ test.describe('Dashboard container', { tag: ['@dashboard'] }, () => { const id = await addGroupAndGetId(); await dashboardPage.addTabToGroup(id); - const tabs = dashboardPage.getGroup(id).getByRole('tab'); + const tabs = dashboardPage.getGroupTabs(id); await expect(tabs).toHaveCount(2); // The URL stores the active tab id; capture it as the source of truth. @@ -222,10 +220,8 @@ test.describe('Dashboard container', { tag: ['@dashboard'] }, () => { .toBe(secondTabId); await dashboardPage.page.reload(); - await expect( - dashboardPage.page.getByTestId('dashboard-page'), - ).toBeVisible(); - const reloadedTabs = dashboardPage.getGroup(id).getByRole('tab'); + await dashboardPage.waitForLoaded(); + const reloadedTabs = dashboardPage.getGroupTabs(id); await expect(reloadedTabs.last()).toHaveAttribute( 'aria-selected', 'true', @@ -236,106 +232,111 @@ test.describe('Dashboard container', { tag: ['@dashboard'] }, () => { }); }); - test('save-and-reload round-trip preserves containers and tabs', async () => { - // Two groups: group A is left default-bordered, group B gets a second - // tab. Bordered toggle is exercised in its own test (line 78) and is - // intentionally kept out of this round-trip. setDashboard for a - // remote dashboard fires PATCH /api/dashboards/{id} without an - // optimistic update, so back-to-back mutations that derive from the - // same in-memory snapshot can clobber each other; this test sequences - // its mutating actions deliberately and waits for the backend to - // confirm before navigating away. - const idA = await addGroupAndGetId(); - const idB = await addGroupAndGetId(); - - await test.step('group B gets a second tab', async () => { - await dashboardPage.addTabToGroup(idB); - const groupBTabs = dashboardPage.getGroup(idB).getByRole('tab'); - await expect(groupBTabs).toHaveCount(2); - }); - - // Capture the dashboard id from the URL while we're still on the page. - const dashboardId = dashboardPage.page - .url() - .match(/\/dashboards\/([^/?#]+)/)?.[1]; - if (!dashboardId) { - throw new Error( - `Could not extract dashboard id from ${dashboardPage.page.url()}`, - ); - } - - await test.step('let pending mutations settle before leaving the page', async () => { - // setDashboard mutations are fire-and-forget; if we navigate before - // the PATCH lands, the change is dropped. Wait for the network to - // quiet before goto. - await dashboardPage.page.waitForLoadState('networkidle'); - }); - - await test.step('navigate away then back', async () => { - await dashboardPage.page.goto('/search'); - await expect(dashboardPage.page).toHaveURL(/.*\/search/); - - await dashboardPage.page.goto(`/dashboards/${dashboardId}`); - await expect( - dashboardPage.page.getByTestId('dashboard-page'), - ).toBeVisible(); - }); - - await test.step('both containers are present in original order', async () => { - // Poll because the reloaded dashboard hydrates via a remote fetch. - await expect - .poll(() => dashboardPage.getGroupOrder()) - .toEqual([idA, idB]); - }); - - await test.step('group B still has two tabs', async () => { - const tabs = dashboardPage.getGroup(idB).getByRole('tab'); - await expect(tabs).toHaveCount(2); - }); - }); - - test('drag-to-reorder rearranges groups and the new order persists', async () => { - test.setTimeout(60000); - - const idA = await addGroupAndGetId(); - const idB = await addGroupAndGetId(); - const idC = await addGroupAndGetId(); - - await test.step('initial order is [A, B, C]', async () => { - expect(await dashboardPage.getGroupOrder()).toEqual([idA, idB, idC]); - }); - - await test.step('drag onto self is a no-op', async () => { - // DashboardDndContext.tsx:67-70 guards - // `activeData.containerId !== overData.containerId`. - await dashboardPage.dragGroupTo(idA, idA); - expect(await dashboardPage.getGroupOrder()).toEqual([idA, idB, idC]); - }); - - await test.step('dragging A onto C produces [B, C, A]', async () => { - // arrayMove(containers, indexOf(A)=0, indexOf(C)=2) yields the @dnd-kit - // documented "shift" semantics: [B, C, A]. See - // useDashboardContainers.handleReorderContainers and - // DashboardDndContext.handleDragEnd. - await dashboardPage.dragGroupTo(idA, idC); - await expect - .poll(() => dashboardPage.getGroupOrder()) - .toEqual([idB, idC, idA]); - }); - - await test.step('the new order persists across navigation', async () => { - const dashboardUrl = dashboardPage.page.url(); - const dashboardId = dashboardUrl.match(/\/dashboards\/([^/?#]+)/)?.[1]; - await dashboardPage.page.goto('/search'); - await expect(dashboardPage.page).toHaveURL(/.*\/search/); - await dashboardPage.page.goto(`/dashboards/${dashboardId}`); - await expect( - dashboardPage.page.getByTestId('dashboard-page'), - ).toBeVisible(); - - await expect - .poll(() => dashboardPage.getGroupOrder()) - .toEqual([idB, idC, idA]); - }); - }); + test( + 'save-and-reload round-trip preserves containers and tabs', + { tag: '@full-stack' }, + async () => { + // Two groups: group A is left default-bordered, group B gets a second + // tab. Bordered toggle is exercised in its own test and is + // intentionally kept out of this round-trip. setDashboard for a + // remote dashboard fires PATCH /api/dashboards/{id} without an + // optimistic update, so back-to-back mutations that derive from the + // same in-memory snapshot can clobber each other; this test sequences + // its mutating actions deliberately and waits for the backend to + // confirm before navigating away. + const idA = await addGroupAndGetId(); + const idB = await addGroupAndGetId(); + + // Start listening for the addTabToGroup PATCH before issuing it, + // because a fast handler can return before the spec reaches a + // post-hoc `waitForResponse`. + const tabPatch = dashboardPage.waitForDashboardPatch(); + + await test.step('group B gets a second tab', async () => { + await dashboardPage.addTabToGroup(idB); + const groupBTabs = dashboardPage.getGroupTabs(idB); + await expect(groupBTabs).toHaveCount(2); + }); + + // Capture the dashboard id from the URL while we're still on the page. + const dashboardId = dashboardPage.getCurrentDashboardId(); + + await test.step('wait for the addTab PATCH to land before leaving', async () => { + // setDashboard mutations are fire-and-forget; if we navigate + // before the PATCH lands, the change is dropped. + await tabPatch; + }); + + await test.step('navigate away then back', async () => { + await dashboardPage.page.goto('/search'); + await expect(dashboardPage.page).toHaveURL(/.*\/search/); + + await dashboardPage.page.goto(`/dashboards/${dashboardId}`); + await dashboardPage.waitForLoaded(); + }); + + await test.step('both containers are present in original order', async () => { + // Poll because the reloaded dashboard hydrates via a remote fetch. + await expect + .poll(() => dashboardPage.getGroupOrder()) + .toEqual([idA, idB]); + }); + + await test.step('group B still has two tabs', async () => { + const tabs = dashboardPage.getGroupTabs(idB); + await expect(tabs).toHaveCount(2); + }); + }, + ); + + test( + 'drag-to-reorder rearranges groups and the new order persists', + { tag: '@full-stack' }, + async () => { + const idA = await addGroupAndGetId(); + const idB = await addGroupAndGetId(); + const idC = await addGroupAndGetId(); + + await test.step('initial order is [A, B, C]', async () => { + expect(await dashboardPage.getGroupOrder()).toEqual([idA, idB, idC]); + }); + + await test.step('drag onto self is a no-op', async () => { + // DashboardDndContext.tsx:67-70 guards + // `activeData.containerId !== overData.containerId`. + await dashboardPage.dragGroupTo(idA, idA); + expect(await dashboardPage.getGroupOrder()).toEqual([idA, idB, idC]); + }); + + // Start listening for the reorder PATCH before issuing the drag. + const reorderPatch = dashboardPage.waitForDashboardPatch(); + + await test.step('dragging A onto C produces [B, C, A]', async () => { + // arrayMove(containers, indexOf(A)=0, indexOf(C)=2) yields the @dnd-kit + // documented "shift" semantics: [B, C, A]. See + // useDashboardContainers.handleReorderContainers and + // DashboardDndContext.handleDragEnd. + await dashboardPage.dragGroupTo(idA, idC); + await expect + .poll(() => dashboardPage.getGroupOrder()) + .toEqual([idB, idC, idA]); + }); + + await test.step('the new order persists across navigation', async () => { + const dashboardId = dashboardPage.getCurrentDashboardId(); + // Wait for the reorder PATCH before leaving the page; otherwise the + // mutation is dropped on the goto. + await reorderPatch; + + await dashboardPage.page.goto('/search'); + await expect(dashboardPage.page).toHaveURL(/.*\/search/); + await dashboardPage.page.goto(`/dashboards/${dashboardId}`); + await dashboardPage.waitForLoaded(); + + await expect + .poll(() => dashboardPage.getGroupOrder()) + .toEqual([idB, idC, idA]); + }); + }, + ); }); diff --git a/packages/app/tests/e2e/page-objects/DashboardPage.ts b/packages/app/tests/e2e/page-objects/DashboardPage.ts index 0211f9ffc2..b8ce94d773 100644 --- a/packages/app/tests/e2e/page-objects/DashboardPage.ts +++ b/packages/app/tests/e2e/page-objects/DashboardPage.ts @@ -101,9 +101,7 @@ export class DashboardPage { this.addTileMenuItem = page.locator( '[data-testid="add-new-tile-menu-item"]', ); - this.addGroupMenuItem = page.locator( - '[data-testid="add-new-group-menu-item"]', - ); + this.addGroupMenuItem = page.getByTestId('add-new-group-menu-item'); this.searchInput = page.locator('[data-testid="search-input"]'); this.searchSubmitButton = page.locator( '[data-testid="search-submit-button"]', @@ -242,7 +240,9 @@ export class DashboardPage { /** * Read the container ids of all groups in DOM order. Each id is parsed - * out of the `group-container-${id}` testid. + * out of the `group-container-${id}` testid. Drops empty entries to + * surface DOM regressions (a group rendered without the testid prefix + * would otherwise show as `""` and silently pass equality checks). */ async getGroupOrder(): Promise { const groups = this.getGroups(); @@ -252,17 +252,75 @@ export class DashboardPage { return testId.replace(/^group-container-/, ''); }), ); - return ids; + return ids.filter(id => id.length > 0); + } + + /** + * Wait for the dashboard page shell to be visible. Used after reload / + * cross-page navigation to a single point of synchronisation. + */ + async waitForLoaded() { + await expect(this.page.getByTestId('dashboard-page')).toBeVisible(); + } + + /** Locator for a group's collapse/expand chevron. */ + getGroupChevron(containerId: string): Locator { + return this.page.getByTestId(`group-chevron-${containerId}`); + } + + /** Locator for the bordered toggle menu item inside a group's overflow menu. */ + getGroupBorderedToggle(containerId: string): Locator { + return this.page.getByTestId(`group-toggle-bordered-${containerId}`); + } + + /** Locator for the visible tabs inside a group's tab bar. */ + getGroupTabs(containerId: string): Locator { + return this.getGroup(containerId).getByRole('tab'); + } + + /** + * Read the inline border state of a group via the `data-bordered` + * attribute. Reading the attribute (rather than inspecting inline + * `style.border`) keeps the spec decoupled from how borders happen to + * be applied. + */ + getGroupBorderedAttr(containerId: string): Promise { + return this.getGroup(containerId).getAttribute('data-bordered'); + } + + /** + * Wait for a backend dashboard PATCH (the fire-and-forget mutation that + * `setDashboard` issues) to land before navigating away. Required between + * any state mutation that the round-trip test relies on and the next + * `goto`/reload, because `setDashboard` is fire-and-forget and a fast + * navigation drops the in-flight request. + */ + async waitForDashboardPatch() { + await this.page.waitForResponse( + r => + r.url().includes('/api/dashboards/') && + r.request().method() === 'PATCH' && + r.ok(), + { timeout: 15000 }, + ); } /** * Hover the group then open its overflow ("...") menu so subsequent * menu-item helpers can click into it. + * + * The menu trigger lives behind `pointer-events: none` until the React + * `hovered` state commits (DashboardContainer.tsx:106-109, + * `hoverControlStyle`). Playwright's auto-wait checks visibility but + * not `pointer-events`, so the click can land on a hidden-from-input + * element. Wait for `pointer-events` to clear before clicking. */ async openGroupMenu(containerId: string) { const group = this.getGroup(containerId); await group.hover(); - await this.page.getByTestId(`group-menu-${containerId}`).click(); + const trigger = this.page.getByTestId(`group-menu-${containerId}`); + await expect(trigger).not.toHaveCSS('pointer-events', 'none'); + await trigger.click(); } /** @@ -299,15 +357,37 @@ export class DashboardPage { const url = new URL(this.page.url()); const raw = url.searchParams.get('activeTabs'); if (!raw) return {}; - try { - return JSON.parse(decodeURIComponent(raw)); - } catch { + const tryParse = (value: string): unknown => { try { - return JSON.parse(raw); + return JSON.parse(value); } catch { - return {}; + return undefined; } + }; + let parsed: unknown; + try { + parsed = tryParse(decodeURIComponent(raw)); + } catch { + parsed = undefined; + } + if (parsed === undefined) parsed = tryParse(raw); + // `JSON.parse` returns `any`. Anything other than a non-array object + // (e.g. `"123"`, `null`, `[…]`) would lie to callers and silently + // fail downstream `expect.poll(() => …[id])` checks. + if ( + typeof parsed !== 'object' || + parsed === null || + Array.isArray(parsed) + ) { + return {}; } + // Build the result explicitly so the `string` value type is enforced + // at runtime (and we don't trust JSON.parse's `any` return). + const result: Record = {}; + for (const [key, value] of Object.entries(parsed)) { + if (typeof value === 'string') result[key] = value; + } + return result; } /** @@ -337,6 +417,17 @@ export class DashboardPage { * before the drag activates, so a single-shot drop never registers. * The multi-step `mouse.move` mirrors the existing histogram-brush * pattern in SearchPage.ts. + * + * Stability notes: + * - Pointerdown is preceded by a `mouse.move(startX, startY)` so the + * pointerdown coalesces deterministically. + * - The activation nudge is 10px (just past the 8px threshold) and + * stays inside the drag handle's bounds so we never leave the hit + * area before activation registers. + * - The target's `boundingBox()` is recomputed immediately before the + * final move, because @dnd-kit applies a 250ms sortable-item + * transform that shifts neighbouring containers while the drag is + * in flight. */ async dragGroupTo(fromContainerId: string, toContainerId: string) { const handle = this.page.getByTestId( @@ -347,20 +438,31 @@ export class DashboardPage { await target.scrollIntoViewIfNeeded(); const handleBox = await handle.boundingBox(); - const targetBox = await target.boundingBox(); - if (!handleBox || !targetBox) { - throw new Error('Drag source or target not visible'); + if (!handleBox) { + throw new Error('Drag source not visible'); } const startX = handleBox.x + handleBox.width / 2; const startY = handleBox.y + handleBox.height / 2; - const endX = targetBox.x + targetBox.width / 2; - const endY = targetBox.y + targetBox.height / 2; await this.page.mouse.move(startX, startY); + await this.page.mouse.move(startX, startY); // stabilise pointerdown coalescing await this.page.mouse.down(); - // Nudge to cross the 8px activation threshold, then traverse to target. - await this.page.mouse.move(startX + 12, startY, { steps: 5 }); + // Nudge to cross the 8px activation threshold while staying inside + // the handle's hit area. + await this.page.mouse.move(startX + 10, startY, { steps: 5 }); + + // Recompute the target box after activation; @dnd-kit shifts + // sibling containers during the drag and a stale box can land the + // pointer on the wrong neighbour. + const targetBox = await target.boundingBox(); + if (!targetBox) { + await this.page.mouse.up(); + throw new Error('Drag target not visible after activation'); + } + const endX = targetBox.x + targetBox.width / 2; + const endY = targetBox.y + targetBox.height / 2; + await this.page.mouse.move(endX, endY, { steps: 15 }); await this.page.mouse.up(); }