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/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/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/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/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/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/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/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/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. 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({}); + }); +}); 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);