From 2b8ab6941512757f7bb9ad1ba69be816c4f6d65f Mon Sep 17 00:00:00 2001 From: VibeSWE <338779468+VibeSWE@users.noreply.github.com> Date: Wed, 11 Nov 2026 09:23:14 -0600 Subject: [PATCH 1/2] fix(frontend): allow administrators to edit shared dashboards --- .../e2e/dashboard-layout-sections.spec.ts | 10 +- frontend/e2e/dashboards.spec.ts | 182 ++++++++++++++++-- frontend/e2e/sql-chart-races.spec.ts | 39 +++- .../dashboards/DashboardEditing.stories.tsx | 8 +- .../features/dashboards/DashboardSections.tsx | 2 +- .../src/features/dashboards/DashboardView.tsx | 8 +- .../features/dashboards/DashboardsList.tsx | 2 +- frontend/src/features/dashboards/owner.ts | 9 +- frontend/src/features/sql/SqlChartPanel.tsx | 22 ++- .../story-tests/dashboard-editing.spec.ts | 2 +- 10 files changed, 244 insertions(+), 40 deletions(-) diff --git a/frontend/e2e/dashboard-layout-sections.spec.ts b/frontend/e2e/dashboard-layout-sections.spec.ts index 7c5f98593..05247dd1b 100644 --- a/frontend/e2e/dashboard-layout-sections.spec.ts +++ b/frontend/e2e/dashboard-layout-sections.spec.ts @@ -16,6 +16,7 @@ async function serve( page: Page, tiles: unknown[] = [tile('First'), tile('Second', 4)], author = classic.author, + privilege = 'admin', ) { const writes: Dashboard[] = []; const queries: any[] = []; @@ -41,7 +42,10 @@ async function serve( return route.fulfill({ json: { id: 'admin', username: 'admin' } }); if (path === '/api/v1/user/admin/role') return route.fulfill({ - json: { roles: { root: { actions: [{ privilege: 'admin' }], roleType: 'internal' } } }, + json: { + roles: { root: { actions: [{ privilege }], roleType: 'internal' } }, + groupRoles: {}, + }, }); if (path === '/api/v1/logstream') return route.fulfill({ json: [{ name: 'app-logs' }] }); if (path.endsWith('/schema')) @@ -183,9 +187,9 @@ test('section add, assignment, rename, reorder, collapse and deletion round trip }); test('read-only dashboards expose no mutation handles', async ({ page }) => { - const server = await serve(page, [tile('First')], 'someone-else'); + const server = await serve(page, [tile('First')], 'someone-else', 'reader'); await expect( - page.getByText('This dashboard is read-only. Only its owner can edit it.'), + page.getByText('This dashboard is read-only. Only its owner or an administrator can edit it.'), ).toBeVisible(); await expect(page.getByRole('button', { name: /^(Move|Resize) tile/ })).toHaveCount(0); await expect(page.getByRole('button', { name: 'Add section', exact: true })).toHaveCount(0); diff --git a/frontend/e2e/dashboards.spec.ts b/frontend/e2e/dashboards.spec.ts index 9f1dddefd..af55bbc0d 100644 --- a/frontend/e2e/dashboards.spec.ts +++ b/frontend/e2e/dashboards.spec.ts @@ -60,6 +60,7 @@ async function mockServer( rows?: DashboardSummary[]; failTitle?: string; privilege?: string; + groupAdmin?: boolean; queryRows?: Record[]; } = {}, ) { @@ -70,8 +71,8 @@ async function mockServer( calls: Call[] = []; const documents = new Map([[current.dashboardId, current]]); await page.context().addCookies([ - { name: 'user_id', value: 'admin', url: 'http://127.0.0.1:5173' }, - { name: 'username', value: 'admin', url: 'http://127.0.0.1:5173' }, + { name: 'user_id', value: 'admin', url: String(test.info().project.use.baseURL) }, + { name: 'username', value: 'admin', url: String(test.info().project.use.baseURL) }, ]); await page.route('**/api/**', async (route) => { const request = route.request(), @@ -105,7 +106,9 @@ async function mockServer( roles: { root: { actions: [{ privilege: options.privilege ?? 'admin' }], roleType: 'internal' }, }, - groupRoles: {}, + groupRoles: options.groupAdmin + ? { operators: { admin: { actions: [{ privilege: 'admin' }], roleType: 'internal' } } } + : {}, }, }); if (path === '/api/v1/logstream') @@ -561,7 +564,7 @@ test('non-owners see read-only tiles, including unknown and unavailable chart pl await page.goto(`${detail}/`); await expect( page.getByText( - 'This dashboard is read-only. Only its owner can edit it. Duplicate it to make an editable copy.', + 'This dashboard is read-only. Only its owner or an administrator can edit it. Duplicate it to make an editable copy.', { exact: true }, ), ).toBeVisible(); @@ -1631,7 +1634,7 @@ test('duplicate and nil tile ids are repaired on load, so editing and deleting a await expect(page.getByText(/The next save will assign new IDs/)).toHaveCount(0); }); -for (const privilege of ['reader', 'admin']) +for (const privilege of ['reader']) test(`${privilege} non-owners with repaired tile IDs can leave and duplicate without unsaved changes`, async ({ page, }) => { @@ -1872,11 +1875,16 @@ test('import rejects files over 5 MB before reading them', async ({ page }) => { expect(server.creates).toEqual([]); }); -for (const privilege of ['reader', 'admin', 'ingestor']) +for (const privilege of ['reader', 'admin', 'superadmin', 'group-admin', 'ingestor']) test(`${privilege} permissions govern all non-owner controls and disabled favourites`, async ({ page, }) => { - const server = await mockServer(page, { privilege, document: { ...classic, author: 'other' } }); + const administrative = ['admin', 'superadmin', 'group-admin'].includes(privilege); + const server = await mockServer(page, { + privilege: privilege === 'group-admin' ? 'reader' : privilege, + groupAdmin: privilege === 'group-admin', + document: { ...classic, author: 'other' }, + }); await page.goto('/dashboards'); await page.waitForLoadState('networkidle'); const favourite = page.getByRole('button', { name: 'Unfavourite Service health', exact: true }); @@ -1888,10 +1896,10 @@ for (const privilege of ['reader', 'admin', 'ingestor']) expect(server.writes).toEqual([]); await page.getByRole('button', { name: 'Actions for Service health', exact: true }).click(); await expect(page.getByRole('menuitem', { name: 'Rename and tags', exact: true })).toHaveCount( - 0, + administrative ? 1 : 0, ); await expect(page.getByRole('menuitem', { name: 'Delete', exact: true })).toHaveCount( - privilege === 'admin' ? 1 : 0, + administrative ? 1 : 0, ); if (privilege === 'ingestor') { const duplicate = page.getByRole('menuitem', { name: 'Duplicate', exact: true }); @@ -1901,19 +1909,165 @@ for (const privilege of ['reader', 'admin', 'ingestor']) } await page.keyboard.press('Escape'); await page.getByRole('link', { name: classic.title, exact: true }).click(); - await expect(page.getByRole('button', { name: 'Save', exact: true })).toHaveCount(0); - await expect(page.getByRole('button', { name: 'Add variable', exact: true })).toHaveCount(0); + await expect(page.getByRole('button', { name: 'Save', exact: true })).toHaveCount( + administrative ? 1 : 0, + ); + await expect(page.getByRole('button', { name: 'Add variable', exact: true })).toHaveCount( + administrative ? 1 : 0, + ); await page.getByRole('button', { name: 'Dashboard actions', exact: true }).click(); await expect(page.getByRole('menuitem', { name: 'Rename and tags', exact: true })).toHaveCount( - 0, + administrative ? 1 : 0, ); await expect(page.getByRole('menuitem', { name: 'Delete dashboard', exact: true })).toHaveCount( - privilege === 'admin' ? 1 : 0, + administrative ? 1 : 0, ); await page.keyboard.press('Escape'); await page.getByRole('button', { name: 'Actions for tile Host load', exact: true }).click(); for (const action of ['Edit', 'Delete tile', 'Duplicate tile', 'Move earlier', 'Move later']) - await expect(page.getByRole('menuitem', { name: action, exact: true })).toHaveCount(0); + await expect(page.getByRole('menuitem', { name: action, exact: true })).toHaveCount( + administrative ? 1 : 0, + ); + }); + +test('non-owner admin changes Percent fill, saves and reloads without changing ownership', async ({ + page, +}) => { + const source: Dashboard = { + ...classic, + author: 'original-owner', + variables: [], + sections: [], + tiles: [ + { + ...classic.tiles[1], + title: 'CPU', + tileType: 'code', + chartType: 'query-value', + config: { type: 'query-value', layout: { unit: '%', precision: 0 } }, + }, + ], + }; + const server = await mockServer(page, { document: source }); + await page.goto(detail); + await tileAction(page, 'CPU', 'Edit'); + const editor = page.getByRole('dialog', { name: 'Edit tile', exact: true }); + await editor.getByLabel('Percent fill', { exact: true }).selectOption('vertical'); + await editor.getByRole('button', { name: 'Apply tile', exact: true }).click(); + await page.getByRole('button', { name: 'Save', exact: true }).click(); + await expect(page.getByText('Dashboard saved.', { exact: true })).toBeVisible(); + expect(server.writes).toHaveLength(1); + expect(server.writes[0]).toMatchObject({ + dashboardId: source.dashboardId, + author: source.author, + tenantId: source.tenantId, + tiles: [{ config: { layout: { percentFill: 'vertical' } } }], + }); + await page.reload(); + await expect(tile(page, 'CPU').locator('.dashboard-stat-fill')).toHaveAttribute( + 'data-direction', + 'vertical', + ); + await tileAction(page, 'CPU', 'Edit'); + await expect(page.getByRole('dialog').getByLabel('Percent fill', { exact: true })).toHaveValue( + 'vertical', + ); + expect(server.current().author).toBe(source.author); +}); + +test('non-owner admin can rename from the list without acquiring ownership', async ({ page }) => { + const server = await mockServer(page, { document: { ...classic, author: 'other' } }); + await page.goto('/dashboards'); + await page.getByRole('button', { name: 'Actions for Service health', exact: true }).click(); + await page.getByRole('menuitem', { name: 'Rename and tags', exact: true }).click(); + const editor = page.getByRole('dialog', { name: 'Rename and tags', exact: true }); + await editor.getByLabel('Dashboard title').fill('Admin rename'); + await editor.locator('[data-dialog-confirm]').click(); + await expect(page.getByRole('link', { name: 'Admin rename', exact: true })).toBeVisible(); + expect(server.writes[0]).toMatchObject({ author: 'other', title: 'Admin rename' }); + await expect( + page.getByRole('button', { name: 'Unfavourite Admin rename', exact: true }), + ).toHaveAttribute('aria-disabled', 'true'); +}); + +test('non-owner admin can save repaired tile IDs while preserving ownership', async ({ page }) => { + const first = { ...classic.tiles[1], tileType: 'code', chartType: 'query-value' }; + const source: Dashboard = { + ...classic, + author: 'other', + variables: [], + sections: [], + tiles: [ + first, + { ...first, title: 'Repeated' }, + { ...first, title: 'Nil', tile_id: '00000000000000000000000000' }, + ], + }; + const server = await mockServer(page, { document: source }); + await page.goto(detail); + await expect(page.getByText(/The next save will assign new IDs/)).toBeVisible(); + await page.getByRole('button', { name: 'Save', exact: true }).click(); + await expect(page.getByText('Dashboard saved.', { exact: true })).toBeVisible(); + const saved = server.writes[0]; + expect(saved.author).toBe('other'); + const ids = saved.tiles!.map((item) => item.tile_id); + expect(new Set(ids).size).toBe(3); + expect(ids).not.toContain('00000000000000000000000000'); + await page.reload(); + await expect(page.getByText(/The next save will assign new IDs/)).toHaveCount(0); + await expect(page.getByRole('button', { name: 'Save', exact: true })).toBeDisabled(); +}); + +test('non-owner admin waits for roles without a read-only flash or premature edit controls', async ({ + page, +}) => { + await mockServer(page, { document: { ...classic, author: 'other' } }); + await page.addInitScript(() => + new MutationObserver(() => { + if (document.body?.textContent?.includes('This dashboard is read-only.')) + document.documentElement.dataset.readOnlyFlash = 'true'; + }).observe(document, { childList: true, subtree: true }), + ); + let release!: () => void; + const gate = new Promise((resolve) => { + release = resolve; + }); + await page.route('**/api/v1/user/admin/role', async (route) => { + await gate; + await route.fallback(); + }); + await page.goto(detail); + await expect(page.getByRole('heading', { name: classic.title, exact: true })).toBeVisible(); + await expect(page.getByRole('button', { name: 'Save', exact: true })).toHaveCount(0); + await expect(page.getByText(/This dashboard is read-only/)).toHaveCount(0); + release(); + await expect(page.getByRole('button', { name: 'Add tile', exact: true })).toBeVisible(); + await expect(page.getByRole('button', { name: 'Save', exact: true })).toBeVisible(); + expect(await page.locator('html').getAttribute('data-read-only-flash')).toBeNull(); +}); + +for (const owned of [false, true]) + test(`role lookup failure ${owned ? 'preserves owner editing' : 'keeps non-owners read-only'}`, async ({ + page, + }) => { + const server = await mockServer(page, { + document: { ...classic, author: owned ? classic.author : 'other' }, + }); + await page.route('**/api/v1/user/admin/role', (route) => + route.fulfill({ status: 503, body: 'Roles unavailable' }), + ); + await page.goto(detail); + if (owned) { + await expect(page.getByRole('button', { name: 'Save', exact: true })).toBeVisible(); + await tileAction(page, 'Errors', 'Edit'); + await expect(page.getByRole('dialog', { name: 'Edit tile', exact: true })).toBeVisible(); + } else { + await expect(page.getByText(/This dashboard is read-only/)).toBeVisible(); + await expect(page.getByRole('button', { name: 'Save', exact: true })).toHaveCount(0); + await page.getByRole('button', { name: 'Actions for tile Errors', exact: true }).click(); + await expect(page.getByRole('menuitem', { name: 'Edit', exact: true })).toHaveCount(0); + } + expect(server.writes).toEqual([]); }); test('favourite toggles the latest row, and list conflicts can be cancelled for further editing', async ({ diff --git a/frontend/e2e/sql-chart-races.spec.ts b/frontend/e2e/sql-chart-races.spec.ts index 07caf28d0..736f5216b 100644 --- a/frontend/e2e/sql-chart-races.spec.ts +++ b/frontend/e2e/sql-chart-races.spec.ts @@ -9,7 +9,7 @@ const rows = [ { service: 'worker', requests: 7 }, ]; -async function serve(page: Page, holdQueries = false) { +async function serve(page: Page, holdQueries = false, privilege = 'admin') { const queries: { sql: string; startTime: string; endTime: string; route: Route }[] = []; const writes: { method: string; body: DashboardRequest }[] = []; let current: Dashboard = structuredClone(classic); @@ -35,7 +35,10 @@ async function serve(page: Page, holdQueries = false) { return route.fulfill({ json: { id: 'admin', username: 'admin' } }); if (path === '/api/v1/user/admin/role') return route.fulfill({ - json: { roles: { root: { actions: [{ privilege: 'admin' }], roleType: 'internal' } } }, + json: { + roles: { root: { actions: [{ privilege }], roleType: 'internal' } }, + groupRoles: {}, + }, }); if (path === '/api/v1/logstream') return route.fulfill({ json: [{ name: 'app-logs' }, { name: 'other-logs' }] }); @@ -71,8 +74,14 @@ async function serve(page: Page, holdQueries = false) { } return route.fulfill({ json: current }); } - if (path === `/api/v1/dashboards/${foreign.dashboardId}`) + if (path === `/api/v1/dashboards/${foreign.dashboardId}`) { + if (request.method() === 'PUT') { + const body: Dashboard = request.postDataJSON(); + writes.push({ method: 'PUT', body: structuredClone(body) }); + Object.assign(foreign, body); + } return route.fulfill({ json: foreign }); + } return route.fulfill({ json: [] }); }); await page.goto('/sql-editor'); @@ -326,10 +335,27 @@ for (const target of ['new', 'existing'] as const) { }); } +test('non-owner admin adds a SQL chart without changing the dashboard owner', async ({ page }) => { + const server = await serve(page); + const original = structuredClone(server.foreign); + await chart(page); + const dialog = await destination(page, original.dashboardId); + await dialog.getByRole('button', { name: 'Add chart', exact: true }).click(); + await expect(dialog).toHaveCount(0); + await expect(page.getByRole('link', { name: original.title, exact: true })).toBeVisible(); + expect(server.writes).toHaveLength(1); + const write = server.writes[0]; + const added = write.body.tiles!.at(-1)!; + expect(write.method).toBe('PUT'); + expect(write.body).toEqual({ ...original, tiles: [...original.tiles!, added] }); + expect(added).toMatchObject({ title: 'Requests by service', chartQuery: executed }); + expect(server.foreign.author).toBe(original.author); +}); + test('dashboard ownership is checked on the loaded document as well as the destination list', async ({ page, }) => { - const server = await serve(page); + const server = await serve(page, false, 'reader'); await chart(page); const dialog = await destination(page, classic.dashboardId); await expect( @@ -342,7 +368,10 @@ test('dashboard ownership is checked on the loaded document as well as the desti server.replace({ ...server.current(), author: 'new-owner' }); await page.getByRole('button', { name: 'Add to Dashboard', exact: true }).click(); await expect( - dialog.getByText('This dashboard is read-only. Only its owner can edit it.', { exact: true }), + dialog.getByText( + 'This dashboard is read-only. Only its owner or an administrator can edit it.', + { exact: true }, + ), ).toBeVisible(); await expect( dialog diff --git a/frontend/src/features/dashboards/DashboardEditing.stories.tsx b/frontend/src/features/dashboards/DashboardEditing.stories.tsx index 9d89c852d..4470dc2d9 100644 --- a/frontend/src/features/dashboards/DashboardEditing.stories.tsx +++ b/frontend/src/features/dashboards/DashboardEditing.stories.tsx @@ -38,6 +38,12 @@ function Playground({ readOnly = false, stacked = false }: Args) { }; return { ...createDemoClient(), + async userRoleSources() { + return { + roles: { viewer: { actions: [{ privilege: 'reader' }], roleType: 'internal' } }, + groupRoles: {}, + }; + }, async getDashboard() { return structuredClone(current); }, @@ -62,7 +68,7 @@ function Playground({ readOnly = false, stacked = false }: Args) { ) : ( diff --git a/frontend/src/features/dashboards/DashboardView.tsx b/frontend/src/features/dashboards/DashboardView.tsx index 2f221f4e9..5d3974b22 100644 --- a/frontend/src/features/dashboards/DashboardView.tsx +++ b/frontend/src/features/dashboards/DashboardView.tsx @@ -9,7 +9,7 @@ import { useAsync } from '../../hooks/useAsync'; import { useRowDeletionFocus } from '../../hooks/useRowDeletionFocus'; import { createLimiter } from '../../lib/concurrency'; import type { Dashboard, DashboardTile as Tile } from '../../lib/types'; -import { useDashboardAccess } from './owner'; +import { canEditDashboard, useDashboardAccess } from './owner'; import { dashboardError, dashboardPresets, duplicateDashboard } from './helpers'; import { moveTile, text } from './tiles'; import { duplicateTile } from './draft'; @@ -65,7 +65,7 @@ function DashboardFields({ original }: { original: Dashboard }) { const [resolutions, setResolutions] = useState({}); const about = useAsync(useCallback((signal) => client.about(signal), [client])); const promqlEnabled = about.data ? about.data.capabilities.promqlDashboard === true : undefined; - const writable = !!access.hash && access.hash === draft.author; + const writable = canEditDashboard(access, draft.author); const editing = writable && !mutation.pending && !presentation.presenting; const limit = useMemo(() => createLimiter(4), [client]); const tileFocus = useRowDeletionFocus( @@ -186,7 +186,7 @@ function DashboardFields({ original }: { original: Dashboard }) { } }), }, - ...(writable || access.isAdmin + ...(writable ? [ { id: 'delete', @@ -243,7 +243,7 @@ function DashboardFields({ original }: { original: Dashboard }) { {!access.loading && !writable && (

- This dashboard is read-only. Only its owner can edit it. + This dashboard is read-only. Only its owner or an administrator can edit it. {access.canCreate ? ' Duplicate it to make an editable copy.' : ''}

)} diff --git a/frontend/src/features/dashboards/DashboardsList.tsx b/frontend/src/features/dashboards/DashboardsList.tsx index dd812ddf9..ecfb627eb 100644 --- a/frontend/src/features/dashboards/DashboardsList.tsx +++ b/frontend/src/features/dashboards/DashboardsList.tsx @@ -146,7 +146,7 @@ export function DashboardsList({ data-row-action={row.dashboardId} disabled={refreshing} items={[ - ...(owned + ...(owned || isAdmin ? [ { id: 'rename', diff --git a/frontend/src/features/dashboards/owner.ts b/frontend/src/features/dashboards/owner.ts index 4a5cd71ea..2d986c5d5 100644 --- a/frontend/src/features/dashboards/owner.ts +++ b/frontend/src/features/dashboards/owner.ts @@ -3,6 +3,13 @@ import { useApp } from '../../app/AppProvider'; import { useAsync } from '../../hooks/useAsync'; import { sha256 } from '../../lib/sha256'; +export function canEditDashboard( + access: { hash?: string; isAdmin: boolean }, + author: string | null | undefined, +): boolean { + return access.isAdmin || (!!access.hash && access.hash === author); +} + export function useDashboardAccess() { const { client, identity, mode } = useApp(); const hash = useAsync( @@ -23,7 +30,7 @@ export function useDashboardAccess() { ].flatMap((role) => role.actions); return { hash: hash.data, - loading: hash.loading, + loading: hash.loading || roles.loading, canCreate: privileges ? privileges.some((item) => ['superadmin', 'admin', 'editor', 'writer', 'reader'].includes(item.privilege), diff --git a/frontend/src/features/sql/SqlChartPanel.tsx b/frontend/src/features/sql/SqlChartPanel.tsx index 1578697c0..c908fc9c2 100644 --- a/frontend/src/features/sql/SqlChartPanel.tsx +++ b/frontend/src/features/sql/SqlChartPanel.tsx @@ -13,7 +13,7 @@ import { TileChart } from '../dashboards/TileChart'; import { TileBindingFields } from '../dashboards/TileBindingFields'; import { TileAppearanceFields } from '../dashboards/TileAppearanceFields'; import { chartIssues } from '../dashboards/chartPicker'; -import { useDashboardAccess } from '../dashboards/owner'; +import { canEditDashboard, useDashboardAccess } from '../dashboards/owner'; import { classicTimeRange } from '../dashboards/timeRange'; import { text, tileDatasets } from '../dashboards/tiles'; import { appendSqlTile, executedSqlTile, type SqlChartConfig } from './sqlChart'; @@ -94,7 +94,7 @@ export function SqlChartPanel({ : invalid.size ? 'Correct the chart options.' : (chartIssues(config)[0] ?? ''); - const writable = selected.data && !!access.hash && selected.data.author === access.hash; + const writable = selected.data && canEditDashboard(access, selected.data.author); const destinationIssue = destination === 'new' ? !access.canCreate @@ -104,9 +104,11 @@ export function SqlChartPanel({ : '' : selected.loading ? 'Loading dashboard…' - : !writable - ? 'This dashboard is read-only. Only its owner can edit it.' - : ''; + : !writable && access.loading + ? 'Loading dashboard permissions…' + : !writable + ? 'This dashboard is read-only. Only its owner or an administrator can edit it.' + : ''; return (
{rows.length > 0 && ( @@ -188,9 +190,11 @@ export function SqlChartPanel({ saveRead.current = controller; const latest = await client.getDashboard(loaded.dashboardId, controller.signal); if (!mutation.isActive()) return; - if (!access.hash || latest.author !== access.hash) { + if (!canEditDashboard(access, latest.author)) { selected.reload(); - throw new Error('This dashboard is read-only. Only its owner can edit it.'); + throw new Error( + 'This dashboard is read-only. Only its owner or an administrator can edit it.', + ); } if (JSON.stringify(latest) !== JSON.stringify(loaded)) { selected.reload(); @@ -239,10 +243,10 @@ export function SqlChartPanel({ ))} diff --git a/frontend/story-tests/dashboard-editing.spec.ts b/frontend/story-tests/dashboard-editing.spec.ts index 711897dde..cc9a7d351 100644 --- a/frontend/story-tests/dashboard-editing.spec.ts +++ b/frontend/story-tests/dashboard-editing.spec.ts @@ -131,7 +131,7 @@ test('read-only production story permits transient collapse and hides editing co }) => { await open(page, 'read-only'); await expect( - page.getByText('This dashboard is read-only. Only its owner can edit it.'), + page.getByText('This dashboard is read-only. Only its owner or an administrator can edit it.'), ).toBeVisible(); await expect(page.getByRole('button', { name: /^(Move|Resize) tile/ })).toHaveCount(0); await expect(page.getByRole('button', { name: 'Add section', exact: true })).toHaveCount(0); From 92eea647eedcbce4c9c4f33df413cdcc6d944722 Mon Sep 17 00:00:00 2001 From: VibeSWE <338779468+VibeSWE@users.noreply.github.com> Date: Wed, 11 Nov 2026 11:05:54 -0600 Subject: [PATCH 2/2] fix(frontend): allow demo admin edits without changing dashboard owners --- frontend/e2e/dashboards.spec.ts | 29 ++++++++++++++++ frontend/src/lib/demoDashboards.test.ts | 45 ++++++++++++++++++++++--- frontend/src/lib/demoDashboards.ts | 6 ++-- 3 files changed, 71 insertions(+), 9 deletions(-) diff --git a/frontend/e2e/dashboards.spec.ts b/frontend/e2e/dashboards.spec.ts index af55bbc0d..1197a92bc 100644 --- a/frontend/e2e/dashboards.spec.ts +++ b/frontend/e2e/dashboards.spec.ts @@ -268,6 +268,35 @@ async function mockServer( }; } +test('demo admin edits and renames a shared dashboard while favourites stay owner-only', async ({ + page, +}) => { + await demo(page, '/dashboards/01M4J000000000000000000003'); + await tileAction(page, 'API events', 'Edit'); + const editor = page.getByRole('dialog', { name: 'Edit tile', exact: true }); + await editor.getByLabel('Tile title').fill('Updated API events'); + await editor.getByRole('button', { name: 'Apply tile', exact: true }).click(); + await page.getByRole('button', { name: 'Save', exact: true }).click(); + await expect(page.getByText('Dashboard saved.', { exact: true })).toBeVisible(); + await page + .getByRole('navigation', { name: 'Dashboard breadcrumb' }) + .getByRole('link', { name: 'Dashboards', exact: true }) + .click(); + await page.getByRole('button', { name: 'Actions for Shared operations', exact: true }).click(); + await page.getByRole('menuitem', { name: 'Rename and tags', exact: true }).click(); + const rename = page.getByRole('dialog', { name: 'Rename and tags', exact: true }); + await rename.getByLabel('Dashboard title').fill('Renamed shared operations'); + await rename.locator('[data-dialog-confirm]').click(); + await expect( + page.getByRole('button', { name: 'Favourite Renamed shared operations', exact: true }), + ).toHaveAttribute('aria-disabled', 'true'); + await page.getByRole('link', { name: 'Renamed shared operations', exact: true }).click(); + await expect(tile(page, 'Updated API events')).toBeVisible(); + await expect( + page.getByRole('heading', { name: 'Renamed shared operations', exact: true }), + ).toBeVisible(); +}); + test('demo CRUD, SQL tile preview/config, duplicate/move and typed deletion', async ({ page }) => { await demo(page); await page.getByRole('button', { name: 'Create dashboard', exact: true }).click(); diff --git a/frontend/src/lib/demoDashboards.test.ts b/frontend/src/lib/demoDashboards.test.ts index ddd65f4b6..7999ec698 100644 --- a/frontend/src/lib/demoDashboards.test.ts +++ b/frontend/src/lib/demoDashboards.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest'; import { createDemoDashboards } from './demoDashboards'; import { dashboard } from './dashboardsContract'; import classic from '../features/dashboards/__fixtures__/classic.json'; +import { appendSqlTile, executedSqlTile } from '../features/sql/sqlChart'; describe('per-client dashboard demo', () => { it('provides SQL, PromQL and another owner fixtures', async () => { const client = createDemoDashboards(), @@ -44,7 +45,44 @@ describe('per-client dashboard demo', () => { await client.deleteDashboard(saved.dashboardId); expect(await createDemoDashboards().listDashboards()).toHaveLength(3); }); - it('uses server validation messages, exact tenant-wide titles and ownership', async () => { + it('allows admin edits, renames and SQL appends while preserving the shared owner', async () => { + const client = createDemoDashboards(); + const row = (await client.listDashboards()).find((item) => item.title === 'Shared operations')!; + const original = await client.getDashboard(row.dashboardId); + const edited = structuredClone(original); + edited.tiles![0].title = 'Updated API events'; + const saved = await client.updateDashboard(row.dashboardId, edited); + expect(saved.author).toBe(original.author); + expect((await client.getDashboard(row.dashboardId)).tiles![0].title).toBe('Updated API events'); + const renamed = await client.updateDashboard(row.dashboardId, { + ...saved, + title: 'Renamed shared operations', + author: 'attempted-owner-change', + }); + expect(renamed.author).toBe(original.author); + const added = executedSqlTile(classic.tiles[0], 'SELECT COUNT(*) AS events FROM "api_logs"'); + const appended = await client.updateDashboard(row.dashboardId, appendSqlTile(renamed, added)); + expect(await client.getDashboard(row.dashboardId)).toEqual(appended); + expect(appended).toMatchObject({ + author: original.author, + created: original.created, + title: renamed.title, + tiles: [ + saved.tiles![0], + expect.objectContaining({ tile_id: added.tile_id, chartQuery: added.chartQuery }), + ], + }); + expect( + (await client.listDashboards()).find((item) => item.dashboardId === row.dashboardId), + ).toMatchObject({ + author: original.author, + title: renamed.title, + }); + await expect(client.setDashboardFavorite(row.dashboardId, true)).rejects.toThrow( + 'you do not have permission', + ); + }); + it('uses server validation messages and exact tenant-wide titles', async () => { const c = createDemoDashboards(); await expect(c.createDashboard({ title: '' })).rejects.toThrow('Title must be provided'); const created = await c.createDashboard({ title: 'One' }); @@ -56,9 +94,6 @@ describe('per-client dashboard demo', () => { 'Dashboard title must be unique', ); const other = (await c.listDashboards()).find((row) => row.title === 'Shared operations')!; - await expect(c.updateDashboard(other.dashboardId, { title: 'New' })).rejects.toThrow( - 'you do not have permission', - ); await c.deleteDashboard(other.dashboardId); await expect(c.getDashboard(other.dashboardId)).rejects.toMatchObject({ status: 400 }); await expect( @@ -100,7 +135,7 @@ describe('per-client dashboard demo', () => { }); }); -it('returns the exact server messages for missing dashboards and validates tiles before owner checks', async () => { +it('returns the exact server messages for missing dashboards and validates shared dashboard tiles', async () => { const client = createDemoDashboards(), id = '01M4J000000000000000000099'; await expect(client.getDashboard(id)).rejects.toThrow( diff --git a/frontend/src/lib/demoDashboards.ts b/frontend/src/lib/demoDashboards.ts index 5947901f1..9694bf904 100644 --- a/frontend/src/lib/demoDashboards.ts +++ b/frontend/src/lib/demoDashboards.ts @@ -224,13 +224,11 @@ export function createDemoDashboards(now = Date.now()): Pick item.tile_id)).size !== tiles.length) fail('Tile IDs must be unique'); - if (original.author !== owner) - fail('Dashboard does not exist or you do not have permission to access it'); unique(body, id); const value: Dashboard = { ...clone(body), version: 'v1', - author: owner, + author: original.author, dashboardId: id, created: original.created, modified: timestamp(), @@ -255,7 +253,7 @@ export function createDemoDashboards(now = Date.now()): Pick item.dashboardId !== id); }, };