From 4632defb40f229e29ea29e13405c6c5282e43534 Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Wed, 22 Jul 2026 16:30:16 +0000 Subject: [PATCH] fix(#370): keep dashboard membership and stars consistent Make Dashboard tiles canonical for panel-query stars and route tile deletion through one atomic membership transform that cleans filter targets, preserves multi-instance membership, normalizes layouts, and mirrors spec.favorite. Add validation rollback, legacy/import, layout fallback, and real-browser Workbench-to-Dashboard reload coverage. Co-Authored-By: OpenAI Codex Claude-Session: unavailable (OpenAI Codex) --- CHANGELOG.md | 10 +++ .../dashboard-authoring-session.ts | 16 +++- src/dashboard/application/tile-membership.ts | 45 ++++++++++ src/state.ts | 9 +- src/ui/dashboard.ts | 24 +++++- src/ui/saved-history.ts | 5 +- tests/e2e/dashboard-membership.html | 86 +++++++++++++++++++ tests/e2e/dashboard-membership.spec.js | 31 +++++++ .../unit/dashboard-authoring-session.test.ts | 49 +++++++++++ tests/unit/dashboard.test.ts | 47 +++++++++- tests/unit/import-planner.test.ts | 11 +++ tests/unit/saved-history.test.ts | 24 ++++++ tests/unit/state.test.ts | 14 ++- tests/unit/tile-membership.test.ts | 75 +++++++++++++++- 14 files changed, 429 insertions(+), 17 deletions(-) create mode 100644 tests/e2e/dashboard-membership.html create mode 100644 tests/e2e/dashboard-membership.spec.js diff --git a/CHANGELOG.md b/CHANGELOG.md index d709ef0..21afee7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,16 @@ auto-generated per-PR notes; this file is the curated, human-readable history. involved (#343 non-goals; membership semantics stay with #370). ### Fixed +- **Dashboard tile deletion now keeps Workbench star membership consistent** + (#370). `dashboard.tiles[]` is the canonical favorite state for panel-role + queries: deleting the final tile clears the compatibility + `spec.favorite` flag, while deleting one of several instances keeps it set. + The same atomic workspace transform removes the selected tile from explicit + filter targets, normalizes the active layout/fallback, and advances the + Dashboard revision once. Legacy or imported `favorite: true` panel queries + without a tile now render unstarred and one click creates membership while + repairing the mirror flag; filter/setup favorites keep their independent + compatibility behavior. - **Migrated the development test stack to Vitest 4** (#372). Vitest and its V8 coverage provider now use the supported 4.x line, removed pool options have been migrated, stricter mock typings are explicit, and the more accurate diff --git a/src/dashboard/application/dashboard-authoring-session.ts b/src/dashboard/application/dashboard-authoring-session.ts index 94e7f98..e1c9278 100644 --- a/src/dashboard/application/dashboard-authoring-session.ts +++ b/src/dashboard/application/dashboard-authoring-session.ts @@ -32,6 +32,7 @@ import { buildDashboardExportBundle } from '../model/dashboard-export.js'; import { defaultLayoutRegistry } from '../layouts/layout-registry.js'; import { applyCommand } from './dashboard-commands.js'; import type { DashboardCommand, DashboardCommandResult } from './dashboard-commands.js'; +import { removeTileMembership } from './tile-membership.js'; import { createEmptyDashboard } from './empty-dashboard.js'; import { createQueryResolver } from './dashboard-query-resolver.js'; import { validateStoredWorkspaceDocument } from '../../workspace/stored-workspace.js'; @@ -108,9 +109,11 @@ export function createDashboardAuthoringSession( /** Validate a candidate dashboard as part of a complete candidate workspace: * structure + references/roles/limits (stored-workspace pipeline), then — * only when that passes — resolve and validate every panel presentation. */ - function validateCandidate(dashboard: DashboardDocumentV1): WorkspaceDiagnostic[] { + function validateCandidate( + dashboard: DashboardDocumentV1, candidateQueries: SavedQueryV2[] = queries, + ): WorkspaceDiagnostic[] { const candidate: StoredWorkspaceV1 = { - storageVersion: 1, id: workspaceId, name: workspaceName, queries, dashboard, + storageVersion: 1, id: workspaceId, name: workspaceName, queries: candidateQueries, dashboard, }; const structural = validateStoredWorkspaceDocument(candidate, codecOptions); if (structural.length) return structural; @@ -151,11 +154,16 @@ export function createDashboardAuthoringSession( const applied = applyCommand(current.document, command, { resolver, genTileId: genId, plugin: resolved.plugin }); if (!applied.ok) return returnFail(applied.diagnostics, baseVersion); - const normalized = resolved.plugin.normalize(applied.dashboard); - const diagnostics = validateCandidate(normalized); + const membership = command.type === 'remove-tile' + ? removeTileMembership(current.document, queries, command.tileId) + : null; + const candidateQueries = membership?.queries ?? queries; + const normalized = resolved.plugin.normalize(membership?.dashboard ?? applied.dashboard); + const diagnostics = validateCandidate(normalized, candidateQueries); if (diagnostics.length) return returnFail(diagnostics, baseVersion); const draftVersion = baseVersion + 1; + queries = candidateQueries; batch(() => { stateSignal.value = { document: normalized, draftVersion, dirty: true, diff --git a/src/dashboard/application/tile-membership.ts b/src/dashboard/application/tile-membership.ts index c2b30a9..3c14f91 100644 --- a/src/dashboard/application/tile-membership.ts +++ b/src/dashboard/application/tile-membership.ts @@ -21,6 +21,12 @@ import { regenerateGridFallback } from '../layouts/grafana-grid-layout.js'; import { createEmptyDashboard } from './empty-dashboard.js'; import type { DashboardDocumentV1, SavedQueryV2 } from '../../generated/json-schema.types.js'; +export interface TileRemovalResult { + dashboard: DashboardDocumentV1; + queries: SavedQueryV2[]; + queryId: string; +} + /** Remove every tile referencing `queryId`, and scrub those tile ids out of * every filter's `targets` — the typed counterpart of saved-query-mutation.ts's * `removeAffectedTiles` (that one operates on unknown/untyped documents). */ @@ -37,6 +43,45 @@ function removeTilesForQuery(dashboard: DashboardDocumentV1, queryId: string): D return { ...dashboard, tiles, filters }; } +/** The Workbench star is canonical tile membership for panel-role queries. + * Filter/setup favorites retain their independent compatibility semantics. */ +export function queryMembershipFavorite( + dashboard: DashboardDocumentV1 | null, + query: SavedQueryV2, +): boolean { + if (queryDashboardRole(query) !== 'panel') return query.spec.favorite === true; + return !!dashboard?.tiles.some((tile) => tile.queryId === query.id); +} + +/** Remove ONE Dashboard tile and synchronize the affected panel query's + * compatibility favorite flag with its post-delete membership. Filter target + * cleanup, layout normalization and grid fallback regeneration are part of + * the same pure transform; revision ownership remains with the commit caller. */ +export function removeTileMembership( + dashboard: DashboardDocumentV1, + queries: SavedQueryV2[], + tileId: string, +): TileRemovalResult | null { + const removedTile = dashboard.tiles.find((tile) => tile.id === tileId); + if (!removedTile) return null; + const tiles = dashboard.tiles.filter((tile) => tile.id !== tileId); + const filters = dashboard.filters.map((filter) => ( + filter.targets + ? { ...filter, targets: filter.targets.filter((target) => target !== tileId) } + : filter + )); + const next = { ...dashboard, tiles, filters }; + const normalized = resolveLayoutPluginSync(next.layout).normalize(next); + regenerateGridFallback(normalized.layout, normalized.tiles); + const member = normalized.tiles.some((tile) => tile.queryId === removedTile.queryId); + const nextQueries = queries.map((query) => ( + query.id === removedTile.queryId && queryDashboardRole(query) === 'panel' + ? { ...query, spec: { ...query.spec, favorite: member } } + : query + )); + return { dashboard: normalized, queries: nextQueries, queryId: removedTile.queryId }; +} + /** * Reflect a Workbench favorite flip onto Dashboard tile membership (#299). * diff --git a/src/state.ts b/src/state.ts index 99d964e..b47e86e 100644 --- a/src/state.ts +++ b/src/state.ts @@ -15,7 +15,9 @@ import { loadStr as loadStrUntyped, } from './core/storage.js'; import { emptyRecentMap as emptyRecentMapUntyped } from './core/recent-values.js'; -import { toggleTileMembership } from './dashboard/application/tile-membership.js'; +import { + queryMembershipFavorite, toggleTileMembership, +} from './dashboard/application/tile-membership.js'; import type { ResultSort } from './core/sort.js'; import { defaultSpecValidationService as defaultSpecValidationServiceUntyped, @@ -1123,7 +1125,7 @@ export async function toggleFavorite( // the desired boolean from the query it displays, and the transform re-checks // applicability against `latest` — tile membership is derived from // `latest.dashboard` (passed as `dashboard` below), never stale `state.dashboard`. - const favorite = !queryFavorite(entry); + const favorite = !queryMembershipFavorite(state.dashboard, entry); return patchSavedSpec(state, id, { favorite }, mutate, validationService, (dashboard, patchedEntry) => toggleTileMembership(dashboard, patchedEntry, favorite, genId)); } @@ -1132,7 +1134,8 @@ export async function toggleFavorite( export function sortedSaved(state: AppState): SavedQueryV2[] { return state.savedQueries .map((q, i): [SavedQueryV2, number] => [q, i]) - .sort((a, b) => (queryFavorite(b[0]) ? 1 : 0) - (queryFavorite(a[0]) ? 1 : 0) || a[1] - b[1]) + .sort((a, b) => (queryMembershipFavorite(state.dashboard, b[0]) ? 1 : 0) + - (queryMembershipFavorite(state.dashboard, a[0]) ? 1 : 0) || a[1] - b[1]) .map(([q]) => q); } diff --git a/src/ui/dashboard.ts b/src/ui/dashboard.ts index 2c4bd80..0662a00 100644 --- a/src/ui/dashboard.ts +++ b/src/ui/dashboard.ts @@ -70,6 +70,7 @@ import { import type { GrafanaGridLayoutModel, GridRenderMode } from '../dashboard/layouts/grafana-grid-layout.js'; import { applyCommand } from '../dashboard/application/dashboard-commands.js'; import type { DashboardCommand } from '../dashboard/application/dashboard-commands.js'; +import { removeTileMembership } from '../dashboard/application/tile-membership.js'; import { createQueryResolver } from '../dashboard/application/dashboard-query-resolver.js'; import { resolveDashboardMode } from '../dashboard/application/session-bundle.js'; import { @@ -875,6 +876,21 @@ export async function renderDashboard(app: DashboardApp): Promise { }; } + /** Apply a route command plus its workspace-level membership semantics. A + * raw remove-tile is first command-validated, then replaced by the shared + * transform that also cleans targets and synchronizes spec.favorite. */ + function applyRouteCommand( + baseDoc: DashboardDocumentV1, command: DashboardCommand, queriesForResolver: SavedQueryV2[], + ) { + const applied = applyCommand(baseDoc, command, ctxFor(baseDoc, queriesForResolver)); + if (!applied.ok) return applied; + if (command.type !== 'remove-tile') return { ...applied, queries: queriesForResolver }; + const membership = removeTileMembership(baseDoc, queriesForResolver, command.tileId); + return membership + ? { ...applied, dashboard: membership.dashboard, queries: membership.queries } + : { ...applied, queries: queriesForResolver }; + } + // ── Structural commands (reorder via drag, preset) ──────────────────────── // move-tile / update-placement / change-layout are the phase-3 authoring // commands; the dashboard UI drives only move-tile (drag) and change-layout @@ -899,7 +915,7 @@ export async function renderDashboard(app: DashboardApp): Promise { // RESULTING document's own engine, so a post-switch grid document is // pruned by the grid plugin (its own `items`), not flow's (which would // only ever see its own fallback surface). - const applied = applyCommand(currentDoc, command, ctxFor(currentDoc, queries)); + const applied = applyRouteCommand(currentDoc, command, queries); // A UI-driven command (drag move-tile, preset change-layout, grid // resize/delete) is always valid; a rejected candidate is simply ignored // (no draft change). @@ -936,12 +952,12 @@ export async function renderDashboard(app: DashboardApp): Promise { observed = latest; if (!latest || !latest.dashboard) return null; const base = latest.dashboard; - const reapplied = applyCommand(base, command, ctxFor(base, latest.queries)); + const reapplied = applyRouteCommand(base, command, latest.queries); if (!reapplied.ok) return null; const committedDoc = resolveLayoutPluginSync(reapplied.dashboard.layout).normalize(reapplied.dashboard); return { candidate: { - storageVersion: 1, id: latest.id, name: latest.name, queries: latest.queries, + storageVersion: 1, id: latest.id, name: latest.name, queries: reapplied.queries, dashboard: { ...committedDoc, revision: base.revision + 1 }, }, }; @@ -1051,7 +1067,7 @@ export async function renderDashboard(app: DashboardApp): Promise { if (!rebased) return; const rebaseQueries = committedWorkspace!.queries; for (const pending of pendingCommands) { - const r = applyCommand(rebased, pending, ctxFor(rebased, rebaseQueries)); + const r = applyRouteCommand(rebased, pending, rebaseQueries); // A replay that no longer applies is simply skipped here — its own // queued `mutateWorkspace` call will independently null-abort and // toast when its turn comes. diff --git a/src/ui/saved-history.ts b/src/ui/saved-history.ts index 945c22f..07c1d94 100644 --- a/src/ui/saved-history.ts +++ b/src/ui/saved-history.ts @@ -15,7 +15,8 @@ import type { AppState, HistoryEntry } from '../state.js'; import { flashToast } from './toast.js'; import { isAutoRunnable } from '../core/sql-split.js'; import { isQuerylessPanel } from '../core/panel-cfg.js'; -import { queryDescription, queryFavorite, queryName, queryPanel, queryView } from '../core/saved-query.js'; +import { queryDescription, queryName, queryPanel, queryView } from '../core/saved-query.js'; +import { queryMembershipFavorite } from '../dashboard/application/tile-membership.js'; import { effectiveDashboardRole, rolePreviewView } from '../core/result-choice.js'; import { filterRoleBadge } from './tabs.js'; import type { App } from './app.types.js'; @@ -122,7 +123,7 @@ function renderSaved(app: App, list: HTMLElement): void { } for (const q of items) { if (app.state.editingSavedId.value === q.id) { list.appendChild(savedEditForm(app, q)); continue; } - const favorite = queryFavorite(q); + const favorite = queryMembershipFavorite(state.dashboard, q); const name = queryName(q); const description = queryDescription(q); const panel = queryPanel(q); diff --git a/tests/e2e/dashboard-membership.html b/tests/e2e/dashboard-membership.html new file mode 100644 index 0000000..2529bf0 --- /dev/null +++ b/tests/e2e/dashboard-membership.html @@ -0,0 +1,86 @@ + + + + + Dashboard membership workflow + + + +
+ + + + diff --git a/tests/e2e/dashboard-membership.spec.js b/tests/e2e/dashboard-membership.spec.js new file mode 100644 index 0000000..42bb998 --- /dev/null +++ b/tests/e2e/dashboard-membership.spec.js @@ -0,0 +1,31 @@ +import { test, expect } from '@playwright/test'; + +test('Workbench star → Dashboard delete → Workbench reload keeps membership consistent', async ({ page }) => { + const pageErrors = []; + page.on('pageerror', (error) => pageErrors.push(error.message)); + await page.goto('/tests/e2e/dashboard-membership.html'); + await page.waitForTimeout(250); + expect(pageErrors).toEqual([]); + await page.waitForFunction(() => window.__ready === true); + + await page.locator('.sv-star[title="Favorite"]').click(); + await page.waitForFunction(async () => (await window.__workspace()).queries[0].spec.favorite === true); + let workspace = await page.evaluate(() => window.__workspace()); + expect(workspace.queries[0].spec.favorite).toBe(true); + expect(workspace.dashboard.tiles).toHaveLength(1); + + await page.getByRole('button', { name: 'Open Dashboard' }).click(); + expect(pageErrors).toEqual([]); + await expect(page.locator('.dash-tile-body')).toContainText('1'); + await page.getByRole('button', { name: 'Remove Revenue from the dashboard' }).click(); + await page.waitForFunction(async () => (await window.__workspace()).queries[0].spec.favorite === false); + workspace = await page.evaluate(() => window.__workspace()); + expect(workspace.queries[0].spec.favorite).toBe(false); + expect(workspace.dashboard.tiles).toEqual([]); + expect(workspace.dashboard.revision).toBe(2); + + await page.reload(); + await page.waitForFunction(() => window.__ready === true); + await expect(page.locator('.sv-star[title="Favorite"]')).toBeVisible(); + await expect(page.locator('.sv-star')).not.toHaveClass(/\bon\b/); +}); diff --git a/tests/unit/dashboard-authoring-session.test.ts b/tests/unit/dashboard-authoring-session.test.ts index 09e0bd8..a138a5a 100644 --- a/tests/unit/dashboard-authoring-session.test.ts +++ b/tests/unit/dashboard-authoring-session.test.ts @@ -224,6 +224,55 @@ describe('DashboardAuthoringSession — membership, export, lifecycle', () => { expect(favOff && favOff.spec.favorite).toBe(false); }); + it('raw remove-tile cleans targets and mirrors membership for final and remaining instances', async () => { + const query = { + ...panelQuery('q1'), sql: 'SELECT {x:String} AS a, 1 AS b', + spec: { ...panelQuery('q1').spec, favorite: true }, + }; + const workspace = workspaceFixture({ + queries: [query], + dashboard: { + ...emptyDash(), + tiles: [{ id: 't1', queryId: 'q1' }, { id: 't2', queryId: 'q1' }], + filters: [{ id: 'f1', parameter: 'x', targets: ['t1', 't2'] }], + }, + } as StoredWorkspaceV1); + const { session } = makeSession({ workspace }); + expect((await session.execute({ type: 'remove-tile', tileId: 't1' })).ok).toBe(true); + let committed = await session.commit(); + expect(committed.ok && committed.workspace.queries[0].spec.favorite).toBe(true); + expect(committed.ok && committed.workspace.dashboard!.filters[0].targets).toEqual(['t2']); + expect(committed.ok && committed.dashboardRevision).toBe(2); + + expect((await session.execute({ type: 'remove-tile', tileId: 't2' })).ok).toBe(true); + committed = await session.commit(); + expect(committed.ok && committed.workspace.queries[0].spec.favorite).toBe(false); + expect(committed.ok && committed.workspace.dashboard!.filters[0].targets).toEqual([]); + expect(committed.ok && committed.dashboardRevision).toBe(3); + }); + + it('a membership validation failure leaves both the Dashboard and favorite mirror unchanged', async () => { + const query = { + ...panelQuery('q1'), sql: 'SELECT {country:String} AS a, 1 AS b', + spec: { ...panelQuery('q1').spec, favorite: true }, + }; + const workspace = workspaceFixture({ + queries: [query, filterQuery('source')], + dashboard: { + ...emptyDash(), tiles: [{ id: 't1', queryId: 'q1' }], + filters: [{ id: 'flt', parameter: 'country', sourceQueryId: 'source', targets: ['t1'] }], + }, + } as StoredWorkspaceV1); + const { session } = makeSession({ workspace }); + const before = JSON.stringify(session.state.value.document); + const result = await session.execute({ type: 'remove-tile', tileId: 't1' }); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.diagnostics.some((d) => d.code === 'filter-selection-no-consumers')).toBe(true); + expect(JSON.stringify(session.state.value.document)).toBe(before); + expect(session.state.value.draftVersion).toBe(0); + expect(session.createPortableBundle().queries.find((q) => q.id === 'q1')?.spec.favorite).toBe(true); + }); + it('builds a portable bundle of only the dependency queries and never increments revision', async () => { const fixed = makeSession({ nowISO: () => '2020-01-01T00:00:00Z' }); await fixed.session.execute({ type: 'add-query', queryId: 'q1' }); diff --git a/tests/unit/dashboard.test.ts b/tests/unit/dashboard.test.ts index 7d5429b..9094d85 100644 --- a/tests/unit/dashboard.test.ts +++ b/tests/unit/dashboard.test.ts @@ -21,6 +21,7 @@ import { makeApp, FakeChart } from '../helpers/fake-app.js'; import { createApp } from '../../src/ui/app.js'; import { createCodeMirrorEditor } from '../../src/editor/codemirror-adapter.js'; import { savedQuery } from '../helpers/saved-query.js'; +import { queryFavorite } from '../../src/core/saved-query.js'; import type { SavedQueryFixture } from '../helpers/saved-query.js'; import type { App } from '../../src/ui/app.types.js'; import type { AppState } from '../../src/state.js'; @@ -3854,6 +3855,42 @@ describe('renderDashboard — the serialized write pipeline (#341)', () => { expect(app.state.dashboard?.revision).toBe(2); }); + it('remove-tile atomically clears the final query favorite and every explicit filter target', async () => { + const workspace = wsWith({ + queries: [q('q1', 'SELECT {x:String}', { favorite: true }), q('q2', 'SELECT 2')], + tiles: [{ id: 't1', queryId: 'q1' }, { id: 't2', queryId: 'q2' }], + filters: [ + { id: 'f1', parameter: 'x', targets: ['t1', 't2'] }, + { id: 'f2', parameter: 'y', targets: ['t1'] }, + ], + layout: { type: 'grafana-grid', version: 1, items: {} }, + }); + const { app, commit } = dashApp({ workspace }); + await render(app); + qs(app.root, '.dash-gg-del').click(); + await flush(); + const candidate = commit.mock.calls[0][0]; + expect(candidate.queries.find((query) => query.id === 'q1')?.spec.favorite).toBe(false); + expect(candidate.dashboard?.filters.map((filter) => filter.targets)).toEqual([['t2'], []]); + expect(candidate.dashboard?.revision).toBe(2); + }); + + it('remove-tile keeps favorite true when another tile instance references the query', async () => { + const workspace = wsWith({ + queries: [q('q1', 'SELECT 1', { favorite: true })], + tiles: [{ id: 't1', queryId: 'q1' }, { id: 't2', queryId: 'q1' }], + layout: { type: 'grafana-grid', version: 1, items: {} }, + }); + const { app, commit } = dashApp({ workspace }); + await render(app); + qs(app.root, '.dash-gg-del').click(); + await flush(); + const candidate = commit.mock.calls[0][0]; + expect(candidate.dashboard?.tiles).toEqual([{ id: 't2', queryId: 'q1' }]); + expect(candidate.queries[0].spec.favorite).toBe(true); + expect(candidate.dashboard?.revision).toBe(2); + }); + it('no persisted aggregate (legacy/empty Dashboard): a command stays optimistic-only — never calls commit', async () => { const { app, commit } = dashApp({ workspace: null, savedQueries: [q('q1', 'SELECT 1', { favorite: true })], @@ -4061,7 +4098,13 @@ describe('renderDashboard — the serialized write pipeline (#341)', () => { .mockImplementation(async (candidate: StoredWorkspaceV1) => ( { ok: true, workspace: candidate, dashboardRevision: candidate.dashboard ? candidate.dashboard.revision : null } )); - const { app } = dashApp({ workspace: twoTilesGrid(), commit }); + const workspace = wsWith({ + queries: [q('q1', 'SELECT {x:String}', { favorite: true }), q('q2', 'SELECT 2')], + tiles: [{ id: 't1', queryId: 'q1' }, { id: 't2', queryId: 'q2' }], + filters: [{ id: 'f1', parameter: 'x', targets: ['t1'] }], + layout: { type: 'grafana-grid', version: 1, items: {} }, + }); + const { app } = dashApp({ workspace, commit }); await render(app); expect(qsa(app.root, '.dash-gg-tile')).toHaveLength(2); qs(app.root, '.dash-gg-del').click(); // remove t1 — its commit fails @@ -4074,6 +4117,8 @@ describe('renderDashboard — the serialized write pipeline (#341)', () => { // truth), and nothing was persisted. expect(qsa(app.root, '.dash-gg-tile')).toHaveLength(2); expect(app.state.dashboard?.tiles.map((t) => t.id)).toEqual(['t1', 't2']); + expect(app.state.dashboard?.filters[0].targets).toEqual(['t1']); + expect(queryFavorite(app.state.savedQueries.find((query) => query.id === 'q1'))).toBe(true); expect((await app.workspace.loadCurrent())?.dashboard?.tiles.map((t) => t.id)).toEqual(['t1', 't2']); // The rebuilt route is fully functional — a later command still commits. qsa(app.root, '.dash-gg-del')[1].click(); diff --git a/tests/unit/import-planner.test.ts b/tests/unit/import-planner.test.ts index 9cdabcf..212cfbd 100644 --- a/tests/unit/import-planner.test.ts +++ b/tests/unit/import-planner.test.ts @@ -248,6 +248,17 @@ describe('planImportQueries', () => { expect(plan.sourceDashboardId).toBeUndefined(); }); + it('imports a favorite panel query without inventing Dashboard membership', () => { + const dash = dashboardDoc({ tiles: [] }); + const ws = workspace({ queries: [panelQuery('a')], dashboard: dash }); + const favorite = panelQuery('b'); + favorite.spec.favorite = true; + const plan = planImportQueries(ws, bundle({ queries: [favorite] }), [], counter()); + expect(plan.candidateWorkspace?.queries.find((query) => query.id === 'b')?.spec.favorite).toBe(true); + expect(plan.candidateWorkspace?.dashboard?.tiles).toEqual([]); + expect(plan.candidateWorkspace?.dashboard).toBe(dash); + }); + it('overwrites the existing entry in place on a replace decision', () => { const ws = workspace({ queries: [panelQuery('a', 'old name')] }); const decisions: QueryDecision[] = [{ sourceId: 'a', action: 'replace' }]; diff --git a/tests/unit/saved-history.test.ts b/tests/unit/saved-history.test.ts index ceab7e3..02fbb1b 100644 --- a/tests/unit/saved-history.test.ts +++ b/tests/unit/saved-history.test.ts @@ -199,6 +199,30 @@ describe('renderSavedHistory', () => { expect(app.queryDoc.revalidateSpecDrafts).toHaveBeenCalled(); }); + it('saved: panel stars and sorting use canonical tile membership, then repair a stale flag in one click', async () => { + const app = makeApp(); + app.state.sidePanel.value = 'saved'; + setSaved(app, [ + { id: 'a', name: 'Stale flag', sql: '1', favorite: true }, + { id: 'b', name: 'Real member', sql: '2', favorite: false }, + ]); + app.state.dashboard = { + documentVersion: 1, id: 'd', title: 'D', revision: 1, + layout: { type: 'flow', version: 1, preset: 'report', items: {} }, + filters: [], tiles: [{ id: 'tb', queryId: 'b' }], + }; + renderSavedHistory(app); + const rows = qsa(savedList(app), '.saved-row'); + expect(rows.map((row) => qs(row, '.name').textContent)).toEqual(['Real member', 'Stale flag']); + expect(qs(rows[0], '.sv-star').classList.contains('on')).toBe(true); + expect(qs(rows[1], '.sv-star').classList.contains('on')).toBe(false); + + click(qs(rows[1], '.sv-star')); + await flush(); + expect(app.state.dashboard.tiles.some((tile) => tile.queryId === 'a')).toBe(true); + expect(queryFavorite(app.state.savedQueries.find((query) => query.id === 'a'))).toBe(true); + }); + it('saved: favorite merges into a linked dirty valid Spec draft', async () => { const app = makeApp(); app.state.sidePanel.value = 'saved'; diff --git a/tests/unit/state.test.ts b/tests/unit/state.test.ts index 71ec692..f65bf91 100644 --- a/tests/unit/state.test.ts +++ b/tests/unit/state.test.ts @@ -432,14 +432,24 @@ describe('saved queries', () => { expect(mutate.commit).toHaveBeenCalledTimes(1); }); - it('favorite ON is idempotent when a tile already references the query', async () => { + it('a stale false flag with a tile toggles from canonical membership and removes it', async () => { const s = savedTestState(); s.savedQueries = [savedQuery({ id: 'p1', sql: 'SELECT 1', favorite: false, dashboard: { role: 'panel' } })]; s.dashboard = { ...blankDashboard(), tiles: [{ id: 't1', queryId: 'p1' }] }; const mutate = fakeMutateWorkspace(s); await toggleFavorite(s, 'p1', mutate, genTileId()); + expect(queryFavorite(s.savedQueries[0])).toBe(false); + expect(s.dashboard!.tiles).toEqual([]); + }); + + it('a stale true flag without a tile is repaired by one click that creates membership', async () => { + const s = savedTestState(); + s.savedQueries = [savedQuery({ id: 'p1', sql: 'SELECT 1', favorite: true, dashboard: { role: 'panel' } })]; + s.dashboard = blankDashboard(); + const mutate = fakeMutateWorkspace(s); + await toggleFavorite(s, 'p1', mutate, genTileId()); expect(queryFavorite(s.savedQueries[0])).toBe(true); - expect(s.dashboard!.tiles).toEqual([{ id: 't1', queryId: 'p1' }]); // no duplicate + expect(s.dashboard!.tiles).toEqual([{ id: 'tile-1', queryId: 'p1' }]); }); it('favorite ON on a filter-role query never creates a tile', async () => { diff --git a/tests/unit/tile-membership.test.ts b/tests/unit/tile-membership.test.ts index 465f709..103e3bd 100644 --- a/tests/unit/tile-membership.test.ts +++ b/tests/unit/tile-membership.test.ts @@ -1,5 +1,7 @@ import { describe, expect, it } from 'vitest'; -import { toggleTileMembership } from '../../src/dashboard/application/tile-membership.js'; +import { + queryMembershipFavorite, removeTileMembership, toggleTileMembership, +} from '../../src/dashboard/application/tile-membership.js'; import type { DashboardDocumentV1, SavedQueryV2 } from '../../src/generated/json-schema.types.js'; const panelQuery = (id: string): SavedQueryV2 => ({ @@ -143,3 +145,74 @@ describe('toggleTileMembership — grafana-grid@1 engine awareness (#291)', () = }); }); }); + +describe('canonical membership and one-tile removal (#370)', () => { + it('reads panel favorites from tiles while preserving non-panel favorite flags', () => { + const panel = { ...panelQuery('p1'), spec: { ...panelQuery('p1').spec, favorite: true } }; + const filter = { ...filterQuery('f1'), spec: { ...filterQuery('f1').spec, favorite: true } }; + expect(queryMembershipFavorite(dashboard(), panel)).toBe(false); + expect(queryMembershipFavorite(dashboard({ tiles: [{ id: 't1', queryId: 'p1' }] }), panel)).toBe(true); + expect(queryMembershipFavorite(null, panel)).toBe(false); + expect(queryMembershipFavorite(null, filter)).toBe(true); + }); + + it('removes the final instance, cleans every explicit target, and clears the compatibility flag', () => { + const query = { ...panelQuery('p1'), spec: { ...panelQuery('p1').spec, favorite: true } }; + const input = dashboard({ + tiles: [{ id: 't1', queryId: 'p1' }, { id: 't2', queryId: 'p2' }], + filters: [ + { id: 'f1', parameter: 'x', targets: ['t1', 't2'] }, + { id: 'f2', parameter: 'y', targets: ['t1'] }, + { id: 'f3', parameter: 'z' }, + ], + }); + const result = removeTileMembership(input, [query, panelQuery('p2')], 't1')!; + expect(result.dashboard.tiles).toEqual([{ id: 't2', queryId: 'p2' }]); + expect(result.dashboard.filters).toEqual([ + { id: 'f1', parameter: 'x', targets: ['t2'] }, + { id: 'f2', parameter: 'y', targets: [] }, + { id: 'f3', parameter: 'z' }, + ]); + expect(result.queries[0].spec.favorite).toBe(false); + expect(input.tiles).toHaveLength(2); + }); + + it('removes only the selected instance and keeps favorite true while another remains', () => { + const query = { ...panelQuery('p1'), spec: { ...panelQuery('p1').spec, favorite: true } }; + const result = removeTileMembership(dashboard({ + tiles: [{ id: 't1', queryId: 'p1' }, { id: 't2', queryId: 'p1' }], + }), [query], 't1')!; + expect(result.dashboard.tiles).toEqual([{ id: 't2', queryId: 'p1' }]); + expect(result.queries[0].spec.favorite).toBe(true); + }); + + it('returns null for a missing tile and does not rewrite non-panel favorites', () => { + const filter = { ...filterQuery('f1'), spec: { ...filterQuery('f1').spec, favorite: true } }; + expect(removeTileMembership(dashboard(), [filter], 'missing')).toBeNull(); + const result = removeTileMembership( + dashboard({ tiles: [{ id: 't1', queryId: 'f1' }] }), [filter], 't1', + )!; + expect(result.queries[0]).toBe(filter); + }); + + it('normalizes grafana-grid primary and fallback placements after one-tile removal', () => { + const input = dashboard({ + tiles: [{ id: 't1', queryId: 'p1' }, { id: 't2', queryId: 'p2' }], + layout: { + type: 'grafana-grid', version: 1, + items: { t1: { colStart: 0, span: 6, height: 2 }, t2: { colStart: 6, span: 6, height: 3 } }, + fallback: { + type: 'flow', version: 1, preset: 'columns-2', + items: { t1: { span: 2, height: 'medium' }, t2: { span: 2, height: 'large' } }, + }, + }, + }); + const result = removeTileMembership(input, [panelQuery('p1'), panelQuery('p2')], 't1')!; + expect((result.dashboard.layout as { items: Record }).items).toEqual({ + t2: { colStart: 6, span: 6, height: 3 }, + }); + expect((result.dashboard.layout as { fallback: { items: Record } }).fallback.items).toEqual({ + t2: { span: 2, height: 'large' }, + }); + }); +});