From ab48475273f4b04326603c5caa825a05c60803b7 Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Sat, 18 Jul 2026 14:44:33 +0000 Subject: [PATCH 1/3] feat(#299): wire Workbench star to Dashboard tile membership MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since dashboard.tiles[] became canonical Dashboard membership (#280/#287), the Workbench star only flipped spec.favorite and never added/removed a tile, so favoriting a query in the Workbench had no effect on /sql/dashboard. toggleFavorite now reflects the favorite flip onto tile membership in the SAME atomic commit, via a new pure toggleTileMembership (src/dashboard/application/ tile-membership.ts). The role gate mirrors the legacy migration precedent (buildLegacyMigrationCandidate): only PANEL-role queries become tiles — a favorited filter/setup-role query stays favorite-only. Unfavoriting removes every tile referencing the query and scrubs those tile ids from filter targets (mirrors saved-query-mutation.ts's removeAffectedTiles); the result is run through flowLayoutPlugin.normalize. patchSavedSpec gains an optional DashboardTransform (default identity, so renameSaved is unaffected) and now projects the committed dashboard back onto state, same convention as savedQueries. spec.favorite stays the star's visual state — the documented dual-write retirement is deferred (Part of #299). Also adds the KEYS.dashFilters constant consumed by the #303 commit (same file). Pure tile-membership logic 100% covered; state/saved-history tests extended. Part of #299 Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01GLMprUnwnz4oaz3cnLRKcz --- src/dashboard/application/tile-membership.ts | 72 +++++++++++++ src/state.ts | 45 +++++++- src/ui/saved-history.ts | 2 +- tests/unit/saved-history.test.ts | 2 +- tests/unit/state.test.ts | 103 ++++++++++++++++-- tests/unit/tile-membership.test.ts | 104 +++++++++++++++++++ 6 files changed, 312 insertions(+), 16 deletions(-) create mode 100644 src/dashboard/application/tile-membership.ts create mode 100644 tests/unit/tile-membership.test.ts diff --git a/src/dashboard/application/tile-membership.ts b/src/dashboard/application/tile-membership.ts new file mode 100644 index 0000000..31349ee --- /dev/null +++ b/src/dashboard/application/tile-membership.ts @@ -0,0 +1,72 @@ +// Wire the Workbench favorite star to Dashboard tile membership (#299). +// `StoredWorkspaceV1.dashboard.tiles[]` is canonical Dashboard membership +// (#287); toggling `spec.favorite` on a saved query must add/remove the +// matching tile IN THE SAME atomic commit as the favorite flip, or the star +// and the Dashboard silently disagree about what's on it. +// +// The role gate mirrors the migration precedent in +// `src/workspace/legacy-migration.ts`'s `buildLegacyMigrationCandidate`: only +// PANEL-role queries ever become tiles — a favorited filter/setup-role query +// stays favorited (the star's own visual state) but never gets a tile. Tile +// removal mirrors `src/dashboard/application/saved-query-mutation.ts`'s +// `removeAffectedTiles`: every tile referencing the query is dropped, and +// those tile ids are scrubbed out of every filter's `targets[]` too. +// +// Pure — no DOM, no persistence; the caller (state.ts's `toggleFavorite`) +// folds the result into the same commit candidate as the favorite patch. + +import { queryDashboardRole } from '../model/workspace-semantics.js'; +import { flowLayoutPlugin } from '../layouts/flow-layout.js'; +import type { DashboardDocumentV1, SavedQueryV2 } from '../../generated/json-schema.types.js'; + +/** 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). */ +function removeTilesForQuery(dashboard: DashboardDocumentV1, queryId: string): DashboardDocumentV1 { + const removed = new Set( + dashboard.tiles.filter((tile) => tile.queryId === queryId).map((tile) => tile.id), + ); + const tiles = dashboard.tiles.filter((tile) => tile.queryId !== queryId); + const filters = dashboard.filters.map((filter) => ( + filter.targets + ? { ...filter, targets: filter.targets.filter((target) => !removed.has(target)) } + : filter + )); + return { ...dashboard, tiles, filters }; +} + +/** + * Reflect a Workbench favorite flip onto Dashboard tile membership (#299). + * + * - Star ON + panel-role query + no existing tile referencing it → append one + * tile `{ id: genTileId(), queryId: query.id }`. + * - Star ON + a tile already references it → unchanged (idempotent). + * - Star ON + filter/setup-role query → unchanged (favorite flip only; never + * becomes a tile, matching `buildLegacyMigrationCandidate`). + * - Star OFF → remove EVERY tile referencing the query and scrub those tile + * ids from every filter's `targets`. + * - `dashboard` null in → `null` out (no Dashboard yet; favorite flip only). + * + * The result is always run through `flowLayoutPlugin.normalize` (a new tile + * gets the flow default placement lazily — nothing to add up front — and a + * removed tile's placement item, if any, is dropped) and is always a fresh + * copy; `dashboard` is never mutated. + */ +export function toggleTileMembership( + dashboard: DashboardDocumentV1 | null, + query: SavedQueryV2, + favorite: boolean, + genTileId: () => string, +): DashboardDocumentV1 | null { + if (!dashboard) return null; + const hasTile = dashboard.tiles.some((tile) => tile.queryId === query.id); + let next = dashboard; + if (favorite) { + if (!hasTile && queryDashboardRole(query) === 'panel') { + next = { ...dashboard, tiles: [...dashboard.tiles, { id: genTileId(), queryId: query.id }] }; + } + } else if (hasTile) { + next = removeTilesForQuery(dashboard, query.id); + } + return flowLayoutPlugin.normalize(next); +} diff --git a/src/state.ts b/src/state.ts index 17207a2..ed18951 100644 --- a/src/state.ts +++ b/src/state.ts @@ -15,6 +15,7 @@ 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 type { ResultSort } from './core/sort.js'; import { defaultSpecValidationService as defaultSpecValidationServiceUntyped, @@ -97,13 +98,16 @@ const asSpecDiagnostics = (diagnostics: readonly WorkspaceDiagnostic[]): SpecDia * carried through unchanged alongside the op's own next `queries` array. */ type WorkspaceCandidateState = Pick; -function buildWorkspaceCandidate(state: WorkspaceCandidateState, queries: SavedQueryV2[]): StoredWorkspaceV1 { +function buildWorkspaceCandidate( + state: WorkspaceCandidateState, queries: SavedQueryV2[], + dashboard: DashboardDocumentV1 | null = state.dashboard, +): StoredWorkspaceV1 { return { storageVersion: 1, id: state.workspaceId, name: state.libraryName.value, queries, - dashboard: state.dashboard, + dashboard, }; } @@ -342,6 +346,11 @@ export const KEYS = { dashCols: 'asb:dashCols', varRecent: 'asb:varRecent', varRecentDisabled: 'asb:varRecentDisabled', + /** Isolated per-dashboard Dashboard-filter persistence (#303 Option B) — a + * single blob keyed `dashboardId -> filterId -> {value,active}`, read/written + * through `dashboard/model/dashboard-filter-store.js`. Deliberately separate + * from the Workbench's `varValues`/`filterActive` keys above. */ + dashFilters: 'asb:dashFilters', }; /** Row-limit options for the result cap selector (shared between state + UI). @@ -750,6 +759,16 @@ export async function commitSavedQuery( return { ok: true, entry: saved }; } +/** A pure transform folded into the SAME commit candidate as a `patchSavedSpec` + * write (#299) — e.g. `toggleFavorite` reflects its favorite flip onto + * Dashboard tile membership atomically alongside the Spec patch. Defaults to + * identity, so `renameSaved`/other callers that don't touch the Dashboard are + * unaffected. Receives the COMMITTED entry (the one about to be sent to + * `commit`, post-patch) so a role/id-dependent transform sees the final Spec. */ +export type DashboardTransform = (dashboard: DashboardDocumentV1 | null, entry: SavedQueryV2) => DashboardDocumentV1 | null; + +const identityDashboardTransform: DashboardTransform = (dashboard) => dashboard; + /** * Generic committed-Spec writer for pencil/star/future controls. The patch is * applied independently to the persisted entry and every linked valid draft, @@ -765,6 +784,7 @@ export async function patchSavedSpec( id: string, patch: SpecPatch, commit: CommitWorkspace, validationService: SpecValidationService = defaultSpecValidationService, + transformDashboard: DashboardTransform = identityDashboardTransform, ): Promise { const invalidTab = invalidSpecTabForSaved(state, id); if (invalidTab) return { ok: false, invalidTab, entry: null }; @@ -788,11 +808,16 @@ export async function patchSavedSpec( // COMPUTE only above — no `state`/`tabs` mutation yet. const nextQueries = state.savedQueries.slice(); nextQueries[index] = entry; - const result = await commit(buildWorkspaceCandidate(state, nextQueries)); + const nextDashboard = transformDashboard(state.dashboard, entry); + const result = await commit(buildWorkspaceCandidate(state, nextQueries, nextDashboard)); if (!result.ok) { return { ok: false, invalidTab: null, entry: null, diagnostics: asSpecDiagnostics(result.diagnostics) }; } state.savedQueries = result.workspace.queries; + // #299: project the committed Dashboard back, same convention as + // `savedQueries` above — the aggregate commit is the single source of truth + // for whether/how tile membership actually landed. + state.dashboard = result.workspace.dashboard; const saved = committedEntry(state.savedQueries, entry.id, entry); for (const update of draftUpdates) { setTabSpecDraft(update.tab, update.spec, { dirty: update.dirty, validationService }); @@ -824,17 +849,27 @@ export async function renameSaved( return patchSavedSpec(state, id, patch, commit, validationService); } -/** Toggle a saved query's favorite flag. */ +/** + * Toggle a saved query's favorite flag, atomically committing any Dashboard + * tile-membership change the flip implies in the SAME candidate (#299): a + * favorited panel-role query gets a tile (unless one already references it), + * an unfavorited query loses every tile that references it. `spec.favorite` + * stays the star's own visual state either way — this only ADDS the tile + * side effect, it does not retire the favorite dual-write. `genTileId` mints + * a fresh tile id (only called when a tile is actually appended). + */ export async function toggleFavorite( state: AppState, id: string, commit: CommitWorkspace, + genTileId: () => string, validationService: SpecValidationService = defaultSpecValidationService, ): Promise { const index = state.savedQueries.findIndex((q) => q.id === id); const entry = index >= 0 ? state.savedQueries[index] : null; if (!entry) return; const favorite = !queryFavorite(entry); - return patchSavedSpec(state, id, { favorite }, commit, validationService); + return patchSavedSpec(state, id, { favorite }, commit, validationService, + (dashboard, patchedEntry) => toggleTileMembership(dashboard, patchedEntry, favorite, genTileId)); } /** Saved queries with favorites first (stable within each group). */ diff --git a/src/ui/saved-history.ts b/src/ui/saved-history.ts index 0fc403e..e7c7068 100644 --- a/src/ui/saved-history.ts +++ b/src/ui/saved-history.ts @@ -136,7 +136,7 @@ function renderSaved(app: App, list: HTMLElement): void { class: 'sv-star' + (favorite ? ' on' : ''), title: favorite ? 'Unfavorite' : 'Favorite', onclick: async (e: Event) => { e.stopPropagation(); - const result = await app.serializeWrite(() => toggleFavorite(state, q.id, app.workspace.commit, app.specValidators)); + const result = await app.serializeWrite(() => toggleFavorite(state, q.id, app.workspace.commit, app.genId, app.specValidators)); if (result && result.invalidTab) app.activateInvalidSpecDraft(result.invalidTab); else if (result && result.ok) { app.queryDoc.revalidateSpecDrafts(); diff --git a/tests/unit/saved-history.test.ts b/tests/unit/saved-history.test.ts index b285802..6e9b09a 100644 --- a/tests/unit/saved-history.test.ts +++ b/tests/unit/saved-history.test.ts @@ -568,7 +568,7 @@ describe('concurrent saved-query writes (#287 review fix)', () => { // Fire a favorite-toggle on q1 and a delete on q2 in the same tick. Without // app.serializeWrite both build a candidate from the same [q1,q2] snapshot // and whichever commit lands last wins — resurrecting the deleted q2. - const pToggle = app.serializeWrite(() => toggleFavorite(app.state, 'q1', app.workspace.commit, app.specValidators)); + const pToggle = app.serializeWrite(() => toggleFavorite(app.state, 'q1', app.workspace.commit, app.genId, app.specValidators)); const pDelete = app.serializeWrite(() => deleteSaved(app.state, 'q2', app.workspace.commit)); await Promise.all([pToggle, pDelete]); expect(app.state.savedQueries.map((q) => q.id)).toEqual(['q1']); // q2 stays deleted diff --git a/tests/unit/state.test.ts b/tests/unit/state.test.ts index ce686c2..79fea5a 100644 --- a/tests/unit/state.test.ts +++ b/tests/unit/state.test.ts @@ -10,7 +10,7 @@ import type { } from '../../src/state.js'; import { queryDescription, queryFavorite, queryName, queryPanel, queryView } from '../../src/core/saved-query.js'; import { savedQuery as savedQueryUntyped } from '../helpers/saved-query.js'; -import type { SavedQueryV2 } from '../../src/generated/json-schema.types.js'; +import type { DashboardDocumentV1, SavedQueryV2 } from '../../src/generated/json-schema.types.js'; import { fakeWorkspaceCommit } from '../helpers/fake-app.js'; afterEach(() => vi.unstubAllGlobals()); @@ -36,6 +36,14 @@ function savedTestState(over: Record = {}): AppState { return s; } +// #299: toggleFavorite now takes an injected tile-id generator (only called +// when it actually appends a tile) — a fresh counter per call keeps ids +// distinct within a test without pulling in a real crypto/uid seam. +const genTileId = (): (() => string) => { + let n = 0; + return () => 'tile-' + (++n); +}; + /** Unwrap a successful `SavedEntryResult`, failing loudly (not silently * returning `undefined`) when a test's own setup produced a rejection — * mirrors the pre-#287 sync code's `!`-asserted non-null return. */ @@ -312,7 +320,7 @@ describe('saved queries', () => { s.tabs.value = [tab, second]; const commit = fakeWorkspaceCommit(); await renameSaved(s, 's1', 'New', 'Description', commit); - await toggleFavorite(s, 's1', commit); + await toggleFavorite(s, 's1', commit, genTileId()); for (const spec of [s.savedQueries[0].spec, tab.specParsed]) { expect(spec).toMatchObject({ name: 'New', description: 'Description', favorite: true, @@ -340,12 +348,89 @@ describe('saved queries', () => { savedQuery({ id: 'c', sql: '3', name: 'C' }), ]; const commit = fakeWorkspaceCommit(); - await toggleFavorite(s, 'c', commit); + await toggleFavorite(s, 'c', commit, genTileId()); expect(queryFavorite(s.savedQueries.find((q) => q.id === 'c'))).toBe(true); - await toggleFavorite(s, 'missing', commit); // no-op + await toggleFavorite(s, 'missing', commit, genTileId()); // no-op expect(sortedSaved(s).map((q) => q.id)).toEqual(['c', 'a', 'b']); expect(commit).toHaveBeenCalledTimes(1); }); + // #299: the Workbench star also drives Dashboard tile membership, atomically + // with the favorite flip — only panel-role queries become tiles (mirrors + // legacy-migration.ts's buildLegacyMigrationCandidate), star OFF removes + // every matching tile and scrubs those tile ids from filter targets (mirrors + // saved-query-mutation.ts's removeAffectedTiles), and a null `state.dashboard` + // means favorite-flip-only (no Dashboard to touch). + describe('toggleFavorite wires Dashboard tile membership (#299)', () => { + const blankDashboard = (): DashboardDocumentV1 => ({ + documentVersion: 1, id: 'dash', title: 'D', revision: 1, + layout: { type: 'flow', version: 1, preset: 'full-width', items: {} }, + filters: [], tiles: [], + }); + + it('favorite ON on a panel-role query appends a tile in the same commit', async () => { + const s = savedTestState(); + s.savedQueries = [savedQuery({ id: 'p1', sql: 'SELECT 1', dashboard: { role: 'panel' } })]; + s.dashboard = blankDashboard(); + const commit = fakeWorkspaceCommit(); + const result = await toggleFavorite(s, 'p1', commit, genTileId()); + expect(result).toMatchObject({ ok: true }); + expect(queryFavorite(s.savedQueries[0])).toBe(true); + expect(s.dashboard!.tiles).toEqual([{ id: 'tile-1', queryId: 'p1' }]); + expect(commit).toHaveBeenCalledTimes(1); + }); + + it('favorite ON is idempotent when a tile already references the query', 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 commit = fakeWorkspaceCommit(); + await toggleFavorite(s, 'p1', commit, genTileId()); + expect(queryFavorite(s.savedQueries[0])).toBe(true); + expect(s.dashboard!.tiles).toEqual([{ id: 't1', queryId: 'p1' }]); // no duplicate + }); + + it('favorite ON on a filter-role query never creates a tile', async () => { + const s = savedTestState(); + s.savedQueries = [savedQuery({ id: 'f1', sql: "SELECT ['a','b'] AS country", dashboard: { role: 'filter' } })]; + s.dashboard = blankDashboard(); + const commit = fakeWorkspaceCommit(); + const result = await toggleFavorite(s, 'f1', commit, genTileId()); + expect(result).toMatchObject({ ok: true }); + expect(queryFavorite(s.savedQueries[0])).toBe(true); + expect(s.dashboard!.tiles).toEqual([]); + }); + + it('favorite OFF removes every tile referencing the query and scrubs filter targets', async () => { + const s = savedTestState(); + s.savedQueries = [ + savedQuery({ id: 'p1', sql: 'SELECT a WHERE c={country:String}', favorite: true, dashboard: { role: 'panel' } }), + savedQuery({ id: 'f1', sql: "SELECT ['a','b'] AS country", dashboard: { role: 'filter' } }), + ]; + s.dashboard = { + ...blankDashboard(), + tiles: [{ id: 't1', queryId: 'p1' }], + filters: [{ id: 'flt', parameter: 'country', sourceQueryId: 'f1', targets: ['t1'] }], + }; + const commit = fakeWorkspaceCommit(); + const result = await toggleFavorite(s, 'p1', commit, genTileId()); + expect(result).toMatchObject({ ok: true }); + expect(queryFavorite(s.savedQueries[0])).toBe(false); + expect(s.dashboard!.tiles).toEqual([]); + expect(s.dashboard!.filters[0].targets).toEqual([]); + expect(commit).toHaveBeenCalledTimes(1); + }); + + it('a null state.dashboard means favorite flip only — no tile change, no crash', async () => { + const s = savedTestState(); + s.savedQueries = [savedQuery({ id: 'p1', sql: 'SELECT 1', dashboard: { role: 'panel' } })]; + expect(s.dashboard).toBeNull(); + const commit = fakeWorkspaceCommit(); + const result = await toggleFavorite(s, 'p1', commit, genTileId()); + expect(result).toMatchObject({ ok: true }); + expect(queryFavorite(s.savedQueries[0])).toBe(true); + expect(s.dashboard).toBeNull(); + }); + }); it('invalid JSON blocks pencil/favorite persistence and identifies the affected tab', async () => { const s = savedTestState(); const tab = s.tabs.value[0]; @@ -358,7 +443,7 @@ describe('saved queries', () => { tab.dirtySpec = true; const commit = fakeWorkspaceCommit(); expect(await renameSaved(s, 's1', 'Overwrite', undefined, commit)).toMatchObject({ ok: false, invalidTab: tab }); - expect(await toggleFavorite(s, 's1', commit)).toMatchObject({ ok: false, invalidTab: tab }); + expect(await toggleFavorite(s, 's1', commit, genTileId())).toMatchObject({ ok: false, invalidTab: tab }); expect(queryName(s.savedQueries[0])).toBe('Original'); expect(queryFavorite(s.savedQueries[0])).toBe(false); expect(commit).not.toHaveBeenCalled(); @@ -373,7 +458,7 @@ describe('saved queries', () => { const entryBlocked: SpecValidationService = { validate: () => [{ path: ['favorite'], severity: 'error', code: 'blocked', message: 'blocked' }], }; - expect(await toggleFavorite(s, 's1', commit, entryBlocked)).toMatchObject({ ok: false, invalidTab: null }); + expect(await toggleFavorite(s, 's1', commit, genTileId(), entryBlocked)).toMatchObject({ ok: false, invalidTab: null }); expect(queryFavorite(s.savedQueries[0])).toBe(false); const draftBlocked: SpecValidationService = { @@ -381,7 +466,7 @@ describe('saved queries', () => { ? [{ path: ['draftOnly'], severity: 'error', code: 'blocked-draft', message: 'blocked draft' }] : [], }; - expect(await toggleFavorite(s, 's1', commit, draftBlocked)).toMatchObject({ ok: false, invalidTab: tab }); + expect(await toggleFavorite(s, 's1', commit, genTileId(), draftBlocked)).toMatchObject({ ok: false, invalidTab: tab }); expect(queryFavorite(s.savedQueries[0])).toBe(false); expect(tab.specParsed!.favorite).toBe(false); expect(commit).not.toHaveBeenCalled(); @@ -412,7 +497,7 @@ describe('saved queries', () => { expect(renamed).toEqual({ ok: false, invalidTab: null, entry: null, diagnostics: expect.any(Array) }); expect(queryName(s.savedQueries[0])).toBe('Original'); - const favorited = await toggleFavorite(s, 's1', failingCommit); + const favorited = await toggleFavorite(s, 's1', failingCommit, genTileId()); expect(favorited).toMatchObject({ ok: false, invalidTab: null, entry: null }); expect(queryFavorite(s.savedQueries[0])).toBe(false); @@ -665,7 +750,7 @@ describe('default persistence', () => { s.tabs.value[0].sqlDraft = 'SELECT 9'; const e = okEntry(await createSavedQuery(s, s.tabs.value[0], 'nine', undefined, commit)); await renameSaved(s, e.id, 'nine!', undefined, commit); - await toggleFavorite(s, e.id, commit); + await toggleFavorite(s, e.id, commit, genTileId()); await deleteSaved(s, 'nope', commit); expect(commit).toHaveBeenCalledTimes(4); expect(s.savedQueries.some((q) => q.id === e.id)).toBe(true); diff --git a/tests/unit/tile-membership.test.ts b/tests/unit/tile-membership.test.ts new file mode 100644 index 0000000..71435d2 --- /dev/null +++ b/tests/unit/tile-membership.test.ts @@ -0,0 +1,104 @@ +import { describe, expect, it } from 'vitest'; +import { toggleTileMembership } from '../../src/dashboard/application/tile-membership.js'; +import type { DashboardDocumentV1, SavedQueryV2 } from '../../src/generated/json-schema.types.js'; + +const panelQuery = (id: string): SavedQueryV2 => ({ + id, sql: 'SELECT 1', specVersion: 1, spec: { name: id, dashboard: { role: 'panel' } }, +} as SavedQueryV2); +const filterQuery = (id: string): SavedQueryV2 => ({ + id, sql: "SELECT ['a'] AS x", specVersion: 1, spec: { name: id, dashboard: { role: 'filter' } }, +} as SavedQueryV2); +const noRoleQuery = (id: string): SavedQueryV2 => ({ + id, sql: 'SELECT 1', specVersion: 1, spec: { name: id }, +} as SavedQueryV2); + +const dashboard = (over: Partial = {}): DashboardDocumentV1 => ({ + documentVersion: 1, id: 'dash', title: 'D', revision: 1, + layout: { type: 'flow', version: 1, preset: 'full-width', items: {} }, + filters: [], tiles: [], ...over, +} as DashboardDocumentV1); + +const genTileId = (): (() => string) => { + let n = 0; + return () => 'tile-' + (++n); +}; + +describe('toggleTileMembership', () => { + it('null dashboard in → null out (no Dashboard yet, favorite flip only)', () => { + expect(toggleTileMembership(null, panelQuery('p1'), true, genTileId())).toBeNull(); + expect(toggleTileMembership(null, panelQuery('p1'), false, genTileId())).toBeNull(); + }); + + it('star ON on a panel-role query with no existing tile appends one', () => { + const next = toggleTileMembership(dashboard(), panelQuery('p1'), true, genTileId())!; + expect(next.tiles).toEqual([{ id: 'tile-1', queryId: 'p1' }]); + }); + + it('a query with no declared role defaults to panel (mirrors queryDashboardRole)', () => { + const next = toggleTileMembership(dashboard(), noRoleQuery('p1'), true, genTileId())!; + expect(next.tiles).toEqual([{ id: 'tile-1', queryId: 'p1' }]); + }); + + it('star ON is idempotent when a tile already references the query', () => { + const d = dashboard({ tiles: [{ id: 't1', queryId: 'p1' }] }); + const next = toggleTileMembership(d, panelQuery('p1'), true, genTileId())!; + expect(next.tiles).toEqual([{ id: 't1', queryId: 'p1' }]); + }); + + it('star ON on a filter-role query creates no tile (favorite flip only)', () => { + const next = toggleTileMembership(dashboard(), filterQuery('f1'), true, genTileId())!; + expect(next.tiles).toEqual([]); + }); + + it('star ON on a setup-role query creates no tile', () => { + const setupQuery: SavedQueryV2 = { + id: 's1', sql: 'CREATE TABLE t (x Int32) ENGINE=Memory', specVersion: 1, + spec: { name: 's1', dashboard: { role: 'setup' } }, + } as SavedQueryV2; + const next = toggleTileMembership(dashboard(), setupQuery, true, genTileId())!; + expect(next.tiles).toEqual([]); + }); + + it('star OFF removes every tile referencing the query and scrubs filter targets', () => { + const d = dashboard({ + tiles: [{ id: 't1', queryId: 'p1' }, { id: 't2', queryId: 'p1' }, { id: 't3', queryId: 'p2' }], + filters: [ + { id: 'flt1', parameter: 'x', targets: ['t1', 't3'] }, + { id: 'flt2', parameter: 'y', targets: ['t2'] }, + { id: 'flt3', parameter: 'z' }, // no targets — untouched + ], + }); + const next = toggleTileMembership(d, panelQuery('p1'), false, genTileId())!; + expect(next.tiles).toEqual([{ id: 't3', queryId: 'p2' }]); + expect(next.filters).toEqual([ + { id: 'flt1', parameter: 'x', targets: ['t3'] }, + { id: 'flt2', parameter: 'y', targets: [] }, + { id: 'flt3', parameter: 'z' }, + ]); + }); + + it('star OFF on a query with no tile is a no-op', () => { + const d = dashboard({ tiles: [{ id: 't1', queryId: 'other' }] }); + const next = toggleTileMembership(d, panelQuery('p1'), false, genTileId())!; + expect(next.tiles).toEqual([{ id: 't1', queryId: 'other' }]); + }); + + it('never mutates the input dashboard', () => { + const d = dashboard({ tiles: [{ id: 't1', queryId: 'p1' }] }); + const snapshot = JSON.parse(JSON.stringify(d)); + toggleTileMembership(d, panelQuery('p1'), false, genTileId()); + expect(d).toEqual(snapshot); + }); + + it('normalizes the result — a removed tile drops its layout placement, a new tile gets none stored', () => { + const d = dashboard({ + layout: { type: 'flow', version: 1, preset: 'full-width', items: { t1: { span: 2, height: 'large' } } }, + tiles: [{ id: 't1', queryId: 'p1' }], + }); + const removed = toggleTileMembership(d, panelQuery('p1'), false, genTileId())!; + expect((removed.layout as { items: Record }).items).toEqual({}); + + const added = toggleTileMembership(dashboard(), panelQuery('p2'), true, genTileId())!; + expect((added.layout as { items: Record }).items).toEqual({}); + }); +}); From aec8dd57b1709fec49589f38fca7b9b9c542fe44 Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Sat, 18 Jul 2026 14:44:45 +0000 Subject: [PATCH 2/3] fix(#300): surface a corrupt-but-present workspace aggregate on boot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The legacy-migration marker keys on raw record existence while loadCurrent() treats an undecodable record as null — so a corrupt-but-present aggregate blocked re-migration AND read as null, letting boot silently continue on the legacy projection until the next CRUD commit orphaned the corrupt record with no user-visible error. WorkspaceRepository gains loadCurrentResult(), distinguishing empty / ok / corrupt (with the codec diagnostics) instead of collapsing empty and corrupt to null; loadCurrent() is untouched. loadWorkspaceOnBoot now uses it: on corrupt it leaves state untouched and flashes a toast with a "Reset workspace" action that clears the unreadable record and rebuilds a fresh aggregate from current local state. Both the OAuth boot (main.ts) and the basic-auth connect action go through loadWorkspaceOnBoot, so both get the surface. flashToast gained an optional, non-auto-dismissing action button. workspace-repository/toast 100% covered; app.ts integration-tested. Closes #300 Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01GLMprUnwnz4oaz3cnLRKcz --- src/ui/app.ts | 79 +++++++++++++++++++------ src/ui/toast.ts | 30 +++++++++- src/workspace/workspace-repository.ts | 27 ++++++++- tests/helpers/fake-app.ts | 3 + tests/unit/app.test.ts | 56 ++++++++++++++++++ tests/unit/toast.test.ts | 51 ++++++++++++++++ tests/unit/workspace-repository.test.ts | 32 ++++++++++ 7 files changed, 258 insertions(+), 20 deletions(-) diff --git a/src/ui/app.ts b/src/ui/app.ts index dd170fc..92f68b1 100644 --- a/src/ui/app.ts +++ b/src/ui/app.ts @@ -1368,24 +1368,34 @@ export function createApp(env: CreateAppEnv = {}): App { // was retired so cap/settings fixes can't apply to only one path. app.renderDashboard = () => renderDashboard(app); + // #286 Phase 4: run the one-shot legacy migration (favorites → tiles, + // dashLayout/dashCols → flow@1) — it only actually does anything when no + // aggregate record exists yet (`migrateLegacyWorkspaceIfNeeded` keys on raw + // record existence via the store, never on `loadCurrent`/`loadCurrentResult` + // validity, so a present-but-corrupt record is never clobbered by a re-run + // — #300). Shared by `loadDashboardWorkspace` below and the boot path + // (`loadWorkspaceOnBoot`) so both read the SAME migration deps built off the + // current `app.state` snapshot. + const runLegacyMigrationIfNeeded = () => migrateLegacyWorkspaceIfNeeded({ + store: workspaceStore, + repository: app.workspace, + legacy: { + name: app.state.libraryName.value, + queries: app.state.savedQueries, + dashLayout: app.state.dashLayout, + dashCols: app.state.dashCols, + }, + genId: () => uid('ws-'), + }); + // #286 Phase 4: resolve the current StoredWorkspaceV1 the Dashboard viewer - // reads from. The one-shot legacy migration (favorites → tiles, - // dashLayout/dashCols → flow@1) runs only when no aggregate exists yet, from - // the flat `asb:*` keys already loaded onto `app.state`; then the aggregate - // is the source of truth. Composition-root wiring — the store lives here, so - // the migration deps are assembled here rather than in the render module. + // reads from. Reads via `loadCurrent` (not `loadCurrentResult`), so a + // corrupt-but-present record still collapses to `null` here — the /dashboard + // route's own render just falls back to its empty state; the user-visible + // corrupt-record surface (#300) lives in the boot path below, which both + // `main.ts`'s OAuth bootstrap and the basic-auth `connect` action go through. app.loadDashboardWorkspace = async () => { - await migrateLegacyWorkspaceIfNeeded({ - store: workspaceStore, - repository: app.workspace, - legacy: { - name: app.state.libraryName.value, - queries: app.state.savedQueries, - dashLayout: app.state.dashLayout, - dashCols: app.state.dashCols, - }, - genId: () => uid('ws-'), - }); + await runLegacyMigrationIfNeeded(); return app.workspace.loadCurrent(); }; @@ -1437,8 +1447,43 @@ export function createApp(env: CreateAppEnv = {}): App { return run; }; + // #300: the Reset action offered on the corrupt-workspace toast below. + // `loadCurrent()`/`clearCurrent()`'s callers rely on a corrupt record + // collapsing to `null`/being silently removable — here that removal is + // deliberate and user-initiated. Clearing makes the store look exactly like + // a fresh install to `migrateLegacyWorkspaceIfNeeded`'s existence check, so + // it rebuilds a brand-new aggregate from the CURRENT legacy/local state + // (favorites, layout prefs) rather than leaving the next random CRUD op to + // silently mint one over nothing. Only projects + re-renders on success — + // an immediately-re-corrupt or still-empty outcome (unexpected; the store + // was just cleared) leaves the legacy `createState()` projection standing, + // same as any other failed load. + const resetCorruptWorkspace = async (): Promise => { + await app.workspace.clearCurrent(); + await runLegacyMigrationIfNeeded(); + const result = await app.workspace.loadCurrentResult(); + if (result.status === 'ok') { + applyCommittedWorkspace(result.workspace); + app.renderApp(); + } + }; + app.loadWorkspaceOnBoot = async () => { - const workspace = await app.loadDashboardWorkspace(); + await runLegacyMigrationIfNeeded(); + const result = await app.workspace.loadCurrentResult(); + if (result.status === 'corrupt') { + // #300: a corrupt-but-present aggregate is surfaced instead of silently + // continuing on the legacy projection (which would otherwise let the + // next saved-query CRUD commit orphan the corrupt record with no + // user-visible error). State is left untouched — same as any other + // null/failed load below. + flashToast( + 'Saved workspace could not be read — your queries and dashboard layout are unaffected until you reset it.', + { document: app.document, action: { label: 'Reset workspace', onClick: () => { void resetCorruptWorkspace(); } } }, + ); + return null; + } + const workspace = result.status === 'ok' ? result.workspace : null; if (workspace) applyCommittedWorkspace(workspace); return workspace; }; diff --git a/src/ui/toast.ts b/src/ui/toast.ts index 8be0e35..eb61b89 100644 --- a/src/ui/toast.ts +++ b/src/ui/toast.ts @@ -9,6 +9,12 @@ export interface ToastOptions { setTimeout?: (handler: () => void, ms: number) => number; clearTimeout?: (id: number) => void; duration?: number; + /** #300: an optional recovery action rendered as a button inside the toast. + * When present, the toast does NOT auto-dismiss (the timer is skipped + * entirely) so the user has time to act; clicking the button runs + * `onClick` then dismisses. Absent, the toast behaves exactly as before + * (text-only, auto-dismisses after `duration`). */ + action?: { label: string; onClick: () => void }; } /** The live toast element, with its own pending-timer id stashed on it (not @@ -34,13 +40,33 @@ export function flashToast(text: string, opts: ToastOptions = {}): HTMLElement { doc.body.appendChild(el); } const toast = el; + // Resets `textContent` (and so wipes any action button a previous call on + // this reused element appended) before this call's own button, if any, is + // (re-)appended below. toast.textContent = text; toast.classList.add('show'); // Timer lives on the element (not the function) so a toast in one document // (e.g. a detached tab's own window) can't clear or clobber a pending timer // that belongs to a toast in a different document's realm. - if (toast._timer) clearTimer(toast._timer); - toast._timer = setTimer(() => { toast._timer = null; toast.classList.remove('show'); }, duration); + if (toast._timer) { clearTimer(toast._timer); toast._timer = null; } + if (opts.action) { + const { label, onClick } = opts.action; + const button = doc.createElement('button'); + button.type = 'button'; + button.className = 'share-toast-action'; + button.textContent = label; + // Stop propagation so the action doesn't also trigger the body's + // click-to-dismiss handler (below) before `onClick` runs. + button.onclick = (event) => { + event.stopPropagation(); + toast.classList.remove('show'); + onClick(); + }; + toast.appendChild(button); + // No auto-dismiss timer: an actionable toast waits for the user. + } else { + toast._timer = setTimer(() => { toast._timer = null; toast.classList.remove('show'); }, duration); + } // Click to dismiss early / reread — rebound each call so it always clears // *this* call's timer (the element is reused across calls, opts may differ). toast.onclick = () => { diff --git a/src/workspace/workspace-repository.ts b/src/workspace/workspace-repository.ts index 8d8e788..4121941 100644 --- a/src/workspace/workspace-repository.ts +++ b/src/workspace/workspace-repository.ts @@ -31,6 +31,18 @@ export type WorkspaceCommitResult = | { ok: true; workspace: StoredWorkspaceV1; dashboardRevision: number | null } | { ok: false; diagnostics: WorkspaceDiagnostic[] }; +/** The #300 tri-state load result `loadCurrentResult` returns — unlike + * `loadCurrent` (below), it distinguishes "no record persisted yet" (`empty`) + * from "a record is persisted but doesn't decode/validate" (`corrupt`, with + * the codec's diagnostics) from a normal successful load (`ok`). Boot-time + * callers that need to surface a corrupt-but-present record to the user + * (rather than silently continuing on the legacy projection) use this + * instead of `loadCurrent`. */ +export type WorkspaceLoadResult = + | { status: 'empty' } + | { status: 'ok'; workspace: StoredWorkspaceV1 } + | { status: 'corrupt'; diagnostics: WorkspaceDiagnostic[] }; + /** The #280 repository contract. Every method is async because the backing * store (IndexedDB) is async. */ export interface WorkspaceRepository { @@ -40,6 +52,10 @@ export interface WorkspaceRepository { * existence via the store, not on this method, so it never re-runs over a * corrupt-but-present record. */ loadCurrent(): Promise; + /** Like `loadCurrent`, but distinguishes "no record" from "record present + * but undecodable" (#300) instead of collapsing both to `null` — see + * `WorkspaceLoadResult`. */ + loadCurrentResult(): Promise; /** Validate the complete candidate, then atomically replace the persisted * aggregate. Publishes committed state only after the write succeeds. */ commit(candidate: StoredWorkspaceV1): Promise; @@ -73,6 +89,15 @@ export function createWorkspaceRepository(deps: WorkspaceRepositoryDeps): Worksp return decoded.ok ? decoded.value : null; } + async function loadCurrentResult(): Promise { + const text = await store.read(); + if (text === null) return { status: 'empty' }; + const decoded = decodeStoredWorkspaceJson(text, codecOptions); + return decoded.ok + ? { status: 'ok', workspace: decoded.value } + : { status: 'corrupt', diagnostics: decoded.diagnostics }; + } + async function commit(candidate: StoredWorkspaceV1): Promise { // Validate + canonically encode the WHOLE candidate before touching the // store; invalid candidates never reach persistence. @@ -104,5 +129,5 @@ export function createWorkspaceRepository(deps: WorkspaceRepositoryDeps): Worksp return store.clear(); } - return { loadCurrent, commit, clearCurrent }; + return { loadCurrent, loadCurrentResult, commit, clearCurrent }; } diff --git a/tests/helpers/fake-app.ts b/tests/helpers/fake-app.ts index 412fcda..6b54946 100644 --- a/tests/helpers/fake-app.ts +++ b/tests/helpers/fake-app.ts @@ -279,6 +279,9 @@ const appDefaults: App = { // with `fakeWorkspaceCommit()` (or its own stub) instead. workspace: { loadCurrent: async () => null, + // #300: default mirrors `loadCurrent`'s own default (no record) — a + // fixture testing the corrupt-record surface overrides this directly. + loadCurrentResult: async () => ({ status: 'empty' }), commit: async (candidate) => ({ ok: true, workspace: candidate, dashboardRevision: candidate.dashboard === null ? null : candidate.dashboard.revision, }), diff --git a/tests/unit/app.test.ts b/tests/unit/app.test.ts index c479fb1..9d8f8f8 100644 --- a/tests/unit/app.test.ts +++ b/tests/unit/app.test.ts @@ -2992,6 +2992,62 @@ describe('share + star + columns', () => { expect(app2.state.workspaceId).toBe(beforeId2); expect(app2.state.dashboard).toBeNull(); }); + it('#300: a corrupt-but-present aggregate surfaces a toast instead of silently continuing, and its Reset action rebuilds a fresh one', async () => { + const app = createApp(env()); + const beforeId = app.state.workspaceId; + const beforeQueries = app.state.savedQueries; + const diagnostics = [{ + path: ['storageVersion'], severity: 'error' as const, + code: 'workspace-version-unsupported', message: 'Unsupported stored-workspace version', + }]; + // Simulate a corrupt-but-present record without hand-rolling raw + // IndexedDB bytes: stub `loadCurrentResult` to report `corrupt` while + // `corrupted` is true, delegating to the REAL (fake-IndexedDB-backed) + // implementation once it flips false — so Reset's rebuild below exercises + // the genuine migrate-then-load path, not another stub. + let corrupted = true; + const realLoadCurrentResult = app.workspace.loadCurrentResult.bind(app.workspace); + app.workspace.loadCurrentResult = vi.fn( + async () => (corrupted ? { status: 'corrupt' as const, diagnostics } : realLoadCurrentResult()), + ); + const clearSpy = vi.spyOn(app.workspace, 'clearCurrent'); + + const workspace = await app.loadWorkspaceOnBoot(); + expect(workspace).toBeNull(); + // State is left exactly as `createState()`'s synchronous legacy + // projection populated it — not overwritten by the corrupt record. + expect(app.state.workspaceId).toBe(beforeId); + expect(app.state.savedQueries).toBe(beforeQueries); + + const toastEl = qs(document, '.share-toast'); + expect(toastEl.textContent).toContain('Saved workspace could not be read'); + const resetBtn = qs(toastEl, 'button.share-toast-action'); + expect(resetBtn.textContent).toBe('Reset workspace'); + expect(clearSpy).not.toHaveBeenCalled(); + + // Invoke the Reset action: clears the (real) store, then re-runs the + // migrate + load path, which now genuinely resolves `ok` and projects. + corrupted = false; + resetBtn.click(); + await flush(); + + expect(clearSpy).toHaveBeenCalledTimes(1); + const rebuilt = await app.workspace.loadCurrent(); + expect(rebuilt).not.toBeNull(); + expect(app.state.workspaceId).toBe(rebuilt!.id); + expect(app.state.workspaceId).not.toBe(beforeId); + }); + it('#300: the empty and ok load-result cases behave exactly as before (no toast, migrate-then-project runs as usual)', async () => { + const app = createApp(env()); + // `env()`'s own #287 default fake IndexedDB: a working store with nothing + // persisted yet resolves `empty`, so `loadWorkspaceOnBoot` migrates and + // projects the freshly-committed aggregate exactly as the pre-#300 test + // above (line ~2963) already covers for `loadCurrent`. + const workspace = await app.loadWorkspaceOnBoot(); + expect(workspace).not.toBeNull(); + expect(app.state.savedQueries).toEqual(workspace!.queries); + expect(document.querySelector('.share-toast')).toBeNull(); + }); it('save popover closes on click outside', () => { const app = createApp(env()); app.renderApp(); diff --git a/tests/unit/toast.test.ts b/tests/unit/toast.test.ts index d63e34b..bd3cf8f 100644 --- a/tests/unit/toast.test.ts +++ b/tests/unit/toast.test.ts @@ -76,4 +76,55 @@ describe('flashToast', () => { expect(otherClear).toHaveBeenCalledWith(2); expect(mainEl.classList.contains('show')).toBe(true); }); + + describe('action button (#300)', () => { + it('renders a button with the given label and does not schedule an auto-dismiss timer', () => { + const setTimeout = fakeSetTimeout(1); + const onClick = vi.fn(); + const el = flashToast('corrupt workspace', { + document, setTimeout, action: { label: 'Reset workspace', onClick }, + }); + const button = el.querySelector('button.share-toast-action') as HTMLButtonElement | null; + expect(button).not.toBeNull(); + expect(button!.textContent).toBe('Reset workspace'); + expect(el.textContent).toContain('corrupt workspace'); + expect(setTimeout).not.toHaveBeenCalled(); + }); + + it('clicking the action button runs onClick, dismisses the toast, and does not also fire the body dismiss handler', () => { + const clearTimeout = vi.fn(); + const onClick = vi.fn(); + const el = flashToast('corrupt workspace', { + document, clearTimeout, action: { label: 'Reset workspace', onClick }, + }); + const button = el.querySelector('button.share-toast-action') as HTMLButtonElement; + button.click(); + expect(onClick).toHaveBeenCalledTimes(1); + expect(el.classList.contains('show')).toBe(false); + // No pending timer was ever set for an actionable toast, so a body + // click-to-dismiss never has anything to clear. + expect(clearTimeout).not.toHaveBeenCalled(); + }); + + it('a later text-only flashToast on the same reused element clears the previous action button', () => { + const el = flashToast('corrupt workspace', { + document, action: { label: 'Reset workspace', onClick: vi.fn() }, + }); + expect(el.querySelector('button.share-toast-action')).not.toBeNull(); + const el2 = flashToast('plain message', { document, setTimeout: fakeSetTimeout(3) }); + expect(el2).toBe(el); + expect(el2.querySelector('button.share-toast-action')).toBeNull(); + expect(el2.textContent).toBe('plain message'); + }); + + it('clicking the toast body (not the button) still dismisses an actionable toast without invoking onClick', () => { + const onClick = vi.fn(); + const el = flashToast('corrupt workspace', { + document, action: { label: 'Reset workspace', onClick }, + }); + el.click(); + expect(onClick).not.toHaveBeenCalled(); + expect(el.classList.contains('show')).toBe(false); + }); + }); }); diff --git a/tests/unit/workspace-repository.test.ts b/tests/unit/workspace-repository.test.ts index e877a34..5d711a4 100644 --- a/tests/unit/workspace-repository.test.ts +++ b/tests/unit/workspace-repository.test.ts @@ -69,6 +69,38 @@ describe('createWorkspaceRepository.loadCurrent', () => { }); }); +describe('createWorkspaceRepository.loadCurrentResult', () => { + it('reports empty when no aggregate record exists', async () => { + const repo = createWorkspaceRepository({ store: memStore(null) }); + expect(await repo.loadCurrentResult()).toEqual({ status: 'empty' }); + }); + + it('reports ok with the decoded workspace when a valid record is stored', async () => { + const ws = withDashboard(); + const encoded = encodeStoredWorkspaceJson(ws); + if (!encoded.ok) throw new Error('fixture should encode'); + const repo = createWorkspaceRepository({ store: memStore(encoded.value) }); + expect(await repo.loadCurrentResult()).toEqual({ status: 'ok', workspace: ws }); + }); + + it('reports corrupt with diagnostics when a present record fails to decode/validate (never collapses to empty)', async () => { + const repo = createWorkspaceRepository({ store: memStore('{"storageVersion":2}') }); + const result = await repo.loadCurrentResult(); + expect(result.status).toBe('corrupt'); + if (result.status !== 'corrupt') throw new Error('unreachable'); + expect(result.diagnostics.length).toBeGreaterThan(0); + expect(result.diagnostics.some((d) => d.code === 'workspace-version-unsupported')).toBe(true); + }); + + it('reports corrupt for text that is not even valid JSON', async () => { + const repo = createWorkspaceRepository({ store: memStore('not json{') }); + const result = await repo.loadCurrentResult(); + expect(result.status).toBe('corrupt'); + if (result.status !== 'corrupt') throw new Error('unreachable'); + expect(result.diagnostics.length).toBeGreaterThan(0); + }); +}); + describe('createWorkspaceRepository.commit', () => { it('validates the whole candidate BEFORE writing; an invalid one is never persisted', async () => { const store = memStore(null); From b4963cf4a2816c1b02c43ced475577283ffa84a5 Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Sat, 18 Jul 2026 14:44:59 +0000 Subject: [PATCH 3/3] fix(#303): persist Dashboard filter values across reload (isolated store) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Regression from the #280 dashboard rewrite: the isolated viewer session initialized every filter purely from def.defaultValue/defaultActive and persisted nothing, so a committed filter value lived only in memory and reset on reload (the Workbench var-strip, which persists asb:varValues/filterActive, was unaffected). Filter runtime value/active now persist to an ISOLATED per-dashboard key (KEYS.dashFilters = asb:dashFilters, dashboardId -> filterId -> {value,active}) — deliberately separate from the Workbench keys (Option B) — and are seeded back on load via a new DashboardViewerDeps.initialFilters. The read/write/merge logic is a pure dashboard/model store (dashboard-filter-store.ts, 100% covered) behind the existing loadJSON/app.saveJSON seam, so the viewer session stays AppState/storage-free (check:arch clean). The persist step gates on a dedicated value/active signature seeded from the session's own initial filter state, so opening a dashboard never writes its defaults back (which would freeze them against later Spec-editor default changes) — only genuine committed changes persist. Closes #303 Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01GLMprUnwnz4oaz3cnLRKcz --- CHANGELOG.md | 30 +++++ .../application/dashboard-viewer-session.ts | 19 ++- src/dashboard/model/dashboard-filter-store.ts | 86 +++++++++++++ src/ui/dashboard.ts | 41 +++++++ tests/unit/dashboard-filter-store.test.ts | 113 ++++++++++++++++++ tests/unit/dashboard-viewer-session.test.ts | 57 +++++++++ tests/unit/dashboard.test.ts | 105 +++++++++++++++- 7 files changed, 448 insertions(+), 3 deletions(-) create mode 100644 src/dashboard/model/dashboard-filter-store.ts create mode 100644 tests/unit/dashboard-filter-store.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 631d864..3c33709 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,36 @@ auto-generated per-PR notes; this file is the curated, human-readable history. never executes queries. ### Fixed +- **The Workbench favorite star now drives Dashboard tile membership** (part of + #299, post-#287 aggregate). Since `dashboard.tiles[]` became the canonical + Dashboard membership (#280/#287), starring a query only flipped `spec.favorite` + and never added a tile, so favoriting/unfavoriting in the Workbench had no + effect on the Dashboard. `toggleFavorite` now reflects the flip onto tile + membership in the **same atomic commit**: a favorited **panel-role** query gains + a tile (mirroring the legacy migration's role gate — a filter/setup-role + favorite stays favorite-only), and unfavoriting removes every tile referencing + the query and scrubs those tile ids from filter targets. `spec.favorite` stays + the star's visual state (the documented dual-write retirement is deferred). +- **A corrupt-but-present workspace aggregate is surfaced on boot instead of + silently swallowed** (#300). `WorkspaceRepository` gains `loadCurrentResult()`, + which distinguishes "no aggregate yet" (`empty`) from "a record is stored but + won't decode/validate" (`corrupt`) — previously both collapsed to `null`, so a + corrupt record let boot continue on the legacy projection and the next CRUD + commit orphaned it with no user-visible error. Boot now shows a toast ("Saved + workspace could not be read…") with a **Reset workspace** action that clears the + unreadable record and rebuilds a fresh aggregate from the current local state. + `flashToast` gained an optional non-auto-dismissing action button. +- **Dashboard filter values persist across a reload again** (#303, regression from + the #280 dashboard rewrite). The isolated viewer session initialized every + filter purely from `def.defaultValue`/`defaultActive` and persisted nothing, so + a committed filter value lived only in memory and reset on reload (the Workbench + var-strip was unaffected). Filter runtime value/active now persist to an + **isolated** per-dashboard key (`asb:dashFilters`, `dashboardId → filterId → + {value,active}`) — deliberately separate from the Workbench's + `asb:varValues`/`asb:filterActive` — seeded back on load. Persistence stays a + pure `dashboard/model` store behind the existing `loadJSON`/`saveJSON` seam; + opening a dashboard does not persist its defaults (only genuine committed + changes write). - **Dashboard filter strips no longer wrap, the visible "Clear all" control and "N active" count are both removed, and the layout switcher moved to the header as a compact select** (#294 + a same-PR 2026-07-18 owner follow-up — diff --git a/src/dashboard/application/dashboard-viewer-session.ts b/src/dashboard/application/dashboard-viewer-session.ts index 6f35663..e53616d 100644 --- a/src/dashboard/application/dashboard-viewer-session.ts +++ b/src/dashboard/application/dashboard-viewer-session.ts @@ -156,6 +156,14 @@ export interface DashboardViewerDeps { onAuthFailed?(): void; /** #171 bound-param recording on a successful tile. */ recordBoundParams?(boundParams: BoundParamSnapshot[]): void; + /** #303: persisted per-filter seed, keyed by filter `def.id` — the shell + * reads this from the isolated `asb:dashFilters` store (never this layer; + * this session stays storage-free) and passes it in so a filter's initial + * runtime value/active reflects the last COMMITTED state instead of always + * resetting to `def.defaultValue`/`defaultActive`. A filter with no entry + * here (absent/empty map, or no key for its `def.id`) is unaffected — its + * initial state is exactly the pre-#303 default-derived behavior. */ + initialFilters?: Record; } export interface DashboardViewerSession { @@ -314,10 +322,17 @@ export function createDashboardViewerSession(deps: DashboardViewerDeps): Dashboa // Filter runtime records, in filter order. const filters: FilterRuntime[] = (Array.isArray(documentRef.filters) ? documentRef.filters : []).map((def) => { const source = typeof def.sourceQueryId === 'string' ? queryById.get(def.sourceQueryId) : undefined; - const active = def.defaultActive ?? (def.defaultValue != null && def.defaultValue !== ''); + const defaultValue = def.defaultValue ?? ''; + const defaultActive = def.defaultActive ?? (def.defaultValue != null && def.defaultValue !== ''); + // #303: a persisted seed for this filter's id overrides the pure-default + // init above (untouched when `initialFilters` is absent/empty, or has no + // entry for `def.id`). + const seed = deps.initialFilters ? deps.initialFilters[def.id] : undefined; + const value = seed !== undefined ? (seed.value ?? defaultValue) : defaultValue; + const active = seed !== undefined ? !!seed.active : defaultActive; const state: ViewerFilterState = { id: def.id, parameter: def.parameter, label: def.label || def.parameter, - active, value: def.defaultValue ?? '', status: 'idle', options: null, + active, value, status: 'idle', options: null, }; return { def, source, gen: 0, abortController: null, provider: null, state }; }); diff --git a/src/dashboard/model/dashboard-filter-store.ts b/src/dashboard/model/dashboard-filter-store.ts new file mode 100644 index 0000000..6ba3705 --- /dev/null +++ b/src/dashboard/model/dashboard-filter-store.ts @@ -0,0 +1,86 @@ +// Isolated per-dashboard Dashboard-filter persistence (#303 Option B). The +// #280 viewer session initializes every filter's runtime state purely from +// `def.defaultValue`/`defaultActive` and never reads persisted values, so a +// committed filter value only lived in memory and reset to defaults on +// reload. The fix is ONE localStorage key (`state.ts`'s `KEYS.dashFilters`) +// holding a map of `dashboardId -> filterId -> { value, active }` — deliberately +// isolated from the Workbench's `asb:varValues`/`asb:filterActive` keys (a +// Dashboard filter is a distinct persisted concern, not a var-strip mirror). +// +// Pure by construction (no DOM, no globals, no storage access of its own): +// the shell (`src/ui/dashboard.ts`) is the only caller that touches +// `core/storage.js`'s `loadJSON`/`saveJSON`; this module only ever receives +// and returns already-parsed JSON values, so it stays testable without a +// storage seam of its own and satisfies the `src/dashboard/model` boundary +// rule (no `state.ts`/`core/storage.js` import here). + +/** One filter's persisted runtime state. */ +export interface DashboardFilterEntry { + value: string; + active: boolean; +} + +/** One dashboard's persisted filter bag, keyed by filter `def.id`. */ +export type DashboardFilterBag = Record; + +/** The whole persisted blob, keyed by `dashboard.id`. */ +export type AllDashboardFilters = Record; + +const isObject = (value: unknown): value is Record => + !!value && typeof value === 'object' && !Array.isArray(value); + +const coerceValue = (value: unknown): string => + (typeof value === 'string' ? value : value == null ? '' : String(value)); + +/** + * Defensively parse an untrusted blob (whatever `loadJSON(KEYS.dashFilters, {})` + * returned) into one dashboard's filter bag. Tolerates a non-object blob, a + * missing dashboard entry, and junk per-filter entries (dropped rather than + * thrown); a present entry has its `value` coerced to a string and its + * `active` coerced to a boolean. Returns `{}` when nothing valid is found. + */ +export function readDashboardFilterBag(all: unknown, dashboardId: string): DashboardFilterBag { + if (!isObject(all)) return {}; + const dashboard = all[dashboardId]; + if (!isObject(dashboard)) return {}; + const out: DashboardFilterBag = {}; + for (const [filterId, entry] of Object.entries(dashboard)) { + if (!isObject(entry)) continue; // junk entry (string/number/array/null) — drop + out[filterId] = { value: coerceValue(entry.value), active: !!entry.active }; + } + return out; +} + +/** Shallow clone of a bag (defends both `writeDashboardFilterBag`'s input and + * its output against later in-place mutation by either side). */ +function cloneBag(bag: DashboardFilterBag): DashboardFilterBag { + const out: DashboardFilterBag = {}; + for (const [filterId, entry] of Object.entries(bag)) out[filterId] = { value: entry.value, active: entry.active }; + return out; +} + +/** + * Return a NEW all-dashboards map with `dashboardId`'s bag replaced by `bag`, + * preserving every other dashboard's entry untouched. Never mutates `all` or + * `bag`. Starts from `{}` when `all` isn't a valid object (first write, or a + * corrupt blob). + */ +export function writeDashboardFilterBag( + all: unknown, dashboardId: string, bag: DashboardFilterBag, +): AllDashboardFilters { + const out: AllDashboardFilters = {}; + if (isObject(all)) { + for (const [id, value] of Object.entries(all)) { + if (id !== dashboardId) out[id] = value as DashboardFilterBag; + } + } + out[dashboardId] = cloneBag(bag); + return out; +} + +/** A stable signature for a bag (sorted-key JSON) so a caller can skip a + * redundant write when nothing has actually changed since the last publish. */ +export function filterBagSignature(bag: DashboardFilterBag): string { + const ids = Object.keys(bag).sort(); + return JSON.stringify(ids.map((id) => [id, bag[id].value, bag[id].active])); +} diff --git a/src/ui/dashboard.ts b/src/ui/dashboard.ts index 977d1ea..7243d50 100644 --- a/src/ui/dashboard.ts +++ b/src/ui/dashboard.ts @@ -52,6 +52,12 @@ import { defaultLayoutRegistry } from '../dashboard/layouts/layout-registry.js'; import { flowLayoutPlugin } from '../dashboard/layouts/flow-layout.js'; import { applyCommand } from '../dashboard/application/dashboard-commands.js'; import { createQueryResolver } from '../dashboard/application/dashboard-query-resolver.js'; +import { + readDashboardFilterBag, writeDashboardFilterBag, filterBagSignature, +} from '../dashboard/model/dashboard-filter-store.js'; +import type { DashboardFilterBag } from '../dashboard/model/dashboard-filter-store.js'; +import { loadJSON } from '../core/storage.js'; +import { KEYS } from '../state.js'; import type { DashboardDocumentV1, DashboardFilterDefinitionV1, FlowPresetV1, SavedQueryV2, StoredWorkspaceV1, } from '../generated/json-schema.types.js'; @@ -91,6 +97,8 @@ export interface DashboardApp { params: Pick; workspace: Pick; loadDashboardWorkspace(): Promise; + /** #303: persists the isolated per-dashboard filter store (`KEYS.dashFilters`). */ + saveJSON(key: string, value: unknown): void; } const valueString = (value: unknown): string => @@ -194,6 +202,14 @@ export async function renderDashboard(app: DashboardApp): Promise { ...currentDoc, filters: [...(currentDoc.filters || []), ...synthesizeImplicitFilters(currentDoc, queryById)], }; + // #303: seed each filter's initial value/active from the isolated + // per-dashboard store (never the Workbench's asb:varValues/asb:filterActive + // keys) — restores committed filter state across a reload. `initialBag` is + // ALSO the baseline the persist effect below compares against, so the very + // first publish (which merely echoes this seed) does not immediately write + // defaults back over it. + const initialBag: DashboardFilterBag = readDashboardFilterBag(loadJSON(KEYS.dashFilters, {}), currentDoc.id); + const session: DashboardViewerSession = createDashboardViewerSession({ document: viewerDoc, queries, @@ -205,6 +221,7 @@ export async function renderDashboard(app: DashboardApp): Promise { isMobile: () => state.isMobile.value, onAuthFailed: () => app.conn.chCtx.onSignedOut(), recordBoundParams: (bp) => app.params.recordBoundParams(bp), + initialFilters: initialBag, }); // ── Header chrome ─────────────────────────────────────────────────────── @@ -459,6 +476,22 @@ export async function renderDashboard(app: DashboardApp): Promise { // ── Effect: reconcile on every publish (and on the mobile-breakpoint flip) ─ let lastMobile = state.isMobile.value; let barSig = ''; + // #303: the committed-filter bag for a published view, built exactly the way + // the persist step below and the seed just under it both need it. + const persistBagOf = (filters: readonly ViewerFilterState[]): DashboardFilterBag => { + const bag: DashboardFilterBag = {}; + for (const f of filters) bag[f.id] = { value: valueString(f.value), active: f.active }; + return bag; + }; + // #303: a SEPARATE signature from `barSig` above — that one also flips when + // curated options arrive (no committed value/active change), which would + // otherwise trigger a redundant write. Seeded from the session's OWN initial + // filter state (post-`initialFilters` seeding + defaults), not the raw stored + // `initialBag`, so the very first publish — which merely echoes that state — + // never writes: an empty/partial store would otherwise differ from the + // default-filled published bag and persist defaults on first open, freezing + // them against later Spec-editor changes to a filter's default. + let lastFilterPersistSig = filterBagSignature(persistBagOf(session.state.value.filters)); effect(() => { const sview = session.state.value; const mobileNow = state.isMobile.value; // tracked so a breakpoint flip re-runs the effect @@ -471,6 +504,14 @@ export async function renderDashboard(app: DashboardApp): Promise { // progress ticks — so in-progress typing is not disturbed mid-wave. const sig = JSON.stringify(sview.filters.map((f) => [f.id, f.active, valueString(f.value), !!(f.options && f.options.length)])); if (sig !== barSig) { barSig = sig; rebuildFilterBar(sview); } + // #303: persist committed filter value/active into the isolated per-dashboard + // store — isolated from the Workbench's asb:varValues/asb:filterActive keys. + const filterBag = persistBagOf(sview.filters); + const persistSig = filterBagSignature(filterBag); + if (persistSig !== lastFilterPersistSig) { + lastFilterPersistSig = persistSig; + app.saveJSON(KEYS.dashFilters, writeDashboardFilterBag(loadJSON(KEYS.dashFilters, {}), currentDoc.id, filterBag)); + } tileCountLabel.textContent = sview.tiles.length + (sview.tiles.length === 1 ? ' tile' : ' tiles'); empty.style.display = sview.tiles.length ? 'none' : ''; // Genuine dashboard-config diagnostics only (a tile whose presentation diff --git a/tests/unit/dashboard-filter-store.test.ts b/tests/unit/dashboard-filter-store.test.ts new file mode 100644 index 0000000..f05c699 --- /dev/null +++ b/tests/unit/dashboard-filter-store.test.ts @@ -0,0 +1,113 @@ +import { describe, expect, it } from 'vitest'; +import { + readDashboardFilterBag, writeDashboardFilterBag, filterBagSignature, +} from '../../src/dashboard/model/dashboard-filter-store.js'; +import type { DashboardFilterBag } from '../../src/dashboard/model/dashboard-filter-store.js'; + +describe('readDashboardFilterBag', () => { + it('round-trips a valid bag for the dashboard id', () => { + const all = { d1: { f1: { value: 'x', active: true }, f2: { value: '', active: false } } }; + expect(readDashboardFilterBag(all, 'd1')).toEqual({ + f1: { value: 'x', active: true }, + f2: { value: '', active: false }, + }); + }); + + it('returns {} for a non-object blob (string, number, array, null, undefined)', () => { + expect(readDashboardFilterBag('nope', 'd1')).toEqual({}); + expect(readDashboardFilterBag(42, 'd1')).toEqual({}); + expect(readDashboardFilterBag([1, 2], 'd1')).toEqual({}); + expect(readDashboardFilterBag(null, 'd1')).toEqual({}); + expect(readDashboardFilterBag(undefined, 'd1')).toEqual({}); + }); + + it('returns {} when the dashboard id is missing from the blob', () => { + expect(readDashboardFilterBag({ other: { f1: { value: 'x', active: true } } }, 'd1')).toEqual({}); + }); + + it('returns {} when the dashboard entry itself is junk (not an object)', () => { + expect(readDashboardFilterBag({ d1: 'nope' }, 'd1')).toEqual({}); + expect(readDashboardFilterBag({ d1: ['x'] }, 'd1')).toEqual({}); + expect(readDashboardFilterBag({ d1: null }, 'd1')).toEqual({}); + }); + + it('drops junk per-filter entries (non-object) while keeping healthy siblings', () => { + const all = { d1: { f1: { value: 'x', active: true }, f2: 'junk', f3: 42, f4: ['j'], f5: null } }; + expect(readDashboardFilterBag(all, 'd1')).toEqual({ f1: { value: 'x', active: true } }); + }); + + it('coerces value to string and active to boolean (number/boolean/missing/nullish)', () => { + const all = { + d1: { + num: { value: 5, active: 1 }, + boolValue: { value: true, active: 0 }, + missingValue: { active: true }, + nullValue: { value: null, active: true }, + undefinedActive: { value: 'v' }, + }, + }; + expect(readDashboardFilterBag(all, 'd1')).toEqual({ + num: { value: '5', active: true }, + boolValue: { value: 'true', active: false }, + missingValue: { value: '', active: true }, + nullValue: { value: '', active: true }, + undefinedActive: { value: 'v', active: false }, + }); + }); +}); + +describe('writeDashboardFilterBag', () => { + it('replaces the named dashboard bag and preserves other dashboards', () => { + const all = { d1: { f1: { value: 'old', active: false } }, d2: { g1: { value: 'keep', active: true } } }; + const next = writeDashboardFilterBag(all, 'd1', { f1: { value: 'new', active: true } }); + expect(next).toEqual({ + d1: { f1: { value: 'new', active: true } }, + d2: { g1: { value: 'keep', active: true } }, + }); + }); + + it('starts from {} when `all` is not a valid object', () => { + expect(writeDashboardFilterBag('nope', 'd1', { f1: { value: 'v', active: true } })) + .toEqual({ d1: { f1: { value: 'v', active: true } } }); + expect(writeDashboardFilterBag(null, 'd1', {})).toEqual({ d1: {} }); + expect(writeDashboardFilterBag(undefined, 'd1', {})).toEqual({ d1: {} }); + }); + + it('adds a brand-new dashboard id without any prior entries', () => { + expect(writeDashboardFilterBag({}, 'd1', { f1: { value: 'v', active: true } })) + .toEqual({ d1: { f1: { value: 'v', active: true } } }); + }); + + it('never mutates the input `all` object or the input `bag`', () => { + const all = { d1: { f1: { value: 'old', active: false } } }; + const allSnapshot = JSON.parse(JSON.stringify(all)); + const bag: DashboardFilterBag = { f1: { value: 'new', active: true } }; + const bagSnapshot = JSON.parse(JSON.stringify(bag)); + const next = writeDashboardFilterBag(all, 'd1', bag); + expect(all).toEqual(allSnapshot); + expect(bag).toEqual(bagSnapshot); + // The written entry is a genuine copy, not a shared reference — mutating + // the source bag's entry after the call must not affect the stored result. + bag.f1.value = 'mutated-after'; + expect(next.d1.f1.value).toBe('new'); + }); +}); + +describe('filterBagSignature', () => { + it('is stable regardless of key insertion order', () => { + const a: DashboardFilterBag = { b: { value: '2', active: true }, a: { value: '1', active: false } }; + const b: DashboardFilterBag = { a: { value: '1', active: false }, b: { value: '2', active: true } }; + expect(filterBagSignature(a)).toBe(filterBagSignature(b)); + }); + + it('differs when a value or active flag differs', () => { + const base: DashboardFilterBag = { a: { value: '1', active: false } }; + expect(filterBagSignature(base)).not.toBe(filterBagSignature({ a: { value: '2', active: false } })); + expect(filterBagSignature(base)).not.toBe(filterBagSignature({ a: { value: '1', active: true } })); + }); + + it('differs when the key set differs, and matches for two empty bags', () => { + expect(filterBagSignature({})).toBe(filterBagSignature({})); + expect(filterBagSignature({})).not.toBe(filterBagSignature({ a: { value: '', active: false } })); + }); +}); diff --git a/tests/unit/dashboard-viewer-session.test.ts b/tests/unit/dashboard-viewer-session.test.ts index ea61192..11eaed0 100644 --- a/tests/unit/dashboard-viewer-session.test.ts +++ b/tests/unit/dashboard-viewer-session.test.ts @@ -208,6 +208,63 @@ describe('createDashboardViewerSession', () => { }); }); +// #303: `initialFilters` seeds each filter's runtime value/active from a +// persisted bag (the shell's isolated per-dashboard store) instead of always +// deriving it from `def.defaultValue`/`defaultActive`. These assert on the +// session's initial `state.value.filters` BEFORE `start()` — the seed is +// applied at construction time, no query execution required. +describe('initialFilters seeding (#303)', () => { + const seededDoc = () => doc({ + filters: [ + { id: 'f1', parameter: 'p1', defaultValue: 'D1', defaultActive: false }, + { id: 'f2', parameter: 'p2', defaultValue: 'D2', defaultActive: true }, + ], + }); + const byId = (session: ReturnType, id: string) => + session.state.value.filters.find((f) => f.id === id)!; + + it('starts a seeded filter with its persisted value+active, overriding the definition defaults', () => { + const session = createDashboardViewerSession(makeDeps({ + document: seededDoc(), initialFilters: { f1: { value: 'seeded', active: true } }, + })); + expect(byId(session, 'f1')).toMatchObject({ value: 'seeded', active: true }); + }); + + it('leaves an unseeded filter (absent from the map) on its definition defaults', () => { + const session = createDashboardViewerSession(makeDeps({ + document: seededDoc(), initialFilters: { f1: { value: 'seeded', active: true } }, + })); + expect(byId(session, 'f2')).toMatchObject({ value: 'D2', active: true }); + }); + + it('behaves identically when initialFilters is absent', () => { + const session = createDashboardViewerSession(makeDeps({ document: seededDoc() })); + expect(byId(session, 'f1')).toMatchObject({ value: 'D1', active: false }); + expect(byId(session, 'f2')).toMatchObject({ value: 'D2', active: true }); + }); + + it('behaves identically when initialFilters is an empty map', () => { + const session = createDashboardViewerSession(makeDeps({ document: seededDoc(), initialFilters: {} })); + expect(byId(session, 'f1')).toMatchObject({ value: 'D1', active: false }); + expect(byId(session, 'f2')).toMatchObject({ value: 'D2', active: true }); + }); + + it('falls back to the definition defaultValue when a seed entry has a nullish value', () => { + const session = createDashboardViewerSession(makeDeps({ + document: seededDoc(), initialFilters: { f1: { value: null, active: true } }, + })); + expect(byId(session, 'f1')).toMatchObject({ value: 'D1', active: true }); + }); + + it('coerces a seeded falsy active flag to false rather than falling back to the default', () => { + const session = createDashboardViewerSession(makeDeps({ + // f2's OWN default is active:true — the seed's explicit false must win. + document: seededDoc(), initialFilters: { f2: { value: 'D2', active: false } }, + })); + expect(byId(session, 'f2')).toMatchObject({ value: 'D2', active: false }); + }); +}); + describe('filters and the #235 execution planner', () => { // A dashboard with a source-backed filter targeting one tile via parameter // declaration, plus an unrelated tile the filter can never affect. diff --git a/tests/unit/dashboard.test.ts b/tests/unit/dashboard.test.ts index 7a1c221..900fad1 100644 --- a/tests/unit/dashboard.test.ts +++ b/tests/unit/dashboard.test.ts @@ -1,9 +1,10 @@ -import { describe, it, expect, vi } from 'vitest'; +import { describe, it, expect, vi, afterEach } from 'vitest'; import { isDashboardRoute, configBase, normalizeDashLayout, normalizeDashCols, DASH_TILE_ROW_CAP, DASH_TILE_BYTE_CAP, DASH_TABLE_DISPLAY_CAP, activeDashboardView, dashboardViewSelection, partitionKpiBands, } from '../../src/core/dashboard.js'; +import { KEYS } from '../../src/state.js'; import { CHART_ROW_CAPS } from '../../src/core/chart-data.js'; import { AUTH_SS_KEYS, AUTH_REQUEST, AUTH_GRANT, @@ -643,6 +644,108 @@ describe('renderDashboard — shared rich filter bar over the viewer (#188)', () }); }); +// #303: the isolated per-dashboard filter store (`asb:dashFilters`) — the +// #280 viewer session used to init every filter purely from +// `def.defaultValue`/`defaultActive`, so a committed value lived only in +// memory and reset on reload. `loadJSON`/`KEYS.dashFilters` reads through the +// REAL default store (not through `app`), so these stub `globalThis.localStorage` +// directly (never touching the ambient real one — Node 25 native Web Storage +// flake, #130) — `app.saveJSON` (a `makeApp()` spy) is asserted on for writes. +describe('renderDashboard — isolated per-dashboard filter persistence (#303)', () => { + function memStore(initial: Record = {}) { + const m = new Map(Object.entries(initial)); + return { + getItem: (k: string) => (m.has(k) ? (m.get(k) as string) : null), + setItem: (k: string, v: unknown) => { m.set(k, String(v)); }, + }; + } + afterEach(() => vi.unstubAllGlobals()); + + const filterWs = (over: WsOver = {}) => wsWith({ + queries: [q('q1', 'SELECT k, v FROM a WHERE n = {n:UInt8}')], + tiles: [{ id: 't1', queryId: 'q1' }], + filters: [{ id: 'n', parameter: 'n', defaultValue: 5, defaultActive: true }], + ...over, + }); + const nField = (app: TestApp): HTMLInputElement => qs(app.root, '.dash-filter-host .var-field input'); + + it("seeds a filter's value/active from a stored bag for the dashboard id", async () => { + vi.stubGlobal('localStorage', memStore({ + [KEYS.dashFilters]: JSON.stringify({ d: { n: { value: '42', active: false } } }), + })); + const { app } = dashApp({ workspace: filterWs() }); + await render(app); + expect(nField(app).value).toBe('42'); + }); + + it('is isolated from the Workbench asb:varValues/asb:filterActive keys (Option B, not shared)', async () => { + vi.stubGlobal('localStorage', memStore({ + [KEYS.varValues]: JSON.stringify({ n: 'workbench-only-value' }), + [KEYS.filterActive]: JSON.stringify({ n: false }), + })); + const { app } = dashApp({ workspace: filterWs() }); + await render(app); + // The dashboard's own default (5) wins — the Workbench keys are never read. + expect(nField(app).value).toBe('5'); + }); + + it('does not write defaults back over an existing stored bag on the initial publish', async () => { + vi.stubGlobal('localStorage', memStore({ + [KEYS.dashFilters]: JSON.stringify({ d: { n: { value: '42', active: false } } }), + })); + const { app } = dashApp({ workspace: filterWs() }); + await render(app); + expect(app.saveJSON).not.toHaveBeenCalled(); + }); + + it('does not persist filter defaults on the initial publish when nothing is stored yet', async () => { + // Empty store + a filter with a non-empty default (n=5, active) — the first + // publish merely echoes the seeded default state, so it must NOT write: + // persisting defaults here would freeze them against a later Spec-editor + // change to the filter's default (regression guard for the review fix). + vi.stubGlobal('localStorage', memStore()); + const { app } = dashApp({ workspace: filterWs() }); + await render(app); + expect(app.saveJSON).not.toHaveBeenCalled(); + }); + + it('persists a committed filter change, keyed by dashboard id + filter id, isolated from the Workbench keys', async () => { + vi.stubGlobal('localStorage', memStore()); + const { app } = dashApp({ workspace: filterWs() }); + await render(app); + const input = nField(app); + input.value = '7'; + input.dispatchEvent(new Event('input', { bubbles: true })); + input.dispatchEvent(new Event('blur', { bubbles: true })); + await Promise.resolve(); await Promise.resolve(); + expect(app.saveJSON).toHaveBeenCalledWith(KEYS.dashFilters, { d: { n: { value: '7', active: true } } }); + // Never touches the Workbench's own keys. + expect(app.saveJSON).not.toHaveBeenCalledWith(KEYS.varValues, expect.anything()); + expect(app.saveJSON).not.toHaveBeenCalledWith(KEYS.filterActive, expect.anything()); + }); + + it('does not write again on a later publish that carries no filter change (e.g. a layout switch)', async () => { + vi.stubGlobal('localStorage', memStore()); + const { app } = dashApp({ + workspace: filterWs({ layout: { type: 'flow', version: 1, preset: 'columns-2', items: {} } }), + }); + await render(app); + const input = nField(app); + input.value = '7'; + input.dispatchEvent(new Event('input', { bubbles: true })); + input.dispatchEvent(new Event('blur', { bubbles: true })); + await Promise.resolve(); await Promise.resolve(); + const saveJSON = app.saveJSON as ReturnType; + const callsAfterCommit = saveJSON.mock.calls.length; + expect(callsAfterCommit).toBeGreaterThan(0); + // A structural republish (preset switch → syncDocument) with the SAME + // filter value/active must not persist again (the dedicated persist + // signature, not the bar-rebuild signature, gates the write). + pickLayout(app.root, 'full-width'); + expect(saveJSON.mock.calls.length).toBe(callsAfterCommit); + }); +}); + function jwt(payload: Record): string { // btoa/atob (not node:crypto's Buffer — no @types/node in this project) — // the same base64url shape core/jwt.js's decodeJwtPayload expects.