diff --git a/CHANGELOG.md b/CHANGELOG.md index 9fde031d..631d8644 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,39 @@ auto-generated per-PR notes; this file is the curated, human-readable history. ## [Unreleased] +### Added +- **Portable open/import/export + a transactional import planner** (#287, Dashboard + v1 Phase 5). The File menu's ambiguous `Append` is gone, replaced by + resource-oriented **Import queries / Import Dashboard / Replace workspace** and + **Export Dashboard / Export workspace** operations, all writing the one canonical + `altinity-sql-browser/portable-bundle` (`PortableBundleV1`) interchange format + through the deterministic canonical encoder. Export never mutates workspace + identity or Dashboard revision; **Export Dashboard** emits a dependency closure + (the Dashboard plus exactly its referenced queries), **Export workspace** emits + the whole catalog plus the zero-or-one Dashboard. Import runs a transactional + planner (`src/workspace/import-planner.ts`): parse → schema validation → select + Dashboard → dependency closure → conflict detection → decisions → complete ID + remapping → central reference rewrite → candidate workspace → whole-workspace + validation → atomic commit, with conflict actions **use-existing** (automatic + only on canonical equality), **copy** (fresh id + full reference rewrite), + **replace**, and **skip** (skipping a required Dashboard dependency invalidates + the Dashboard import rather than silently dropping a tile). A multi-Dashboard + bundle asks which Dashboard to import. Legacy Library v1/v2 files remain + **importable** (normalized to an in-memory bundle with `dashboards: []`); no new + Library-only JSON is written. + +### Changed +- **The `StoredWorkspaceV1` aggregate is now the single source of truth for the + saved-query collection** (#287). All query CRUD (create/edit/rename/delete/star) + commits the whole workspace through the atomic `WorkspaceRepository` + (validate-before-publish per #280) instead of the flat `asb:saved` localStorage + key, which is retired (read once only as the legacy-migration source); boot loads + and projects the aggregate. Saved-query writes are serialized so two rapid + concurrent edits can't interleave and resurrect a just-deleted query. The + external-bundle **trust preflight** and durable cross-tab transport are deferred + to Phase 6 (#288); every Phase-5 file operation only commits to the workspace and + never executes queries. + ### Fixed - **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 diff --git a/README.md b/README.md index 695e08f5..acef2073 100644 --- a/README.md +++ b/README.md @@ -24,10 +24,10 @@ See the [**feature tour, deployment guide and screenshots**](https://docs.altini on the project site. Try it live on the Antalya demo cluster: **https://antalya.demo.altinity.cloud/sql**. The [**ontime chart demo**](docs/ONTIME-CHART-DEMO.md) is a ready-made library of 10 queries (load [`examples/ontime-charts.json`](examples/ontime-charts.json) via -**File ▾ → Open**) that walks through every chart type and feature against the public +**File ▾ → Import queries**) that walks through every chart type and feature against the public `ontime` flight dataset. The [**system explorer demo**](docs/SYSTEM-EXPLORER-DEMO.md) is a 14-query library (load [`examples/system-explorer-charts.json`](examples/system-explorer-charts.json) -via **File ▾ → Append**) that introspects ClickHouse's own `system` database — +via **File ▾ → Import queries**) that introspects ClickHouse's own `system` database — running queries, merges/replication health, and historical query/part/error activity — with a shared From/To filter driving every time-ranged Dashboard tile at once. @@ -93,7 +93,7 @@ with no per-favorite name, description, or statistics footer, spanning every Dashboard layout (Full width/Report/2/3 columns). Consecutive explicit KPI favorites merge into one shared band. The complete [`kpi-panel.json`](examples/kpi-panel.json) Library example can be opened from -**File ▾ → Open** to see both. +**File ▾ → Import queries** to see both. When constructing a named tuple from expressions, either enable alias-derived member names for the query: @@ -422,40 +422,51 @@ read; DDL secrets remain masked unless the role separately holds ## Saved queries & the Library -Queries you save (★ **Save** next to Run, or `⌘S`) land in the sidebar **★ Library** +Queries you save (★ **Save** next to Run, or `⌘S`) land in the sidebar **★ Queries** panel. Each carries a name, an optional **description**, and — when set — its remembered result view and chart config. Saving or editing a query opens a small form with both a name and a description field; the description shows under the row and is included in Markdown/SQL exports. -The whole collection is treated as a **document — the Library** — with a name and -an unsaved-changes dot, managed from the header **File ▾** menu: +The queries plus the zero-or-one editable Dashboard form one **current workspace** +(a `StoredWorkspaceV1` aggregate). The workspace is persisted **atomically** in the +browser (IndexedDB) — a reload restores queries, Dashboard, layout, and workspace +name together; every saved-query edit commits the whole workspace and only +publishes once persistence succeeds. The header **File ▾** menu carries a name, an +unsaved-changes dot (changes since the last export or import), and these +resource-oriented operations (#287): -- **New Library** — clears to an empty, default-named library (confirms first +- **New workspace…** — clears to an empty, default-named workspace (confirms first when non-empty). Open editor tabs are unaffected. -- **Save JSON** (`.json`) — downloads the whole Library in the versioned - `altinity-sql-browser/saved-queries` envelope. Version 2 stores each query as - `{id, sql, specVersion, spec}`: `spec` is the complete, lossless query - definition (`name`, `description`, `favorite`, `view`, `panel`, `dashboard`, - and future extension fields). New files include a canonical `$schema` hint - and RFC 3339 `exportedAt` timestamp and validate against the - [complete Library contract](docs/library-json-schema.md). The filename derives from the Library name; - saving clears the unsaved-changes dot. Version 1 Library files remain - importable and are upgraded in memory; new exports always use version 2. -- **Open… / Append…** — load a `.json` file: Open swaps the Library and - adopts the file's base name (confirms when the current Library is non-empty); - Append merges via the existing dedupe and reports `Added N · updated N · - skipped N`. **JSON is the only importable format**, and imported SQL is never - run automatically. +- **Import queries…** — merge a file's queries into the current collection. When + an incoming query's id collides with an existing one, a **conflict dialog** + offers a default action plus per-row overrides — *use existing*, *copy* (import + under a fresh id), *replace*, or *skip*; a byte-identical incoming query is + reused automatically. The Dashboard is untouched (imported favorite flags never + add tiles). +- **Import Dashboard…** — replace the current Dashboard with one from a file + (confirms first when a Dashboard already exists), importing only its referenced + queries. A multi-Dashboard file asks which one. +- **Replace workspace…** — atomically replace **both** the query collection and + the Dashboard from a file (confirms first). A multi-Dashboard file asks which + Dashboard to adopt (or none). +- **Export Dashboard… / Export workspace…** (`.json`) — write the one canonical + **`altinity-sql-browser/portable-bundle`** interchange format. *Export Dashboard* + emits the selected Dashboard plus exactly its dependency-closure of queries; + *Export workspace* emits every saved query (catalog order) plus the zero-or-one + Dashboard. Both use the deterministic canonical encoder and never mutate the + workspace's identity or Dashboard revision. Legacy Library v1/v2 files remain + **importable** (decoded to an in-memory bundle); no new Library-only JSON is + written. See the [schema contracts](docs/library-json-schema.md). Imported SQL + is never run automatically. - **Share / publish** — **Download Markdown** (`.md`, a `### heading` + fenced ` ```sql ` cookbook) and **Download SQL** (`.sql`, `/* name + description */` comment blocks, `;`-delimited). Both are **one-way** — lossy by design (no ids - or Spec metadata), so JSON stays the canonical round-trip format. + or Spec metadata), so the portable bundle stays the canonical round-trip format. -The Library name is editable inline (click it in the header) and is persisted -separately from the queries. The **•** dot appears after any change that hasn't -been written to a file yet (save/rename/delete/favorite/append/rename) and clears -on Save JSON / Open / New. +The workspace name is editable inline (click it in the header). The **•** dot +appears after any change not yet written to a file and clears on export / import / +New workspace. ### Dashboard Filter sources @@ -471,7 +482,7 @@ ordinary parameter field when a source, consumer type, or provider conflicts. Filter sources run and reconcile saved values before any Panel query starts. The complete [`query-log-explorer.json`](examples/query-log-explorer.json) -Library example (load via **File ▾ → Append**) demonstrates every filter +Library example (load via **File ▾ → Import queries**) demonstrates every filter variant against `system.query_log` on any cluster: three Filter sources, one per option shape (`Array(Tuple(value, label))`, `Map(String, String)`, plain `Array(T)`), alongside plain auto-detected numeric/text fields — a KPI panel, diff --git a/docs/ADR-0001-reactivity.md b/docs/ADR-0001-reactivity.md index 7eaafa08..52b38a0b 100644 --- a/docs/ADR-0001-reactivity.md +++ b/docs/ADR-0001-reactivity.md @@ -338,3 +338,32 @@ re-opens the decision via a fresh spike: If re-opened, the candidate is **Preact** (`spike/preact-schema` stands as evidence, +6.8 KB gzip), never React (+45 KB). + +## Addendum — the workspace aggregate is the saved-query source of truth (#287, Dashboard v1 Phase 5) + +Phase 5 made the persisted `StoredWorkspaceV1` aggregate (via the atomic +`WorkspaceRepository`, IndexedDB) the single source of truth for the saved-query +collection, retiring the flat `asb:saved` localStorage write path (read once only +as the legacy-migration source). This is a state-flow change worth recording +against this ADR because it re-shapes how `state.savedQueries` relates to +reactivity: + +- `state.savedQueries` is now a **projection** of the committed workspace, not a + directly-mutated array. Boot loads the aggregate and projects it (queries, + Dashboard, workspace id/name); every file operation commits and re-projects + through one shared `app.applyCommittedWorkspace` helper. +- All query CRUD is **strict async, validate-before-publish** (#280): each op + computes a candidate, `await`s `WorkspaceRepository.commit` (which validates the + whole candidate, then atomically replaces the record), and only then mutates + in-memory state + tabs. A failed commit mutates nothing and keeps the draft + dirty. The array is not a signal (it never was — see #276); the render surfaces + repaint explicitly after a commit, exactly as the pre-#287 synchronous code did. +- Because commits are async, saved-query writes are **serialized** per app + (`app.serializeWrite`) so two overlapping ops can't each build a candidate from + the same stale snapshot and have the later commit resurrect a just-deleted query + or clobber a concurrent edit. This is last-commit-wins within one tab; #280's + multi-tab "last successful commit wins, no compare-and-swap" policy is unchanged. + +No framework pull here: the change is about persistence atomicity and validation, +not a render model. The imperative-islands + signals-for-invalidation decision +stands. diff --git a/docs/ONTIME-CHART-DEMO.md b/docs/ONTIME-CHART-DEMO.md index 2e62c059..da06463e 100644 --- a/docs/ONTIME-CHART-DEMO.md +++ b/docs/ONTIME-CHART-DEMO.md @@ -20,7 +20,7 @@ dataset (`ontime`, ~230M rows, 1987–2025) on the Antalya demo cluster. 3. In the header, click **File ▾ → Open…** and pick the file. The library is renamed **ontime-charts** and fills with 10 saved queries (confirm the replace if you already had queries saved). -4. Click any query in the **Library** panel — it runs and opens straight into its chart. +4. Click any query in the **Queries** panel — it runs and opens straight into its chart. Switch **Table / JSON / Chart** at the top of the results, or change the **Type / X / Y / Series** dropdowns to re-encode any chart live. diff --git a/src/application/saved-query-service.ts b/src/application/saved-query-service.ts index 35559da7..fb4cc37a 100644 --- a/src/application/saved-query-service.ts +++ b/src/application/saved-query-service.ts @@ -40,6 +40,8 @@ import { queryPanel, withQuerySpec } from '../core/saved-query.js'; import { isQuerylessPanel } from '../core/panel-cfg.js'; import { encodeShare } from '../core/share.js'; import type { SavedQueryV2 } from '../generated/json-schema.types.js'; +import type { WorkspaceRepository } from '../workspace/workspace-repository.js'; +import type { WorkspaceDiagnostic } from '../dashboard/model/workspace-diagnostics.js'; // ── Construction deps ──────────────────────────────────────────────────────── @@ -49,7 +51,8 @@ import type { SavedQueryV2 } from '../generated/json-schema.types.js'; * `createSavedQuery`/`commitSavedQuery`/`recordHistory` (state.ts, all three * narrowed to their own exact reads) need between them. */ export interface SavedQueryServiceDeps { - state: Pick; + state: Pick; saveJSON: SaveJSON; /** `createSavedQuery`'s minting timestamp — a genuine wall-clock read * (production wires this to `() => Date.now()`, called at the exact @@ -64,17 +67,25 @@ export interface SavedQueryServiceDeps { * `createSavedQuery`/`commitSavedQuery` only ever call `.validate`). * Production passes `app.specValidators` (structurally assignable). */ specValidators: SpecValidationService; + /** The strict aggregate-commit seam (#287 W4) `createSavedQuery`/ + * `commitSavedQuery` await instead of the retired flat `asb:saved` write. + * Production passes `app.workspace` (structurally assignable — only + * `.commit` is ever called). */ + workspace: Pick; } // ── Result types ───────────────────────────────────────────────────────────── export type CreateSavedResult = | { ok: true; entry: SavedQueryV2 } - /** `createSavedQuery` itself returned null (already-linked tab, blank SQL - * on a non-text panel, blank name, or a blocking validation diagnostic) — - * the pre-extraction inline code never distinguished a reason here either - * (silent no-op), so neither does this result. */ - | { ok: false }; + /** `createSavedQuery` itself rejected the entry — either a pre-commit + * compute guard (already-linked tab, blank SQL on a non-text panel, blank + * name, or a blocking validation diagnostic — the pre-#287 inline code + * never distinguished a reason here either, so neither does this result; + * `diagnostics` absent), or the aggregate strictly rejected the whole- + * workspace commit (#287 W4 — `diagnostics` present, straight from + * `WorkspaceRepository.commit`; nothing was mutated). */ + | { ok: false; diagnostics?: WorkspaceDiagnostic[] }; export type CommitLinkedResult = | { ok: true; entry: SavedQueryV2 } @@ -85,10 +96,13 @@ export type CommitLinkedResult = | 'invalid-spec' /** Blank SQL on a panel type that isn't SQL-optional (#166's text panel). */ | 'empty' - /** `commitSavedQuery` itself returned null (tab no longer linked, or its - * own re-validation against the normalized Spec rejected it) — a - * defensive case the pre-extraction inline code never toasted either. */ + /** `commitSavedQuery` itself rejected the commit — either its own + * defensive re-check against the normalized Spec (tab no longer linked — + * the pre-#287 inline code never toasted this either, `diagnostics` + * absent), or the aggregate strictly rejected the whole-workspace commit + * (#287 W4 — `diagnostics` present; nothing was mutated). */ | 'rejected'; + diagnostics?: WorkspaceDiagnostic[]; }; export type ShareResult = @@ -116,12 +130,12 @@ export interface SavedQueryService { * saved query from an unsaved tab's current `sqlDraft`/`specParsed` plus * `name`/`description`. Rejects (silently — see `CreateSavedResult`) an * already-linked tab, per `createSavedQuery`'s own guard. */ - create(tab: QueryTab, name: unknown, description: unknown): CreateSavedResult; + create(tab: QueryTab, name: unknown, description: unknown): Promise; /** Update-in-place path (app.ts's `commitLinkedQuery`, the "Save" button on * an already-linked tab): persist `evaluated` as the linked saved query's * new Spec. Takes the Spec evaluation as an input (see this module's * header comment) rather than evaluating it itself. */ - commit(tab: QueryTab, evaluated: { parsed: unknown; diagnostics: SpecValidationDiagnostic[] }): CommitLinkedResult; + commit(tab: QueryTab, evaluated: { parsed: unknown; diagnostics: SpecValidationDiagnostic[] }): Promise; /** Record a successful run in history (state.ts's own `recordHistory`) — * never touches rendering; app.ts's own `app.recordHistory` delegate * conditionally repaints the History side panel itself after calling @@ -136,14 +150,16 @@ export interface SavedQueryService { * validation, no defaulting; the caller supplies every field exactly as it * wants it used. */ export function createSavedQueryService(deps: SavedQueryServiceDeps): SavedQueryService { - function create(tab: QueryTab, name: unknown, description: unknown): CreateSavedResult { - const entry = createSavedQuery(deps.state, tab, name, description, deps.saveJSON, deps.now(), deps.specValidators); - return entry ? { ok: true, entry } : { ok: false }; + async function create(tab: QueryTab, name: unknown, description: unknown): Promise { + const result = await createSavedQuery( + deps.state, tab, name, description, deps.workspace.commit, deps.now(), deps.specValidators, + ); + return result.ok ? { ok: true, entry: result.entry } : { ok: false, diagnostics: result.diagnostics }; } - function commit( + async function commit( tab: QueryTab, evaluated: { parsed: unknown; diagnostics: SpecValidationDiagnostic[] }, - ): CommitLinkedResult { + ): Promise { if (!evaluated.parsed || hasBlockingSpecErrors(evaluated.diagnostics)) { return { ok: false, reason: 'invalid-spec' }; } @@ -151,8 +167,12 @@ export function createSavedQueryService(deps: SavedQueryServiceDeps): SavedQuery if (!String(tab.sqlDraft || '').trim() && !isQuerylessPanel(panel)) { return { ok: false, reason: 'empty' }; } - const entry = commitSavedQuery(deps.state, tab, evaluated.parsed as QuerySpecDraft | null, deps.saveJSON, deps.specValidators); - return entry ? { ok: true, entry } : { ok: false, reason: 'rejected' }; + const result = await commitSavedQuery( + deps.state, tab, evaluated.parsed as QuerySpecDraft | null, deps.workspace.commit, deps.specValidators, + ); + return result.ok + ? { ok: true, entry: result.entry } + : { ok: false, reason: 'rejected', diagnostics: result.diagnostics }; } function recordHistoryFn(tab: QueryTab, sqlText?: string): void { diff --git a/src/core/saved-io.ts b/src/core/saved-io.ts index d0cdcd00..cb36258c 100644 --- a/src/core/saved-io.ts +++ b/src/core/saved-io.ts @@ -1,17 +1,15 @@ -// Pure import/export/merge for saved-query documents. No DOM or globals. -// Envelope v1 is accepted and upgraded; v2 is the only emitted format. Every -// live/exported query uses { id, sql, specVersion, spec } and all extensibility +// Pure Library-document import + one-way share/publish exports. No DOM or +// globals. Envelope v1 is accepted and upgraded; v2 is the only import format +// otherwise recognized (#287 W5 retired the JSON-Library WRITE path — +// `parseImportDoc` stays the read/migration path `normalizeLegacyLibraryToBundle` +// builds on via `decodeLibraryJson` directly, and the example-file regression +// suite (`tests/unit/spec-examples.test.js`) exercises it independently). Every +// live/imported query uses { id, sql, specVersion, spec } and all extensibility // stays inside the complete, losslessly-cloned Spec. -import { - cloneJson, isPlainObject, queryContentKey, - queryDescription, queryName, queryPanel, upgradeSavedQuery, upgradeV1Query, - withQuerySpec, -} from './saved-query.js'; +import { cloneJson, queryDescription, queryName, queryPanel, upgradeSavedQuery } from './saved-query.js'; import type { QueryRoot } from './saved-query.js'; -import { - decodeLibraryJson, encodeLibraryDocument, throwingValue, validateSavedQueryDocument, -} from './library-codec.js'; +import { decodeLibraryJson, throwingValue } from './library-codec.js'; import type { SpecDiagnostic } from '../editor/spec-editor.types.js'; // ── Injected Spec/Library validation seam ─────────────────────────────────── @@ -29,49 +27,11 @@ export interface SavedIoValidationService { validate(spec: unknown, context?: SavedIoValidationContext): SpecDiagnostic[]; } -/** Build the canonical v2 export envelope. `nowISO` is injected for tests. */ -export function buildExportDoc(queries: unknown, nowISO?: string): Record { - return throwingValue(encodeLibraryDocument(queries, { nowISO })); -} - function invalidSpecError(query: { id?: unknown }, index: number, diag: SpecDiagnostic): never { const identity = query.id ? `Query ${JSON.stringify(query.id)}` : `Query at index ${index}`; throw new Error(`${identity}: ${diag.message}.`); } -/** Validate canonical/upgraded queries before any Library mutation. */ -function validateLibraryEntries( - entries: { raw: unknown; index: number }[], validationService: SavedIoValidationService | null, -): QueryRoot[] { - return entries.map(({ raw, index }) => { - // `as`: when `raw` already looks like a spec-bearing entry ('spec'/ - // 'specVersion' present) it is trusted to carry the same {id?, sql, - // specVersion, spec} shape `upgradeV1Query`'s branch always returns — - // only object-shape checked structurally above (the same ingress - // convention as e.g. panel-cfg.ts's clonePanelCfg). - const query = (isPlainObject(raw) && ('spec' in raw || 'specVersion' in raw) - ? cloneJson(raw) - : upgradeV1Query(raw)) as QueryRoot; - // State compatibility callers historically mint a missing id after this - // validation step. Portable Library decoding is strict and never uses this - // placeholder path. - const checked: QueryRoot = query.id ? query : { ...query, id: `__compat-${index}` }; - const structural = validateSavedQueryDocument(checked).find((item) => item.severity === 'error'); - if (structural) throw new Error(structural.message); - if (validationService) { - const feature = validationService.validate(query.spec, { sql: query.sql, query }).find((item) => item.severity === 'error'); - if (feature) invalidSpecError(query, index, feature); - } - return { id: query.id, sql: query.sql, specVersion: query.specVersion, spec: cloneJson(query.spec) }; - }); -} - -export function validateLibraryQueries( - queries: readonly unknown[], validationService: SavedIoValidationService | null = null, -): QueryRoot[] { - return validateLibraryEntries(queries.map((raw, index) => ({ raw, index })), validationService); -} - /** `parseImportDoc`'s result: the decoded envelope metadata plus the * canonical, cloned, (optionally feature-validated) queries. */ export interface ParsedImportDoc { @@ -110,64 +70,6 @@ export function parseImportDoc( }; } -/** `mergeSaved`'s result: the merged canonical list plus the three tallies. */ -export interface MergeSavedResult { - merged: (QueryRoot & { id: string })[]; - added: number; - updated: number; - skipped: number; -} - -/** - * Merge canonical/upgradable queries without mutating either input. Content - * identity is SQL + specVersion + the COMPLETE Spec (object key order ignored, - * array order retained); id is identity, not content. A by-id update replaces - * the complete incoming Spec, so extensions are never reconstructed. - */ -export function mergeSaved( - existing: readonly unknown[], incoming: readonly unknown[], genId: () => string, -): MergeSavedResult { - // `as`: every entry `mergeSaved` returns below is either an already-id'd - // `existing` entry (the Library's own persisted identity) or one this loop - // mints/keeps a real string id for (`current.id` or `freshId()`) — - // `upgradeSavedQuery`/`withQuerySpec`'s own general QueryRoot contract - // (`id: string | null | undefined`) doesn't encode that narrower, - // mergeSaved-specific guarantee. - const merged = existing.map(upgradeSavedQuery) as (QueryRoot & { id: string })[]; - const seen = new Set(merged.map(queryContentKey)); - const ids = new Set(merged.map((query) => query.id).filter((id): id is string => Boolean(id))); - let added = 0, updated = 0, skipped = 0; - - const freshId = (): string => { - let id: string; - do { id = genId(); } while (!id || ids.has(id)); - return id; - }; - - for (const rawIncoming of incoming) { - const inc = upgradeSavedQuery(rawIncoming); - const index = inc.id ? merged.findIndex((query) => query.id === inc.id) : -1; - if (index >= 0) { - const current = merged[index]; - if (queryContentKey(current) === queryContentKey(inc)) { skipped++; continue; } - seen.delete(queryContentKey(current)); - merged[index] = withQuerySpec({ ...inc, id: current.id }, inc.spec) as QueryRoot & { id: string }; - seen.add(queryContentKey(merged[index])); - updated++; - continue; - } - const key = queryContentKey(inc); - if (seen.has(key)) { skipped++; continue; } - const id = inc.id && !ids.has(inc.id) ? inc.id : freshId(); - const entry = withQuerySpec({ ...inc, id }, inc.spec) as QueryRoot & { id: string }; - ids.add(id); - merged.push(entry); - seen.add(key); - added++; - } - return { merged, added, updated, skipped }; -} - // ── One-way share/publish exports ─────────────────────────────────────────── // Markdown and SQL are lossy by design; JSON is the canonical round-trip form. diff --git a/src/dashboard/application/dashboard-authoring-session.ts b/src/dashboard/application/dashboard-authoring-session.ts index 31434704..aad14518 100644 --- a/src/dashboard/application/dashboard-authoring-session.ts +++ b/src/dashboard/application/dashboard-authoring-session.ts @@ -28,10 +28,7 @@ import type { SpecSchemaService } from '../../core/spec-schema.js'; import { diagnostic, sortDiagnostics } from '../model/workspace-diagnostics.js'; import type { WorkspaceDiagnostic } from '../model/workspace-diagnostics.js'; import { resolveDashboardPresentations } from '../model/presentation-resolver.js'; -import { dashboardDependencyQueryIds } from '../model/bundle-order.js'; -import { - CURRENT_PORTABLE_BUNDLE_VERSION, PORTABLE_BUNDLE_FORMAT, PORTABLE_BUNDLE_V1_SCHEMA_ID, -} from '../model/portable-bundle-codec.js'; +import { buildDashboardExportBundle } from '../model/dashboard-export.js'; import { resolveActiveLayoutPlugin } from '../layouts/flow-layout.js'; import { applyCommand } from './dashboard-commands.js'; import type { DashboardCommand, DashboardCommandResult } from './dashboard-commands.js'; @@ -228,26 +225,9 @@ export function createDashboardAuthoringSession( } function createPortableBundle(): PortableBundleV1 { - const dashboard = cloneJson(stateSignal.value.document); - const byId = new Map(); - for (const query of queries) { - if (isObject(query) && typeof query.id === 'string' && !byId.has(query.id)) byId.set(query.id, query); - } - const bundleQueries: SavedQueryV2[] = []; - for (const id of dashboardDependencyQueryIds(dashboard)) { - const query = byId.get(id); - if (query) bundleQueries.push(cloneJson(query)); - } // Export never mutates workspace identity or revision — nothing here - // touches `committedRevision` or the draft. - return { - $schema: PORTABLE_BUNDLE_V1_SCHEMA_ID as PortableBundleV1['$schema'], - format: PORTABLE_BUNDLE_FORMAT as PortableBundleV1['format'], - version: CURRENT_PORTABLE_BUNDLE_VERSION as PortableBundleV1['version'], - exportedAt: nowISO(), - queries: bundleQueries, - dashboards: [dashboard], - }; + // touches `committedRevision` or the draft; the builder deep-clones. + return buildDashboardExportBundle(stateSignal.value.document, queries, nowISO()); } function destroy(): void { diff --git a/src/dashboard/model/dashboard-export.ts b/src/dashboard/model/dashboard-export.ts new file mode 100644 index 00000000..337e7446 --- /dev/null +++ b/src/dashboard/model/dashboard-export.ts @@ -0,0 +1,73 @@ +// Portable-bundle export builders (#280/#287 "Canonical output ordering", +// "Portable dependency closure"). Pure structural assembly only — callers +// pass the returned PortableBundleV1 to `encodePortableBundleJson` +// (portable-bundle-codec.ts) for validation/canonical encoding; this module +// never encodes or validates itself so it stays independently unit-testable. +// +// Two builders, two ordering contracts: +// - `buildDashboardExportBundle` — one Dashboard's dependency closure: only +// the queries it actually references (bundle-order.ts), in dependency +// order, so a single-Dashboard export never drags in unrelated queries. +// - `buildWorkspaceExportBundle` — the full workspace catalog: every saved +// query in its existing catalog/authoring order (never reordered by +// Dashboard usage), plus the zero-or-one current Dashboard as-is. +// +// Both deep-clone every resource they emit — an export must never let the +// caller's in-memory workspace/session state (including Dashboard `revision`) +// be mutated through the returned bundle, and must never mutate its inputs. + +import { cloneJson } from '../../core/saved-query.js'; +import { dashboardDependencyQueryIds } from './bundle-order.js'; +import { + CURRENT_PORTABLE_BUNDLE_VERSION, PORTABLE_BUNDLE_FORMAT, PORTABLE_BUNDLE_V1_SCHEMA_ID, +} from './portable-bundle-codec.js'; +import type { + DashboardDocumentV1, PortableBundleV1, SavedQueryV2, StoredWorkspaceV1, +} from '../../generated/json-schema.types.js'; + +function bundleEnvelope( + nowISO: string, queries: SavedQueryV2[], dashboards: DashboardDocumentV1[], +): PortableBundleV1 { + return { + $schema: PORTABLE_BUNDLE_V1_SCHEMA_ID as PortableBundleV1['$schema'], + format: PORTABLE_BUNDLE_FORMAT as PortableBundleV1['format'], + version: CURRENT_PORTABLE_BUNDLE_VERSION as PortableBundleV1['version'], + exportedAt: nowISO, + queries, + dashboards, + }; +} + +/** Build a portable bundle for exporting ONE Dashboard: the Dashboard plus + * exactly the queries it depends on (tiles then filter sources, each once, + * in `dashboardDependencyQueryIds` order); a dependency id absent from + * `queries` is skipped rather than failing. Unrelated catalog queries never + * appear. Deep-clones both the Dashboard and every emitted query — the + * input `dashboard` (and its `revision`) is left byte-for-byte unchanged. */ +export function buildDashboardExportBundle( + dashboard: DashboardDocumentV1, queries: readonly SavedQueryV2[], nowISO: string, +): PortableBundleV1 { + const byId = new Map(); + for (const query of queries) { + if (!byId.has(query.id)) byId.set(query.id, query); + } + const bundleQueries: SavedQueryV2[] = []; + for (const id of dashboardDependencyQueryIds(dashboard)) { + const query = byId.get(id); + if (query) bundleQueries.push(cloneJson(query)); + } + return bundleEnvelope(nowISO, bundleQueries, [cloneJson(dashboard)]); +} + +/** Build a portable bundle for exporting the WHOLE workspace: every saved + * query in its existing catalog order (never reordered by Dashboard tile + * usage) plus the zero-or-one current Dashboard as-is. Deep-clones every + * emitted resource — the input `workspace` (including its Dashboard + * `revision`) is left byte-for-byte unchanged. */ +export function buildWorkspaceExportBundle( + workspace: StoredWorkspaceV1, nowISO: string, +): PortableBundleV1 { + const queries = cloneJson(workspace.queries); + const dashboards = workspace.dashboard ? [cloneJson(workspace.dashboard)] : []; + return bundleEnvelope(nowISO, queries, dashboards); +} diff --git a/src/dashboard/model/legacy-bundle.ts b/src/dashboard/model/legacy-bundle.ts new file mode 100644 index 00000000..a5a4de58 --- /dev/null +++ b/src/dashboard/model/legacy-bundle.ts @@ -0,0 +1,73 @@ +// Legacy Library v1/v2 → in-memory PortableBundleV1 normalizer (#280/#287 +// Phase 5 "Legacy Library decoding normalizes to an in-memory bundle"). +// Reuses the canonical Library codec (core/library-codec.ts) to parse, +// migrate, and validate legacy Library JSON, then wraps the decoded queries +// into a fresh, empty-dashboards PortableBundleV1 and re-validates the whole +// bundle through the one canonical portable-bundle validator — never a +// partial import. Pure: JSON-limit guards, the clock, and the validation +// service are all injected exactly like the sibling codecs in this +// directory. + +import { decodeLibraryDocument } from '../../core/library-codec.js'; +import type { JsonSchemaValidationService } from '../../core/json-schema-validation.js'; +import type { PortableBundleV1 } from '../../generated/json-schema.types.js'; +import { parseJsonWithLimits } from './json-limits.js'; +import type { JsonLimitOptions } from './json-limits.js'; +import { diagnostic } from './workspace-diagnostics.js'; +import type { WorkspaceDiagnostic } from './workspace-diagnostics.js'; +import { + CURRENT_PORTABLE_BUNDLE_VERSION, PORTABLE_BUNDLE_FORMAT, PORTABLE_BUNDLE_V1_SCHEMA_ID, + validatePortableBundleDocument, +} from './portable-bundle-codec.js'; + +export interface NormalizeLegacyLibraryOptions extends JsonLimitOptions { + validationService?: JsonSchemaValidationService; + /** Fallback `exportedAt` used only when the decoded legacy Library carries + * none of its own (a bare v1 document, or a v2 document that omitted the + * optional field). Never defaulted to `Date.now()` here — this module is + * pure; callers inject the clock. */ + nowISO?: string; + generateId?: (index: number, attempt: number) => string; +} + +export type NormalizeLegacyLibraryResult = + | { ok: true; value: PortableBundleV1 } + | { ok: false; diagnostics: WorkspaceDiagnostic[] }; + +/** Decode legacy Library v1/v2 JSON text into a fresh, empty-dashboards + * in-memory `PortableBundleV1`. Legacy formats stay readable — v1 is + * migrated to v2 first — but nothing is ever written back out as + * Library-only JSON; the only output shape is the portable bundle, fully + * re-validated end to end. Any decode/migration/validation failure returns + * `{ok:false}` with precise diagnostics rather than an incomplete bundle. */ +export function normalizeLegacyLibraryToBundle( + text: unknown, options: NormalizeLegacyLibraryOptions = {}, +): NormalizeLegacyLibraryResult { + const { validationService, nowISO, generateId, maxBytes, maxDepth } = options; + const parsed = parseJsonWithLimits(text, { maxBytes, maxDepth }); + if (!parsed.ok) return parsed; + + const decoded = decodeLibraryDocument(parsed.value, { nowISO, generateId, validationService }); + if (!decoded.ok) { + return { + ok: false, + diagnostics: decoded.diagnostics.map((item) => diagnostic(item.path, item.code, item.message)), + }; + } + + const exportedAt = typeof decoded.value.exportedAt === 'string' && decoded.value.exportedAt + ? decoded.value.exportedAt + : nowISO; + const document: Record = { + $schema: PORTABLE_BUNDLE_V1_SCHEMA_ID, + format: PORTABLE_BUNDLE_FORMAT, + version: CURRENT_PORTABLE_BUNDLE_VERSION, + ...(exportedAt === undefined ? {} : { exportedAt }), + queries: decoded.value.queries, + dashboards: [], + }; + + const diagnostics = validatePortableBundleDocument(document, { validationService }); + if (diagnostics.length) return { ok: false, diagnostics }; + return { ok: true, value: document as unknown as PortableBundleV1 }; +} diff --git a/src/main.ts b/src/main.ts index b4f5db97..4c351daf 100644 --- a/src/main.ts +++ b/src/main.ts @@ -42,6 +42,12 @@ export interface BootstrapApp { * main.ts's own `string | null` sentinel (`null` means "no callback * error"), so this contract states what's actually passed here. */ showLogin(msg?: string | null): void; + /** #287 W4: resolve the current StoredWorkspaceV1 aggregate (migrating the + * legacy flat state once, if needed) and project it onto `app.state` + * before the first `renderApp()` — see `App.loadWorkspaceOnBoot`'s own doc + * comment (app.types.ts). The real return value is never read here + * (`Promise` is enough for `bootstrap`'s own purposes). */ + loadWorkspaceOnBoot(): Promise; } /** `app.state.resultView`'s value union, reused at the one cast below. */ @@ -168,7 +174,15 @@ export async function bootstrap(app: BootstrapApp, env: BootstrapEnv): Promise<{ // ch_auth=basic username, not the raw email claim) on first paint. // (ensureConfig is a no-op in basic mode.) await app.conn.ensureConfig(); - if (dash) app.renderDashboard(); else app.renderApp(); + if (dash) app.renderDashboard(); + else { + // #287 W4: resolve + project the aggregate before the Workbench's first + // paint so the saved-query sidebar/Save flow already treat it as the + // single source of truth (the /dashboard branch above keeps resolving + // its own copy via `renderDashboard` → `loadDashboardWorkspace`). + await app.loadWorkspaceOnBoot(); + app.renderApp(); + } } else { app.showLogin(callbackError); } diff --git a/src/state.ts b/src/state.ts index 438508ad..17207a2d 100644 --- a/src/state.ts +++ b/src/state.ts @@ -3,7 +3,6 @@ // every operation is unit-testable with a spy and no real localStorage. import { clamp as clampUntyped } from './core/format.js'; -import { mergeSaved as mergeSavedUntyped, validateLibraryQueries as validateLibraryQueriesUntyped } from './core/saved-io.js'; import { SPEC_VERSION, cloneJson, patchQuerySpec, queryDescription, queryFavorite, queryName, queryPanel, queryView, withQuerySpec, @@ -13,7 +12,7 @@ import { decodeStoredSavedQueries as decodeStoredSavedQueriesUntyped } from './c import { normalizeDashLayout, normalizeDashCols } from './core/dashboard.js'; import { loadJSON as loadJSONUntyped, saveJSON as saveJSONUntyped, - loadStr as loadStrUntyped, saveStr as saveStrUntyped, + loadStr as loadStrUntyped, } from './core/storage.js'; import { emptyRecentMap as emptyRecentMapUntyped } from './core/recent-values.js'; import type { ResultSort } from './core/sort.js'; @@ -26,8 +25,10 @@ import { } from './core/spec-draft.js'; import { signal } from '@preact/signals-core'; import type { Signal } from '@preact/signals-core'; -import type { QuerySpecV1, SavedQueryV2 } from './generated/json-schema.types.js'; +import type { QuerySpecV1, SavedQueryV2, DashboardDocumentV1, StoredWorkspaceV1 } from './generated/json-schema.types.js'; import type { SpecDiagnostic } from './editor/spec-editor.types.js'; +import type { WorkspaceCommitResult } from './workspace/workspace-repository.js'; +import type { WorkspaceDiagnostic } from './dashboard/model/workspace-diagnostics.js'; // ── Persisted-data types (schema-generated) ───────────────────────────────── @@ -52,6 +53,68 @@ export interface StateReader { export type SaveJSON = (key: string, value: unknown) => void; export type SaveStr = (key: string, value: string) => void; +// ── Aggregate persistence seam (#287 W4 — strict async commit) ───────────── +// The saved-query CRUD ops below persist through the StoredWorkspaceV1 +// aggregate (app.workspace: WorkspaceRepository, IndexedDB), not the flat +// `asb:saved` localStorage key: a caller builds the WHOLE candidate workspace +// (this workbench never touches `dashboard`, just carries it through +// unchanged) and awaits this seam BEFORE mutating any in-memory state or +// tabs. `app.ts` injects `app.workspace.commit` directly (its signature is +// exactly `CommitWorkspace`); tests inject an in-memory fake repository +// (mirrors workspace-repository.test.ts's own `memStore` convention) so this +// module never imports a concrete IndexedDB adapter. + +/** The async aggregate-commit seam every saved-query CRUD op below takes in + * place of the old sync `save: SaveJSON` write. */ +export type CommitWorkspace = (candidate: StoredWorkspaceV1) => Promise; + +/** A saved-query CRUD op's async result once its candidate is strictly + * committed (validate-then-atomically-replace — see WorkspaceRepository.commit). + * `entry: null` on failure covers BOTH a pre-commit compute guard (bad name, + * blank SQL, a blocking Spec diagnostic — the same early-return semantics + * the pre-#287 sync code had, `diagnostics` absent) and a real repository + * rejection (`diagnostics` present, straight from the aggregate's whole- + * candidate validation). Either way NOTHING is mutated — no `state`/`tabs` + * write happens until `commit` resolves `ok: true`. */ +export type SavedEntryResult = + | { ok: true; entry: SavedQueryV2 } + | { ok: false; entry: null; diagnostics?: WorkspaceDiagnostic[] }; + +/** Bridge a workspace-commit rejection's diagnostics onto the narrower + * `SpecDiagnostic` shape `PatchSavedResult`/the Spec editor already expect + * (`WorkspaceDiagnostic.severity` adds `'information'`, not a member of + * `SpecDiagnostic.severity`'s narrower union — this maps it to `'warning'`, + * the closest non-error severity, so nothing here needs a third severity + * tier just for this one bridge). */ +const asSpecDiagnostics = (diagnostics: readonly WorkspaceDiagnostic[]): SpecDiagnostic[] => + diagnostics.map((d) => ({ + message: d.message, code: d.code, path: d.path, + severity: d.severity === 'warning' || d.severity === 'error' ? d.severity : 'warning', + })); + +/** The exact slice of `AppState` every saved-query CRUD op needs to build its + * whole-workspace commit candidate — the current workspace id/name/Dashboard, + * carried through unchanged alongside the op's own next `queries` array. */ +type WorkspaceCandidateState = Pick; + +function buildWorkspaceCandidate(state: WorkspaceCandidateState, queries: SavedQueryV2[]): StoredWorkspaceV1 { + return { + storageVersion: 1, + id: state.workspaceId, + name: state.libraryName.value, + queries, + dashboard: state.dashboard, + }; +} + +/** The committed entry, re-read from the just-committed canonical `queries` + * array (not the locally-computed candidate entry) — the aggregate's commit + * is the single source of truth for what actually persisted. Falls back to + * `fallback` defensively; every real call site's `id` is always present in + * the array it just committed. */ +const committedEntry = (queries: SavedQueryV2[], id: string, fallback: SavedQueryV2): SavedQueryV2 => + queries.find((q) => q.id === id) ?? fallback; + // ── Spec validation seam types ────────────────────────────────────────────── /** Context handed to Spec validators (core/spec-schema.js): the linked SQL @@ -85,22 +148,6 @@ const clamp: (v: number, lo: number, hi: number) => number = clampUntyped; const loadJSON: StateReader['loadJSON'] = loadJSONUntyped; const saveJSON: SaveJSON = saveJSONUntyped; const loadStr: StateReader['loadStr'] = loadStrUntyped; -const saveStr: SaveStr = saveStrUntyped; - -// mergeSaved mints a fresh string id for every merged entry that lacks one, so -// the canonical persisted SavedQueryV2 shape holds for the whole merged list. -const mergeSaved: ( - existing: SavedQueryV2[], incoming: readonly unknown[], genId: () => string, -) => { merged: SavedQueryV2[]; added: number; updated: number; skipped: number } = mergeSavedUntyped; - -// validateLibraryQueries upgrades/validates each raw entry into the canonical -// `{id, sql, specVersion, spec}` shape (throwing on the first invalid one). -// `as`: the .js default parameter (`validationService = null`) makes TS infer -// the param as `null | undefined`; the runtime accepts any object with -// `.validate` (the SpecValidationService seam). -const validateLibraryQueries = validateLibraryQueriesUntyped as ( - queries: readonly unknown[], validationService: SpecValidationService | null, -) => SavedQueryV2[]; // decodeStoredSavedQueries fails closed: `ok: false` carries diagnostics and // no usable value (createState substitutes []); `ok: true` value entries are @@ -230,6 +277,22 @@ export interface AppState { history: HistoryEntry[]; libraryName: Signal; libraryDirty: Signal; + /** #287 W4: the current committed StoredWorkspaceV1's Dashboard document — + * the Workbench NEVER mutates this (that's the /dashboard route's job); it + * is only carried through, unchanged, in every saved-query CRUD commit + * candidate. `null` until the boot projection (app.ts's + * `loadWorkspaceOnBoot`) resolves the aggregate, or when the workspace + * genuinely has no Dashboard yet. */ + dashboard: DashboardDocumentV1 | null; + /** #287 W4: the current committed StoredWorkspaceV1's id, carried forward + * unchanged by every saved-query CRUD commit candidate. `createState` + * mints a session-local placeholder synchronously (never blank — the + * stored-workspace schema requires a non-empty id), so a CRUD op run + * before the boot projection resolves still succeeds, committing the + * first-ever aggregate under that id; `loadWorkspaceOnBoot` overwrites it + * with the real committed id (existing or freshly migrated) once + * resolved. See `createState`'s own comment on `mintWorkspaceId`. */ + workspaceId: string; libraryFilter: string; shortcutsOpen: Signal; isMobile: Signal; @@ -469,6 +532,21 @@ export function createState(read: StateReader = { loadJSON, loadStr }): AppState // resets on reload. Read/write via `.value`. libraryName: signal(read.loadStr(KEYS.libraryName, DEFAULT_LIBRARY_NAME)), libraryDirty: signal(false), + // #287 W4: the aggregate projection. `dashboard` has no aggregate to read + // yet at this synchronous constructor — it starts `null` and is populated + // once app.ts's async boot step (`loadWorkspaceOnBoot`) resolves the real + // StoredWorkspaceV1 (after the one-shot legacy migration). `workspaceId` + // is minted here rather than left blank: the stored-workspace schema + // requires a non-empty id, so a save attempted in the window before boot + // projection completes (or by a fixture that never runs it at all — e.g. + // a unit test driving `createApp` directly) still succeeds, committing + // the FIRST-ever aggregate under this freshly-minted id; `loadCurrent`'s + // migration marker is keyed on store record existence, so that commit is + // simply treated as "already migrated" rather than raced/overwritten. + // `loadWorkspaceOnBoot` overwrites this with the real committed id once + // it resolves (a pre-existing aggregate, or the one migration just built). + dashboard: null, + workspaceId: mintWorkspaceId(), // Transient search text for the Library/History side panel (session-only, // cleared on a tab switch); never persisted. libraryFilter: '', @@ -520,6 +598,11 @@ export function allocTabId(state: AppState): string { const rnd = () => Math.random().toString(36).slice(2, 6); const makeId = (prefix: string, now: number) => prefix + now + rnd(); + +// #287 W4: `createState`'s synchronous placeholder workspace id (see its own +// doc comment) — a session-local id, distinct enough that two tabs opened at +// once don't collide before either resolves the real aggregate. +const mintWorkspaceId = (): string => 'ws-' + Date.now().toString(36) + rnd() + rnd(); // Narrowed to `Pick` (#276 Phase 4C) — the only field read // — so `patchSavedSpec`'s own narrowed `state` param (below) can pass it // through unchanged; every real caller already passes a full `AppState`, @@ -581,13 +664,13 @@ export function savedForTab( * (app.ts's own `SavedQueryService`) already has a full `AppState` to pass, * which satisfies this directly. */ -export function createSavedQuery( - state: Pick, - tab: QueryTab | null | undefined, name: unknown, description?: unknown, - save: SaveJSON = saveJSON, now: number = Date.now(), +export async function createSavedQuery( + state: Pick, + tab: QueryTab | null | undefined, name: unknown, description: unknown, + commit: CommitWorkspace, now: number = Date.now(), validationService: SpecValidationService = defaultSpecValidationService, -): SavedQueryV2 | null { - if (!tab || tab.savedId) return null; +): Promise { + if (!tab || tab.savedId) return { ok: false, entry: null }; const sql = String(tab.sqlDraft || ''); const nm = String(name || '').trim(); const panel = tabPanel(tab); @@ -596,7 +679,7 @@ export function createSavedQuery( // (`cfg!`: every panel this save path sees carries a cfg — the schema marks // cfg optional only for forward compatibility.) const sqlOptional = panel && panel.cfg!.type === 'text'; - if ((!sql.trim() && !sqlOptional) || !nm) return null; + if ((!sql.trim() && !sqlOptional) || !nm) return { ok: false, entry: null }; const desc = String(description || '').trim(); // Remember the current result view (Table/JSON/Panel) so a restore reopens the // same data representation; the transient raw view isn't persisted. @@ -610,45 +693,61 @@ export function createSavedQuery( view, }); const entry = asSavedEntry(withQuerySpec({ ...draft, id: makeId('s', now), sql }, normalizeSpec(draft.spec))); - if (hasBlockingSpecErrors(validationService.validate(entry.spec, { sql, query: entry, tab }))) return null; - state.savedQueries.unshift(entry); - tab.savedId = entry.id; + if (hasBlockingSpecErrors(validationService.validate(entry.spec, { sql, query: entry, tab }))) { + return { ok: false, entry: null }; + } + // COMPUTE only above this line — no `state`/`tab` mutation yet. Build the + // whole-workspace candidate and await its strict commit before touching + // anything (#287 W4: the aggregate is the single source of truth). + const nextQueries = [entry, ...state.savedQueries]; + const result = await commit(buildWorkspaceCandidate(state, nextQueries)); + if (!result.ok) return { ok: false, entry: null, diagnostics: result.diagnostics }; + // APPLY only after `ok: true` — project the canonical committed queries, + // then mutate the tab exactly as the pre-#287 sync code did. + state.savedQueries = result.workspace.queries; + const saved = committedEntry(state.savedQueries, entry.id, entry); + tab.savedId = saved.id; tab.specVersion = SPEC_VERSION; - tab.sqlDraft = entry.sql; + tab.sqlDraft = saved.sql; tab.dirtySql = false; - tab.name = queryName(entry); - setTabSpecDraft(tab, entry.spec, { validationService }); + tab.name = queryName(saved); + setTabSpecDraft(tab, saved.spec, { validationService }); state.libraryDirty.value = true; - save(KEYS.saved, state.savedQueries); - return entry; + return { ok: true, entry: saved }; } -/** Atomically persist both documents of a linked tab in one Library write. - * Narrowed to `Pick` (#276 Phase - * 4C), same convention as `createSavedQuery` above. */ -export function commitSavedQuery( - state: Pick, tab: QueryTab, spec: QuerySpecDraft | null | undefined, - save: SaveJSON = saveJSON, +/** Atomically persist both documents of a linked tab in one strict aggregate + * commit. Narrowed to the exact fields `buildWorkspaceCandidate` also needs + * (#287 W4), same convention as `createSavedQuery` above. */ +export async function commitSavedQuery( + state: Pick, + tab: QueryTab, spec: QuerySpecDraft | null | undefined, + commit: CommitWorkspace, validationService: SpecValidationService = defaultSpecValidationService, -): SavedQueryV2 | null { +): Promise { const index = tab && tab.savedId ? state.savedQueries.findIndex((query) => query.id === tab.savedId) : -1; - if (index < 0 || !spec) return null; + if (index < 0 || !spec) return { ok: false, entry: null }; const normalized = normalizeSpec(spec); const sql = String(tab.sqlDraft || ''); const diagnostics = validationService.validate(normalized, { sql, tab }); - if (hasBlockingSpecErrors(diagnostics)) return null; + if (hasBlockingSpecErrors(diagnostics)) return { ok: false, entry: null }; const panel = queryPanel({ spec: normalized }); - if (!sql.trim() && panel?.cfg?.type !== 'text') return null; + if (!sql.trim() && panel?.cfg?.type !== 'text') return { ok: false, entry: null }; const current = state.savedQueries[index]; const entry = asSavedEntry(withQuerySpec({ id: current.id, sql }, normalized)); - state.savedQueries[index] = entry; + // COMPUTE only above — build the next array without mutating `state.savedQueries` in place. + const nextQueries = state.savedQueries.slice(); + nextQueries[index] = entry; + const result = await commit(buildWorkspaceCandidate(state, nextQueries)); + if (!result.ok) return { ok: false, entry: null, diagnostics: result.diagnostics }; + state.savedQueries = result.workspace.queries; + const saved = committedEntry(state.savedQueries, entry.id, entry); tab.specVersion = SPEC_VERSION; - tab.name = queryName(entry); + tab.name = queryName(saved); tab.dirtySql = false; - setTabSpecDraft(tab, entry.spec, { validationService }); + setTabSpecDraft(tab, saved.spec, { validationService }); state.libraryDirty.value = true; - save(KEYS.saved, state.savedQueries); - return entry; + return { ok: true, entry: saved }; } /** @@ -661,11 +760,12 @@ export function commitSavedQuery( * `commitSavedQuery`. `renameSaved`/`toggleFavorite` below keep passing a * full `AppState` through unchanged (it satisfies this directly). */ -export function patchSavedSpec( - state: Pick, id: string, patch: SpecPatch, - save: SaveJSON = saveJSON, +export async function patchSavedSpec( + state: Pick, + id: string, patch: SpecPatch, + commit: CommitWorkspace, validationService: SpecValidationService = defaultSpecValidationService, -): PatchSavedResult { +): Promise { const invalidTab = invalidSpecTabForSaved(state, id); if (invalidTab) return { ok: false, invalidTab, entry: null }; const index = state.savedQueries.findIndex((query) => query.id === id); @@ -685,14 +785,21 @@ export function patchSavedSpec( return { ok: false, invalidTab: update.tab, entry: null, diagnostics }; } } - state.savedQueries[index] = entry; + // COMPUTE only above — no `state`/`tabs` mutation yet. + const nextQueries = state.savedQueries.slice(); + nextQueries[index] = entry; + const result = await commit(buildWorkspaceCandidate(state, nextQueries)); + if (!result.ok) { + return { ok: false, invalidTab: null, entry: null, diagnostics: asSpecDiagnostics(result.diagnostics) }; + } + state.savedQueries = result.workspace.queries; + const saved = committedEntry(state.savedQueries, entry.id, entry); for (const update of draftUpdates) { setTabSpecDraft(update.tab, update.spec, { dirty: update.dirty, validationService }); update.tab.name = queryName({ spec: update.spec }); } state.libraryDirty.value = true; - save(KEYS.saved, state.savedQueries); - return { ok: true, invalidTab: null, entry }; + return { ok: true, invalidTab: null, entry: saved }; } /** @@ -700,11 +807,11 @@ export function patchSavedSpec( * `description` is provided (not undefined) it is set/cleared too; pass * undefined to leave the existing description untouched (name-only rename). */ -export function renameSaved( - state: AppState, id: string, name: unknown, description?: string | null, - save: SaveJSON = saveJSON, +export async function renameSaved( + state: AppState, id: string, name: unknown, description: string | null | undefined, + commit: CommitWorkspace, validationService: SpecValidationService = defaultSpecValidationService, -): PatchSavedResult | undefined { +): Promise { const nm = String(name || '').trim(); const index = state.savedQueries.findIndex((q) => q.id === id); const entry = index >= 0 ? state.savedQueries[index] : null; @@ -714,20 +821,20 @@ export function renameSaved( const desc = String(description || '').trim(); // match saveQuery: null/non-string → '' → cleared patch.description = desc || undefined; } - return patchSavedSpec(state, id, patch, save, validationService); + return patchSavedSpec(state, id, patch, commit, validationService); } /** Toggle a saved query's favorite flag. */ -export function toggleFavorite( +export async function toggleFavorite( state: AppState, id: string, - save: SaveJSON = saveJSON, + commit: CommitWorkspace, validationService: SpecValidationService = defaultSpecValidationService, -): PatchSavedResult | undefined { +): 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 }, save, validationService); + return patchSavedSpec(state, id, { favorite }, commit, validationService); } /** Saved queries with favorites first (stable within each group). */ @@ -758,110 +865,27 @@ export function filterHistory(list: HistoryEntry[], query: unknown): HistoryEntr return list.filter((ent) => (ent.sql || '').toLowerCase().includes(q)); } -/** - * Merge imported queries into savedQueries (dedupe by content, update by id, - * else add). Returns { added, updated, skipped }. - */ -export function importSaved( - state: AppState, queries: readonly unknown[], - save: SaveJSON = saveJSON, genId: () => string = () => makeId('s', Date.now()), -): { added: number; updated: number; skipped: number } { - const { merged, added, updated, skipped } = mergeSaved(state.savedQueries, queries, genId); - state.savedQueries = merged; - state.libraryDirty.value = true; - save(KEYS.saved, state.savedQueries); - return { added, updated, skipped }; -} - -/** Delete a saved query by id and clear any tab pointer to it. */ -export function deleteSaved(state: AppState, id: string, save: SaveJSON = saveJSON): void { - state.savedQueries = state.savedQueries.filter((q) => q.id !== id); +/** The strict-commit result of `deleteSaved`/similar whole-collection ops + * that have no single "entry" to hand back. */ +export type CommitOnlyResult = { ok: true } | { ok: false; diagnostics: WorkspaceDiagnostic[] }; + +/** Delete a saved query by id and clear any tab pointer to it — a strict + * aggregate commit (#287 W4): on `ok: false` NOTHING is mutated (the query + * and every tab pointer to it are left exactly as they were). */ +export async function deleteSaved( + state: Pick, + id: string, commit: CommitWorkspace, +): Promise { + const nextQueries = state.savedQueries.filter((q) => q.id !== id); + const result = await commit(buildWorkspaceCandidate(state, nextQueries)); + if (!result.ok) return { ok: false, diagnostics: result.diagnostics }; + state.savedQueries = result.workspace.queries; for (const t of tabsForSaved(state, id)) { t.savedId = null; t.editorMode = 'sql'; } state.libraryDirty.value = true; - save(KEYS.saved, state.savedQueries); -} - -// ── Library document ops ──────────────────────────────────────────────────── -// The saved-query collection is a named, savable document. These ops back the -// header File menu (New / Save / Replace / Append) and the editable library -// name + unsaved-changes dot. - -/** Clear tab→saved links whose entry no longer exists (after New/Replace), so a - * kept tab doesn't show "Saved" against a query that's gone. */ -function pruneTabLinks(state: AppState): void { - const ids = new Set(state.savedQueries.map((q) => q.id)); - for (const t of state.tabs.value) { - if (t.savedId && !ids.has(t.savedId)) { - t.savedId = null; - t.editorMode = 'sql'; - } - } -} - -/** Rename the library (blank → the default name). Marks dirty; persists name. */ -export function renameLibrary(state: AppState, name: unknown, saveName: SaveStr = saveStr): void { - state.libraryName.value = String(name || '').trim() || DEFAULT_LIBRARY_NAME; - state.libraryDirty.value = true; - saveName(KEYS.libraryName, state.libraryName.value); -} - -/** Start an empty, default-named library. Clears dirty; open tabs are kept - * (their now-dangling saved links are pruned). */ -export function newLibrary(state: AppState, save: SaveJSON = saveJSON, saveName: SaveStr = saveStr): void { - state.savedQueries = []; - pruneTabLinks(state); - state.libraryName.value = DEFAULT_LIBRARY_NAME; - state.libraryDirty.value = false; - save(KEYS.saved, state.savedQueries); - saveName(KEYS.libraryName, state.libraryName.value); -} - -/** Replace the library with `queries`, adopting the loaded file's base name. - * Unique ids are kept (lossless round-trip); missing OR duplicate ids get a fresh id. - * Clears dirty; open tabs are kept (dangling links pruned). */ -export function replaceLibrary( - state: AppState, queries: readonly Record[], fileName: unknown, - save: SaveJSON = saveJSON, saveName: SaveStr = saveStr, - genId: () => string = () => makeId('s', Date.now()), - validationService: SpecValidationService | false = defaultSpecValidationService, -): void { - const validated = validationService === false ? queries : validateLibraryQueries(queries, validationService); - const seen = new Set(); - state.savedQueries = validated.map((q) => { - // Mint a fresh id for a missing OR already-seen id so every saved row has a - // unique id. The sidebar addresses rows by id (find/filter), so a duplicate - // id would let one delete remove several rows and rename/favorite hit the - // wrong one. (mergeSaved-based import already collapsed dup ids; keep parity.) - let id = q.id; - if (!id || seen.has(id)) { do { id = genId(); } while (seen.has(id)); } - seen.add(id); - return asSavedEntry(withQuerySpec({ ...q, id }, q.spec)); - }); - pruneTabLinks(state); - const base = String(fileName || '').replace(/\.[^.]+$/, '').trim(); - state.libraryName.value = base || DEFAULT_LIBRARY_NAME; - state.libraryDirty.value = false; - save(KEYS.saved, state.savedQueries); - saveName(KEYS.libraryName, state.libraryName.value); -} - -/** Append `queries` into the library via the standard merge dedupe (sets dirty - * through importSaved). Returns { added, updated, skipped }. */ -export function appendLibrary( - state: AppState, queries: readonly Record[], - save: SaveJSON = saveJSON, - genId: () => string = () => makeId('s', Date.now()), - validationService: SpecValidationService | false = defaultSpecValidationService, -): { added: number; updated: number; skipped: number } { - return importSaved(state, validationService === false ? queries : validateLibraryQueries(queries, validationService), save, genId); -} - -/** Mark the library as saved to a file (clears the unsaved-changes dot). */ -export function markLibrarySaved(state: AppState): void { - state.libraryDirty.value = false; + return { ok: true }; } // Push one history entry (most-recent first, capped at 50). Internal — the diff --git a/src/styles.css b/src/styles.css index b5385d77..4d3742e3 100644 --- a/src/styles.css +++ b/src/styles.css @@ -412,6 +412,24 @@ body { .fm-dialog-cancel:hover { background: var(--bg-hover); } .fm-dialog-confirm { border: none; background: var(--accent); color: #fff; font-weight: 600; } .fm-dialog-confirm:hover { filter: brightness(1.08); } +/* Import conflict dialog + multi-dashboard picker (#287 W5) — wider variant + of the confirm card above, plus the conflict-row/select controls. */ +.fm-dialog-card--wide { width: 460px; } +.fm-select { + font-family: inherit; font-size: 12px; padding: 4px 6px; border-radius: 6px; + border: 1px solid var(--border); background: var(--bg-modal); color: var(--fg); +} +.fm-conflict-global { + display: flex; align-items: center; justify-content: space-between; gap: 10px; + margin-top: 8px; padding-top: 10px; border-top: 1px solid var(--border-faint); +} +.fm-conflict-rows { max-height: 220px; overflow-y: auto; margin-top: 6px; } +.fm-conflict-row { + display: flex; align-items: center; justify-content: space-between; gap: 10px; + padding: 6px 0; border-bottom: 1px solid var(--border-faint); +} +.fm-conflict-row:last-child { border-bottom: none; } +.fm-picker-list { display: flex; flex-direction: column; gap: 2px; max-height: 260px; overflow-y: auto; } /* ------------ main row ------------ */ /* position:relative so the mobile sidebar overlay + backdrop (#126) can anchor diff --git a/src/ui/app.ts b/src/ui/app.ts index e5041ebf..dd170fc5 100644 --- a/src/ui/app.ts +++ b/src/ui/app.ts @@ -12,7 +12,7 @@ import { normalizeRowLimit, } from '../state.js'; import type { QueryTab, AppState, SpecValidationService } from '../state.js'; -import type { SavedQueryV2 } from '../generated/json-schema.types.js'; +import type { SavedQueryV2, StoredWorkspaceV1 } from '../generated/json-schema.types.js'; import { splitStatements } from '../core/sql-split.js'; import { analysisView, fieldControls, fieldControlKind } from '../core/param-pipeline.js'; import { hasOptionalBlocks } from '../core/optional-blocks.js'; @@ -282,6 +282,7 @@ export function createApp(env: CreateAppEnv = {}): App { saveJSON, now: () => Date.now(), specValidators, + workspace: app.workspace, }); app.saved = saved; app.sqlEditor.onDocChange((value) => { @@ -1200,19 +1201,25 @@ export function createApp(env: CreateAppEnv = {}): App { return { close }; } - function commitLinkedQuery(): SavedQueryV2 | null { + async function commitLinkedQuery(): Promise { const tab = app.activeTab(); const evaluated = queryDoc.evaluateSpecDraft(tab, tab.specText, { dirty: tab.dirtySpec }); - const result = saved.commit(tab, evaluated); + // Serialized with every other saved-query write so a save can't interleave + // with a concurrent star/delete and commit a stale whole-workspace candidate + // (#287 review fix — see `app.serializeWrite`). + const result = await app.serializeWrite(() => saved.commit(tab, evaluated)); if (!result.ok) { - // 'rejected' (commit's own defensive re-check inside the service) stays - // a silent no-op, same as the pre-extraction inline code's own bare - // `if (!entry) return null;`. + // 'rejected' (commit's own defensive re-check inside the service, OR the + // aggregate strictly rejecting the whole-workspace commit — #287 W4) + // stays a silent no-op for the tab/editor state (nothing was mutated), + // but a real commit rejection still surfaces its first diagnostic. if (result.reason === 'invalid-spec') { queryDoc.revealFirstSpecError(tab); flashToast('Fix Spec errors before saving', { document: doc }); } else if (result.reason === 'empty') { flashToast('Nothing to save', { document: doc }); + } else if (result.diagnostics?.length) { + flashToast('Save failed: ' + result.diagnostics[0].message, { document: doc }); } return null; } @@ -1227,7 +1234,7 @@ export function createApp(env: CreateAppEnv = {}): App { return result.entry; } - function saveActiveQuery(): SavedQueryV2 | null | undefined { + async function saveActiveQuery(): Promise { if (savedForTab(app.state, app.activeTab())) return commitLinkedQuery(); openSavePopover(); return undefined; @@ -1248,10 +1255,13 @@ export function createApp(env: CreateAppEnv = {}): App { const input = h('input', { class: 'sp-input', value: prefill }); const descInput = h('textarea', { class: 'sp-desc', rows: '3', placeholder: 'What this query does — included in Markdown export' }); let close: () => void; - const commit = (): void => { + const commit = async (): Promise => { if (!input.value.trim()) return; - const result = saved.create(tab, input.value, descInput.value); - if (!result.ok) return; + const result = await app.serializeWrite(() => saved.create(tab, input.value, descInput.value)); + if (!result.ok) { + if (result.diagnostics?.length) flashToast('Save failed: ' + result.diagnostics[0].message, { document: doc }); + return; + } close(); queryDoc.revalidateSpecDrafts(); app.specEditor.syncFromState(); @@ -1379,6 +1389,60 @@ export function createApp(env: CreateAppEnv = {}): App { return app.workspace.loadCurrent(); }; + // #287 W4: the async boot-init step — migrate-if-needed + loadCurrent (via + // `loadDashboardWorkspace` above), then PROJECT the resolved aggregate onto + // `state` so the Workbench (not only the /dashboard route) treats it as the + // saved-query collection's single source of truth. `main.ts`'s `bootstrap` + // awaits this before the first `renderApp()`. A null/failed load leaves + // `state` exactly as `createState()`'s synchronous legacy read already + // populated it (a brand-new install with nothing to migrate yet, or a + // degraded IndexedDB) — including its own synchronously-minted + // `workspaceId` placeholder (see `createState`'s `mintWorkspaceId`), so a + // saved-query CRUD op run in this window (or by a caller that never awaits + // this step at all) still succeeds rather than failing closed. + // #287 W5: the projection every commit of the aggregate onto `state` + // shares — extracted from this same assignment's pre-W5 inline body so + // `loadWorkspaceOnBoot` and every file-menu.js write (New/Import/Replace/ + // rename) apply it identically. `libraryDirty` clears here too: a workspace + // that was JUST committed (boot's own load, or a file-menu op's commit) is + // by construction in sync with what's persisted, matching the pre-#287 + // New/Replace-clears-dirty behavior file-menu.js's own ops used to apply + // directly. + const applyCommittedWorkspace = (workspace: StoredWorkspaceV1): void => { + app.state.savedQueries = workspace.queries; + app.state.dashboard = workspace.dashboard; + app.state.workspaceId = workspace.id; + app.state.libraryName.value = workspace.name; + app.state.libraryDirty.value = false; + }; + app.applyCommittedWorkspace = applyCommittedWorkspace; + // #287 W5: the shared WorkspaceIdGen seam file-menu.js's New workspace / + // Import / Replace operations mint fresh ids through — the same generator + // `loadDashboardWorkspace`'s one-shot legacy migration already uses inline + // above (`uid('ws-')`). + app.genId = () => uid('ws-'); + + // #287 review fix: serialize saved-query writes so overlapping async CRUD + // commits can't interleave. Without this, a delete and a star toggle fired in + // rapid succession each build a candidate from the same stale + // `state.savedQueries` snapshot, and whichever commits LAST wins — resurrecting + // a just-deleted query (or clobbering a concurrent edit). Chaining each op + // after the previous one fully resolves means the next op reads the freshest + // projected state. The chain swallows rejections so one failed op never + // wedges the queue; the op's own result/rejection still reaches its caller. + let writeChain: Promise = Promise.resolve(); + app.serializeWrite = (op: () => Promise): Promise => { + const run = writeChain.then(op, op); + writeChain = run.then(() => undefined, () => undefined); + return run; + }; + + app.loadWorkspaceOnBoot = async () => { + const workspace = await app.loadDashboardWorkspace(); + if (workspace) applyCommittedWorkspace(workspace); + return workspace; + }; + // Open the dashboard in a new tab and stand ready to hand it our credentials // — the cross-tab auth-handoff GRANT side itself is the session's job now // (`conn.grantHandoffTo`, #276 Phase 2); this stays app-side only because @@ -1398,7 +1462,14 @@ export function createApp(env: CreateAppEnv = {}): App { closeTab: (id) => closeTab(app, id), loadIntoNewTab: (queryOrName, sql) => { loadIntoNewTab(app, queryOrName, sql); toEditorOnMobile(); }, login: (idpId, targetOrigin) => conn.beginOAuth(idpId, targetOrigin), - connect: async (input) => { await conn.connectBasic(input); app.renderApp(); }, + // Basic-auth login renders in-page (no page reload), so — unlike the OAuth + // path, where `main.ts`'s `bootstrap` awaits it — this is the ONLY place the + // aggregate load + legacy migration runs for a username/password session. + // Without it a first basic-auth session would render on the placeholder + // workspaceId and skip `migrateLegacyWorkspaceIfNeeded`, so the first CRUD + // commit would mint an orphan aggregate the migration marker then treats as + // "already migrated" — permanently stranding legacy favorites/layout (#287). + connect: async (input) => { await conn.connectBasic(input); await app.loadWorkspaceOnBoot(); app.renderApp(); }, share, copyResult, // `ActionsRegistry.copySnapshot`'s public `result: Json | null` is looser diff --git a/src/ui/app.types.ts b/src/ui/app.types.ts index 7c6243fd..61f489f5 100644 --- a/src/ui/app.types.ts +++ b/src/ui/app.types.ts @@ -134,11 +134,12 @@ export interface ActionsRegistry { exportDirect(sqlInput: string, waveMs: number): Promise; cancelExport(): void; cancelExportScript(): void; - /** null: nothing to save (empty draft); undefined: the create-popover opened - * instead of returning a result. A committed/created save resolves the real - * generated `SavedQueryV2` entry (state.ts's `commitSavedQuery`/ - * `createSavedQuery`) — `Json` undersold it as opaque. */ - save(): SavedQueryV2 | null | undefined; + /** null: nothing to save (empty draft, or the aggregate strictly rejected + * the commit — #287 W4); undefined: the create-popover opened instead of + * returning a result. A committed/created save resolves the real generated + * `SavedQueryV2` entry (state.ts's `commitSavedQuery`/`createSavedQuery`, + * both async now) — `Json` undersold it as opaque. */ + save(): Promise; openUserMenu(): void; formatQuery(): Promise; formatSpec(): void; @@ -366,6 +367,42 @@ export interface App { * exists. Returns null when neither an aggregate nor a migratable legacy * workspace is available. */ loadDashboardWorkspace(): Promise; + /** #287 W4: the async boot-init step — runs `loadDashboardWorkspace` + * (migrate-if-needed, then `workspace.loadCurrent()`) and, when it + * resolves a real aggregate, PROJECTS it onto `state` (`savedQueries`, + * `dashboard`, `workspaceId`, `libraryName`) so the whole app (not only + * the /dashboard route) treats the aggregate as the saved-query + * collection's single source of truth. `main.ts`'s `bootstrap` awaits + * this before the first `renderApp()`. On a null/failed load, `state` + * keeps whatever the legacy-projected `createState()` synchronous read + * already populated (a brand-new install, or a degraded IndexedDB). */ + loadWorkspaceOnBoot(): Promise; + /** #287 W5: project a committed `StoredWorkspaceV1` onto `state` + * (`savedQueries`/`dashboard`/`workspaceId`/`libraryName`, and clear + * `libraryDirty` — a fresh committed workspace is, by construction, in + * sync with what's persisted) — the exact projection `loadWorkspaceOnBoot` + * inlined pre-#287 W5, now shared with every file-menu commit (New/Import/ + * Replace/rename) so they never fork from the boot projection. Repaint + * (`updateSaveBtn`/`updateEditorModeUi`/`renderSavedHistory`) is the + * caller's job — this never touches `app.dom` (it also runs during boot, + * before the first `renderApp()`/mount). */ + applyCommittedWorkspace(workspace: StoredWorkspaceV1): void; + /** #287 W5: a fresh, unguessable id — the same generator + * `loadDashboardWorkspace`'s legacy migration already uses internally + * (`uid('ws-')`), exposed here as the injected `WorkspaceIdGen` the + * file-menu's New workspace / Import / Replace operations pass to + * `createNewWorkspace`/the import planner. One shared generator: a minted + * id only needs to be unique, never to encode which op minted it. */ + genId(): string; + /** #287 review fix: serialize saved-query write operations per-app so two + * overlapping async CRUD commits can't interleave. Each queued op runs only + * after the previous fully resolved (compute → commit → project), so it + * reads the freshest `state.savedQueries` — without this, a delete and a + * star toggle fired in rapid succession could each build a candidate from the + * same stale snapshot and the later commit would resurrect the deleted query + * (or clobber a concurrent edit). Rejections propagate to the caller; the + * queue itself never rejects. */ + serializeWrite(op: () => Promise): Promise; actions: ActionsRegistry; } diff --git a/src/ui/dashboard.ts b/src/ui/dashboard.ts index a034b7c0..977d1eae 100644 --- a/src/ui/dashboard.ts +++ b/src/ui/dashboard.ts @@ -294,7 +294,7 @@ export async function renderDashboard(app: DashboardApp): Promise { const filterDiagnosticsHost = h('div', { class: 'dash-filter-diagnostics' }); const grid = h('div', { class: 'dash-grid' }); const empty = h('div', { class: 'dash-empty', style: { display: currentDoc.tiles.length ? 'none' : '' } }, - 'No tiles yet — star a query in the Library to add it to the dashboard.'); + 'No tiles yet — star a query in the Queries panel to add it to the dashboard.'); // ── Structural commands (reorder via drag, preset) ──────────────────────── // move-tile / update-placement / change-layout are the phase-3 authoring diff --git a/src/ui/file-menu.ts b/src/ui/file-menu.ts index 399ea37b..38b2f6c2 100644 --- a/src/ui/file-menu.ts +++ b/src/ui/file-menu.ts @@ -1,28 +1,53 @@ -// The header "File ▾" menu: the saved-query collection treated as a savable -// document ("the Library"). New / Save (JSON) / Open (replace) / Append, plus one-way -// Markdown/SQL "share" downloads, an editable library name, and an -// unsaved-changes dot. Render module over the `app` controller; every side -// effect goes through an injected seam (app.saveJSON / app.saveStr / -// app.downloadFile / app.FileReader / app.document), so it is fully testable. +// The header "File ▾" menu (#287 W5): resource-oriented portable-bundle +// workspace operations — New workspace / Import queries / Import Dashboard / +// Replace workspace / Export Dashboard / Export workspace — plus the kept +// "Open as dashboard" (#149), the Variable history section, and the one-way +// Markdown/SQL "share" downloads (buildMarkdownDoc/buildSqlDoc, unchanged). +// The legacy Library New/Save-JSON/Open-replace/Append ops are gone; every +// write here resolves a `PortableBundleImportPlan` (workspace/import-planner.js) +// or a repository-level primitive (workspace/workspace-operations.js), then +// commits the WHOLE candidate through `app.workspace.commit` and projects the +// result onto `state` via `app.applyCommittedWorkspace` before repainting +// (`afterLibraryChange`). External read-only "Open for viewing" + a trust +// preflight are DEFERRED to #288 — every operation here only COMMITS to the +// workspace, never executes a query. Render module over the `app` +// controller; every side effect goes through an injected seam +// (app.workspace.commit / app.downloadFile / app.FileReader / app.document / +// app.genId / app.wallNow), so it is fully testable. import { h, fixedAnchor, attachBackdropClose } from './dom.js'; import { Icon } from './icons.js'; import { flashToast } from './toast.js'; import { renderSavedHistory } from './saved-history.js'; -import { buildExportDoc, parseImportDoc, buildMarkdownDoc, buildSqlDoc } from '../core/saved-io.js'; -import { queryFavorite } from '../core/saved-query.js'; -import { newLibrary, replaceLibrary, appendLibrary, renameLibrary, markLibrarySaved } from '../state.js'; +import { buildMarkdownDoc, buildSqlDoc } from '../core/saved-io.js'; +import { queryFavorite, queryName } from '../core/saved-query.js'; +import { decodePortableBundleJson, encodePortableBundleJson } from '../dashboard/model/portable-bundle-codec.js'; +import { normalizeLegacyLibraryToBundle } from '../dashboard/model/legacy-bundle.js'; +import { buildDashboardExportBundle, buildWorkspaceExportBundle } from '../dashboard/model/dashboard-export.js'; +import { dashboardDependencyQueryIds } from '../dashboard/model/bundle-order.js'; +import { + planImportQueries, planImportDashboard, planReplaceWorkspace, + detectQueryConflicts, autoResolveConflicts, listBundleDashboards, +} from '../workspace/import-planner.js'; +import type { + QueryDecision, QueryConflict, QueryConflictAction, DashboardSummary, PortableBundleImportPlan, +} from '../workspace/import-planner.js'; +import { createNewWorkspace, renameWorkspace } from '../workspace/workspace-operations.js'; import type { App } from './app.types.js'; +import type { PortableBundleV1, SavedQueryV2, StoredWorkspaceV1 } from '../generated/json-schema.types.js'; +import type { WorkspaceDiagnostic } from '../dashboard/model/workspace-diagnostics.js'; -/** Library name → safe file base (strips path/illegal chars, collapses spaces). */ +/** Workspace/library name → safe file base (strips path/illegal chars, + * collapses spaces). */ const fileBase = (name: unknown): string => (String(name || '')).replace(/[\\/:*?"<>|]+/g, '-').replace(/\s+/g, ' ').trim() || 'queries'; const queries = (n: number): string => n + (n === 1 ? ' query' : ' queries'); +const first = (diagnostics: readonly WorkspaceDiagnostic[], fallback: string): string => diagnostics[0]?.message || fallback; -/** Build the header File button + editable library title; returns the nodes to - * splice into the app header (after the connection chip). */ +/** Build the header File button + editable workspace title; returns the nodes + * to splice into the app header (after the connection chip). */ export function libraryControls(app: App): HTMLElement[] { app.dom.fileBtn = h('button', { - class: 'hd-file-btn', title: 'File — save or load your library', + class: 'hd-file-btn', title: 'File — workspace and dashboard import/export', onclick: () => openFileMenu(app), }, h('span', null, 'File'), Icon.chevDown()); app.dom.libraryTitle = h('div', { class: 'lib-title' }); @@ -30,7 +55,7 @@ export function libraryControls(app: App): HTMLElement[] { return [app.dom.fileBtn, app.dom.libraryTitle]; } -/** (Re)render the library title into its slot: a click-to-rename name button +/** (Re)render the workspace title into its slot: a click-to-rename name button * with an unsaved-changes dot, or an inline rename input while editing. */ export function renderLibraryTitle(app: App): void { const slot = app.dom.libraryTitle; @@ -45,11 +70,13 @@ export function renderLibraryTitle(app: App): void { const finish = (commit: boolean): void => { if (done) return; done = true; - // Leave edit mode first, so the renameLibrary write below repaints the - // button view via the title effect rather than a transient input. + // Leave edit mode first, so the async rename commit below repaints the + // button view via the libraryName/libraryDirty effect in createApp + // rather than a transient input. app.editingLibrary = false; - if (commit && input.value.trim()) renameLibrary(state, input.value, app.saveStr); + const nextName = input.value; renderLibraryTitle(app); // explicit: the cancel/no-op path changes no signal + if (commit && nextName.trim()) void renameWorkspaceAction(app, nextName); }; input.addEventListener('keydown', (e) => { if (e.key === 'Enter') { e.preventDefault(); finish(true); } @@ -61,10 +88,10 @@ export function renderLibraryTitle(app: App): void { return; } slot.appendChild(h('button', { - class: 'lib-name', title: 'Rename library', + class: 'lib-name', title: 'Rename workspace', onclick: () => { app.editingLibrary = true; renderLibraryTitle(app); }, }, h('span', { class: 'lib-name-text' }, state.libraryName.value), - state.libraryDirty.value ? h('span', { class: 'lib-dirty', title: 'Unsaved changes since last save / load' }) : null)); + state.libraryDirty.value ? h('span', { class: 'lib-dirty', title: 'Changes since the last export or import' }) : null)); } /** Open the File dropdown anchored under the File button (Esc / outside-click close). */ @@ -79,18 +106,21 @@ export function openFileMenu(app: App): void { }; const onKey = (e: KeyboardEvent): void => { if (e.key === 'Escape') { e.preventDefault(); close(); } }; - const replaceInput = pickerInput(app, (f) => onReplaceFile(app, f)); - const appendInput = pickerInput(app, (f) => onAppendFile(app, f)); + const importQueriesInput = pickerInput(app, (f) => onImportQueriesFile(app, f)); + const importDashboardInput = pickerInput(app, (f) => onImportDashboardFile(app, f)); + const replaceWorkspaceInput = pickerInput(app, (f) => onReplaceWorkspaceFile(app, f)); const item = (icon: Node, label: string, meta: string | null, onClick: () => void): HTMLButtonElement => h('button', { class: 'fm-item', onclick: onClick }, h('span', { class: 'fm-icon' }, icon), h('span', { class: 'fm-label' }, label), meta ? h('span', { class: 'fm-meta' }, meta) : null); const sep = (): HTMLDivElement => h('div', { class: 'fm-sep' }); const empty = list.length === 0; const hasFav = list.some(queryFavorite); + const hasDashboard = !!app.state.dashboard; - const newLibraryItem = item(Icon.plus(), 'New Library', null, () => { close(); newLibraryAction(app); }); - // Open the favorited subset of the Library as a standalone dashboard (#149). - // Enabled only when at least one query is starred; otherwise it explains why. + const newWorkspaceItem = item(Icon.plus(), 'New workspace…', null, () => { close(); newWorkspaceAction(app); }); + // Open the favorited subset of the workspace as a standalone dashboard + // (#149). Enabled only when at least one query is starred; otherwise it + // explains why. const dashboardItem = item(Icon.layers(), 'Open as dashboard', hasFav ? null : 'no favorites', () => { close(); if (!hasFav) { flashToast('Star a query to add it to the dashboard', { document: app.document }); return; } @@ -119,7 +149,7 @@ export function openFileMenu(app: App): void { flashToast('Cleared recent variable values', { document: app.document }); }); const menu = h('div', { class: 'file-menu' }, - newLibraryItem, + newWorkspaceItem, sep(), h('div', { class: 'fm-section' }, 'Dashboard'), dashboardItem, @@ -128,18 +158,24 @@ export function openFileMenu(app: App): void { historyToggle, clearAllRecentItem, sep(), - h('div', { class: 'fm-section' }, 'Save library'), - item(Icon.download(), 'Save JSON', '.json', () => { close(); saveJsonAction(app); }), + h('div', { class: 'fm-section' }, 'Import / replace'), + item(Icon.upload(), 'Import queries…', null, () => { importQueriesInput.click(); close(); }), + item(Icon.upload(), 'Import Dashboard…', null, () => { importDashboardInput.click(); close(); }), + item(Icon.refresh(), 'Replace workspace…', null, () => { replaceWorkspaceInput.click(); close(); }), sep(), - h('div', { class: 'fm-section' }, 'Load from file'), - item(Icon.upload(), 'Open…', null, () => { replaceInput.click(); close(); }), - item(Icon.upload(), 'Append…', null, () => { appendInput.click(); close(); }), + h('div', { class: 'fm-section' }, 'Export'), + item(Icon.download(), 'Export Dashboard…', hasDashboard ? '.json' : 'no dashboard', () => { + close(); + if (!hasDashboard) { flashToast('No dashboard to export', { document: app.document }); return; } + exportDashboardAction(app); + }), + item(Icon.download(), 'Export workspace…', '.json', () => { close(); exportWorkspaceAction(app); }), sep(), h('div', { class: 'fm-section' }, 'Share / publish'), item(Icon.download(), 'Download Markdown', '.md', () => { close(); downloadAction(app, 'md'); }), item(Icon.download(), 'Download SQL', '.sql', () => { close(); downloadAction(app, 'sql'); }), - h('div', { class: 'fm-count' }, empty ? 'Library is empty' : queries(list.length) + ' in Library'), - replaceInput, appendInput); + h('div', { class: 'fm-count' }, empty ? 'Workspace is empty' : queries(list.length) + ' in workspace'), + importQueriesInput, importDashboardInput, replaceWorkspaceInput); const overlay = h('div', { class: 'fm-overlay', onclick: close }); app.dom.fileMenuOverlay = overlay; @@ -156,10 +192,10 @@ export function openFileMenu(app: App): void { menu.style.left = a.left + 'px'; doc.body.appendChild(menu); doc.addEventListener('keydown', onKey, true); - setTimeout(() => newLibraryItem.focus()); + setTimeout(() => newWorkspaceItem.focus()); } -// ── file pickers + JSON read ──────────────────────────────────────────────── +// ── file pickers + bundle decode ──────────────────────────────────────────── function pickerInput(app: App, onPick: (file: File) => void): HTMLInputElement { return h('input', { @@ -173,120 +209,375 @@ function pickerInput(app: App, onPick: (file: File) => void): HTMLInputElement { }); } -/** Read + parse a JSON library file, then `cb(queries)`. Bad files toast. */ -function readJsonFile(app: App, file: File, cb: (queries: unknown[]) => void): void { +// `identifyPortableBundle` (portable-bundle-codec.ts) returns EXACTLY one of +// these codes, and only these, when the document isn't recognizable as a +// portable bundle at all (before any structural validation runs) — the +// signal that legacy Library JSON is worth trying. Any OTHER decode failure +// (malformed JSON, a resource-limit guard, a structurally-invalid portable +// bundle) means the file WAS a portable bundle attempt, so falling back would +// only replace a precise diagnostic with a useless "Unrecognized file format". +const BUNDLE_IDENTITY_CODES = new Set([ + 'bundle-invalid-root', 'bundle-invalid-format', 'bundle-version-missing', + 'bundle-version-invalid', 'bundle-version-unsupported', +]); +const isUnrecognizedBundleFormat = (diagnostics: readonly WorkspaceDiagnostic[]): boolean => + diagnostics.length === 1 && BUNDLE_IDENTITY_CODES.has(diagnostics[0].code); + +/** Read + decode one portable-bundle (or legacy Library v1/v2) JSON file, then + * `onBundle(bundle)`. `decodePortableBundleJson` runs first; a + * format/version-identity failure falls back to + * `normalizeLegacyLibraryToBundle` (legacy Library → an in-memory bundle with + * `dashboards: []`). Any other failure (or a failed fallback) toasts the + * first diagnostic and aborts — never a partial import. */ +function readBundleFile(app: App, file: File, onBundle: (bundle: PortableBundleV1) => void): void { const reader = new (app.FileReader || globalThis.FileReader)(); reader.onload = () => { - try { - cb(parseImportDoc(String(reader.result), null, { - nowISO: new Date(app.wallNow()).toISOString(), - }).queries); + const text = String(reader.result); + const nowISO = new Date(app.wallNow()).toISOString(); + // `decodePortableBundleJson` validates an EXISTING document (it always + // carries its own `exportedAt`) — `nowISO` is only meaningful for the + // legacy fallback below, which builds a fresh bundle envelope around + // queries that may have no `exportedAt` of their own. + const decoded = decodePortableBundleJson(text); + if (decoded.ok) { onBundle(decoded.value); return; } + if (!isUnrecognizedBundleFormat(decoded.diagnostics)) { + flashToast('✕ ' + first(decoded.diagnostics, 'Could not read file'), { document: app.document }); + return; } - catch (e) { flashToast('✕ ' + ((e && (e as Error).message) || e), { document: app.document }); } + const legacy = normalizeLegacyLibraryToBundle(text, { nowISO }); + if (legacy.ok) { onBundle(legacy.value); return; } + flashToast('✕ ' + first(legacy.diagnostics, 'Unrecognized file format'), { document: app.document }); }; reader.onerror = () => flashToast('✕ Could not read file', { document: app.document }); reader.readAsText(file); } -// ── actions ───────────────────────────────────────────────────────────────── +// ── current workspace + commit/project ────────────────────────────────────── -function saveJsonAction(app: App): void { - const qs = app.state.savedQueries; - if (!qs.length) { flashToast('Nothing to save', { document: app.document }); return; } - app.downloadFile(fileBase(app.state.libraryName.value) + '.json', 'application/json', - JSON.stringify(buildExportDoc(qs, new Date(app.wallNow()).toISOString()), null, 2)); - markLibrarySaved(app.state); // clears libraryDirty → title effect drops the dot - flashToast('Saved ' + queries(qs.length) + ' → .json', { document: app.document }); +/** The current committed aggregate, reconstructed from `state` — W4 keeps + * `state.savedQueries`/`dashboard`/`workspaceId`/`libraryName` as a live + * projection of it, so this never needs its own read of `app.workspace`. */ +function currentWorkspace(app: App): StoredWorkspaceV1 { + return { + storageVersion: 1, + id: app.state.workspaceId, + name: app.state.libraryName.value, + queries: app.state.savedQueries, + dashboard: app.state.dashboard, + }; } -function downloadAction(app: App, fmt: 'md' | 'sql'): void { - const qs = app.state.savedQueries; - if (!qs.length) { flashToast('Nothing to save', { document: app.document }); return; } - if (fmt === 'md') app.downloadFile(fileBase(app.state.libraryName.value) + '.md', 'text/markdown', buildMarkdownDoc(qs)); - else app.downloadFile(fileBase(app.state.libraryName.value) + '.sql', 'application/sql', buildSqlDoc(qs)); - flashToast('Saved ' + queries(qs.length) + ' → .' + fmt, { document: app.document }); +/** Re-sync the surfaces a workspace change touches: Save button (tab links + * may be pruned) and the saved list (count + rows). The title (name + dirty + * dot) repaints itself via the libraryName/libraryDirty effect in createApp. */ +function afterLibraryChange(app: App): void { + app.updateSaveBtn(); + // Always defined by the time a file-menu action can run (post-boot, + // post-first-renderApp()) — app.types.ts only marks it optional because it's + // absent before that first render. + app.updateEditorModeUi!(); + renderSavedHistory(app); +} + +/** Commit one whole candidate `StoredWorkspaceV1`, then project + repaint on + * success. A rejected commit toasts the first diagnostic and leaves `state` + * untouched — never a partial write. */ +async function commitWorkspace(app: App, candidate: StoredWorkspaceV1, successMsg?: string): Promise { + const result = await app.workspace.commit(candidate); + if (!result.ok) { + flashToast('✕ ' + first(result.diagnostics, 'Could not save workspace'), { document: app.document }); + return false; + } + app.applyCommittedWorkspace(result.workspace); + afterLibraryChange(app); + if (successMsg) flashToast(successMsg, { document: app.document }); + return true; } -function onReplaceFile(app: App, file: File): void { - readJsonFile(app, file, (qs) => { - if (!qs.length) { flashToast('✕ No queries in file', { document: app.document }); return; } - if (app.state.savedQueries.length) confirmReplace(app, file.name, qs); - else doReplace(app, qs, file.name); +/** Commit an import-planner plan, or toast + abort when it invalidated + * (`candidateWorkspace: null` — e.g. a skipped required Dashboard + * dependency). Never commits a partial/invalid candidate. */ +async function commitPlan(app: App, plan: PortableBundleImportPlan, successMsg: string): Promise { + if (!plan.candidateWorkspace) { + flashToast('✕ ' + first(plan.diagnostics, 'Import failed'), { document: app.document }); + return; + } + await commitWorkspace(app, plan.candidateWorkspace, successMsg); +} + +// ── conflict resolution (global default + per-row override) ──────────────── + +/** Detect id conflicts between `existing`/`incoming`, silently auto-resolve + * the canonically-identical ones, and — only when conflicts with genuinely + * different content remain — open the conflict dialog for the rest before + * calling `run` with the combined `QueryDecision[]`. `run` is never called + * if the dialog is cancelled. */ +function withQueryDecisions( + app: App, existing: readonly SavedQueryV2[], incoming: readonly SavedQueryV2[], + run: (decisions: QueryDecision[]) => void, +): void { + const conflicts = detectQueryConflicts(existing, incoming); + if (!conflicts.length) { run([]); return; } + const auto = autoResolveConflicts(conflicts); + const remaining = conflicts.filter((c) => !c.canonicalEqual); + if (!remaining.length) { run(auto); return; } + openConflictDialog(app, remaining, (decisions) => run([...auto, ...decisions])); +} + +const CONFLICT_ACTIONS: { value: QueryConflictAction; label: string }[] = [ + { value: 'use-existing', label: 'Keep existing' }, + { value: 'copy', label: 'Keep both (copy)' }, + { value: 'replace', label: 'Replace existing' }, + { value: 'skip', label: 'Skip' }, +]; + +function actionSelect( + initial: QueryConflictAction | '', onChange: (v: QueryConflictAction | '') => void, withDefaultOption: boolean, +): HTMLSelectElement { + const select = h('select', { + class: 'fm-select', + onchange: (e: Event) => onChange((e.target as HTMLSelectElement).value as QueryConflictAction | ''), + }) as HTMLSelectElement; + const options = withDefaultOption + ? [{ value: '' as const, label: 'Use default' }, ...CONFLICT_ACTIONS] + : CONFLICT_ACTIONS; + for (const o of options) select.appendChild(h('option', { value: o.value }, o.label)); + select.value = initial; + return select; +} + +/** One global default action selector, plus a per-row override list — the + * owner-decided conflict UI (#287 W5). A row left on "Use default" resolves + * to whichever action the global selector currently holds AT SUBMIT time + * (not when the row was rendered). */ +function openConflictDialog( + app: App, conflicts: readonly QueryConflict[], onResolve: (decisions: QueryDecision[]) => void, +): void { + let globalAction: QueryConflictAction = 'use-existing'; + const overrides = new Map(); + // No `if (v)` guard here (unlike the per-row selects below): the global + // select never offers the "Use default" option (`withDefaultOption: false`), + // so its onchange value is structurally always one of the four real actions. + const globalSelect = actionSelect(globalAction, (v) => { globalAction = v as QueryConflictAction; }, false); + const rows = conflicts.map((c) => { + const rowSelect = actionSelect('', (v) => { + if (v) overrides.set(c.sourceId, v); else overrides.delete(c.sourceId); + }, true); + return h('div', { class: 'fm-conflict-row' }, + h('span', { class: 'fm-label' }, queryName(c.existing)), rowSelect); }); + const handle = openDialogShell(app, `Resolve ${conflicts.length} conflicting ${conflicts.length === 1 ? 'query' : 'queries'}`, [ + h('div', { class: 'fm-dialog-body' }, + 'These incoming queries share an id with an existing query whose content differs.', + h('div', { class: 'fm-conflict-global' }, + h('span', { class: 'fm-label' }, 'Default action for every row below'), globalSelect), + h('div', { class: 'fm-conflict-rows' }, rows)), + h('div', { class: 'fm-dialog-actions' }, + h('button', { class: 'fm-dialog-cancel', onclick: () => handle.close() }, 'Cancel'), + h('button', { class: 'fm-dialog-confirm', onclick: () => { + const decisions: QueryDecision[] = conflicts.map((c) => ( + { sourceId: c.sourceId, action: overrides.get(c.sourceId) ?? globalAction } + )); + handle.close(); + onResolve(decisions); + } }, 'Apply')), + ], 'fm-dialog-card--wide'); } -function doReplace(app: App, qs: unknown[], fileName: string): void { - replaceLibrary(app.state, qs as Record[], fileName, app.saveJSON, app.saveStr, undefined, false); - afterLibraryChange(app); - flashToast('Opened library · ' + queries(qs.length), { document: app.document }); +// ── multi-dashboard picker ─────────────────────────────────────────────────── + +/** Show a picker over `dashboards` (bundle array order — presentation order, + * not re-sorted); `allowNone` adds a "No dashboard" row (Replace workspace's + * own owner decision — Import Dashboard never offers it, since it must + * import exactly one). Cancelling calls neither branch of `onPick`. */ +function openDashboardPicker( + app: App, title: string, dashboards: readonly DashboardSummary[], allowNone: boolean, + onPick: (id: string | null) => void, +): void { + const rows = dashboards.map((d) => h('button', { + class: 'fm-item', onclick: () => { handle.close(); onPick(d.id); }, + }, + h('span', { class: 'fm-label' }, d.title), + h('span', { class: 'fm-meta' }, `${d.tileCount} ${d.tileCount === 1 ? 'tile' : 'tiles'} · ${d.filterCount} ${d.filterCount === 1 ? 'filter' : 'filters'}`))); + const noneRow = allowNone ? h('button', { + class: 'fm-item', onclick: () => { handle.close(); onPick(null); }, + }, h('span', { class: 'fm-label' }, 'No dashboard')) : null; + const handle = openDialogShell(app, title, [ + h('div', { class: 'fm-dialog-body fm-picker-list' }, rows, noneRow), + h('div', { class: 'fm-dialog-actions' }, + h('button', { class: 'fm-dialog-cancel', onclick: () => handle.close() }, 'Cancel')), + ], 'fm-dialog-card--wide'); +} + +// ── actions: New workspace ─────────────────────────────────────────────────── + +function newWorkspaceAction(app: App): void { + const cur = currentWorkspace(app); + if (cur.queries.length || cur.dashboard) confirmNewWorkspace(app, cur); + else void doNewWorkspace(app); +} + +async function doNewWorkspace(app: App): Promise { + const candidate = createNewWorkspace(app.genId); + await commitWorkspace(app, candidate, 'Started a new workspace'); } -function onAppendFile(app: App, file: File): void { - readJsonFile(app, file, (qs) => { - if (!qs.length) { flashToast('✕ No queries in file', { document: app.document }); return; } - const { added, updated, skipped } = appendLibrary(app.state, qs as Record[], app.saveJSON, undefined, false); - afterLibraryChange(app); - flashToast('Added ' + added + ' · updated ' + updated + ' · skipped ' + skipped, { document: app.document }); +function confirmNewWorkspace(app: App, cur: StoredWorkspaceV1): void { + openConfirm(app, { + title: 'Start a new workspace?', + body: ['This clears your current ', h('b', null, String(cur.queries.length)), ' saved ', + cur.queries.length === 1 ? 'query' : 'queries', cur.dashboard ? ' and your Dashboard' : '', + '. Open editor tabs are unaffected. Export first if you want to keep them.'], + confirmLabel: 'New workspace', + onConfirm: () => void doNewWorkspace(app), }); } -function newLibraryAction(app: App): void { - if (app.state.savedQueries.length) { confirmNew(app); return; } - doNew(app); +// ── actions: rename ────────────────────────────────────────────────────────── + +async function renameWorkspaceAction(app: App, name: string): Promise { + const candidate = renameWorkspace(currentWorkspace(app), name); + await commitWorkspace(app, candidate); } -function doNew(app: App): void { - newLibrary(app.state, app.saveJSON, app.saveStr); - afterLibraryChange(app); - flashToast('Started a new library', { document: app.document }); +// ── actions: Import queries ───────────────────────────────────────────────── + +function onImportQueriesFile(app: App, file: File): void { + readBundleFile(app, file, (bundle) => startImportQueries(app, bundle)); } -/** Re-sync the surfaces a library change touches: Save button (tab links may be - * pruned) and the saved list (count + rows). The title (name + dirty dot) - * repaints itself via the libraryName/libraryDirty effect in createApp. */ -function afterLibraryChange(app: App): void { - app.updateSaveBtn(); - // Always defined by the time a library action can run (post-boot, - // post-first-renderApp()) — app.types.ts only marks it optional because it's - // absent before that first render. - app.updateEditorModeUi!(); - renderSavedHistory(app); +function startImportQueries(app: App, bundle: PortableBundleV1): void { + const workspace = currentWorkspace(app); + withQueryDecisions(app, workspace.queries, bundle.queries, (decisions) => { + const plan = planImportQueries(workspace, bundle, decisions, app.genId); + void commitPlan(app, plan, 'Imported ' + queries(bundle.queries.length)); + }); } -// ── confirm dialogs (reuse the modal-backdrop/card visual language) ────────── +// ── actions: Import Dashboard ──────────────────────────────────────────────── -function confirmReplace(app: App, fileName: string, qs: unknown[]): void { - const cur = app.state.savedQueries.length; - openConfirm(app, { - title: 'Open and replace current library?', - body: [h('span', { class: 'fm-mono' }, fileName), ' contains ', h('b', null, String(qs.length)), ' ', - qs.length === 1 ? 'query' : 'queries', '. Opening it will replace your current ', - h('b', null, String(cur)), ' saved ', cur === 1 ? 'query' : 'queries', - '. Open editor tabs are unaffected. Use Append instead to keep both.'], - confirmLabel: 'Open', - onConfirm: () => doReplace(app, qs, fileName), +function onImportDashboardFile(app: App, file: File): void { + readBundleFile(app, file, (bundle) => startImportDashboard(app, bundle)); +} + +function startImportDashboard(app: App, bundle: PortableBundleV1): void { + const dashboards = listBundleDashboards(bundle); + if (!dashboards.length) { flashToast('✕ No dashboard in file', { document: app.document }); return; } + if (dashboards.length === 1) { runImportDashboard(app, bundle, dashboards[0].id); return; } + openDashboardPicker(app, 'Import which dashboard?', dashboards, false, (id) => { + if (id) runImportDashboard(app, bundle, id); + }); +} + +function runImportDashboard(app: App, bundle: PortableBundleV1, dashboardId: string): void { + // v1 holds at most one Dashboard, so importing one REPLACES the current + // Dashboard (its tiles/layout/filters). Confirm first when that would discard + // an existing Dashboard — matching New/Replace-workspace, which also gate + // destructive commits (#287; flagged in review — silent, unrecoverable loss). + if (app.state.dashboard) { + openConfirm(app, { + title: 'Import and replace current Dashboard?', + body: ['This replaces your current Dashboard (its tiles, layout, and filters) with the imported one. ', + 'Its saved queries are kept and merged. Open editor tabs are unaffected. ', + 'Export your Dashboard first if you want to keep it.'], + confirmLabel: 'Import Dashboard', + onConfirm: () => doImportDashboard(app, bundle, dashboardId), + }); + return; + } + doImportDashboard(app, bundle, dashboardId); +} + +function doImportDashboard(app: App, bundle: PortableBundleV1, dashboardId: string): void { + const workspace = currentWorkspace(app); + const source = bundle.dashboards.find((d) => d.id === dashboardId); + const closureIds = new Set(dashboardDependencyQueryIds(source)); + const closureQueries = bundle.queries.filter((q) => closureIds.has(q.id)); + withQueryDecisions(app, workspace.queries, closureQueries, (decisions) => { + // 'copy' mints a fresh Dashboard id/revision for the imported Dashboard, + // which then REPLACES the workspace's zero-or-one current Dashboard + // (v1 single-Dashboard model, #280 "Import Dashboard replaces the current + // Dashboard"). The confirm above gates the destructive case. + const plan = planImportDashboard(workspace, bundle, dashboardId, decisions, 'copy', app.genId); + void commitPlan(app, plan, 'Imported dashboard'); }); } -function confirmNew(app: App): void { - const cur = app.state.savedQueries.length; +// ── actions: Replace workspace ─────────────────────────────────────────────── + +function onReplaceWorkspaceFile(app: App, file: File): void { + readBundleFile(app, file, (bundle) => startReplaceWorkspace(app, bundle)); +} + +function startReplaceWorkspace(app: App, bundle: PortableBundleV1): void { + const dashboards = listBundleDashboards(bundle); + if (dashboards.length > 1) { + openDashboardPicker(app, 'Replace workspace — which dashboard?', dashboards, true, (id) => { + confirmReplaceWorkspace(app, bundle, id === null ? undefined : id); + }); + return; + } + confirmReplaceWorkspace(app, bundle, dashboards[0]?.id); +} + +function confirmReplaceWorkspace(app: App, bundle: PortableBundleV1, sourceDashboardId: string | undefined): void { + const cur = currentWorkspace(app); openConfirm(app, { - title: 'Start a new library?', - body: ['This clears your current ', h('b', null, String(cur)), ' saved ', cur === 1 ? 'query' : 'queries', - ' and starts an empty library. Open editor tabs are unaffected. Save first if you want to keep them.'], - confirmLabel: 'New Library', - onConfirm: () => doNew(app), + title: 'Replace workspace?', + body: ['This replaces your current ', h('b', null, String(cur.queries.length)), ' saved ', + cur.queries.length === 1 ? 'query' : 'queries', cur.dashboard ? ' and your Dashboard' : '', + ' with ', h('b', null, String(bundle.queries.length)), ' ', queries(bundle.queries.length), + sourceDashboardId ? ' and the selected Dashboard' : '', ' from the file. Open editor tabs are unaffected.'], + confirmLabel: 'Replace', + onConfirm: () => { + const workspace = currentWorkspace(app); + withQueryDecisions(app, workspace.queries, bundle.queries, (decisions) => { + const plan = planReplaceWorkspace(workspace, bundle, sourceDashboardId, decisions, app.genId); + void commitPlan(app, plan, 'Replaced workspace'); + }); + }, }); } -interface ConfirmOpts { - title: string; - body: unknown[]; - confirmLabel: string; - onConfirm: () => void; +// ── actions: Export ────────────────────────────────────────────────────────── + +function downloadEncodedBundle(app: App, bundle: PortableBundleV1, baseName: string): void { + const encoded = encodePortableBundleJson({ queries: bundle.queries, dashboards: bundle.dashboards, nowISO: bundle.exportedAt }); + if (!encoded.ok) { flashToast('✕ ' + first(encoded.diagnostics, 'Could not export'), { document: app.document }); return; } + app.downloadFile(fileBase(baseName) + '.json', 'application/json', encoded.value); + flashToast('Exported → .json', { document: app.document }); } -function openConfirm(app: App, { title, body, confirmLabel, onConfirm }: ConfirmOpts): void { +function exportDashboardAction(app: App): void { + // `!`: the only call site (the "Export Dashboard…" item's onclick, above) + // already checked `hasDashboard` and returns early with its own toast + // otherwise — this is never invoked with a null Dashboard. + const dashboard = app.state.dashboard!; + const bundle = buildDashboardExportBundle(dashboard, app.state.savedQueries, new Date(app.wallNow()).toISOString()); + downloadEncodedBundle(app, bundle, dashboard.title || app.state.libraryName.value); +} + +function exportWorkspaceAction(app: App): void { + const bundle = buildWorkspaceExportBundle(currentWorkspace(app), new Date(app.wallNow()).toISOString()); + downloadEncodedBundle(app, bundle, app.state.libraryName.value); +} + +function downloadAction(app: App, fmt: 'md' | 'sql'): void { + const qs = app.state.savedQueries; + if (!qs.length) { flashToast('Nothing to save', { document: app.document }); return; } + if (fmt === 'md') app.downloadFile(fileBase(app.state.libraryName.value) + '.md', 'text/markdown', buildMarkdownDoc(qs)); + else app.downloadFile(fileBase(app.state.libraryName.value) + '.sql', 'application/sql', buildSqlDoc(qs)); + flashToast('Saved ' + queries(qs.length) + ' → .' + fmt, { document: app.document }); +} + +// ── dialogs (shared `.fm-dialog-*` visual language) ───────────────────────── + +interface DialogHandle { close(): void; } + +/** Mount one `.fm-dialog-backdrop`/`.fm-dialog-card` (title + caller-supplied + * content nodes), wired for Esc + outside-click close — the shared shell + * `openConfirm`/the conflict dialog/the dashboard picker all build on. */ +function openDialogShell(app: App, title: string, content: unknown[], extraCardClass?: string): DialogHandle { const doc = app.document; const close = (): void => { doc.removeEventListener('keydown', onKey, true); @@ -294,15 +585,28 @@ function openConfirm(app: App, { title, body, confirmLabel, onConfirm }: Confirm if (app.dom.fileDialog) { app.dom.fileDialog.remove(); app.dom.fileDialog = undefined; } }; const onKey = (e: KeyboardEvent): void => { if (e.key === 'Escape') { e.preventDefault(); close(); } }; - const card = h('div', { class: 'fm-dialog-card' }, - h('div', { class: 'fm-dialog-title' }, title), - h('div', { class: 'fm-dialog-body' }, body), - h('div', { class: 'fm-dialog-actions' }, - h('button', { class: 'fm-dialog-cancel', onclick: close }, 'Cancel'), - h('button', { class: 'fm-dialog-confirm', onclick: () => { close(); onConfirm(); } }, confirmLabel))); + const card = h('div', { class: extraCardClass ? `fm-dialog-card ${extraCardClass}` : 'fm-dialog-card' }, + h('div', { class: 'fm-dialog-title' }, title), content); const backdrop = h('div', { class: 'fm-dialog-backdrop' }, card); const detachBackdrop = attachBackdropClose(backdrop, close); app.dom.fileDialog = backdrop; doc.body.appendChild(backdrop); doc.addEventListener('keydown', onKey, true); + return { close }; +} + +interface ConfirmOpts { + title: string; + body: unknown[]; + confirmLabel: string; + onConfirm: () => void; +} + +function openConfirm(app: App, { title, body, confirmLabel, onConfirm }: ConfirmOpts): void { + const handle = openDialogShell(app, title, [ + h('div', { class: 'fm-dialog-body' }, body), + h('div', { class: 'fm-dialog-actions' }, + h('button', { class: 'fm-dialog-cancel', onclick: () => handle.close() }, 'Cancel'), + h('button', { class: 'fm-dialog-confirm', onclick: () => { handle.close(); onConfirm(); } }, confirmLabel)), + ]); } diff --git a/src/ui/saved-history.ts b/src/ui/saved-history.ts index 019d2099..0fc403e0 100644 --- a/src/ui/saved-history.ts +++ b/src/ui/saved-history.ts @@ -12,6 +12,7 @@ import { deleteHistory, invalidSpecTabForSaved, SAVED_VIEWS, } from '../state.js'; import type { AppState, HistoryEntry } from '../state.js'; +import { flashToast } from './toast.js'; import { isAutoRunnable } from '../core/sql-split.js'; import { isQuerylessPanel } from '../core/panel-cfg.js'; import { queryDescription, queryFavorite, queryName, queryPanel, queryView } from '../core/saved-query.js'; @@ -54,7 +55,7 @@ export function renderSavedHistory(app: App): void { h('button', { class: 'side-tab' + (state.sidePanel.value === 'saved' ? ' active' : ''), onclick: () => switchTo('saved'), - }, Icon.layers(), h('span', null, 'Library'), + }, Icon.layers(), h('span', null, 'Queries'), count ? h('span', { class: 'side-count' }, '· ' + count) : null), h('button', { class: 'side-tab' + (state.sidePanel.value === 'history' ? ' active' : ''), @@ -133,13 +134,15 @@ function renderSaved(app: App, list: HTMLElement): void { const launchView = rolePreview || queryView(q); const star = h('button', { class: 'sv-star' + (favorite ? ' on' : ''), title: favorite ? 'Unfavorite' : 'Favorite', - onclick: (e: Event) => { + onclick: async (e: Event) => { e.stopPropagation(); - const result = toggleFavorite(state, q.id, app.saveJSON, app.specValidators); + const result = await app.serializeWrite(() => toggleFavorite(state, q.id, app.workspace.commit, app.specValidators)); if (result && result.invalidTab) app.activateInvalidSpecDraft(result.invalidTab); else if (result && result.ok) { app.queryDoc.revalidateSpecDrafts(); app.specEditor.syncFromState(); + } else if (result && !result.ok && result.diagnostics?.length) { + flashToast('Couldn’t update favorite: ' + result.diagnostics[0].message, { document: app.document }); } renderSavedHistory(app); }, @@ -184,11 +187,15 @@ function renderSaved(app: App, list: HTMLElement): void { }, Icon.pencil()), h('button', { class: 'sv-act', title: 'Delete', - onclick: (e: Event) => { + onclick: async (e: Event) => { e.stopPropagation(); - deleteSaved(state, q.id, app.saveJSON); - app.updateSaveBtn(); - app.updateEditorModeUi?.(); + const result = await app.serializeWrite(() => deleteSaved(state, q.id, app.workspace.commit)); + if (result.ok) { + app.updateSaveBtn(); + app.updateEditorModeUi?.(); + } else if (result.diagnostics.length) { + flashToast('Couldn’t delete: ' + result.diagnostics[0].message, { document: app.document }); + } renderSavedHistory(app); }, }, Icon.trash())), @@ -211,13 +218,15 @@ function savedEditForm(app: App, q: SavedQueryV2): HTMLDivElement { const descInput = h('textarea', { class: 'sv-edit-desc', rows: '3', placeholder: 'What this query does (shown in Markdown export)' }); descInput.value = queryDescription(q); let done = false; - const finish = (commit: boolean): void => { + const finish = async (commit: boolean): Promise => { if (done) return; done = true; if (commit && nameInput.value.trim()) { - const result = renameSaved(state, q.id, nameInput.value, descInput.value, app.saveJSON, app.specValidators); + const result = await app.serializeWrite(() => renameSaved(state, q.id, nameInput.value, descInput.value, app.workspace.commit, app.specValidators)); if (result && result.invalidTab) app.activateInvalidSpecDraft(result.invalidTab); - else { + else if (result && !result.ok && result.diagnostics?.length) { + flashToast('Couldn’t rename: ' + result.diagnostics[0].message, { document: app.document }); + } else { app.queryDoc.revalidateSpecDrafts(); app.specEditor.syncFromState(); app.actions.rerenderTabs(); diff --git a/src/workspace/import-planner.ts b/src/workspace/import-planner.ts new file mode 100644 index 00000000..f65c81e5 --- /dev/null +++ b/src/workspace/import-planner.ts @@ -0,0 +1,417 @@ +// Transactional PortableBundle import planner (#280 "Transactional import +// planner" / "Cross-resource semantic validation", Phase 5 of #287). Pure: no +// DOM, no storage, no crypto import — the ID generator is injected exactly +// like workspace-operations.ts's WorkspaceIdGen, so minting is deterministic +// in tests and unguessable in production. +// +// A PortableBundle import always resolves to one COMPLETE candidate +// StoredWorkspaceV1 built from the repository-level primitives in +// workspace-operations.ts, then validated in one pass through +// validateStoredWorkspaceDocument — exactly the same "build the whole +// candidate, validate once, never commit an invalid one" discipline +// saved-query-mutation.ts uses for in-place mutations. Nothing here mutates +// application state; the caller commits the returned candidate atomically +// through the Phase-2 repository, or does not commit at all. +// +// Query-identity conflicts are resolved BY ID, never by content-based dedup +// (#280): an incoming query conflicts with an existing one only when their +// `id`s match. `canonicalEqual` (over SAVED_QUERY_SHAPE) then decides whether +// that conflict can auto-resolve to "use existing" or needs a caller decision. + +import { canonicalEqual, SAVED_QUERY_SHAPE } from '../dashboard/model/canonical-json.js'; +import { dashboardDependencyQueryIds } from '../dashboard/model/bundle-order.js'; +import { diagnostic, sortDiagnostics } from '../dashboard/model/workspace-diagnostics.js'; +import type { WorkspaceDiagnostic } from '../dashboard/model/workspace-diagnostics.js'; +import { cloneJson } from '../core/saved-query.js'; +import { + importQueries, replaceWorkspaceContents, +} from './workspace-operations.js'; +import type { WorkspaceIdGen } from './workspace-operations.js'; +import { validateStoredWorkspaceDocument } from './stored-workspace.js'; +import type { WorkspaceCodecOptions } from './stored-workspace.js'; +import type { + DashboardDocumentV1, PortableBundleV1, SavedQueryV2, StoredWorkspaceV1, +} from '../generated/json-schema.types.js'; + +// --- Dashboard listing ------------------------------------------------------- + +/** One bundled Dashboard's presentation summary for an import picker. */ +export interface DashboardSummary { + id: string; + title: string; + tileCount: number; + filterCount: number; +} + +/** Summarize every Dashboard in a bundle, preserving `bundle.dashboards` + * ARRAY ORDER — array order is import-time presentation order, not a + * catalog to re-sort (owner decision, #280/#287). */ +export function listBundleDashboards(bundle: PortableBundleV1): DashboardSummary[] { + return bundle.dashboards.map((dashboard) => ({ + id: dashboard.id, + title: dashboard.title, + tileCount: dashboard.tiles.length, + filterCount: dashboard.filters.length, + })); +} + +// --- Query conflict detection + resolution ---------------------------------- + +export type QueryConflictAction = 'use-existing' | 'copy' | 'replace' | 'skip'; + +/** One incoming query whose `id` matches an existing query's `id`. + * `canonicalEqual` is `canonicalEqual(existing, incoming, SAVED_QUERY_SHAPE)`. */ +export interface QueryConflict { + sourceId: string; + existing: SavedQueryV2; + incoming: SavedQueryV2; + canonicalEqual: boolean; +} + +/** Detect every incoming/existing query-id collision. Matches BY ID ONLY — + * not content-based dedup: two different ids with identical content are not + * a conflict here. */ +export function detectQueryConflicts( + existing: readonly SavedQueryV2[], incoming: readonly SavedQueryV2[], +): QueryConflict[] { + const existingById = new Map(existing.map((query) => [query.id, query] as const)); + const out: QueryConflict[] = []; + for (const query of incoming) { + const match = existingById.get(query.id); + if (!match) continue; + out.push({ + sourceId: query.id, + existing: match, + incoming: query, + canonicalEqual: canonicalEqual(match, query, SAVED_QUERY_SHAPE), + }); + } + return out; +} + +export interface QueryDecision { + sourceId: string; + action: QueryConflictAction; + /** Only meaningful for `action: 'copy'` — a caller-preferred fresh id. + * Honored when it is actually free; a taken/omitted value falls back to + * the injected `genId()` retry loop. */ + targetId?: string; +} + +/** Auto-resolve the conflicts that are safe to resolve without asking: a + * conflict whose incoming content is canonically IDENTICAL to the existing + * query auto-resolves to 'use-existing'. A conflict whose content differs is + * omitted — the caller (a UI or a scripted import) must supply an explicit + * decision for it. */ +export function autoResolveConflicts(conflicts: readonly QueryConflict[]): QueryDecision[] { + return conflicts + .filter((conflict) => conflict.canonicalEqual) + .map((conflict) => ({ sourceId: conflict.sourceId, action: 'use-existing' as const, targetId: conflict.existing.id })); +} + +// --- Query id mapping -------------------------------------------------------- + +/** Per-source-query-id resolution: `targetId: null` means the query is + * skipped (dropped from the candidate); otherwise `targetId` is the id the + * query lands under in the candidate workspace. */ +export type IdMapping = Record; + +const MAX_FRESH_ID_ATTEMPTS = 1000; + +/** + * Resolve every incoming query to a target id: a non-conflicting query keeps + * its own id (recorded with action `'copy'` — it lands in the candidate + * as-is, no rename needed). A conflicting query without a matching decision + * defaults to `'skip'` (never silently import an undecided collision). A + * conflicting query WITH a decision resolves per that decision: + * - 'use-existing' / 'replace' — same id as the conflict (ids already + * match by definition of "conflict"); + * - 'skip' — `targetId: null`; + * - 'copy' — a caller-preferred `decision.targetId` when free, else a fresh + * id from `genId()`, retried (mirrors `mergeSaved`/`library-migrations`'s + * freshId loop) against existing ids, incoming ids, and ids already + * minted in this same call so two colliding copies in one import never + * collide with each other either. + */ +export function buildQueryIdMapping( + incoming: readonly SavedQueryV2[], existing: readonly SavedQueryV2[], + decisions: readonly QueryDecision[], genId: WorkspaceIdGen, +): IdMapping { + const existingIds = new Set(existing.map((query) => query.id)); + const incomingIds = new Set(incoming.map((query) => query.id)); + const decisionBySourceId = new Map(decisions.map((decision) => [decision.sourceId, decision] as const)); + const minted = new Set(); + const isTaken = (id: string): boolean => existingIds.has(id) || incomingIds.has(id) || minted.has(id); + const freshId = (): string => { + for (let attempt = 0; attempt < MAX_FRESH_ID_ATTEMPTS; attempt++) { + const id = genId(); + if (id && !isTaken(id)) { minted.add(id); return id; } + } + throw new Error('Unable to mint a unique saved-query id for the import plan'); + }; + + const mapping: IdMapping = {}; + for (const query of incoming) { + const sourceId = query.id; + if (!existingIds.has(sourceId)) { + mapping[sourceId] = { targetId: sourceId, action: 'copy' }; + continue; + } + const decision = decisionBySourceId.get(sourceId); + if (!decision) { + mapping[sourceId] = { targetId: null, action: 'skip' }; + continue; + } + if (decision.action === 'skip') { + mapping[sourceId] = { targetId: null, action: 'skip' }; + } else if (decision.action === 'use-existing' || decision.action === 'replace') { + mapping[sourceId] = { targetId: sourceId, action: decision.action }; + } else { + const requested = decision.targetId; + let targetId: string; + if (requested && !isTaken(requested)) { targetId = requested; minted.add(requested); } else { targetId = freshId(); } + mapping[sourceId] = { targetId, action: 'copy' }; + } + } + return mapping; +} + +// --- Dashboard reference rewriting ------------------------------------------- + +function resolveMapping( + mapping: Map | IdMapping, id: string, +): { mapped: boolean; targetId: string | null } { + if (mapping instanceof Map) { + return mapping.has(id) ? { mapped: true, targetId: mapping.get(id) ?? null } : { mapped: false, targetId: null }; + } + return Object.hasOwn(mapping, id) + ? { mapped: true, targetId: mapping[id].targetId } + : { mapped: false, targetId: null }; +} + +export interface RewriteDashboardReferencesResult { + dashboard: DashboardDocumentV1; + invalidated: boolean; + missingRequiredIds: string[]; +} + +/** + * Bulk generalization of `saved-query-mutation.ts`'s `remapQuery`: rewrite + * every `tile.queryId` and `filter.sourceQueryId` through `mapping`. A + * reference that maps to `null` (skipped) or has no mapping entry at all + * sets `invalidated: true` and collects the source id in + * `missingRequiredIds` — the reference is left as-is (never silently + * dropped) so the caller sees exactly what broke; `invalidated` is the + * signal that the candidate must not be committed. Deep-clones; never + * mutates `dashboard`. + */ +export function rewriteDashboardReferences( + dashboard: DashboardDocumentV1, mapping: Map | IdMapping, +): RewriteDashboardReferencesResult { + const next = cloneJson(dashboard); + const missing = new Set(); + let invalidated = false; + + const remap = (id: string): string => { + const { mapped, targetId } = resolveMapping(mapping, id); + if (!mapped || targetId === null) { + invalidated = true; + missing.add(id); + return id; + } + return targetId; + }; + + // `queryId` is a required string per DashboardTileV1 — no runtime guard + // needed (structurally-invalid tiles are the schema layer's job). + next.tiles = next.tiles.map((tile) => ({ ...tile, queryId: remap(tile.queryId) })); + next.filters = next.filters.map((filter) => ( + typeof filter.sourceQueryId === 'string' ? { ...filter, sourceQueryId: remap(filter.sourceQueryId) } : filter + )); + + return { dashboard: next, invalidated, missingRequiredIds: [...missing] }; +} + +// --- Candidate query-set assembly -------------------------------------------- + +/** Resolve one incoming query to its post-mapping content under `targetId`. + * Callers only invoke this for a 'copy'/'replace' entry, which + * `buildQueryIdMapping` always pairs with a non-null `targetId` — the null + * ('skip') and same-as-existing ('use-existing') cases are handled by the + * caller before this runs. */ +function resolvedQueryContent(query: SavedQueryV2, targetId: string): SavedQueryV2 { + // Always deep-clone so the committed candidate never aliases an object from + // the caller's decoded bundle — a caller that retained and later mutated its + // bundle would otherwise corrupt the persisted workspace (review hazard). + return targetId === query.id ? cloneJson(query) : { ...cloneJson(query), id: targetId }; +} + +/** MERGE incoming queries into the existing catalog (Import queries / Import + * Dashboard): existing queries keep their catalog position; a 'replace' + * decision overwrites its entry in place; 'use-existing' is a no-op (the + * existing entry already stands); a 'copy' (fresh id or non-conflicting own + * id) is a genuinely new catalog entry, appended in bundle order. */ +function mergeIncomingQueries( + incoming: readonly SavedQueryV2[], existing: readonly SavedQueryV2[], mapping: IdMapping, +): SavedQueryV2[] { + const replaceById = new Map(); + const additions: SavedQueryV2[] = []; + for (const query of incoming) { + const entry = mapping[query.id]; + if (!entry || entry.action === 'skip' || entry.action === 'use-existing') continue; + // action is 'copy' or 'replace' — always paired with a non-null targetId. + const content = resolvedQueryContent(query, entry.targetId as string); + if (entry.action === 'replace') replaceById.set(content.id, content); + else additions.push(content); + } + // `buildQueryIdMapping` only ever assigns a 'replace' action to a source id + // that already exists in `existing` (it is the conflicting-id branch), so + // every `replaceById` target is guaranteed present in `existing` — no + // append fallback needed here. + const merged = existing.map((query) => replaceById.get(query.id) ?? query); + return [...merged, ...additions]; +} + +/** REPLACE the query catalog wholesale (Replace workspace): only queries + * reachable from the incoming bundle survive, in bundle order; 'use-existing' + * keeps the existing query's own content (still under the shared id) rather + * than the incoming content. */ +function replaceIncomingQueries( + incoming: readonly SavedQueryV2[], existing: readonly SavedQueryV2[], mapping: IdMapping, +): SavedQueryV2[] { + const existingById = new Map(existing.map((query) => [query.id, query] as const)); + const out: SavedQueryV2[] = []; + for (const query of incoming) { + const entry = mapping[query.id]; + if (!entry || entry.action === 'skip') continue; + if (entry.action === 'use-existing') { + // 'use-existing' is only ever assigned for a source id already present + // in `existing` — guaranteed found. + out.push(existingById.get(entry.targetId as string) as SavedQueryV2); + continue; + } + out.push(resolvedQueryContent(query, entry.targetId as string)); + } + return out; +} + +// --- Plans -------------------------------------------------------------------- + +export interface PortableBundleImportPlan { + sourceDashboardId?: string; + queryMappings: IdMapping; + candidateWorkspace: StoredWorkspaceV1 | null; + diagnostics: WorkspaceDiagnostic[]; +} + +function invalidPlan( + diagnostics: WorkspaceDiagnostic[], queryMappings: IdMapping, sourceDashboardId?: string, +): PortableBundleImportPlan { + return { + ...(sourceDashboardId === undefined ? {} : { sourceDashboardId }), + queryMappings, candidateWorkspace: null, diagnostics: sortDiagnostics(diagnostics), + }; +} + +function validatedPlan( + candidate: StoredWorkspaceV1, queryMappings: IdMapping, options: WorkspaceCodecOptions, sourceDashboardId?: string, +): PortableBundleImportPlan { + const diagnostics = validateStoredWorkspaceDocument(candidate, options); + if (diagnostics.length) return invalidPlan(diagnostics, queryMappings, sourceDashboardId); + return { + ...(sourceDashboardId === undefined ? {} : { sourceDashboardId }), + queryMappings, candidateWorkspace: candidate, diagnostics: [], + }; +} + +function dashboardNotFoundPlan( + sourceDashboardId: string, queryMappings: IdMapping, +): PortableBundleImportPlan { + return invalidPlan( + [diagnostic(['dashboards'], 'import-dashboard-not-found', + `Bundle contains no dashboard with id ${JSON.stringify(sourceDashboardId)}`, sourceDashboardId)], + queryMappings, sourceDashboardId, + ); +} + +function invalidatedDashboardPlan( + sourceDashboardId: string, queryMappings: IdMapping, missingRequiredIds: readonly string[], +): PortableBundleImportPlan { + return invalidPlan( + [diagnostic(['dashboard'], 'dashboard-import-invalid', + `Dashboard import is missing required saved-query dependencies: ${missingRequiredIds.join(', ')}`, + sourceDashboardId)], + queryMappings, sourceDashboardId, + ); +} + +/** Queries-only import (Dashboard untouched): merge the bundle's queries into + * the workspace's query catalog per `decisions`, and validate the result. */ +export function planImportQueries( + workspace: StoredWorkspaceV1, bundle: PortableBundleV1, + decisions: readonly QueryDecision[], genId: WorkspaceIdGen, + options: WorkspaceCodecOptions = {}, +): PortableBundleImportPlan { + const mapping = buildQueryIdMapping(bundle.queries, workspace.queries, decisions, genId); + const nextQueries = mergeIncomingQueries(bundle.queries, workspace.queries, mapping); + const candidate = importQueries(workspace, nextQueries); + return validatedPlan(candidate, mapping, options); +} + +/** Import one bundled Dashboard plus its dependency closure of queries. + * `mode: 'copy'` mints a fresh Dashboard id (revision reset to 1); `mode: + * 'replace'` keeps the imported Dashboard's own id and revision. A skipped + * or unmapped required dependency invalidates the plan (`candidateWorkspace: + * null`) rather than silently dropping the reference. */ +export function planImportDashboard( + workspace: StoredWorkspaceV1, bundle: PortableBundleV1, sourceDashboardId: string, + decisions: readonly QueryDecision[], mode: 'copy' | 'replace', genId: WorkspaceIdGen, + options: WorkspaceCodecOptions = {}, +): PortableBundleImportPlan { + const source = bundle.dashboards.find((dashboard) => dashboard.id === sourceDashboardId); + if (!source) return dashboardNotFoundPlan(sourceDashboardId, {}); + + const closureIds = new Set(dashboardDependencyQueryIds(source)); + const closureQueries = bundle.queries.filter((query) => closureIds.has(query.id)); + const mapping = buildQueryIdMapping(closureQueries, workspace.queries, decisions, genId); + const nextQueries = mergeIncomingQueries(closureQueries, workspace.queries, mapping); + + const rewritten = rewriteDashboardReferences(source, mapping); + if (rewritten.invalidated) { + return invalidatedDashboardPlan(sourceDashboardId, mapping, rewritten.missingRequiredIds); + } + const finalDashboard: DashboardDocumentV1 = mode === 'copy' + ? { ...rewritten.dashboard, id: genId(), revision: 1 } + : rewritten.dashboard; + + const candidate = replaceWorkspaceContents(workspace, { queries: nextQueries, dashboard: finalDashboard }); + return validatedPlan(candidate, mapping, options, sourceDashboardId); +} + +/** Replace the workspace's queries AND Dashboard atomically (preserving + * workspace `id`/`name`): only queries reachable from the bundle survive, + * and the selected Dashboard (if any) becomes the workspace's sole + * Dashboard, keeping its own id/revision. Omit `sourceDashboardId` to + * replace with a query-only workspace (Dashboard cleared to `null`). */ +export function planReplaceWorkspace( + workspace: StoredWorkspaceV1, bundle: PortableBundleV1, sourceDashboardId: string | undefined, + decisions: readonly QueryDecision[], genId: WorkspaceIdGen, + options: WorkspaceCodecOptions = {}, +): PortableBundleImportPlan { + const mapping = buildQueryIdMapping(bundle.queries, workspace.queries, decisions, genId); + const nextQueries = replaceIncomingQueries(bundle.queries, workspace.queries, mapping); + + let dashboard: DashboardDocumentV1 | null = null; + if (sourceDashboardId !== undefined) { + const source = bundle.dashboards.find((candidate) => candidate.id === sourceDashboardId); + if (!source) return dashboardNotFoundPlan(sourceDashboardId, mapping); + const rewritten = rewriteDashboardReferences(source, mapping); + if (rewritten.invalidated) { + return invalidatedDashboardPlan(sourceDashboardId, mapping, rewritten.missingRequiredIds); + } + dashboard = rewritten.dashboard; + } + + const candidate = replaceWorkspaceContents(workspace, { queries: nextQueries, dashboard }); + return validatedPlan(candidate, mapping, options, sourceDashboardId); +} diff --git a/tests/helpers/fake-app.ts b/tests/helpers/fake-app.ts index fd206813..412fcda8 100644 --- a/tests/helpers/fake-app.ts +++ b/tests/helpers/fake-app.ts @@ -25,6 +25,8 @@ import dagre from '@dagrejs/dagre'; import { createState, activeTab } from '../../src/state.js'; import { createNoopPort } from '../../src/editor/editor-port.js'; import { createNoopSpecEditor } from '../../src/editor/spec-editor.js'; +import { createWorkspaceRepository } from '../../src/workspace/workspace-repository.js'; +import type { WorkspaceStore } from '../../src/workspace/workspace-store.types.js'; import type { App, ActionsRegistry, AppDom, ChCtx } from '../../src/ui/app.types.js'; import type { AppState } from '../../src/state.js'; import type { ConfigDoc, ResolvedIdpConfig } from '../../src/net/oauth-config.js'; @@ -41,12 +43,36 @@ import type { WorkbenchParameterSession } from '../../src/application/workbench- import type { ExportService } from '../../src/application/export-service.js'; import type { QueryDocumentSession } from '../../src/application/query-document-session.js'; import type { WorkspaceRepository } from '../../src/workspace/workspace-repository.js'; +import type { StoredWorkspaceV1 } from '../../src/generated/json-schema.types.js'; import type { SavedQueryService, CreateSavedResult, CommitLinkedResult, ShareResult, } from '../../src/application/saved-query-service.js'; import { assembleReferenceData, buildCompletions } from '../../src/core/completions.js'; import type { AssembledReference } from '../../src/core/completions.js'; +// #287 W4: an in-memory `WorkspaceStore` fake (mirrors +// workspace-repository.test.ts's own `memStore`) — every saved-query CRUD op +// now awaits a real `WorkspaceRepository.commit`, not a bare stub, so a test +// exercising `createSavedQuery`/`toggleFavorite`/`deleteSaved`/etc. through +// `app.workspace.commit` gets genuine whole-candidate schema validation +// rather than an always-succeeds/always-fails placeholder. +export function memWorkspaceStore(initial: string | null = null): WorkspaceStore { + let value = initial; + return { + read: async () => value, + write: async (text: string) => { value = text; }, + clear: async () => { value = null; }, + }; +} + +/** A fresh real `WorkspaceRepository.commit` (backed by its own private + * `memWorkspaceStore`), wrapped in a `vi.fn` spy so a test can assert call + * counts the same way it used to assert on the retired `save: SaveJSON` + * spy — `vi.fn(impl)` still delegates to the real implementation. */ +export function fakeWorkspaceCommit() { + return vi.fn(createWorkspaceRepository({ store: memWorkspaceStore() }).commit); +} + // `app.conn.chCtx`'s defaults (#276 Phase 2; Phase 5 deleted the flat // `App.chCtx` alias this file used to also mirror it onto). const chCtxDefaults: ChCtx = { @@ -201,8 +227,8 @@ const queryDocDefaults: QueryDocumentSession = { // saved-query-service.test.ts's job); this just satisfies the `App.saved` // contract. const savedDefaults: SavedQueryService = { - create: vi.fn((): CreateSavedResult => ({ ok: false })), - commit: vi.fn((): CommitLinkedResult => ({ ok: false, reason: 'empty' })), + create: vi.fn(async (): Promise => ({ ok: false })), + commit: vi.fn(async (): Promise => ({ ok: false, reason: 'empty' })), recordHistory: vi.fn(), buildShareUrl: vi.fn((): ShareResult => ({ ok: false, reason: 'empty' })), }; @@ -243,12 +269,31 @@ const appDefaults: App = { params: paramsDefaults, graph: graphDefaults, prefs: prefsDefaults, + // Default commit is a permissive ECHO (not real schema validation, unlike + // `fakeWorkspaceCommit()` above): it always succeeds and publishes back + // exactly the candidate it was given, so a generic UI-focused fixture that + // never overrides `workspace` still gets a coherent `state.savedQueries = + // result.workspace.queries` projection (#287 W4) without also having to set + // `state.workspaceId` to satisfy the real repository's non-empty-id schema + // rule. A test asserting real validate-then-persist behavior overrides this + // with `fakeWorkspaceCommit()` (or its own stub) instead. workspace: { loadCurrent: async () => null, - commit: async () => ({ ok: true, workspace: {} as never, dashboardRevision: null }), + commit: async (candidate) => ({ + ok: true, workspace: candidate, dashboardRevision: candidate.dashboard === null ? null : candidate.dashboard.revision, + }), clearCurrent: async () => {}, }, loadDashboardWorkspace: async () => null, + loadWorkspaceOnBoot: async () => null, + // Inert placeholders — `base` below overrides both with real, state-backed + // implementations (mirroring app.ts's own #287 W5 wiring) so a file-menu + // fixture that never overrides `workspace.commit` still observes a coherent + // post-commit projection. + applyCommittedWorkspace: () => {}, + genId: () => 'gen-id', + // Inert passthrough — `base` overrides with a real per-instance queue. + serializeWrite: (op: () => Promise): Promise => op(), sqlEditor: {} as App['sqlEditor'], specEditor: {} as App['specEditor'], CodeViewer: () => ({ setText: () => {}, setLanguage: () => {}, setWrap: () => {}, focus: () => {}, destroy: () => {} }), @@ -323,6 +368,13 @@ const appDefaults: App = { * convenience key (#276 Phase 5 deleted `App.chCtx` — `app.conn.chCtx` is * the only live alias now), kept so existing `makeApp({ chCtx: {...} })` * call sites don't all need to become `conn: { chCtx: {...} }`. */ +// Exported (as `MakeAppOverrides`, distinct from any test file's own local +// `AppOverrides`) so a fixture's own `mount()`-style wrapper (e.g. +// file-menu.test.ts's, which needs `workspace: { commit }` — a genuinely +// nested-partial override `Partial` alone can't express) can type its +// parameter as exactly what `makeApp` itself accepts, instead of +// re-declaring a narrower `Partial` that would reject it. +export type MakeAppOverrides = AppOverrides; type AppOverrides = Partial> & { /** Partial like the rest (#286 Phase 4) — the Dashboard viewer reads a * StoredWorkspaceV1 through `loadDashboardWorkspace`/`workspace.loadCurrent`; @@ -409,6 +461,31 @@ export function makeApp>(override entityDoc: vi.fn(async () => ''), }, toggleTheme: vi.fn(), + // #287 W5: real, state-backed implementations (mirroring app.ts's own + // projection + WorkspaceIdGen wiring) so a file-menu fixture exercising a + // real commit (default echo `workspace.commit`, or `fakeWorkspaceCommit()`) + // observes a coherent post-commit projection without overriding either. + // `genId` mints a fresh, test-deterministic id per call — its own counter, + // not `appDefaults`' shared placeholder, so ids never leak across tests. + applyCommittedWorkspace: (workspace: StoredWorkspaceV1) => { + state.savedQueries = workspace.queries; + state.dashboard = workspace.dashboard; + state.workspaceId = workspace.id; + state.libraryName.value = workspace.name; + state.libraryDirty.value = false; + }, + genId: (() => { let n = 0; return () => 'gen-' + (++n); })(), + // #287 review fix: a real per-instance serialization queue (mirrors app.ts) + // so a test firing two overlapping CRUD ops observes them applied in order, + // not interleaved. Per-instance (own `chain`), so no leak across makeApp calls. + serializeWrite: (() => { + let chain: Promise = Promise.resolve(); + return (op: () => Promise): Promise => { + const run = chain.then(op, op); + chain = run.then(() => undefined, () => undefined); + return run; + }; + })(), // The one deliberate delegate survivor of #276 Phase 5's params-group // cleanup — see `App.saveVarRecent`'s own doc comment. saveVarRecent: vi.fn(), diff --git a/tests/helpers/fake-idb.ts b/tests/helpers/fake-idb.ts new file mode 100644 index 00000000..620c6b28 --- /dev/null +++ b/tests/helpers/fake-idb.ts @@ -0,0 +1,73 @@ +// A minimal, always-succeeding in-memory fake `IDBFactory` (#287 W4). Not +// under src/, so it does not count toward coverage. +// +// Real `createApp(env)`-based fixtures (app.test.ts's `env()`) inject this by +// default so `app.workspace.commit` — the seam every saved-query CRUD op now +// awaits — actually persists under happy-dom, which has no real `indexedDB`. +// Without it every commit would reject with a `workspace-persist-failed` +// diagnostic (see `indexeddb-workspace-store.ts`'s `openDb` — a missing +// factory rejects the open), so every save/rename/favorite/delete flow driven +// through the real app would silently do nothing. +// +// Mirrors `indexeddb-workspace-store.test.ts`'s own richer local `fakeFactory` +// (which adds per-call error-injection knobs this generic, always-succeeds +// fixture doesn't need) and `dashboard.test.ts`'s own local `fakeIndexedDb` +// (same shape, kept separate there since it predates this shared helper). +type Handler = (() => void) | null; + +class FakeRequest { + result: unknown; + error: unknown = null; + onsuccess: Handler = null; + onerror: Handler = null; + onupgradeneeded: Handler = null; +} + +class FakeObjectStore { + data: Map; + constructor(data: Map) { this.data = data; } + get(key: string): FakeRequest { + const req = new FakeRequest(); + queueMicrotask(() => { req.result = this.data.get(key); req.onsuccess?.(); }); + return req; + } + put(value: unknown, key: string): FakeRequest { this.data.set(key, value); return new FakeRequest(); } + delete(key: string): FakeRequest { this.data.delete(key); return new FakeRequest(); } +} + +class FakeTx { + oncomplete: Handler = null; + onerror: Handler = null; + onabort: Handler = null; + db: FakeDB; + constructor(db: FakeDB) { + this.db = db; + queueMicrotask(() => this.oncomplete?.()); + } + objectStore(name: string): FakeObjectStore { return new FakeObjectStore(this.db.stores.get(name)!); } +} + +class FakeDB { + stores = new Map>(); + objectStoreNames = { contains: (n: string): boolean => this.stores.has(n) }; + createObjectStore(name: string): void { this.stores.set(name, new Map()); } + transaction(_names: string[], _mode: string): FakeTx { return new FakeTx(this); } +} + +/** A fresh always-succeeding fake `IDBFactory` — one independent in-memory + * database per call (construct one per test/app unless deliberately sharing + * across several `createApp()` calls in the same test). */ +export function fakeIndexedDbFactory(): IDBFactory { + const db = new FakeDB(); + return { + open(_name: string, _version?: number) { + const req = new FakeRequest(); + queueMicrotask(() => { + req.result = db; + req.onupgradeneeded?.(); + req.onsuccess?.(); + }); + return req as unknown as IDBOpenDBRequest; + }, + } as unknown as IDBFactory; +} diff --git a/tests/unit/app.test.ts b/tests/unit/app.test.ts index 199b28f0..c479fb13 100644 --- a/tests/unit/app.test.ts +++ b/tests/unit/app.test.ts @@ -12,6 +12,7 @@ import { emptyRecentMap, recordRecent } from '../../src/core/recent-values.js'; import { queryDescription } from '../../src/core/saved-query.js'; import { createSpecValidatorRegistry } from '../../src/core/spec-draft.js'; import { savedQuery } from '../helpers/saved-query.js'; +import { fakeIndexedDbFactory } from '../helpers/fake-idb.js'; import { decodeShare } from '../../src/core/share.js'; import type { CreateAppEnv } from '../../src/env.types.js'; import type { App } from '../../src/ui/app.types.js'; @@ -342,6 +343,15 @@ function env(over: Partial = {}): CreateAppEnv { now: () => 0, retryMs: 0, // instant script-statement retry in tests (no real 250ms wait) navigator: { clipboard: asClipboard({ writeText: vi.fn(async (_data: string) => {}) }) }, + // #287 W4: every saved-query CRUD op now awaits `app.workspace.commit` — + // a real IndexedDB-backed commit, not the retired flat `asb:saved` write. + // Default to a working in-memory fake factory (mirrors dashboard.test.ts's + // own local `fakeIndexedDb`) so the many real `createApp(env())`-driven + // save/rename/favorite/delete flows below actually persist under + // happy-dom (which has no real `indexedDB`) instead of silently failing + // closed on every commit. A test can still override `indexedDB: undefined` + // to exercise the no-factory fallback path deliberately. + indexedDB: fakeIndexedDbFactory(), ...over, }; } @@ -356,10 +366,11 @@ describe('createApp basics', () => { expect(app.specValidators).toBe(service); // identity — the seam's own service passes straight through }); it('constructs the atomic StoredWorkspaceV1 repository (#284) behind the injected IndexedDB seam', () => { - // Default env: no env.indexedDB → falls back to win.indexedDB (absent under - // happy-dom). The repository still constructs (lazy — no DB opened yet) and - // exposes the full #280 contract. - const app = createApp(env()); + // `indexedDB: undefined` overrides `env()`'s own #287 W4 default fake + // factory → falls back to win.indexedDB (absent under happy-dom). The + // repository still constructs (lazy — no DB opened yet) and exposes the + // full #280 contract. + const app = createApp(env({ indexedDB: undefined })); expect(typeof app.workspace.loadCurrent).toBe('function'); expect(typeof app.workspace.commit).toBe('function'); expect(typeof app.workspace.clearCurrent).toBe('function'); @@ -2840,6 +2851,14 @@ describe('credentials (basic) sign-in', () => { }); }); +// #287 W4: `commitLinkedQuery`'s Save-button path and the popover's own Save +// button both await the aggregate commit before mutating anything; a DOM +// `click`/keydown dispatch can't be awaited directly (the handler's promise +// isn't returned to the dispatcher), so a macrotask flush lets it settle +// before a test's post-click assertions run (same convention as +// saved-history.test.ts's own `flush`). +const flush = (): Promise => new Promise((resolve) => setTimeout(resolve, 0)); + describe('share + star + columns', () => { it('share copies a link to the clipboard', async () => { const e = env({ window: asWindow({ history: { replaceState: vi.fn() }, navigator: {} }) }); @@ -2856,16 +2875,17 @@ describe('share + star + columns', () => { app.activeTab().sqlDraft = ' '; expect(() => app.actions.share()).not.toThrow(); }); - it('save opens a name popover; Save commits, links the tab, and the button reads "Saved"', () => { + it('save opens a name popover; Save commits, links the tab, and the button reads "Saved"', async () => { const app = createApp(env()); app.renderApp(); app.activeTab().sqlDraft = 'SELECT 42'; - app.actions.save(); + app.actions.save(); // opens the popover synchronously (before any await) const pop = qs(document, '.save-popover'); expect(pop).not.toBeNull(); expect(qs(pop, '.sp-input').value).toBe('SELECT 42'); // inferred name qs(pop, '.sp-input').value = 'My fave'; qs(pop, '.sp-save').dispatchEvent(new Event('click')); + await flush(); // the popover's Save button awaits the aggregate commit (#287 W4) expect(app.state.savedQueries).toHaveLength(1); expect(app.state.savedQueries[0]).toMatchObject({ sql: 'SELECT 42', spec: { name: 'My fave', favorite: false } }); expect(app.activeTab().savedId).toBe(app.state.savedQueries[0].id); @@ -2873,7 +2893,7 @@ describe('share + star + columns', () => { expect(app.dom.saveBtn!.textContent).toContain('Saved'); expect(qs(document, '.save-popover')).toBeNull(); // closed }); - it('save popover: re-opening is idempotent, Esc closes, dirty edit flips "Saved"→"Save"', () => { + it('save popover: re-opening is idempotent, Esc closes, dirty edit flips "Saved"→"Save"', async () => { const app = createApp(env()); app.renderApp(); app.activeTab().sqlDraft = 'SELECT 1'; @@ -2882,6 +2902,7 @@ describe('share + star + columns', () => { expect(qsa(document, '.save-popover')).toHaveLength(1); qs(document, '.save-popover .sp-input').value = 'Q'; qs(document, '.save-popover .sp-save').dispatchEvent(new Event('click')); + await flush(); expect(app.dom.saveBtn!.textContent).toContain('Saved'); // edit → button reverts to "Save" app.activeTab().sqlDraft = 'SELECT 2'; @@ -2902,17 +2923,75 @@ describe('share + star + columns', () => { expect(qs(document, '.save-popover')).toBeNull(); expect(qs(document, '.share-toast').textContent).toBe('Nothing to save'); }); - it('linked Save also retains the empty-SQL guard', () => { + it('linked Save also retains the empty-SQL guard', async () => { const app = createApp(env()); app.renderApp(); app.state.savedQueries = [savedQueryFixture({ id: 's9', name: 'Fav', sql: 'SELECT 9' })]; app.actions.loadIntoNewTab(asQueryOrName(app.state.savedQueries[0])); app.sqlEditor.replaceDocument(''); - app.actions.save(); + await app.actions.save(); expect(app.state.savedQueries[0].sql).toBe('SELECT 9'); expect(qs(document, '.share-toast').textContent).toBe('Nothing to save'); expect(app.activeTab().dirtySql).toBe(true); }); + it('#287 W4: the Save popover surfaces a toast (and mutates nothing) when the aggregate commit is rejected', async () => { + const app = createApp(env()); + app.renderApp(); + const diagnostics = [{ path: [], severity: 'error' as const, code: 'test-fail', message: 'boom' }]; + app.workspace.commit = vi.fn(async () => ({ ok: false as const, diagnostics })); + app.activeTab().sqlDraft = 'SELECT 1'; + app.actions.save(); // opens the popover synchronously + qs(document, '.save-popover .sp-input').value = 'Q'; + qs(document, '.save-popover .sp-save').dispatchEvent(new Event('click')); + await flush(); + expect(app.state.savedQueries).toEqual([]); + expect(app.activeTab().savedId).toBeNull(); + expect(qs(document, '.share-toast').textContent).toBe('Save failed: boom'); + }); + it('#287 W4: linked Save surfaces a toast (and mutates nothing) when the aggregate commit is rejected', async () => { + const app = createApp(env()); + app.renderApp(); + app.state.savedQueries = [savedQueryFixture({ id: 's9', name: 'Fav', sql: 'SELECT 9' })]; + app.actions.loadIntoNewTab(asQueryOrName(app.state.savedQueries[0])); + const diagnostics = [{ path: [], severity: 'error' as const, code: 'test-fail', message: 'boom' }]; + app.workspace.commit = vi.fn(async () => ({ ok: false as const, diagnostics })); + app.sqlEditor.replaceDocument('SELECT 99'); + await app.actions.save(); + expect(app.state.savedQueries[0].sql).toBe('SELECT 9'); // unchanged + expect(qs(document, '.share-toast').textContent).toBe('Save failed: boom'); + }); + it('#287 W4: loadWorkspaceOnBoot projects the resolved aggregate onto state, or leaves it untouched on a null/failed load', async () => { + // A working IndexedDB (env()'s own #287 default fake): the one-shot legacy + // migration builds + commits a fresh aggregate (no favorites yet, but the + // Dashboard is still created — see legacy-migration.ts), so the resolved + // workspace is non-null and every field gets projected. + const app = createApp(env()); + app.state.libraryName.value = 'Before boot'; + const before = app.state.workspaceId; + const workspace = await app.loadWorkspaceOnBoot(); + expect(workspace).not.toBeNull(); + expect(app.state.savedQueries).toEqual(workspace!.queries); + expect(app.state.dashboard).toEqual(workspace!.dashboard); + expect(app.state.workspaceId).toBe(workspace!.id); + expect(app.state.workspaceId).not.toBe(before); // overwritten by the real committed id + expect(app.state.libraryName.value).toBe(workspace!.name); + + // The one-shot migration's own commit rejected (a real repository + // rejection, not an unavailable store — see `workspace-persist-failed`): + // nothing was ever written, so the still-real, working `loadCurrent()` + // finds no record and resolves null → state is left exactly as it was + // (the synchronous `createState()` legacy projection, including its + // minted placeholder id). + const app2 = createApp(env()); + const beforeId2 = app2.state.workspaceId; + app2.workspace.commit = vi.fn(async () => ({ + ok: false as const, diagnostics: [{ path: [], severity: 'error' as const, code: 'test-fail', message: 'boom' }], + })); + const workspace2 = await app2.loadWorkspaceOnBoot(); + expect(workspace2).toBeNull(); + expect(app2.state.workspaceId).toBe(beforeId2); + expect(app2.state.dashboard).toBeNull(); + }); it('save popover closes on click outside', () => { const app = createApp(env()); app.renderApp(); @@ -3002,7 +3081,7 @@ describe('share + star + columns', () => { const before = structuredClone(app.state.savedQueries[0]); store.setItem.mockClear(); - app.actions.save(); + await app.actions.save(); app.actions.setEditorMode('sql'); app.actions.share(); await Promise.resolve(); @@ -3100,7 +3179,7 @@ describe('share + star + columns', () => { error(); warning(); expect(app.dom.specStatus!.hidden).toBe(true); }); - it('synchronously reruns registered blocking validation inside linked Save', () => { + it('synchronously reruns registered blocking validation inside linked Save', async () => { const store = { getItem: vi.fn(() => null), setItem: vi.fn() }; vi.stubGlobal('localStorage', store); const app = createApp(env()); @@ -3115,19 +3194,19 @@ describe('share + star + columns', () => { // Simulate stale presentation state: Save must not trust this array. app.activeTab().specDiagnostics = []; store.setItem.mockClear(); - app.actions.save(); + await app.actions.save(); expect(app.state.savedQueries[0].sql).toBe('SELECT 9'); expect(store.setItem).not.toHaveBeenCalled(); expect(app.activeTab().specDiagnostics[0]).toMatchObject({ code: 'runtime-blocked' }); }); - it('linked Save commits SQL and authoritative Spec directly without a popover', () => { + it('linked Save commits SQL and authoritative Spec directly without a popover', async () => { const app = createApp(env()); app.renderApp(); app.state.savedQueries = [savedQueryFixture({ id: 's9', name: 'Fav', sql: 'SELECT 9', description: 'why' })]; app.actions.loadIntoNewTab(asQueryOrName(app.state.savedQueries[0])); app.sqlEditor.replaceDocument('SELECT 10'); app.specEditor.replaceDocument('{"name":" Renamed ","description":" updated reason ","favorite":false,"future":{"kept":true}}'); - app.actions.save(); + await app.actions.save(); expect(qs(document, '.save-popover')).toBeNull(); expect(app.state.savedQueries[0].sql).toBe('SELECT 10'); expect(app.state.savedQueries[0].spec).toEqual({ @@ -3178,7 +3257,7 @@ describe('exhaustive controller coverage', () => { expect(e.sessionStorage!.getItem('oauth_refresh_token')).toBe('rt2'); }); - it('clicks every header + toolbar control', () => { + it('clicks every header + toolbar control', async () => { const e = env({ window: fakeWin(), navigator: { clipboard: asClipboard({ writeText: vi.fn(async () => {}) }) } }); const app = createApp(e); app.renderApp(); @@ -3188,6 +3267,7 @@ describe('exhaustive controller coverage', () => { app.dom.saveBtn!.dispatchEvent(new Event('click')); // open save popover qs(document, '.save-popover .sp-input').value = 'Q'; qs(document, '.save-popover .sp-save').dispatchEvent(new Event('click')); // commit + await flush(); // the popover's Save button awaits the aggregate commit (#287 W4) app.dom.shareBtn!.dispatchEvent(new Event('click')); // share expect(app.state.tabs.value.length).toBeGreaterThan(1); expect(app.state.savedQueries.length).toBe(1); diff --git a/tests/unit/dashboard-export.test.ts b/tests/unit/dashboard-export.test.ts new file mode 100644 index 00000000..71e6b019 --- /dev/null +++ b/tests/unit/dashboard-export.test.ts @@ -0,0 +1,131 @@ +import { describe, expect, it } from 'vitest'; +import { + buildDashboardExportBundle, buildWorkspaceExportBundle, +} from '../../src/dashboard/model/dashboard-export.js'; +import type { + DashboardDocumentV1, SavedQueryV2, StoredWorkspaceV1, +} from '../../src/generated/json-schema.types.js'; + +const query = (id: string): SavedQueryV2 => ({ + id, sql: `SELECT '${id}'`, specVersion: 1, spec: { name: id }, +}); + +const dashboard = ( + id: string, tileQueryIds: string[], filterSourceIds: string[] = [], +): DashboardDocumentV1 => ({ + documentVersion: 1, id, title: `Dashboard ${id}`, revision: 3, + layout: { type: 'flow', version: 1, preset: 'full-width', items: {} }, + tiles: tileQueryIds.map((queryId, index) => ({ + id: `${id}-t${index}`, queryId, presentation: { kind: 'table' }, + } as unknown as DashboardDocumentV1['tiles'][number])), + filters: filterSourceIds.map((sourceQueryId, index) => ({ + id: `${id}-f${index}`, parameter: `p${index}`, sourceQueryId, + } as unknown as DashboardDocumentV1['filters'][number])), +}); + +describe('buildDashboardExportBundle', () => { + it('emits the dependency closure (tiles then filter sources, each once), excluding unrelated queries', () => { + const dash = dashboard('d1', ['q3', 'q1', 'q3'], ['q1', 'q9']); + const queries = [query('q1'), query('q2'), query('q3'), query('q9'), query('unrelated')]; + const bundle = buildDashboardExportBundle(dash, queries, '2020-01-01T00:00:00Z'); + + expect(bundle.format).toBe('altinity-sql-browser/portable-bundle'); + expect(bundle.version).toBe(1); + expect(bundle.exportedAt).toBe('2020-01-01T00:00:00Z'); + expect(bundle.queries.map((q) => q.id)).toEqual(['q3', 'q1', 'q9']); + expect(bundle.dashboards).toHaveLength(1); + expect(bundle.dashboards[0]?.id).toBe('d1'); + }); + + it('skips a dependency id that has no matching query', () => { + const dash = dashboard('d1', ['q1', 'missing']); + const queries = [query('q1')]; + const bundle = buildDashboardExportBundle(dash, queries, '2020-01-01T00:00:00Z'); + expect(bundle.queries.map((q) => q.id)).toEqual(['q1']); + }); + + it('deep-clones the dashboard and queries — mutating the result never touches the inputs', () => { + const dash = dashboard('d1', ['q1']); + const queries = [query('q1')]; + const bundle = buildDashboardExportBundle(dash, queries, '2020-01-01T00:00:00Z'); + + expect(bundle.dashboards[0]).not.toBe(dash); + expect(bundle.queries[0]).not.toBe(queries[0]); + + (bundle.dashboards[0] as DashboardDocumentV1).title = 'mutated'; + (bundle.dashboards[0] as DashboardDocumentV1).revision = 999; + (bundle.queries[0] as SavedQueryV2).sql = 'DROP TABLE x'; + + expect(dash.title).toBe('Dashboard d1'); + expect(dash.revision).toBe(3); + expect(queries[0]?.sql).toBe("SELECT 'q1'"); + }); + + // REGRESSION: an export must never mutate the caller's Dashboard identity + // or persisted revision — this is the deep-clone proof the export path + // relies on to keep `commit()`'s revision counter honest (#280/#287). + it('REGRESSION: leaves the input dashboard revision and identity unchanged', () => { + const dash = dashboard('d1', ['q1']); + const originalRevision = dash.revision; + const originalRef = dash; + buildDashboardExportBundle(dash, [query('q1')], '2020-01-01T00:00:00Z'); + expect(dash).toBe(originalRef); + expect(dash.revision).toBe(originalRevision); + }); +}); + +describe('buildWorkspaceExportBundle', () => { + const workspaceFixture = (over: Partial = {}): StoredWorkspaceV1 => ({ + storageVersion: 1, id: 'ws', name: 'WS', + queries: [query('q2'), query('q1'), query('q3')], + dashboard: dashboard('d1', ['q1']), + ...over, + } as StoredWorkspaceV1); + + it('emits every query in catalog order, not reordered by Dashboard tile usage', () => { + const ws = workspaceFixture(); + const bundle = buildWorkspaceExportBundle(ws, '2020-01-01T00:00:00Z'); + expect(bundle.queries.map((q) => q.id)).toEqual(['q2', 'q1', 'q3']); + expect(bundle.dashboards).toHaveLength(1); + expect(bundle.dashboards[0]?.id).toBe('d1'); + }); + + it('emits zero dashboards when the workspace has none', () => { + const ws = workspaceFixture({ dashboard: null }); + const bundle = buildWorkspaceExportBundle(ws, '2020-01-01T00:00:00Z'); + expect(bundle.dashboards).toEqual([]); + expect(bundle.queries.map((q) => q.id)).toEqual(['q2', 'q1', 'q3']); + }); + + it('sets format/version/exportedAt fields', () => { + const bundle = buildWorkspaceExportBundle(workspaceFixture(), '2021-06-15T12:00:00Z'); + expect(bundle.format).toBe('altinity-sql-browser/portable-bundle'); + expect(bundle.version).toBe(1); + expect(bundle.exportedAt).toBe('2021-06-15T12:00:00Z'); + }); + + it('deep-clones queries and the dashboard — mutating the result never touches the input workspace', () => { + const ws = workspaceFixture(); + const bundle = buildWorkspaceExportBundle(ws, '2020-01-01T00:00:00Z'); + + expect(bundle.queries[0]).not.toBe(ws.queries[0]); + expect(bundle.dashboards[0]).not.toBe(ws.dashboard); + + (bundle.queries[0] as SavedQueryV2).sql = 'DROP TABLE x'; + (bundle.dashboards[0] as DashboardDocumentV1).revision = 999; + + expect(ws.queries[0]?.sql).toBe("SELECT 'q2'"); + expect(ws.dashboard?.revision).toBe(3); + }); + + // REGRESSION: a full-workspace export must never mutate the caller's + // Dashboard identity or persisted revision (#280/#287 deep-clone proof). + it('REGRESSION: leaves the input workspace.dashboard revision and identity unchanged', () => { + const ws = workspaceFixture(); + const originalDashboardRef = ws.dashboard; + const originalRevision = ws.dashboard?.revision; + buildWorkspaceExportBundle(ws, '2020-01-01T00:00:00Z'); + expect(ws.dashboard).toBe(originalDashboardRef); + expect(ws.dashboard?.revision).toBe(originalRevision); + }); +}); diff --git a/tests/unit/file-menu.test.ts b/tests/unit/file-menu.test.ts index fe867c4c..2935bbf7 100644 --- a/tests/unit/file-menu.test.ts +++ b/tests/unit/file-menu.test.ts @@ -1,10 +1,13 @@ import { describe, it, expect, vi, afterEach } from 'vitest'; import { libraryControls, renderLibraryTitle, openFileMenu } from '../../src/ui/file-menu.js'; import { queryName } from '../../src/core/saved-query.js'; +import { decodePortableBundleJson } from '../../src/dashboard/model/portable-bundle-codec.js'; import { makeApp } from '../helpers/fake-app.js'; +import type { MakeAppOverrides } from '../helpers/fake-app.js'; import { savedQuery } from '../helpers/saved-query.js'; import type { SavedQueryFixture } from '../helpers/saved-query.js'; import type { App } from '../../src/ui/app.types.js'; +import type { DashboardDocumentV1, PortableBundleV1, SavedQueryV2 } from '../../src/generated/json-schema.types.js'; const click = (el: Element): boolean => el.dispatchEvent(new Event('click', { bubbles: true })); const key = (target: EventTarget, k: string, mods: KeyboardEventInit = {}): boolean => @@ -15,6 +18,10 @@ const toast = (): string | null => document.querySelector('.share-toast')!.textC const setSaved = (app: App, queries: SavedQueryFixture[]): void => { app.state.savedQueries = queries.map((q) => savedQuery(q)); }; +// Flush the microtask queue — every commit path here is async (`app.workspace +// .commit` always returns a Promise), so an assertion made right after firing +// a UI event needs one tick before the projection lands. +const flush = (): Promise => new Promise((r) => setTimeout(r)); // A FileReader stub: readAsText resolves synchronously with `content` (or errors). // Implements the full (mostly-unused) `FileReader` interface honestly — rather @@ -46,15 +53,36 @@ const fakeReader = (content: string, fail?: boolean): typeof FileReader => class removeEventListener(): void {} dispatchEvent(): boolean { return true; } }; -const envFile = (queries: unknown[]): string => JSON.stringify({ format: 'altinity-sql-browser/saved-queries', version: 1, queries }); + +// ── portable-bundle / Dashboard fixtures (mirror import-planner.test.ts) ──── + +const panelQuery = (id: string, name = id, sql = 'SELECT 1'): SavedQueryV2 => ({ + id, sql, specVersion: 1, spec: { name, panel: { cfg: { type: 'bar', x: 0, y: [1] } } }, +}); +const dashboardDoc = (over: Partial = {}): DashboardDocumentV1 => ({ + documentVersion: 1, id: 'd1', title: 'D', revision: 1, + layout: { type: 'flow', version: 1, preset: 'full-width', items: {} }, + filters: [], tiles: [], ...over, +}); +const bundleDoc = (over: Partial = {}): PortableBundleV1 => ({ + format: 'altinity-sql-browser/portable-bundle', version: 1, + exportedAt: '2026-07-17T00:00:00.000Z', queries: [], dashboards: [], ...over, +}); +const bundleText = (over: Partial = {}): string => JSON.stringify(bundleDoc(over)); +const legacyFile = (queries: unknown[]): string => + JSON.stringify({ format: 'altinity-sql-browser/saved-queries', version: 1, queries }); // Build an app with the header controls mounted (File button + title slot in the DOM). -function mount = Record>(over: O = {} as O) { +function mount>(over: O = {} as O) { const app = makeApp(over); for (const node of libraryControls(app)) document.body.appendChild(node); return app; } const picker = (i: number): HTMLInputElement => document.querySelectorAll('.file-menu input[type=file]')[i]; +const pickFile = (input: HTMLInputElement, name = 'file.json'): void => { + Object.defineProperty(input, 'files', { configurable: true, value: [{ name }] }); + input.dispatchEvent(new Event('change', { bubbles: true })); +}; afterEach(() => document.body.replaceChildren()); @@ -110,29 +138,27 @@ describe('variable history (#171)', () => { }); }); -describe('library title', () => { - it('renders the name + dirty dot; inline rename commits on Enter and persists', () => { +describe('workspace title', () => { + it('renders the name + dirty dot; inline rename commits on Enter through the workspace aggregate', async () => { const app = mount(); - app.state.libraryName.value = 'My queries'; + app.state.libraryName.value = 'My workspace'; app.state.libraryDirty.value = true; renderLibraryTitle(app); - expect(app.dom.libraryTitle!.querySelector('.lib-name-text')!.textContent).toBe('My queries'); + expect(app.dom.libraryTitle!.querySelector('.lib-name-text')!.textContent).toBe('My workspace'); expect(app.dom.libraryTitle!.querySelector('.lib-dirty')).not.toBeNull(); click(app.dom.libraryTitle!.querySelector('.lib-name')!); expect(app.editingLibrary).toBe(true); const input = app.dom.libraryTitle!.querySelector('.lib-name-input')!; - expect(input.value).toBe('My queries'); + expect(input.value).toBe('My workspace'); input.value = 'Renamed'; key(input, 'Enter'); + expect(app.editingLibrary).toBe(false); // leaves edit mode immediately, before the commit resolves + await flush(); expect(app.state.libraryName.value).toBe('Renamed'); - expect(app.editingLibrary).toBe(false); - expect(app.saveStr).toHaveBeenCalled(); - app.state.libraryDirty.value = false; - renderLibraryTitle(app); - expect(app.dom.libraryTitle!.querySelector('.lib-dirty')).toBeNull(); + expect(app.state.libraryDirty.value).toBe(false); // a fresh commit is never "unsaved" }); - it('inline rename: Escape cancels, blur commits, empty commit is a no-op, double-fire guarded', () => { + it('inline rename: Escape cancels, blur commits, empty commit is a no-op, double-fire guarded', async () => { const app = mount(); app.state.libraryName.value = 'Orig'; renderLibraryTitle(app); @@ -153,8 +179,10 @@ describe('library title', () => { input = app.dom.libraryTitle!.querySelector('.lib-name-input')!; input.value = 'Blurred'; input.dispatchEvent(new Event('blur')); + await flush(); expect(app.state.libraryName.value).toBe('Blurred'); key(input, 'Enter'); + await flush(); expect(app.state.libraryName.value).toBe('Blurred'); }); @@ -163,36 +191,34 @@ describe('library title', () => { }); }); -describe('file menu', () => { +describe('file menu structure', () => { it('lists every section + item, reflects the (pluralized) count, and re-open is a no-op', () => { const app = mount(); - setSaved(app, [ - { id: 's1', name: 'A', sql: '1', favorite: false }, - { id: 's2', name: 'B', sql: '2', favorite: false }, - ]); + app.state.savedQueries = [panelQuery('s1', 'A'), panelQuery('s2', 'B')]; openFileMenu(app); expect([...document.querySelectorAll('.fm-label')].map((l) => l.textContent)).toEqual([ - 'New Library', 'Open as dashboard', 'Remember recent variable values', 'Clear all recent values', - 'Save JSON', 'Open…', 'Append…', 'Download Markdown', 'Download SQL', + 'New workspace…', 'Open as dashboard', 'Remember recent variable values', 'Clear all recent values', + 'Import queries…', 'Import Dashboard…', 'Replace workspace…', + 'Export Dashboard…', 'Export workspace…', 'Download Markdown', 'Download SQL', ]); expect([...document.querySelectorAll('.fm-section')].map((s) => s.textContent)).toEqual( - ['Dashboard', 'Variable history', 'Save library', 'Load from file', 'Share / publish']); - expect(document.querySelector('.fm-count')!.textContent).toBe('2 queries in Library'); + ['Dashboard', 'Variable history', 'Import / replace', 'Export', 'Share / publish']); + expect(document.querySelector('.fm-count')!.textContent).toBe('2 queries in workspace'); openFileMenu(app); expect(document.querySelectorAll('.file-menu')).toHaveLength(1); }); - it('autofocuses the first item (New Library) on open', async () => { + it('autofocuses the first item (New workspace…) on open', async () => { const app = mount(); openFileMenu(app); - await new Promise((r) => setTimeout(r)); - expect(document.activeElement).toBe(item(/New Library/)); + await flush(); + expect(document.activeElement).toBe(item(/New workspace/)); }); it('footer shows the empty state when there are no queries', () => { const app = mount(); openFileMenu(app); - expect(document.querySelector('.fm-count')!.textContent).toBe('Library is empty'); + expect(document.querySelector('.fm-count')!.textContent).toBe('Workspace is empty'); }); it('closes on overlay click and on Escape (ignores other keys)', () => { @@ -208,32 +234,65 @@ describe('file menu', () => { }); }); -describe('Save JSON / Markdown / SQL downloads', () => { - it('Save JSON: empty → toast; non-empty → download envelope, clear dirty', () => { +describe('Export', () => { + it('Export Dashboard is disabled (with a reason) and toasts when there is no Dashboard', () => { const app = mount(); openFileMenu(app); - click(item(/Save JSON/)!); + const btn = item(/Export Dashboard/)!; + expect(btn.textContent).toContain('no dashboard'); + click(btn); expect(app.downloadFile).not.toHaveBeenCalled(); - expect(toast()).toBe('Nothing to save'); - setSaved(app, [{ id: 's1', name: 'A', sql: '1', favorite: true }]); - app.state.libraryName.value = 'My Lib'; - app.state.libraryDirty.value = true; + expect(toast()).toBe('No dashboard to export'); + }); + + it('Export Dashboard downloads a valid bundle containing only its query dependencies', () => { + const app = mount(); + app.state.dashboard = dashboardDoc({ title: 'Ops', tiles: [{ id: 't1', queryId: 'p1' }] }); + app.state.savedQueries = [panelQuery('p1', 'Panel'), panelQuery('unrelated', 'Unrelated')]; openFileMenu(app); - click(item(/Save JSON/)!); + click(item(/Export Dashboard/)!); const [fname, mime, content] = app.downloadFile.mock.calls[0]; - expect(fname).toBe('My Lib.json'); + expect(fname).toBe('Ops.json'); expect(mime).toBe('application/json'); - expect(JSON.parse(content as string)).toMatchObject({ - $schema: 'https://altinity.com/schemas/altinity-sql-browser/library-v2.schema.json', - format: 'altinity-sql-browser/saved-queries', - version: 2, - exportedAt: '1970-01-01T00:00:00.000Z', - }); - expect(app.state.libraryDirty.value).toBe(false); - expect(toast()).toBe('Saved 1 query → .json'); + const decoded = decodePortableBundleJson(content as string); + expect(decoded.ok).toBe(true); + if (decoded.ok) { + expect(decoded.value.queries.map((q) => q.id)).toEqual(['p1']); // unrelated query excluded + expect(decoded.value.dashboards).toHaveLength(1); + } + expect(toast()).toBe('Exported → .json'); }); - it('Download Markdown + SQL: empty → toast; non-empty → files named from the library', () => { + it('a role-incompatible Dashboard toasts the encode diagnostic instead of downloading', () => { + const filterQuery: SavedQueryV2 = { + id: 'f1', sql: 'SELECT 1', specVersion: 1, spec: { name: 'F', dashboard: { role: 'filter' } }, + }; + const app = mount(); + // A tile referencing a filter-role query — legal to ASSEMBLE (Wave 1's + // pure builder never re-validates), but `encodePortableBundleJson`'s own + // re-validation catches the role mismatch. + app.state.dashboard = dashboardDoc({ tiles: [{ id: 't1', queryId: 'f1' }] }); + app.state.savedQueries = [filterQuery]; + openFileMenu(app); + click(item(/Export Dashboard/)!); + expect(app.downloadFile).not.toHaveBeenCalled(); + expect(toast()).toMatch(/^✕ /); + }); + + it('Export workspace downloads a valid bundle containing the whole catalog', () => { + const app = mount(); + app.state.libraryName.value = 'My Lib'; + app.state.savedQueries = [panelQuery('p1'), panelQuery('p2')]; + openFileMenu(app); + click(item(/Export workspace/)!); + const [fname, , content] = app.downloadFile.mock.calls[0]; + expect(fname).toBe('My Lib.json'); + const decoded = decodePortableBundleJson(content as string); + expect(decoded.ok).toBe(true); + if (decoded.ok) expect(decoded.value.queries.map((q) => q.id).sort()).toEqual(['p1', 'p2']); + }); + + it('Download Markdown + SQL: empty → toast; non-empty → files named from the workspace', () => { const app = mount(); openFileMenu(app); click(item(/Download Markdown/)!); @@ -247,7 +306,7 @@ describe('Save JSON / Markdown / SQL downloads', () => { openFileMenu(app); click(item(/Download SQL/)!); expect(app.downloadFile.mock.calls.at(-1)!.slice(0, 2)).toEqual(['Lib.sql', 'application/sql']); - // an unnamed / whitespace-only library name falls back to "queries" + // an unnamed / whitespace-only workspace name falls back to "queries" app.state.libraryName.value = ''; openFileMenu(app); click(item(/Download Markdown/)!); @@ -259,138 +318,345 @@ describe('Save JSON / Markdown / SQL downloads', () => { }); }); -describe('Open / Append (JSON only)', () => { - it('Open item closes the menu and opens the picker; a non-empty library confirms first', () => { - const app = mount({ FileReader: fakeReader(envFile([{ id: 'x', name: 'New', sql: 'S' }, { name: 'New2', sql: 'S2' }])) }); - setSaved(app, [ - { id: 's1', name: 'Old', sql: '1', favorite: false }, - { id: 's2', name: 'Old2', sql: '2', favorite: false }, - ]); +describe('Import queries', () => { + it('the menu item closes the menu, opens the picker, and a picked file imports without a conflict dialog', async () => { + const app = mount({ FileReader: fakeReader(bundleText({ queries: [panelQuery('q1', 'Q1'), panelQuery('q2', 'Q2')] })) }); + app.state.savedQueries = [panelQuery('q1', 'Q1')]; // canonically identical to the incoming q1 → auto-resolved openFileMenu(app); - const replaceInput = picker(0); - replaceInput.click = vi.fn(); - click(item(/Open…/)!); - expect(document.querySelector('.file-menu')).toBeNull(); // menu closed - expect(replaceInput.click).toHaveBeenCalled(); - // user picks a file → confirm dialog (current library non-empty, plural copy) - Object.defineProperty(replaceInput, 'files', { configurable: true, value: [{ name: 'team.json' }] }); - replaceInput.dispatchEvent(new Event('change', { bubbles: true })); - const dialog = document.querySelector('.fm-dialog-card')!; - expect(dialog.textContent).toContain('Open and replace current library?'); - expect(dialog.textContent).toContain('contains 2 queries'); - expect(dialog.textContent).toContain('current 2 saved queries'); - click(document.querySelector('.fm-dialog-confirm')!); - expect(app.state.savedQueries.map(queryName)).toEqual(['New', 'New2']); - expect(app.state.libraryName.value).toBe('team'); - expect(app.updateSaveBtn).toHaveBeenCalled(); - expect(app.updateEditorModeUi).toHaveBeenCalled(); - expect(toast()).toBe('Opened library · 2 queries'); + const input = picker(0); + input.click = vi.fn(); + click(item(/Import queries/)!); + expect(document.querySelector('.file-menu')).toBeNull(); + expect(input.click).toHaveBeenCalled(); + pickFile(input); + await flush(); + expect(document.querySelector('.fm-dialog-card')).toBeNull(); + expect(app.state.savedQueries.map((q) => q.id).sort()).toEqual(['q1', 'q2']); + expect(toast()).toBe('Imported 2 queries'); + }); + + it('imports directly (no dialog at all) when no incoming id overlaps an existing one', async () => { + const app = mount({ FileReader: fakeReader(bundleText({ queries: [panelQuery('new1'), panelQuery('new2')] })) }); + app.state.savedQueries = [panelQuery('existing')]; + openFileMenu(app); + pickFile(picker(0)); + await flush(); + expect(document.querySelector('.fm-dialog-card')).toBeNull(); + expect(app.state.savedQueries.map((q) => q.id).sort()).toEqual(['existing', 'new1', 'new2']); }); - it('Open into an empty library loads directly (no confirm); cancelling the picker is a no-op', () => { - const app = mount({ FileReader: fakeReader(envFile([{ name: 'New', sql: 'S' }])) }); + it('picking no file is a no-op', () => { + const app = mount({ FileReader: fakeReader(bundleText({ queries: [panelQuery('q1')] })) }); openFileMenu(app); const input = picker(0); - // cancel (no file chosen) → nothing happens Object.defineProperty(input, 'files', { configurable: true, value: [] }); input.dispatchEvent(new Event('change', { bubbles: true })); expect(app.state.savedQueries).toEqual([]); - // pick a file → loaded directly, name adopted, no dialog - Object.defineProperty(input, 'files', { configurable: true, value: [{ name: 'lib.json' }] }); - input.dispatchEvent(new Event('change', { bubbles: true })); + }); + + it('opens a conflict dialog for genuinely differing ids; the global default resolves an unmarked row', async () => { + const app = mount({ FileReader: fakeReader(bundleText({ queries: [panelQuery('q1', 'NewName')] })) }); + app.state.savedQueries = [panelQuery('q1', 'OldName')]; + openFileMenu(app); + pickFile(picker(0)); + const dialog = document.querySelector('.fm-dialog-card')!; + expect(dialog.textContent).toContain('Resolve 1 conflicting query'); + expect(dialog.textContent).toContain('OldName'); // row shows the EXISTING query's name + click(document.querySelector('.fm-dialog-confirm')!); // Apply with the default (use-existing) + await flush(); + expect(app.state.savedQueries.map((q) => queryName(q))).toEqual(['OldName']); + expect(toast()).toBe('Imported 1 query'); + }); + + it('a per-row override wins over the global default', async () => { + const app = mount({ FileReader: fakeReader(bundleText({ queries: [panelQuery('q1', 'NewName')] })) }); + app.state.savedQueries = [panelQuery('q1', 'OldName')]; + openFileMenu(app); + pickFile(picker(0)); + const rowSelect = document.querySelectorAll('.fm-select')[1]; // [0] is the global select + rowSelect.value = 'replace'; + rowSelect.dispatchEvent(new Event('change', { bubbles: true })); + click(document.querySelector('.fm-dialog-confirm')!); + await flush(); + expect(app.state.savedQueries.map((q) => queryName(q))).toEqual(['NewName']); + }); + + it('resetting a row back to "Use default" falls back to the global action', async () => { + const app = mount({ FileReader: fakeReader(bundleText({ queries: [panelQuery('q1', 'NewName')] })) }); + app.state.savedQueries = [panelQuery('q1', 'OldName')]; + openFileMenu(app); + pickFile(picker(0)); + const [globalSelect, rowSelect] = document.querySelectorAll('.fm-select'); + globalSelect.value = 'skip'; + globalSelect.dispatchEvent(new Event('change', { bubbles: true })); + rowSelect.value = 'replace'; + rowSelect.dispatchEvent(new Event('change', { bubbles: true })); + rowSelect.value = ''; // back to "Use default" — the row no longer overrides + rowSelect.dispatchEvent(new Event('change', { bubbles: true })); + click(document.querySelector('.fm-dialog-confirm')!); + await flush(); + // 'skip' (the global default) dropped the incoming row entirely + expect(app.state.savedQueries.map((q) => queryName(q))).toEqual(['OldName']); + }); + + it('the global default action applies to every unmarked conflicting row ("copy" keeps both)', async () => { + const app = mount({ FileReader: fakeReader(bundleText({ queries: [panelQuery('q1', 'NewName')] })) }); + app.state.savedQueries = [panelQuery('q1', 'OldName')]; + openFileMenu(app); + pickFile(picker(0)); + const globalSelect = document.querySelector('.fm-select')!; + globalSelect.value = 'copy'; + globalSelect.dispatchEvent(new Event('change', { bubbles: true })); + click(document.querySelector('.fm-dialog-confirm')!); + await flush(); + expect(app.state.savedQueries.map((q) => queryName(q)).sort()).toEqual(['NewName', 'OldName']); + expect(app.state.savedQueries).toHaveLength(2); + }); + + it('"skip" drops the incoming query, leaving the existing one untouched', async () => { + const app = mount({ FileReader: fakeReader(bundleText({ queries: [panelQuery('q1', 'NewName')] })) }); + app.state.savedQueries = [panelQuery('q1', 'OldName')]; + openFileMenu(app); + pickFile(picker(0)); + const globalSelect = document.querySelector('.fm-select')!; + globalSelect.value = 'skip'; + globalSelect.dispatchEvent(new Event('change', { bubbles: true })); + click(document.querySelector('.fm-dialog-confirm')!); + await flush(); + expect(app.state.savedQueries.map((q) => queryName(q))).toEqual(['OldName']); + }); + + it('cancelling the conflict dialog aborts the import (no commit)', () => { + const commit = vi.fn(async (candidate) => ({ ok: true as const, workspace: candidate, dashboardRevision: null })); + const app = mount({ + FileReader: fakeReader(bundleText({ queries: [panelQuery('q1', 'NewName')] })), + workspace: { commit }, + }); + app.state.savedQueries = [panelQuery('q1', 'OldName')]; + openFileMenu(app); + pickFile(picker(0)); + click(document.querySelector('.fm-dialog-cancel')!); expect(document.querySelector('.fm-dialog-card')).toBeNull(); - expect(app.state.savedQueries.map(queryName)).toEqual(['New']); - expect(app.state.libraryName.value).toBe('lib'); + expect(commit).not.toHaveBeenCalled(); + expect(app.state.savedQueries.map((q) => queryName(q))).toEqual(['OldName']); }); +}); - it('Append item closes the menu, merges the file, and toasts counts', () => { - const app = mount({ FileReader: fakeReader(envFile([{ id: 's1', name: 'A', sql: '1' }, { name: 'B', sql: '2' }])) }); - setSaved(app, [{ id: 's1', name: 'A', sql: '1', favorite: false }]); +describe('Import Dashboard', () => { + it('the menu item closes the menu and opens the picker; a single-dashboard file imports directly, minting a fresh id/revision', async () => { + const dep = panelQuery('p1', 'Panel'); + const dash = dashboardDoc({ id: 'src-d', title: 'Sales', tiles: [{ id: 't1', queryId: 'p1' }] }); + const app = mount({ FileReader: fakeReader(bundleText({ queries: [dep], dashboards: [dash] })) }); openFileMenu(app); - const appendInput = picker(1); - appendInput.click = vi.fn(); - click(item(/Append/)!); + const input = picker(1); + input.click = vi.fn(); + click(item(/Import Dashboard/)!); expect(document.querySelector('.file-menu')).toBeNull(); - expect(appendInput.click).toHaveBeenCalled(); - Object.defineProperty(appendInput, 'files', { configurable: true, value: [{ name: 'more.json' }] }); - appendInput.dispatchEvent(new Event('change', { bubbles: true })); - expect(app.state.savedQueries.map(queryName)).toEqual(['A', 'B']); - expect(toast()).toBe('Added 1 · updated 0 · skipped 1'); // the duplicate A is skipped + expect(input.click).toHaveBeenCalled(); + pickFile(input); + await flush(); + expect(document.querySelector('.fm-dialog-card')).toBeNull(); + expect(app.state.dashboard).not.toBeNull(); + expect(app.state.dashboard!.id).not.toBe('src-d'); // mode 'copy' mints a fresh id + expect(app.state.dashboard!.revision).toBe(1); + expect(app.state.dashboard!.title).toBe('Sales'); + expect(app.state.savedQueries.map((q) => q.id)).toContain('p1'); + expect(toast()).toBe('Imported dashboard'); }); - it('Open / Append with no queries in the file → error toast', () => { - const app = mount({ FileReader: fakeReader(envFile([])) }); + it('confirms before discarding an existing Dashboard, then replaces it on confirm', async () => { + const dep = panelQuery('p1', 'Panel'); + const dash = dashboardDoc({ id: 'src-d', title: 'Sales', tiles: [{ id: 't1', queryId: 'p1' }] }); + const app = mount({ FileReader: fakeReader(bundleText({ queries: [dep], dashboards: [dash] })) }); + app.state.dashboard = dashboardDoc({ id: 'old', title: 'My existing dashboard' }); openFileMenu(app); - Object.defineProperty(picker(0), 'files', { configurable: true, value: [{ name: 'empty.json' }] }); - picker(0).dispatchEvent(new Event('change', { bubbles: true })); - expect(toast()).toBe('✕ No queries in file'); + pickFile(picker(1)); + const dialog = document.querySelector('.fm-dialog-card')!; + expect(dialog.textContent).toContain('Import and replace current Dashboard?'); + click(document.querySelector('.fm-dialog-confirm')!); + await flush(); + expect(app.state.dashboard!.title).toBe('Sales'); + expect(app.state.dashboard!.id).not.toBe('old'); // mode 'copy' mints a fresh id + expect(toast()).toBe('Imported dashboard'); + }); + + it('cancelling the discard-Dashboard confirm keeps the current Dashboard and imports nothing', () => { + const dash = dashboardDoc({ id: 'src-d', title: 'Sales' }); + const app = mount({ FileReader: fakeReader(bundleText({ dashboards: [dash] })) }); + app.state.dashboard = dashboardDoc({ id: 'old', title: 'My existing dashboard' }); openFileMenu(app); - Object.defineProperty(picker(1), 'files', { configurable: true, value: [{ name: 'empty.json' }] }); - picker(1).dispatchEvent(new Event('change', { bubbles: true })); - expect(toast()).toBe('✕ No queries in file'); + pickFile(picker(1)); + expect(document.querySelector('.fm-dialog-card')!.textContent).toContain('Import and replace current Dashboard?'); + click(document.querySelector('.fm-dialog-cancel')!); + expect(app.state.dashboard!.id).toBe('old'); + expect(app.state.dashboard!.title).toBe('My existing dashboard'); }); - it('invalid JSON → error toast; a read error → error toast', () => { - const bad = mount({ FileReader: fakeReader('{not json') }); - openFileMenu(bad); - Object.defineProperty(picker(0), 'files', { configurable: true, value: [{ name: 'bad.json' }] }); - picker(0).dispatchEvent(new Event('change', { bubbles: true })); - expect(toast()).toBe('✕ Not a valid JSON file'); - document.body.replaceChildren(); - const err = mount({ FileReader: fakeReader('', true) }); - openFileMenu(err); - Object.defineProperty(picker(0), 'files', { configurable: true, value: [{ name: 'x.json' }] }); - picker(0).dispatchEvent(new Event('change', { bubbles: true })); - expect(toast()).toBe('✕ Could not read file'); + it('shows a picker (no "No dashboard" option) for a multi-dashboard bundle; selecting one imports only that one', async () => { + const dep1 = panelQuery('p1', 'Panel1'); + const dep2 = panelQuery('p2', 'Panel2'); + const dashA = dashboardDoc({ id: 'a', title: 'Alpha', tiles: [{ id: 't1', queryId: 'p1' }] }); + const dashB = dashboardDoc({ id: 'b', title: 'Beta', tiles: [{ id: 't2', queryId: 'p2' }] }); + const app = mount({ FileReader: fakeReader(bundleText({ queries: [dep1, dep2], dashboards: [dashA, dashB] })) }); + openFileMenu(app); + pickFile(picker(1)); + const dialog = document.querySelector('.fm-dialog-card')!; + expect(dialog.textContent).toContain('Import which dashboard?'); + expect(dialog.textContent).toContain('Alpha'); + expect(dialog.textContent).toContain('Beta'); + expect([...dialog.querySelectorAll('.fm-label')].some((l) => l.textContent === 'No dashboard')).toBe(false); + const betaRow = [...dialog.querySelectorAll('.fm-item')].find((b) => (b.textContent || '').includes('Beta'))!; + click(betaRow); + await flush(); + expect(app.state.dashboard!.title).toBe('Beta'); + expect(app.state.savedQueries.map((q) => q.id)).toContain('p2'); }); - it('rejects a complete invalid document before Open or Append can mutate state', () => { - const invalid = JSON.stringify({ - format: 'altinity-sql-browser/saved-queries', version: 2, - queries: [ - { id: 'same', sql: '1', specVersion: 1, spec: { name: 'A' } }, - { id: 'same', sql: '2', specVersion: 1, spec: { name: 'B' } }, - ], - }); - const app = mount({ FileReader: fakeReader(invalid) }); - setSaved(app, [{ id: 'keep', name: 'Keep', sql: 'SELECT 1', favorite: false }]); - openFileMenu(app); - const inputs = [picker(0), picker(1)]; - for (const input of inputs) { - Object.defineProperty(input, 'files', { configurable: true, value: [{ name: 'invalid.json' }] }); - input.dispatchEvent(new Event('change', { bubbles: true })); - expect(app.state.savedQueries.map((query) => query.id)).toEqual(['keep']); - expect(document.querySelector('.fm-dialog-card')).toBeNull(); - expect(toast()).toContain('duplicates'); - } + it('cancelling the multi-dashboard picker imports nothing', () => { + const dashA = dashboardDoc({ id: 'a', title: 'Alpha' }); + const dashB = dashboardDoc({ id: 'b', title: 'Beta' }); + const app = mount({ FileReader: fakeReader(bundleText({ dashboards: [dashA, dashB] })) }); + openFileMenu(app); + pickFile(picker(1)); + click(document.querySelector('.fm-dialog-cancel')!); + expect(document.querySelector('.fm-dialog-card')).toBeNull(); + expect(app.state.dashboard).toBeNull(); + }); + + it('aborts (no commit) when a skipped conflict breaks a required tile dependency', async () => { + const dash = dashboardDoc({ id: 'src', title: 'D', tiles: [{ id: 't1', queryId: 'p1' }] }); + const app = mount({ FileReader: fakeReader(bundleText({ queries: [panelQuery('p1', 'Incoming')], dashboards: [dash] })) }); + app.state.savedQueries = [panelQuery('p1', 'Existing')]; // conflicting id, different content + openFileMenu(app); + pickFile(picker(1)); + const rowSelect = document.querySelectorAll('.fm-select')[1]; + rowSelect.value = 'skip'; + rowSelect.dispatchEvent(new Event('change', { bubbles: true })); + click(document.querySelector('.fm-dialog-confirm')!); + await flush(); + expect(app.state.dashboard).toBeNull(); + expect(app.state.savedQueries.map((q) => queryName(q))).toEqual(['Existing']); + expect(toast()).toContain('missing required saved-query dependencies'); + }); + + it('toasts and aborts when the file has no dashboard', () => { + const app = mount({ FileReader: fakeReader(bundleText({ queries: [] })) }); + openFileMenu(app); + pickFile(picker(1)); + expect(toast()).toBe('✕ No dashboard in file'); + }); +}); + +describe('Replace workspace', () => { + it('the menu item closes the menu, opens the picker, and — after confirming — replaces the catalog + Dashboard', async () => { + const dep = panelQuery('p1', 'Panel'); + const dash = dashboardDoc({ id: 'd1', title: 'Ops', tiles: [{ id: 't1', queryId: 'p1' }] }); + const app = mount({ FileReader: fakeReader(bundleText({ queries: [dep], dashboards: [dash] })) }); + app.state.savedQueries = [panelQuery('old', 'Old')]; + openFileMenu(app); + const input = picker(2); + input.click = vi.fn(); + click(item(/Replace workspace/)!); + expect(document.querySelector('.file-menu')).toBeNull(); + expect(input.click).toHaveBeenCalled(); + pickFile(input); + const dialog = document.querySelector('.fm-dialog-card')!; + expect(dialog.textContent).toContain('Replace workspace?'); + expect(dialog.textContent).toContain('current 1 saved query'); + expect(dialog.textContent).toContain('and the selected Dashboard'); + click(document.querySelector('.fm-dialog-confirm')!); + await flush(); + expect(app.state.savedQueries.map((q) => q.id)).toEqual(['p1']); + expect(app.state.dashboard!.id).toBe('d1'); // Replace keeps the bundle Dashboard's own id/revision + expect(toast()).toBe('Replaced workspace'); + }); + + it('cancelling the confirm dialog leaves the workspace untouched', () => { + const app = mount({ FileReader: fakeReader(bundleText({ queries: [panelQuery('p1')] })) }); + app.state.savedQueries = [panelQuery('old', 'Old')]; + openFileMenu(app); + pickFile(picker(2)); + click(document.querySelector('.fm-dialog-cancel')!); + expect(document.querySelector('.fm-dialog-card')).toBeNull(); + expect(app.state.savedQueries.map((q) => q.id)).toEqual(['old']); + }); + + it('shows a picker with a "No dashboard" option for a multi-dashboard bundle; picking it clears the Dashboard', async () => { + const dashA = dashboardDoc({ id: 'a', title: 'Alpha' }); + const dashB = dashboardDoc({ id: 'b', title: 'Beta' }); + const app = mount({ FileReader: fakeReader(bundleText({ dashboards: [dashA, dashB] })) }); + app.state.dashboard = dashboardDoc({ id: 'existing', title: 'Existing' }); + openFileMenu(app); + pickFile(picker(2)); + const dialog = document.querySelector('.fm-dialog-card')!; + expect(dialog.textContent).toContain('Replace workspace — which dashboard?'); + const noneRow = [...dialog.querySelectorAll('.fm-item')].find((b) => (b.textContent || '').includes('No dashboard'))!; + click(noneRow); + expect(document.querySelector('.fm-dialog-card')!.textContent).toContain('Replace workspace?'); + click(document.querySelector('.fm-dialog-confirm')!); + await flush(); + expect(app.state.dashboard).toBeNull(); + }); + + it('auto-picks the sole Dashboard in a single-dashboard bundle (no picker)', () => { + const dash = dashboardDoc({ id: 'only', title: 'Only' }); + const app = mount({ FileReader: fakeReader(bundleText({ dashboards: [dash] })) }); + openFileMenu(app); + pickFile(picker(2)); + expect(document.querySelector('.fm-dialog-card')!.textContent).toContain('Replace workspace?'); + }); + + it('a queries-only bundle (no Dashboard) clears an existing Dashboard', async () => { + const app = mount({ FileReader: fakeReader(bundleText({ queries: [panelQuery('p1')] })) }); + app.state.dashboard = dashboardDoc({ id: 'existing', title: 'Existing' }); + app.state.savedQueries = []; + openFileMenu(app); + pickFile(picker(2)); + expect(document.querySelector('.fm-dialog-card')!.textContent).toContain('current 0 saved queries'); + click(document.querySelector('.fm-dialog-confirm')!); + await flush(); + expect(app.state.dashboard).toBeNull(); + expect(app.state.savedQueries.map((q) => q.id)).toEqual(['p1']); }); }); -describe('New Library + confirm dialogs', () => { - it('New Library: empty → clears directly; non-empty → confirm → New resets to the default', () => { +describe('New workspace', () => { + it('commits directly (no confirm) when the workspace is already empty', async () => { const app = mount(); + const oldId = app.state.workspaceId; openFileMenu(app); - click(item(/New Library/)!); + click(item(/New workspace/)!); + await flush(); expect(document.querySelector('.fm-dialog-backdrop')).toBeNull(); - expect(toast()).toBe('Started a new library'); - expect(app.updateEditorModeUi).toHaveBeenCalled(); - setSaved(app, [{ id: 's1', name: 'A', sql: '1', favorite: false }]); - app.state.libraryName.value = 'Old'; + expect(app.state.savedQueries).toEqual([]); + expect(app.state.libraryName.value).toBe('SQL Library'); + expect(app.state.workspaceId).not.toBe(oldId); + expect(toast()).toBe('Started a new workspace'); + }); + + it('confirms first when there are saved queries', async () => { + const app = mount(); + app.state.savedQueries = [panelQuery('q1')]; openFileMenu(app); - click(item(/New Library/)!); - expect(document.querySelector('.fm-dialog-card')!.textContent).toContain('Start a new library?'); + click(item(/New workspace/)!); + expect(document.querySelector('.fm-dialog-card')!.textContent).toContain('Start a new workspace?'); click(document.querySelector('.fm-dialog-confirm')!); + await flush(); expect(app.state.savedQueries).toEqual([]); - expect(app.state.libraryName.value).toBe('SQL Library'); + }); + + it('confirms first when a Dashboard exists even with no saved queries', () => { + const app = mount(); + app.state.dashboard = dashboardDoc(); + openFileMenu(app); + click(item(/New workspace/)!); + expect(document.querySelector('.fm-dialog-card')!.textContent).toContain('Start a new workspace?'); }); it('confirm dialog: Cancel, backdrop click, and Escape all dismiss; a card click does not', () => { const app = mount(); - setSaved(app, [ // two queries → exercises the plural dialog copy - { id: 's1', name: 'A', sql: '1', favorite: false }, - { id: 's2', name: 'B', sql: '2', favorite: false }, - ]); - const openNew = (): void => { openFileMenu(app); click(item(/New Library/)!); }; + app.state.savedQueries = [panelQuery('q1'), panelQuery('q2')]; // two → exercises the plural dialog copy + const openNew = (): void => { openFileMenu(app); click(item(/New workspace/)!); }; // Cancel openNew(); click(document.querySelector('.fm-dialog-cancel')!); @@ -413,9 +679,9 @@ describe('New Library + confirm dialogs', () => { it('a gesture starting on the card and ending on the backdrop does not dismiss it (#110)', () => { const app = mount(); - setSaved(app, [{ id: 's1', name: 'A', sql: '1', favorite: false }]); + app.state.savedQueries = [panelQuery('q1')]; openFileMenu(app); - click(item(/New Library/)!); + click(item(/New workspace/)!); const backdrop = document.querySelector('.fm-dialog-backdrop')!; const card = document.querySelector('.fm-dialog-card')!; card.dispatchEvent(new MouseEvent('mousedown', { bubbles: true })); @@ -423,3 +689,63 @@ describe('New Library + confirm dialogs', () => { expect(document.querySelector('.fm-dialog-backdrop')).not.toBeNull(); }); }); + +describe('decode failures', () => { + it('malformed JSON toasts the parse diagnostic (no legacy fallback)', () => { + const app = mount({ FileReader: fakeReader('{not json') }); + openFileMenu(app); + pickFile(picker(0)); + expect(toast()).toBe('✕ Not a valid JSON file'); + }); + + it('a read error toasts', () => { + const app = mount({ FileReader: fakeReader('', true) }); + openFileMenu(app); + pickFile(picker(0)); + expect(toast()).toBe('✕ Could not read file'); + }); + + it('falls back to the legacy Library decoder for a v1/v2 saved-queries file', async () => { + const app = mount({ FileReader: fakeReader(legacyFile([{ id: 'x', name: 'New', sql: 'S' }])) }); + openFileMenu(app); + pickFile(picker(0)); + await flush(); + expect(app.state.savedQueries.map((q) => queryName(q))).toEqual(['New']); + expect(toast()).toBe('Imported 1 query'); + }); + + it('toasts the legacy diagnostic when neither decoder recognizes the file', () => { + const app = mount({ FileReader: fakeReader(JSON.stringify({ foo: 1 })) }); + openFileMenu(app); + pickFile(picker(0)); + expect(toast()).toMatch(/^✕ /); + }); + + it('a structurally-invalid portable bundle toasts its OWN diagnostic, never falling back to "Unrecognized file format"', () => { + const bad = JSON.stringify({ + format: 'altinity-sql-browser/portable-bundle', version: 1, exportedAt: 'x', + queries: [{ id: 'a' }], dashboards: [], + }); + const app = mount({ FileReader: fakeReader(bad) }); + openFileMenu(app); + pickFile(picker(0)); + expect(toast()).toMatch(/^✕ /); + expect(toast()).not.toContain('Unrecognized file format'); + }); +}); + +describe('commit failure', () => { + it('a rejected commit toasts the first diagnostic and leaves state untouched', async () => { + const commit = vi.fn(async () => ({ + ok: false as const, diagnostics: [{ path: [], severity: 'error' as const, code: 'x', message: 'nope' }], + })); + const app = mount({ workspace: { commit } }); + app.state.savedQueries = [panelQuery('q1')]; + openFileMenu(app); + click(item(/New workspace/)!); + click(document.querySelector('.fm-dialog-confirm')!); + await flush(); + expect(toast()).toBe('✕ nope'); + expect(app.state.savedQueries.map((q) => q.id)).toEqual(['q1']); + }); +}); diff --git a/tests/unit/import-planner.test.ts b/tests/unit/import-planner.test.ts new file mode 100644 index 00000000..5fbbde9b --- /dev/null +++ b/tests/unit/import-planner.test.ts @@ -0,0 +1,428 @@ +import { describe, expect, it } from 'vitest'; +import { + autoResolveConflicts, buildQueryIdMapping, detectQueryConflicts, listBundleDashboards, + planImportDashboard, planImportQueries, planReplaceWorkspace, rewriteDashboardReferences, +} from '../../src/workspace/import-planner.js'; +import type { + IdMapping, QueryConflict, QueryDecision, +} from '../../src/workspace/import-planner.js'; +import type { + DashboardDocumentV1, PortableBundleV1, SavedQueryV2, StoredWorkspaceV1, +} from '../../src/generated/json-schema.types.js'; + +// --- fixtures ---------------------------------------------------------------- + +const panelQuery = (id: string, name = id): SavedQueryV2 => ({ + id, sql: 'SELECT 1', specVersion: 1, + spec: { name, panel: { cfg: { type: 'bar', x: 0, y: [1] } } }, +}); + +const filterQuery = (id: string, name = id): SavedQueryV2 => ({ + id, sql: 'SELECT 1', specVersion: 1, + spec: { name, dashboard: { role: 'filter' } }, +}); + +const dashboardDoc = (over: Partial = {}): DashboardDocumentV1 => ({ + documentVersion: 1, id: 'd1', title: 'D', revision: 1, + layout: { type: 'flow', version: 1, preset: 'full-width', items: {} }, + filters: [], tiles: [], ...over, +}); + +const workspace = (over: Partial = {}): StoredWorkspaceV1 => ({ + storageVersion: 1, id: 'w1', name: 'Workspace', queries: [], dashboard: null, ...over, +}); + +const bundle = (over: Partial = {}): PortableBundleV1 => ({ + format: 'altinity-sql-browser/portable-bundle', version: 1, + exportedAt: '2026-07-17T00:00:00.000Z', queries: [], dashboards: [], ...over, +}); + +// A deterministic counter ID generator for the tests. +const counter = (prefix = 'id') => { + let n = 0; + return () => `${prefix}-${++n}`; +}; + +const ids = (queries: readonly SavedQueryV2[]): string[] => queries.map((q) => q.id); + +// --- listBundleDashboards ----------------------------------------------------- + +describe('listBundleDashboards', () => { + it('preserves bundle.dashboards ARRAY ORDER (no re-sort)', () => { + const zeta = dashboardDoc({ + id: 'zeta', title: 'Zeta', tiles: [{ id: 't1', queryId: 'p1' }], filters: [], + }); + const alpha = dashboardDoc({ + id: 'alpha', title: 'Alpha', filters: [{ id: 'f1', parameter: 'p' }], + }); + const summaries = listBundleDashboards(bundle({ dashboards: [zeta, alpha] })); + expect(summaries).toEqual([ + { id: 'zeta', title: 'Zeta', tileCount: 1, filterCount: 0 }, + { id: 'alpha', title: 'Alpha', tileCount: 0, filterCount: 1 }, + ]); + }); + + it('returns an empty summary list for an empty bundle', () => { + expect(listBundleDashboards(bundle())).toEqual([]); + }); +}); + +// --- detectQueryConflicts / autoResolveConflicts ------------------------------ + +describe('detectQueryConflicts', () => { + it('matches BY ID ONLY, and flags canonicalEqual per matched pair', () => { + const existing = [panelQuery('a'), panelQuery('b', 'B existing')]; + const incoming = [panelQuery('a'), panelQuery('b', 'B incoming'), panelQuery('c')]; + const conflicts = detectQueryConflicts(existing, incoming); + expect(conflicts).toHaveLength(2); + expect(conflicts[0]).toEqual({ + sourceId: 'a', existing: existing[0], incoming: incoming[0], canonicalEqual: true, + }); + expect(conflicts[1].sourceId).toBe('b'); + expect(conflicts[1].canonicalEqual).toBe(false); + }); + + it('reports no conflicts when no ids match', () => { + expect(detectQueryConflicts([panelQuery('a')], [panelQuery('b')])).toEqual([]); + }); +}); + +describe('autoResolveConflicts', () => { + it('auto-resolves ONLY canonically-equal conflicts to use-existing; non-equal ones are omitted', () => { + const conflicts: QueryConflict[] = [ + { sourceId: 'a', existing: panelQuery('a'), incoming: panelQuery('a'), canonicalEqual: true }, + { sourceId: 'b', existing: panelQuery('b'), incoming: panelQuery('b', 'different'), canonicalEqual: false }, + ]; + expect(autoResolveConflicts(conflicts)).toEqual([ + { sourceId: 'a', action: 'use-existing', targetId: 'a' }, + ]); + }); +}); + +// --- buildQueryIdMapping ------------------------------------------------------- + +describe('buildQueryIdMapping', () => { + it('keeps a non-conflicting incoming query under its own id (action copy)', () => { + const mapping = buildQueryIdMapping([panelQuery('x')], [], [], counter()); + expect(mapping).toEqual({ x: { targetId: 'x', action: 'copy' } }); + }); + + it('defaults an undecided conflict to skip', () => { + const mapping = buildQueryIdMapping([panelQuery('a')], [panelQuery('a')], [], counter()); + expect(mapping.a).toEqual({ targetId: null, action: 'skip' }); + }); + + it('honors an explicit skip decision on a conflict', () => { + const decisions: QueryDecision[] = [{ sourceId: 'a', action: 'skip' }]; + const mapping = buildQueryIdMapping([panelQuery('a')], [panelQuery('a')], decisions, counter()); + expect(mapping.a).toEqual({ targetId: null, action: 'skip' }); + }); + + it('honors use-existing and replace decisions under the shared conflict id', () => { + const decisions: QueryDecision[] = [ + { sourceId: 'a', action: 'use-existing' }, { sourceId: 'b', action: 'replace' }, + ]; + const mapping = buildQueryIdMapping( + [panelQuery('a'), panelQuery('b')], [panelQuery('a'), panelQuery('b')], decisions, counter(), + ); + expect(mapping.a).toEqual({ targetId: 'a', action: 'use-existing' }); + expect(mapping.b).toEqual({ targetId: 'b', action: 'replace' }); + }); + + it('honors a caller-preferred fresh copy targetId when it is free', () => { + const decisions: QueryDecision[] = [{ sourceId: 'a', action: 'copy', targetId: 'a-copy' }]; + const mapping = buildQueryIdMapping([panelQuery('a')], [panelQuery('a')], decisions, counter()); + expect(mapping.a).toEqual({ targetId: 'a-copy', action: 'copy' }); + }); + + it('mints a fresh id via genId when a copy decision omits targetId', () => { + const decisions: QueryDecision[] = [{ sourceId: 'a', action: 'copy' }]; + const mapping = buildQueryIdMapping([panelQuery('a')], [panelQuery('a')], decisions, counter('fresh')); + expect(mapping.a).toEqual({ targetId: 'fresh-1', action: 'copy' }); + }); + + it('falls back to genId when the requested copy targetId collides, retrying past collisions', () => { + const decisions: QueryDecision[] = [{ sourceId: 'a', action: 'copy', targetId: 'a' }]; // 'a' is taken (existing) + let calls = 0; + const genId = () => { calls += 1; return calls === 1 ? 'a' /* still taken */ : 'fresh-2'; }; + const mapping = buildQueryIdMapping([panelQuery('a')], [panelQuery('a')], decisions, genId); + expect(mapping.a).toEqual({ targetId: 'fresh-2', action: 'copy' }); + expect(calls).toBe(2); + }); + + it('never mints the same fresh id twice within one call, even if two decisions request the same free id', () => { + const decisions: QueryDecision[] = [ + { sourceId: 'a', action: 'copy', targetId: 'new' }, + { sourceId: 'b', action: 'copy', targetId: 'new' }, + ]; + const genId = counter('minted'); + const mapping = buildQueryIdMapping( + [panelQuery('a'), panelQuery('b')], [panelQuery('a'), panelQuery('b')], decisions, genId, + ); + expect(mapping.a).toEqual({ targetId: 'new', action: 'copy' }); + expect(mapping.b.targetId).not.toBe('new'); + expect(mapping.b.action).toBe('copy'); + }); + + it('throws when genId can never produce a free id within the retry budget', () => { + const decisions: QueryDecision[] = [{ sourceId: 'a', action: 'copy' }]; + expect(() => buildQueryIdMapping([panelQuery('a')], [panelQuery('a')], decisions, () => 'a')).toThrow(); + }); +}); + +// --- rewriteDashboardReferences ------------------------------------------------ + +describe('rewriteDashboardReferences', () => { + const dashboard = dashboardDoc({ + tiles: [{ id: 't1', queryId: 'p1' }], + filters: [{ id: 'flt1', parameter: 'p', sourceQueryId: 'f1' }, { id: 'flt2', parameter: 'q' }], + layout: { type: 'flow', version: 1, preset: 'full-width', items: { t1: {} } }, + }); + + it('rewrites BOTH tile.queryId and filter.sourceQueryId via an IdMapping', () => { + const mapping: IdMapping = { + p1: { targetId: 'p1-copy', action: 'copy' }, + f1: { targetId: 'f1-copy', action: 'copy' }, + }; + const result = rewriteDashboardReferences(dashboard, mapping); + expect(result.invalidated).toBe(false); + expect(result.missingRequiredIds).toEqual([]); + expect(result.dashboard.tiles[0].queryId).toBe('p1-copy'); + expect(result.dashboard.filters[0].sourceQueryId).toBe('f1-copy'); + expect(result.dashboard.filters[1].sourceQueryId).toBeUndefined(); // no sourceQueryId — untouched + }); + + it('rewrites via a plain Map too', () => { + const mapping = new Map([['p1', 'p2'], ['f1', 'f2']]); + const result = rewriteDashboardReferences(dashboard, mapping); + expect(result.invalidated).toBe(false); + expect(result.dashboard.tiles[0].queryId).toBe('p2'); + expect(result.dashboard.filters[0].sourceQueryId).toBe('f2'); + }); + + it('invalidates when a required reference maps to null (skipped)', () => { + const mapping: IdMapping = { + p1: { targetId: null, action: 'skip' }, + f1: { targetId: 'f1', action: 'use-existing' }, + }; + const result = rewriteDashboardReferences(dashboard, mapping); + expect(result.invalidated).toBe(true); + expect(result.missingRequiredIds).toEqual(['p1']); + // Never silently dropped — the original reference is retained. + expect(result.dashboard.tiles[0].queryId).toBe('p1'); + }); + + it('invalidates when a required reference has no mapping entry at all (Map variant, too)', () => { + const mapping = new Map([['f1', 'f1']]); // p1 absent + const result = rewriteDashboardReferences(dashboard, mapping); + expect(result.invalidated).toBe(true); + expect(result.missingRequiredIds).toEqual(['p1']); + }); + + it('invalidates when a required reference has no IdMapping (Record) entry at all', () => { + const mapping: IdMapping = { f1: { targetId: 'f1', action: 'use-existing' } }; // p1 absent + const result = rewriteDashboardReferences(dashboard, mapping); + expect(result.invalidated).toBe(true); + expect(result.missingRequiredIds).toEqual(['p1']); + }); + + it('never mutates the input dashboard (deep clone)', () => { + const mapping: IdMapping = { p1: { targetId: 'p2', action: 'copy' }, f1: { targetId: 'f1', action: 'use-existing' } }; + const result = rewriteDashboardReferences(dashboard, mapping); + result.dashboard.tiles[0].queryId = 'mutated'; + expect(dashboard.tiles[0].queryId).toBe('p1'); + }); +}); + +// --- planImportQueries --------------------------------------------------------- + +describe('planImportQueries', () => { + it('imports a non-conflicting query and leaves the Dashboard byte-for-byte unchanged', () => { + const dash = dashboardDoc(); + const ws = workspace({ queries: [panelQuery('a')], dashboard: dash }); + const plan = planImportQueries(ws, bundle({ queries: [panelQuery('b')] }), [], counter()); + expect(plan.candidateWorkspace).not.toBeNull(); + expect(ids(plan.candidateWorkspace!.queries)).toEqual(['a', 'b']); + expect(plan.candidateWorkspace!.dashboard).toBe(dash); + expect(plan.diagnostics).toEqual([]); + expect(plan.sourceDashboardId).toBeUndefined(); + }); + + it('overwrites the existing entry in place on a replace decision', () => { + const ws = workspace({ queries: [panelQuery('a', 'old name')] }); + const decisions: QueryDecision[] = [{ sourceId: 'a', action: 'replace' }]; + const plan = planImportQueries(ws, bundle({ queries: [panelQuery('a', 'new name')] }), decisions, counter()); + expect(ids(plan.candidateWorkspace!.queries)).toEqual(['a']); + expect(plan.candidateWorkspace!.queries[0].spec.name).toBe('new name'); + }); + + it('allows skip on a conflicting query with no Dashboard dependency (queries-only skip is fine)', () => { + const ws = workspace({ queries: [panelQuery('a')] }); + const decisions: QueryDecision[] = [{ sourceId: 'a', action: 'skip' }]; + const plan = planImportQueries(ws, bundle({ queries: [panelQuery('a', 'incoming')] }), decisions, counter()); + expect(plan.candidateWorkspace).not.toBeNull(); + expect(ids(plan.candidateWorkspace!.queries)).toEqual(['a']); + expect(plan.candidateWorkspace!.queries[0].spec.name).toBe('a'); // existing content retained + }); + + it('use-existing keeps the existing entry and does not duplicate it', () => { + const ws = workspace({ queries: [panelQuery('a', 'existing')] }); + const decisions: QueryDecision[] = [{ sourceId: 'a', action: 'use-existing' }]; + const plan = planImportQueries(ws, bundle({ queries: [panelQuery('a', 'incoming')] }), decisions, counter()); + expect(ids(plan.candidateWorkspace!.queries)).toEqual(['a']); + expect(plan.candidateWorkspace!.queries[0].spec.name).toBe('existing'); + }); + + it('returns candidateWorkspace: null with sorted diagnostics when the candidate fails validation', () => { + const ws = workspace(); + const badQuery = { id: 'a', sql: 'SELECT 1', specVersion: 9, spec: {} } as unknown as SavedQueryV2; + const plan = planImportQueries(ws, bundle({ queries: [badQuery] }), [], counter()); + expect(plan.candidateWorkspace).toBeNull(); + expect(plan.diagnostics.length).toBeGreaterThan(0); + expect(plan.diagnostics.some((d) => d.code === 'spec-version-unsupported')).toBe(true); + }); +}); + +// --- planImportDashboard -------------------------------------------------------- + +describe('planImportDashboard', () => { + const buildBundle = () => bundle({ + queries: [panelQuery('p1', 'incoming p1'), filterQuery('f1', 'incoming f1')], + dashboards: [dashboardDoc({ + id: 'd1', revision: 5, + tiles: [{ id: 't1', queryId: 'p1' }], + filters: [{ id: 'flt1', parameter: 'p', sourceQueryId: 'f1' }], + layout: { type: 'flow', version: 1, preset: 'full-width', items: { t1: {} } }, + })], + }); + + it('copy mode rewrites BOTH tile.queryId and filter.sourceQueryId, mints a fresh Dashboard id, and resets revision to 1', () => { + const ws = workspace({ queries: [panelQuery('p1', 'existing p1'), filterQuery('f1', 'existing f1')] }); + const decisions: QueryDecision[] = [ + { sourceId: 'p1', action: 'copy', targetId: 'p1-copy' }, + { sourceId: 'f1', action: 'copy', targetId: 'f1-copy' }, + ]; + const genId = counter('new-dash'); + const plan = planImportDashboard(ws, buildBundle(), 'd1', decisions, 'copy', genId); + expect(plan.diagnostics).toEqual([]); + const candidate = plan.candidateWorkspace!; + expect(candidate.dashboard!.id).toBe('new-dash-1'); + expect(candidate.dashboard!.revision).toBe(1); + expect(candidate.dashboard!.tiles[0].queryId).toBe('p1-copy'); + expect(candidate.dashboard!.filters[0].sourceQueryId).toBe('f1-copy'); + // existing catalog entries keep their position; new copies are appended. + expect(ids(candidate.queries)).toEqual(['p1', 'f1', 'p1-copy', 'f1-copy']); + expect(plan.sourceDashboardId).toBe('d1'); + }); + + it('replace mode keeps the imported Dashboard id and revision', () => { + const ws = workspace(); // no existing queries — both incoming ids are non-conflicting + const plan = planImportDashboard(ws, buildBundle(), 'd1', [], 'replace', counter()); + const candidate = plan.candidateWorkspace!; + expect(candidate.dashboard!.id).toBe('d1'); + expect(candidate.dashboard!.revision).toBe(5); + expect(candidate.dashboard!.tiles[0].queryId).toBe('p1'); + expect(candidate.dashboard!.filters[0].sourceQueryId).toBe('f1'); + }); + + it('invalidates when skipping a required Dashboard dependency (candidate null, missingRequiredIds populated)', () => { + const ws = workspace({ queries: [panelQuery('p1', 'existing p1'), filterQuery('f1', 'existing f1')] }); + const decisions: QueryDecision[] = [ + { sourceId: 'p1', action: 'skip' }, { sourceId: 'f1', action: 'use-existing' }, + ]; + const plan = planImportDashboard(ws, buildBundle(), 'd1', decisions, 'copy', counter()); + expect(plan.candidateWorkspace).toBeNull(); + expect(plan.diagnostics).toHaveLength(1); + expect(plan.diagnostics[0].code).toBe('dashboard-import-invalid'); + expect(plan.diagnostics[0].message).toContain('p1'); + }); + + it('reports a not-found diagnostic for an unknown sourceDashboardId', () => { + const plan = planImportDashboard(workspace(), buildBundle(), 'missing', [], 'copy', counter()); + expect(plan.candidateWorkspace).toBeNull(); + expect(plan.queryMappings).toEqual({}); + expect(plan.diagnostics).toEqual([ + { path: ['dashboards'], severity: 'error', code: 'import-dashboard-not-found', resource: 'missing', message: 'Bundle contains no dashboard with id "missing"' }, + ]); + expect(plan.sourceDashboardId).toBe('missing'); + }); + + it('returns candidateWorkspace: null with sorted diagnostics when the rewritten candidate still fails validation', () => { + const ws = workspace(); + const badBundle = bundle({ + queries: [panelQuery('p1')], + dashboards: [dashboardDoc({ + id: 'd1', + tiles: [{ id: 't1', queryId: 'p1' }], + // 'ghost' names no tile — layout-orphan-placement, unrelated to query mapping. + layout: { type: 'flow', version: 1, preset: 'full-width', items: { t1: {}, ghost: {} } }, + })], + }); + const plan = planImportDashboard(ws, badBundle, 'd1', [], 'replace', counter()); + expect(plan.candidateWorkspace).toBeNull(); + expect(plan.diagnostics.some((d) => d.code === 'layout-orphan-placement')).toBe(true); + }); +}); + +// --- planReplaceWorkspace -------------------------------------------------------- + +describe('planReplaceWorkspace', () => { + it('preserves workspace identity and replaces the query catalog wholesale (dropping unreferenced existing queries)', () => { + const ws = workspace({ + id: 'w1', name: 'Mine', queries: [panelQuery('p1', 'existing p1'), panelQuery('old', 'unreferenced')], + }); + const decisions: QueryDecision[] = [{ sourceId: 'p1', action: 'use-existing' }]; + const plan = planReplaceWorkspace( + ws, bundle({ queries: [panelQuery('p1', 'bundle p1'), panelQuery('p2')] }), undefined, decisions, counter(), + ); + const candidate = plan.candidateWorkspace!; + expect(candidate.id).toBe('w1'); + expect(candidate.name).toBe('Mine'); + expect(ids(candidate.queries)).toEqual(['p1', 'p2']); // 'old' dropped + expect(candidate.queries[0].spec.name).toBe('existing p1'); // use-existing kept existing content + expect(candidate.dashboard).toBeNull(); + expect(plan.sourceDashboardId).toBeUndefined(); + }); + + it('replaces queries AND Dashboard atomically when a source Dashboard is selected, including standalone queries', () => { + const ws = workspace(); + const bundleWithDashboard = bundle({ + queries: [panelQuery('p1'), filterQuery('f1'), panelQuery('standalone')], + dashboards: [dashboardDoc({ + id: 'd1', revision: 2, + tiles: [{ id: 't1', queryId: 'p1' }], + filters: [{ id: 'flt1', parameter: 'p', sourceQueryId: 'f1' }], + layout: { type: 'flow', version: 1, preset: 'full-width', items: { t1: {} } }, + })], + }); + const plan = planReplaceWorkspace(ws, bundleWithDashboard, 'd1', [], counter()); + const candidate = plan.candidateWorkspace!; + expect(ids(candidate.queries)).toEqual(['p1', 'f1', 'standalone']); + expect(candidate.dashboard!.id).toBe('d1'); + expect(candidate.dashboard!.revision).toBe(2); + expect(plan.sourceDashboardId).toBe('d1'); + }); + + it('reports a not-found diagnostic for an unknown sourceDashboardId', () => { + const plan = planReplaceWorkspace(workspace(), bundle({ queries: [panelQuery('p1')] }), 'missing', [], counter()); + expect(plan.candidateWorkspace).toBeNull(); + expect(plan.diagnostics[0].code).toBe('import-dashboard-not-found'); + }); + + it('invalidates when a required Dashboard dependency is skipped', () => { + const ws = workspace({ queries: [panelQuery('p1', 'existing')] }); + const bundleWithDashboard = bundle({ + queries: [panelQuery('p1', 'incoming')], + dashboards: [dashboardDoc({ + id: 'd1', + tiles: [{ id: 't1', queryId: 'p1' }], + layout: { type: 'flow', version: 1, preset: 'full-width', items: { t1: {} } }, + })], + }); + const decisions: QueryDecision[] = [{ sourceId: 'p1', action: 'skip' }]; + const plan = planReplaceWorkspace(ws, bundleWithDashboard, 'd1', decisions, counter()); + expect(plan.candidateWorkspace).toBeNull(); + expect(plan.diagnostics[0].code).toBe('dashboard-import-invalid'); + expect(plan.diagnostics[0].message).toContain('p1'); + }); +}); diff --git a/tests/unit/legacy-bundle.test.ts b/tests/unit/legacy-bundle.test.ts new file mode 100644 index 00000000..0ff9b586 --- /dev/null +++ b/tests/unit/legacy-bundle.test.ts @@ -0,0 +1,112 @@ +import { describe, expect, it } from 'vitest'; +import { normalizeLegacyLibraryToBundle } from '../../src/dashboard/model/legacy-bundle.js'; +import { LIBRARY_FORMAT } from '../../src/core/library-codec.js'; +import { PORTABLE_BUNDLE_FORMAT, PORTABLE_BUNDLE_V1_SCHEMA_ID } from '../../src/dashboard/model/portable-bundle-codec.js'; + +const query = (id = 'q', spec: Record = {}): Record => + ({ id, sql: 'SELECT 1', specVersion: 1, spec }); + +const libraryV2 = (queries: unknown[] = [query()], over: Record = {}): Record => ({ + format: LIBRARY_FORMAT, version: 2, exportedAt: '2026-07-14T00:00:00.000Z', queries, ...over, +}); + +describe('normalizeLegacyLibraryToBundle', () => { + it('normalizes a valid Library v2 document into a bundle with dashboards:[] and preserved queries', () => { + const source = libraryV2([query('q1', { extension: { nested: [1] } })]); + const result = normalizeLegacyLibraryToBundle(JSON.stringify(source)); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.value.format).toBe(PORTABLE_BUNDLE_FORMAT); + expect(result.value.$schema).toBe(PORTABLE_BUNDLE_V1_SCHEMA_ID); + expect(result.value.version).toBe(1); + expect(result.value.exportedAt).toBe('2026-07-14T00:00:00.000Z'); + expect(result.value.dashboards).toEqual([]); + expect(result.value.queries).toHaveLength(1); + expect(result.value.queries[0].id).toBe('q1'); + // Deep-cloned, not aliased to the source document. + expect(result.value.queries).not.toBe(source.queries); + }); + + it('migrates a legacy Library v1 document, minting ids and falling back to the injected clock', () => { + const source = { + format: LIBRARY_FORMAT, version: 1, queries: [ + { id: 'old', name: 'Old', sql: '1', chart: { cfg: { type: 'pie', x: 0, y: [1] } } }, + { sql: '2' }, + ], + }; + const result = normalizeLegacyLibraryToBundle(JSON.stringify(source), { + nowISO: '2026-07-18T00:00:00.000Z', generateId: (index: number) => `g${index}`, + }); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.value.exportedAt).toBe('2026-07-18T00:00:00.000Z'); + expect(result.value.dashboards).toEqual([]); + expect(result.value.queries.map((item) => item.id)).toEqual(['old', 'g1']); + }); + + it('rejects malformed JSON without producing a partial bundle', () => { + const result = normalizeLegacyLibraryToBundle('{not json'); + expect(result).toEqual({ + ok: false, + diagnostics: [{ path: [], severity: 'error', code: 'json-syntax', message: 'Not a valid JSON file' }], + }); + }); + + it('rejects a Library document that fails codec decoding (e.g. unsupported version) with mapped diagnostics', () => { + const source = { format: LIBRARY_FORMAT, version: 9, queries: [] }; + const result = normalizeLegacyLibraryToBundle(JSON.stringify(source)); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.diagnostics).toEqual([ + { path: ['version'], severity: 'error', code: 'library-version-unsupported', message: 'Unsupported Library version 9' }, + ]); + }); + + it('rejects input over the byte limit before parsing, with no partial import', () => { + const source = libraryV2([query('q1')]); + const text = JSON.stringify(source); + const result = normalizeLegacyLibraryToBundle(text, { maxBytes: 4 }); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.diagnostics).toEqual([ + { path: [], severity: 'error', code: 'limit-json-bytes', message: expect.stringContaining('maximum is 4') }, + ]); + }); + + it('rejects input over the depth limit before parsing', () => { + const nested = '['.repeat(70) + ']'.repeat(70); + const result = normalizeLegacyLibraryToBundle(nested, { maxDepth: 64 }); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.diagnostics[0].code).toBe('limit-json-depth'); + }); + + it('rejects a decoded Library whose wrapped bundle fails portable-bundle validation, without partial import', () => { + // A v2 document with no `exportedAt` of its own, decoded with no injected + // `nowISO` fallback, produces a structurally-valid decoded Library but an + // exportedAt-less wrapped bundle — the final re-validation against + // PortableBundleV1 (which requires exportedAt) must still catch this, + // proving the wrap step is genuinely re-validated rather than trusted. + const source = libraryV2([query('q1')]); + delete source.exportedAt; + const result = normalizeLegacyLibraryToBundle(JSON.stringify(source)); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.diagnostics.some((item) => item.path[0] === 'exportedAt')).toBe(true); + }); + + it('passes the validation service through to both the Library decode and bundle re-validation steps', () => { + const calls: string[] = []; + const validationService = { + validate: (schemaId: string, value: unknown) => { + calls.push(schemaId); + return []; + }, + getSchema: () => ({}), + }; + const source = libraryV2([query('q1')]); + const result = normalizeLegacyLibraryToBundle(JSON.stringify(source), { validationService }); + expect(result.ok).toBe(true); + expect(calls.length).toBeGreaterThan(0); + }); +}); diff --git a/tests/unit/main.test.ts b/tests/unit/main.test.ts index 456c442d..888bad9a 100644 --- a/tests/unit/main.test.ts +++ b/tests/unit/main.test.ts @@ -54,6 +54,11 @@ function fakeApp(over: Partial> & { conn?: Partial null), ...rest, } as FakeApp; return self; diff --git a/tests/unit/query-document-session.test.ts b/tests/unit/query-document-session.test.ts index 0035c9fd..a18f7297 100644 --- a/tests/unit/query-document-session.test.ts +++ b/tests/unit/query-document-session.test.ts @@ -275,17 +275,22 @@ describe('resolveEditorMode', () => { expect(gate).toEqual({ ok: false, message: 'Save this query to create an editable Spec.' }); }); - it('allows entering spec mode once the tab is linked to a saved query', () => { + it('allows entering spec mode once the tab is linked to a saved query', async () => { const tab = newTabObj('t1'); tab.sqlDraft = 'SELECT 1'; const state = { tabs: signal([tab]), savedQueries: [] as AppState['savedQueries'], resultView: signal<'table' | 'json' | 'panel' | 'filter'>('table'), libraryDirty: signal(false), + libraryName: signal('Lib'), workspaceId: 'w1', dashboard: null as AppState['dashboard'], }; // Link the tab via the real state.ts create path — the exact invariant - // `savedForTab` (this session's own dependency) reads. - const entry = createSavedQuery(state, tab, 'My query', '', vi.fn(), 0); - expect(entry).not.toBeNull(); + // `savedForTab` (this session's own dependency) reads. #287 W4: the + // aggregate commit is a trivial always-succeeds echo — this test only + // needs a real linked entry, not real persistence semantics. + const result = await createSavedQuery(state, tab, 'My query', '', async (candidate) => ({ + ok: true, workspace: candidate, dashboardRevision: null, + }), 0); + expect(result.ok).toBe(true); const { deps } = makeDeps({ tab, state }); const gate = createQueryDocumentSession(deps).resolveEditorMode(tab, 'spec'); expect(gate).toEqual({ ok: true }); diff --git a/tests/unit/saved-history.test.ts b/tests/unit/saved-history.test.ts index fcd8efcf..b285802d 100644 --- a/tests/unit/saved-history.test.ts +++ b/tests/unit/saved-history.test.ts @@ -5,13 +5,18 @@ import { queryDescription, queryFavorite, queryName } from '../../src/core/saved import { makeApp } from '../helpers/fake-app.js'; import { savedQuery } from '../helpers/saved-query.js'; import type { SavedQueryFixture } from '../helpers/saved-query.js'; -import { setTabSpecDraft } from '../../src/state.js'; +import { setTabSpecDraft, toggleFavorite, deleteSaved } from '../../src/state.js'; import type { App } from '../../src/ui/app.types.js'; import type { AppState, HistoryEntry } from '../../src/state.js'; type ResultView = AppState['resultView']['value']; const click = (el: Element) => el.dispatchEvent(new Event('click', { bubbles: true })); +// #287 W4: toggleFavorite/renameSaved/deleteSaved's onclick handlers are now +// async (they await the aggregate commit before mutating state/re-rendering) +// — a macrotask flush lets every pending microtask (the commit promise chain) +// settle before a test's post-click assertions run. +const flush = (): Promise => new Promise((resolve) => setTimeout(resolve, 0)); const setSaved = (app: App, queries: SavedQueryFixture[]) => { app.state.savedQueries = queries.map((q) => savedQuery(q)); }; @@ -51,7 +56,7 @@ describe('renderSavedHistory', () => { expect(savedList(app).textContent).toContain('No saved queries yet.'); }); - it('saved: lists rows, loads on click, deletes via trash + refreshes Save button', () => { + it('saved: lists rows, loads on click, deletes via trash + refreshes Save button', async () => { const app = makeApp(); app.state.sidePanel.value = 'saved'; const panel = { cfg: { type: 'pie', x: 0, y: [1], series: null }, key: 'k' }; @@ -66,6 +71,7 @@ describe('renderSavedHistory', () => { expect(app.actions.loadIntoNewTab).toHaveBeenCalledWith(app.state.savedQueries[0]); expect(app.actions.run).toHaveBeenCalledWith({ view: 'panel' }); byTitle(row, 'Delete').dispatchEvent(new Event('click', { bubbles: true })); + await flush(); expect(app.state.savedQueries).toHaveLength(0); expect(app.updateSaveBtn).toHaveBeenCalled(); expect(app.updateEditorModeUi).toHaveBeenCalled(); @@ -163,7 +169,7 @@ describe('renderSavedHistory', () => { expect(app.state.resultView.value).toBe('filter'); // role wins, not the dormant 'panel' }); - it('saved: live count + star toggles favorite and re-sorts favorites first', () => { + it('saved: live count + star toggles favorite and re-sorts favorites first', async () => { const app = makeApp(); app.state.sidePanel.value = 'saved'; setSaved(app, [ @@ -176,12 +182,13 @@ describe('renderSavedHistory', () => { expect(names()).toEqual(['A', 'B']); const stars = qsa(savedList(app), '.sv-star'); stars[1].dispatchEvent(new Event('click', { bubbles: true })); // favorite B + await flush(); expect(queryFavorite(app.state.savedQueries.find((q) => q.id === 'b'))).toBe(true); expect(names()).toEqual(['B', 'A']); expect(app.queryDoc.revalidateSpecDrafts).toHaveBeenCalled(); }); - it('saved: favorite merges into a linked dirty valid Spec draft', () => { + it('saved: favorite merges into a linked dirty valid Spec draft', async () => { const app = makeApp(); app.state.sidePanel.value = 'saved'; setSaved(app, [{ id: 's1', name: 'A', sql: '1', favorite: false }]); @@ -190,10 +197,10 @@ describe('renderSavedHistory', () => { setTabSpecDraft(tab, { name: 'Draft', favorite: false, future: { keep: true } }, { dirty: true }); renderSavedHistory(app); click(qs(savedList(app), '.sv-star')); + await flush(); expect(queryFavorite(app.state.savedQueries[0])).toBe(true); expect(tab.specParsed).toMatchObject({ name: 'Draft', favorite: true, future: { keep: true } }); expect(tab.dirtySpec).toBe(true); - expect(app.saveJSON).toHaveBeenCalledTimes(1); }); it('saved: pencil focuses an invalid linked Spec draft instead of opening', () => { @@ -212,7 +219,7 @@ describe('renderSavedHistory', () => { expect(app.activateInvalidSpecDraft).toHaveBeenCalledWith(tab); }); - it('saved: favorite blocks on invalid JSON without persistence', () => { + it('saved: favorite blocks on invalid JSON without persistence', async () => { const app = makeApp(); app.state.sidePanel.value = 'saved'; setSaved(app, [{ id: 's1', name: 'A', sql: '1', favorite: false }]); @@ -224,12 +231,59 @@ describe('renderSavedHistory', () => { tab.dirtySpec = true; renderSavedHistory(app); click(qs(savedList(app), '.sv-star')); + await flush(); expect(queryFavorite(app.state.savedQueries[0])).toBe(false); expect(app.activateInvalidSpecDraft).toHaveBeenCalledWith(tab); - expect(app.saveJSON).not.toHaveBeenCalled(); }); - it('saved: pencil opens the edit form; Name(Enter)+Description commit via renameSaved; double-fire is guarded', () => { + const failingCommit = () => vi.fn(async () => ({ + ok: false as const, + diagnostics: [{ path: [], severity: 'error' as const, code: 'test-fail', message: 'boom' }], + })); + + it('#287 W4: star surfaces a toast (and mutates nothing) when the aggregate commit is rejected', async () => { + const commit = failingCommit(); + const app = makeApp({ workspace: { commit } }); + app.state.sidePanel.value = 'saved'; + setSaved(app, [{ id: 's1', name: 'A', sql: '1', favorite: false }]); + renderSavedHistory(app); + click(qs(savedList(app), '.sv-star')); + await flush(); + expect(queryFavorite(app.state.savedQueries[0])).toBe(false); + expect(qs(document, '.share-toast').textContent).toBe('Couldn’t update favorite: boom'); + }); + + it('#287 W4: delete surfaces a toast (and mutates nothing) when the aggregate commit is rejected', async () => { + const commit = failingCommit(); + const app = makeApp({ workspace: { commit } }); + app.state.sidePanel.value = 'saved'; + setSaved(app, [{ id: 's1', name: 'A', sql: '1', favorite: false }]); + app.activeTab().savedId = 's1'; + renderSavedHistory(app); + byTitle(qs(savedList(app), '.saved-row'), 'Delete').dispatchEvent(new Event('click', { bubbles: true })); + await flush(); + expect(app.state.savedQueries).toHaveLength(1); + expect(app.activeTab().savedId).toBe('s1'); + expect(app.updateSaveBtn).not.toHaveBeenCalled(); + expect(qs(document, '.share-toast').textContent).toBe('Couldn’t delete: boom'); + }); + + it('#287 W4: rename surfaces a toast (and mutates nothing) when the aggregate commit is rejected', async () => { + const commit = failingCommit(); + const app = makeApp({ workspace: { commit } }); + app.state.sidePanel.value = 'saved'; + setSaved(app, [{ id: 's1', name: 'Old', sql: '1', favorite: false }]); + renderSavedHistory(app); + byTitle(savedList(app), 'Edit name & description').dispatchEvent(new Event('click', { bubbles: true })); + qs(savedList(app), '.sv-edit-name').value = 'New'; + qs(savedList(app), '.sv-edit-name') + .dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true })); + await flush(); + expect(queryName(app.state.savedQueries[0])).toBe('Old'); + expect(qs(document, '.share-toast').textContent).toBe('Couldn’t rename: boom'); + }); + + it('saved: pencil opens the edit form; Name(Enter)+Description commit via renameSaved; double-fire is guarded', async () => { const app = makeApp(); app.state.sidePanel.value = 'saved'; setSaved(app, [{ id: 's1', name: 'Old', sql: '1', favorite: false }]); @@ -243,6 +297,7 @@ describe('renderSavedHistory', () => { nameInput.value = 'New'; descInput.value = 'a description'; nameInput.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true })); + await flush(); expect(app.state.savedQueries[0].spec).toMatchObject({ name: 'New', description: 'a description' }); expect(app.state.editingSavedId.value).toBeNull(); expect(app.actions.rerenderTabs).toHaveBeenCalled(); @@ -250,6 +305,7 @@ describe('renderSavedHistory', () => { // a second commit on the now-detached field is a no-op (the `done` guard) nameInput.value = 'AGAIN'; nameInput.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true })); + await flush(); expect(queryName(app.state.savedQueries[0])).toBe('New'); // re-open and press Escape on the name field → cancels without saving byTitle(savedList(app), 'Edit name & description').dispatchEvent(new Event('click', { bubbles: true })); @@ -259,7 +315,7 @@ describe('renderSavedHistory', () => { expect(app.state.editingSavedId.value).toBeNull(); expect(queryName(app.state.savedQueries[0])).toBe('New'); }); - it('saved: edit form — description prefilled; ⌘/Ctrl+Enter + Save commit, Escape/Cancel + empty name revert', () => { + it('saved: edit form — description prefilled; ⌘/Ctrl+Enter + Save commit, Escape/Cancel + empty name revert', async () => { const app = makeApp(); app.state.sidePanel.value = 'saved'; setSaved(app, [{ id: 's1', name: 'Old', sql: '1', favorite: false, description: 'd0' }]); @@ -271,12 +327,14 @@ describe('renderSavedHistory', () => { expect(descInput.value).toBe('d0'); descInput.value = 'd1'; descInput.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', metaKey: true, bubbles: true })); + await flush(); expect(queryDescription(app.state.savedQueries[0])).toBe('d1'); // Ctrl+Enter also commits open(); descInput = qs(savedList(app), '.sv-edit-desc'); descInput.value = 'd2'; descInput.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', ctrlKey: true, bubbles: true })); + await flush(); expect(queryDescription(app.state.savedQueries[0])).toBe('d2'); // Escape on the description cancels without saving open(); @@ -310,13 +368,13 @@ describe('renderSavedHistory', () => { expect(rows[1].querySelector('.desc')).toBeNull(); }); - it('saved: the tab is labelled "Library" with a live count and no Export/Import row', () => { + it('saved: the tab is labelled "Queries" with a live count and no Export/Import row', () => { const app = makeApp(); app.state.sidePanel.value = 'saved'; setSaved(app, [{ id: 's1', name: 'A', sql: '1', favorite: false }]); renderSavedHistory(app); const savedTab = qsa(savedTabsRow(app), '.side-tab')[0]; - expect(savedTab.textContent).toContain('Library'); + expect(savedTab.textContent).toContain('Queries'); expect(savedTab.textContent).not.toContain('Saved'); expect(qs(savedTab, '.side-count').textContent).toContain('1'); // the old bottom Export/Import row is gone (moved to the header File menu) @@ -499,3 +557,21 @@ describe('drag a row into the editor', () => { expect(setData).toHaveBeenCalledWith(SUBQUERY_MIME, 'SELECT 2'); }); }); + +describe('concurrent saved-query writes (#287 review fix)', () => { + it('serializes overlapping ops so a delete is not resurrected by a stale toggle', async () => { + const app = makeApp(); + setSaved(app, [ + { id: 'q1', name: 'Q1', sql: 'SELECT 1', favorite: false }, + { id: 'q2', name: 'Q2', sql: 'SELECT 2' }, + ]); + // 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 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 + expect(queryFavorite(app.state.savedQueries[0])).toBe(true); // q1 toggle applied + }); +}); diff --git a/tests/unit/saved-io.test.ts b/tests/unit/saved-io.test.ts index 59855cec..d7eb7142 100644 --- a/tests/unit/saved-io.test.ts +++ b/tests/unit/saved-io.test.ts @@ -1,54 +1,12 @@ import { describe, it, expect } from 'vitest'; -import { - buildExportDoc, parseImportDoc, mergeSaved, buildMarkdownDoc, buildSqlDoc, upgradeSavedEntry, -} from '../../src/core/saved-io.js'; +import { parseImportDoc, buildMarkdownDoc, buildSqlDoc, upgradeSavedEntry } from '../../src/core/saved-io.js'; const FORMAT = 'altinity-sql-browser/saved-queries'; -const SCHEMA = 'https://altinity.com/schemas/altinity-sql-browser/library-v2.schema.json'; -const NOW = '2026-07-13T00:00:00.000Z'; const v2 = (id: string, sql: string, spec: Record = {}) => ({ id, sql, specVersion: 1, spec }); const envelope = (version: number, queries: unknown[], over: Record = {}) => JSON.stringify({ format: FORMAT, version, queries, ...over }); const CHART = { cfg: { type: 'pie', x: 0, y: [1], series: null }, key: 'k' }; -// `buildExportDoc` returns a bare `Record` (the generic export -// envelope shape) — this module's own tests read a query's canonical fields -// back off it, so a single, local, honestly-loose shape (not `any`) narrows -// `doc.queries` for every assertion below. -interface ExportedQuery { - id: string; - sql: string; - specVersion: number; - spec: Record; -} -const queriesOf = (doc: Record): ExportedQuery[] => doc.queries as ExportedQuery[]; - -describe('buildExportDoc', () => { - it('writes only the canonical v2 envelope and preserves the complete Spec', () => { - const extension = { nested: [{ x: 1 }] }; - const doc = buildExportDoc([v2('s1', 'SELECT 1', { - name: 'A', favorite: true, panel: { cfg: { type: 'table' }, fieldConfig: { defaults: {} } }, extension, - })], '2026-07-13T00:00:00.000Z'); - expect(doc).toEqual({ - $schema: SCHEMA, format: FORMAT, version: 2, exportedAt: NOW, - queries: [v2('s1', 'SELECT 1', { - name: 'A', favorite: true, panel: { cfg: { type: 'table' }, fieldConfig: { defaults: {} } }, extension, - })], - }); - expect(queriesOf(doc)[0].spec.extension).not.toBe(extension); - expect('name' in queriesOf(doc)[0]).toBe(false); - expect('panel' in queriesOf(doc)[0]).toBe(false); - expect('chart' in queriesOf(doc)[0]).toBe(false); - }); - - it('upgrades defensive v1 input on export and handles an empty list', () => { - const doc = buildExportDoc([{ id: 'old', name: 'Old', sql: '1', chart: CHART }], NOW); - expect(doc.version).toBe(2); - expect(queriesOf(doc)[0]).toEqual(v2('old', '1', { name: 'Old', favorite: false, panel: CHART })); - expect(queriesOf(buildExportDoc([], NOW))).toEqual([]); - }); -}); - describe('parseImportDoc — v1 migration', () => { it('upgrades supported flat entries, defaults a missing name, and skips malformed rows', () => { const { queries } = parseImportDoc(envelope(1, [ @@ -152,57 +110,10 @@ describe('parseImportDoc — v2 validation', () => { }); }); -describe('mergeSaved', () => { - const generator = () => { let n = 0; return () => 'gen' + (++n); }; - - it('adds, updates by id, generates unique ids, and skips complete-Spec duplicates', () => { - const existing = [v2('s1', '1', { name: 'A', favorite: false, extension: { b: 2, a: 1 } })]; - const incoming = [ - v2('different-id', '1', { extension: { a: 1, b: 2 }, favorite: false, name: 'A' }), - v2('s1', '1b', { name: 'A2', favorite: true, extension: { incoming: [1] } }), - { name: 'B', sql: '2' }, - v2('s2', '3', { name: 'C', favorite: false }), - ]; - const result = mergeSaved(existing, incoming, generator()); - expect(result).toMatchObject({ added: 2, updated: 1, skipped: 1 }); - expect(result.merged.map((q) => q.id)).toEqual(['s1', 'gen1', 's2']); - expect(result.merged[0].spec).toEqual({ name: 'A2', favorite: true, extension: { incoming: [1] } }); - expect(existing[0].spec.name).toBe('A'); - }); - - it('replaces the complete incoming Spec by id, including extension removal/addition', () => { - const existing = [v2('s1', '1', { name: 'A', oldExtension: { keep: false } })]; - const incoming = [v2('s1', '1', { name: 'A', newExtension: { nested: [1, 2] } })]; - const result = mergeSaved(existing, incoming, () => 'unused'); - expect(result.updated).toBe(1); - expect(result.merged[0].spec).toEqual(incoming[0].spec); - expect(result.merged[0].spec).not.toBe(incoming[0].spec); - }); - - it('upgrades v1 input, preserves array-order semantics, and avoids duplicate incoming ids', () => { - const existing = [v2('taken', 'x', { name: 'X', list: [1, 2] })]; - const incoming = [ - { id: 'old', name: 'Old', sql: '1', chart: CHART }, - v2('taken', 'x', { name: 'X', list: [2, 1] }), - v2('old', '2', { name: 'Other' }), - ]; - const result = mergeSaved(existing, incoming, generator()); - expect(result).toMatchObject({ added: 1, updated: 2, skipped: 0 }); - expect(result.merged.find((q) => q.id === 'old')!.sql).toBe('2'); +describe('upgradeSavedEntry', () => { + it('re-exports upgradeSavedQuery unchanged (a v1 flat entry upgrades to canonical v2)', () => { expect(upgradeSavedEntry({ name: 'Alias', sql: 'a' }).spec.name).toBe('Alias'); }); - - it('dedups a v1 null-key chart against its live key-less twin (no spurious duplicate)', () => { - // Live editing stores a null schema key by OMITTING it: {cfg}. A v1 file - // (or legacy share) carrying the same chart with an absent/null key must - // upgrade to the same key-less shape, so a different-id import is skipped - // as an exact duplicate rather than added as a second row. - const live = v2('live1', 'SELECT 1', { name: 'Chart Q', favorite: false, panel: { cfg: { type: 'bar', x: 'x', y: 'y' } } }); - const v1file = { id: 'other', name: 'Chart Q', sql: 'SELECT 1', favorite: false, chart: { cfg: { type: 'bar', x: 'x', y: 'y' } } }; - const result = mergeSaved([live], [v1file], () => 'unused'); - expect(result).toMatchObject({ added: 0, updated: 0, skipped: 1 }); - expect(result.merged).toHaveLength(1); - }); }); describe('one-way Markdown/SQL exports', () => { diff --git a/tests/unit/saved-query-service.test.ts b/tests/unit/saved-query-service.test.ts index 7fdb9850..a566ff33 100644 --- a/tests/unit/saved-query-service.test.ts +++ b/tests/unit/saved-query-service.test.ts @@ -8,6 +8,7 @@ import type { SpecValidationDiagnostic } from '../../src/core/spec-draft.js'; import { encodeShare, decodeShare } from '../../src/core/share.js'; import { withQuerySpec } from '../../src/core/saved-query.js'; import type { SavedQueryV2 } from '../../src/generated/json-schema.types.js'; +import { fakeWorkspaceCommit } from '../helpers/fake-app.js'; // SavedQueryService (#276 Phase 4C) — the saved-query create/commit policy, // history recording, and share-URL building extracted from app.ts, unit- @@ -20,7 +21,8 @@ import type { SavedQueryV2 } from '../../src/generated/json-schema.types.js'; // ── Fakes ──────────────────────────────────────────────────────────────────── -type StateSlice = Pick; +type StateSlice = Pick; function makeState(over: Partial = {}): StateSlice { return { @@ -28,6 +30,9 @@ function makeState(over: Partial = {}): StateSlice { resultView: over.resultView ?? signal<'table' | 'json' | 'panel' | 'filter'>('table'), libraryDirty: over.libraryDirty ?? signal(false), history: over.history ?? [], + libraryName: over.libraryName ?? signal('Lib'), + workspaceId: over.workspaceId ?? 'w1', + dashboard: over.dashboard ?? null, }; } @@ -41,16 +46,22 @@ function makeDeps(over: { saveJSON?: ReturnType; now?: () => number; specValidators?: SpecValidationService; -} = {}): { deps: SavedQueryServiceDeps; state: StateSlice; saveJSON: ReturnType } { + workspace?: ReturnType; +} = {}): { + deps: SavedQueryServiceDeps; state: StateSlice; saveJSON: ReturnType; + commit: ReturnType; +} { const state = over.state || makeState(); const saveJSON = over.saveJSON || vi.fn(); + const commit = over.workspace || fakeWorkspaceCommit(); const deps: SavedQueryServiceDeps = { state, saveJSON: saveJSON as unknown as SaveJSON, now: over.now || (() => 1700000000000), specValidators: over.specValidators || ALWAYS_VALID, + workspace: { commit }, }; - return { deps, state, saveJSON }; + return { deps, state, saveJSON, commit }; } const validEvaluated = (parsed: unknown = { name: 'Q', favorite: false }): { parsed: unknown; diagnostics: SpecValidationDiagnostic[] } => @@ -59,59 +70,71 @@ const validEvaluated = (parsed: unknown = { name: 'Q', favorite: false }): { par // ── create ─────────────────────────────────────────────────────────────────── describe('create', () => { - it('creates and persists a new saved query from an unsaved tab', () => { + it('creates and persists (via the aggregate commit) a new saved query from an unsaved tab', async () => { const tab = newTabObj('t1'); tab.sqlDraft = 'SELECT 1'; - const { deps, state, saveJSON } = makeDeps(); - const result = createSavedQueryService(deps).create(tab, 'My query', 'a desc'); + const { deps, state, commit } = makeDeps(); + const result = await createSavedQueryService(deps).create(tab, 'My query', 'a desc'); expect(result.ok).toBe(true); if (!result.ok) throw new Error('unreachable'); expect(result.entry).toMatchObject({ sql: 'SELECT 1', spec: { name: 'My query', description: 'a desc' } }); expect(state.savedQueries).toEqual([result.entry]); expect(tab.savedId).toBe(result.entry.id); expect(state.libraryDirty.value).toBe(true); - expect(saveJSON).toHaveBeenCalledWith(KEYS.saved, state.savedQueries); + expect(commit).toHaveBeenCalledTimes(1); expect(result.entry.id.startsWith('s1700000000000')).toBe(true); // deps.now() feeds the minted id }); - it('rejects (no persistence) a tab already linked to a saved query', () => { + it('rejects (no persistence) a tab already linked to a saved query', async () => { const tab = newTabObj('t1'); tab.sqlDraft = 'SELECT 1'; tab.savedId = 'already-linked'; - const { deps, state, saveJSON } = makeDeps(); - const result = createSavedQueryService(deps).create(tab, 'My query', ''); - expect(result).toEqual({ ok: false }); + const { deps, state, commit } = makeDeps(); + const result = await createSavedQueryService(deps).create(tab, 'My query', ''); + expect(result).toEqual({ ok: false, diagnostics: undefined }); expect(state.savedQueries).toEqual([]); - expect(saveJSON).not.toHaveBeenCalled(); + expect(commit).not.toHaveBeenCalled(); }); - it('rejects (no persistence) blank SQL on a non-text-panel tab', () => { + it('rejects (no persistence) blank SQL on a non-text-panel tab', async () => { const tab = newTabObj('t1'); tab.sqlDraft = ' '; - const { deps, state, saveJSON } = makeDeps(); - const result = createSavedQueryService(deps).create(tab, 'My query', ''); - expect(result).toEqual({ ok: false }); + const { deps, state, commit } = makeDeps(); + const result = await createSavedQueryService(deps).create(tab, 'My query', ''); + expect(result).toEqual({ ok: false, diagnostics: undefined }); expect(state.savedQueries).toEqual([]); - expect(saveJSON).not.toHaveBeenCalled(); + expect(commit).not.toHaveBeenCalled(); }); - it('rejects (no persistence) a blank name', () => { + it('rejects (no persistence) a blank name', async () => { const tab = newTabObj('t1'); tab.sqlDraft = 'SELECT 1'; const { deps, state } = makeDeps(); - const result = createSavedQueryService(deps).create(tab, ' ', ''); - expect(result).toEqual({ ok: false }); + const result = await createSavedQueryService(deps).create(tab, ' ', ''); + expect(result).toEqual({ ok: false, diagnostics: undefined }); + expect(state.savedQueries).toEqual([]); + }); + + it('rejects (no persistence) when the injected specValidators blocks the resulting Spec', async () => { + const tab = newTabObj('t1'); + tab.sqlDraft = 'SELECT 1'; + const { deps, state, commit } = makeDeps({ specValidators: ALWAYS_BLOCKING }); + const result = await createSavedQueryService(deps).create(tab, 'My query', ''); + expect(result).toEqual({ ok: false, diagnostics: undefined }); expect(state.savedQueries).toEqual([]); + expect(commit).not.toHaveBeenCalled(); }); - it('rejects (no persistence) when the injected specValidators blocks the resulting Spec', () => { + it('rejects with the aggregate\'s diagnostics when the whole-workspace commit itself is rejected', async () => { const tab = newTabObj('t1'); tab.sqlDraft = 'SELECT 1'; - const { deps, state, saveJSON } = makeDeps({ specValidators: ALWAYS_BLOCKING }); - const result = createSavedQueryService(deps).create(tab, 'My query', ''); - expect(result).toEqual({ ok: false }); + const diagnostics = [{ path: [], severity: 'error' as const, code: 'test-fail', message: 'boom' }]; + const failingCommit = vi.fn(async () => ({ ok: false as const, diagnostics })); + const { deps, state } = makeDeps({ workspace: failingCommit }); + const result = await createSavedQueryService(deps).create(tab, 'My query', ''); + expect(result).toEqual({ ok: false, diagnostics }); expect(state.savedQueries).toEqual([]); - expect(saveJSON).not.toHaveBeenCalled(); + expect(tab.savedId).toBeNull(); }); }); @@ -124,82 +147,94 @@ describe('commit', () => { return makeState({ savedQueries: [entry] }); } - it('rejects with invalid-spec when the evaluated Spec has no parsed draft', () => { + it('rejects with invalid-spec when the evaluated Spec has no parsed draft', async () => { const tab = newTabObj('t1'); const state = linkedState(tab); - const { deps, saveJSON } = makeDeps({ state }); - const result = createSavedQueryService(deps).commit(tab, { parsed: null, diagnostics: [] }); + const { deps, commit } = makeDeps({ state }); + const result = await createSavedQueryService(deps).commit(tab, { parsed: null, diagnostics: [] }); expect(result).toEqual({ ok: false, reason: 'invalid-spec' }); - expect(saveJSON).not.toHaveBeenCalled(); + expect(commit).not.toHaveBeenCalled(); }); - it('rejects with invalid-spec when the evaluated Spec carries a blocking diagnostic', () => { + it('rejects with invalid-spec when the evaluated Spec carries a blocking diagnostic', async () => { const tab = newTabObj('t1'); const state = linkedState(tab); - const { deps, saveJSON } = makeDeps({ state }); + const { deps, commit } = makeDeps({ state }); const evaluated = { parsed: { name: 'Q', favorite: false }, diagnostics: [{ path: [], severity: 'error' as const, code: 'x', message: 'bad' }], }; - const result = createSavedQueryService(deps).commit(tab, evaluated); + const result = await createSavedQueryService(deps).commit(tab, evaluated); expect(result).toEqual({ ok: false, reason: 'invalid-spec' }); - expect(saveJSON).not.toHaveBeenCalled(); + expect(commit).not.toHaveBeenCalled(); }); - it('rejects with empty when SQL is blank on a non-queryless panel', () => { + it('rejects with empty when SQL is blank on a non-queryless panel', async () => { const tab = newTabObj('t1'); tab.sqlDraft = ' '; const state = linkedState(tab, ''); - const { deps, saveJSON } = makeDeps({ state }); - const result = createSavedQueryService(deps).commit(tab, validEvaluated()); + const { deps, commit } = makeDeps({ state }); + const result = await createSavedQueryService(deps).commit(tab, validEvaluated()); expect(result).toEqual({ ok: false, reason: 'empty' }); - expect(saveJSON).not.toHaveBeenCalled(); + expect(commit).not.toHaveBeenCalled(); }); - it('accepts blank SQL on a queryless (text) panel', () => { + it('accepts blank SQL on a queryless (text) panel', async () => { const tab = newTabObj('t1'); tab.sqlDraft = ''; const state = linkedState(tab, ''); - const { deps, saveJSON, state: s } = makeDeps({ state }); + const { deps, commit } = makeDeps({ state }); const evaluated = validEvaluated({ name: 'Q', favorite: false, panel: { cfg: { type: 'text', content: 'hi' } } }); - const result = createSavedQueryService(deps).commit(tab, evaluated); + const result = await createSavedQueryService(deps).commit(tab, evaluated); expect(result.ok).toBe(true); - expect(saveJSON).toHaveBeenCalledWith(KEYS.saved, s.savedQueries); + expect(commit).toHaveBeenCalledTimes(1); }); - it('rejects with rejected when commitSavedQuery itself declines (tab no longer linked)', () => { + it('rejects with rejected when commitSavedQuery itself declines (tab no longer linked)', async () => { const tab = newTabObj('t1'); tab.sqlDraft = 'SELECT 1'; tab.savedId = 'does-not-exist'; // linked id absent from savedQueries → index < 0 - const { deps, saveJSON } = makeDeps(); - const result = createSavedQueryService(deps).commit(tab, validEvaluated()); - expect(result).toEqual({ ok: false, reason: 'rejected' }); - expect(saveJSON).not.toHaveBeenCalled(); + const { deps, commit } = makeDeps(); + const result = await createSavedQueryService(deps).commit(tab, validEvaluated()); + expect(result).toEqual({ ok: false, reason: 'rejected', diagnostics: undefined }); + expect(commit).not.toHaveBeenCalled(); + }); + + it('rejects with rejected when the injected specValidators blocks the normalized Spec even though the input evaluation was clean', async () => { + const tab = newTabObj('t1'); + tab.sqlDraft = 'SELECT 1'; + const state = linkedState(tab); + const { deps, commit } = makeDeps({ state, specValidators: ALWAYS_BLOCKING }); + const result = await createSavedQueryService(deps).commit(tab, validEvaluated()); + expect(result).toEqual({ ok: false, reason: 'rejected', diagnostics: undefined }); + expect(commit).not.toHaveBeenCalled(); }); - it('rejects with rejected when the injected specValidators blocks the normalized Spec even though the input evaluation was clean', () => { + it('rejects with rejected + diagnostics when the whole-workspace commit itself is rejected', async () => { const tab = newTabObj('t1'); tab.sqlDraft = 'SELECT 1'; const state = linkedState(tab); - const { deps, saveJSON } = makeDeps({ state, specValidators: ALWAYS_BLOCKING }); - const result = createSavedQueryService(deps).commit(tab, validEvaluated()); - expect(result).toEqual({ ok: false, reason: 'rejected' }); - expect(saveJSON).not.toHaveBeenCalled(); + const diagnostics = [{ path: [], severity: 'error' as const, code: 'test-fail', message: 'boom' }]; + const failingCommit = vi.fn(async () => ({ ok: false as const, diagnostics })); + const { deps } = makeDeps({ state, workspace: failingCommit }); + const result = await createSavedQueryService(deps).commit(tab, validEvaluated()); + expect(result).toEqual({ ok: false, reason: 'rejected', diagnostics }); + expect(state.savedQueries[0]).toMatchObject({ id: 's1', sql: 'SELECT 1' }); // unchanged }); - it('commits and persists an update to the linked saved query', () => { + it('commits and persists an update to the linked saved query', async () => { const tab = newTabObj('t1'); tab.sqlDraft = 'SELECT 2'; const state = linkedState(tab); - const { deps, saveJSON, state: s } = makeDeps({ state }); + const { deps, commit, state: s } = makeDeps({ state }); const evaluated = validEvaluated({ name: 'Renamed', favorite: true }); - const result = createSavedQueryService(deps).commit(tab, evaluated); + const result = await createSavedQueryService(deps).commit(tab, evaluated); expect(result.ok).toBe(true); if (!result.ok) throw new Error('unreachable'); expect(result.entry).toMatchObject({ id: 's1', sql: 'SELECT 2', spec: { name: 'Renamed', favorite: true } }); expect(s.savedQueries[0]).toEqual(result.entry); expect(s.libraryDirty.value).toBe(true); - expect(saveJSON).toHaveBeenCalledWith(KEYS.saved, s.savedQueries); + expect(commit).toHaveBeenCalledTimes(1); }); }); @@ -306,11 +341,11 @@ describe('buildShareUrl', () => { // depends on) still resolves against the narrowed slice this service itself // persists into — proves the two extractions share one consistent state shape. describe('cross-check with savedForTab', () => { - it('a committed query is resolvable via savedForTab against the same state', () => { + it('a committed query is resolvable via savedForTab against the same state', async () => { const tab = newTabObj('t1'); tab.sqlDraft = 'SELECT 1'; const { deps, state } = makeDeps(); - const result = createSavedQueryService(deps).create(tab, 'Q', ''); + const result = await createSavedQueryService(deps).create(tab, 'Q', ''); expect(result.ok).toBe(true); expect(savedForTab(state, tab)).toEqual(result.ok ? result.entry : null); }); diff --git a/tests/unit/state.test.ts b/tests/unit/state.test.ts index f93e06e9..ce686c21 100644 --- a/tests/unit/state.test.ts +++ b/tests/unit/state.test.ts @@ -2,14 +2,16 @@ import { describe, it, expect, vi, afterEach } from 'vitest'; import { KEYS, DEFAULT_LIBRARY_NAME, newTabObj, createState, activeTab, allocTabId, effectiveFilterActive, createSavedQuery, commitSavedQuery, savedForTab, renameSaved, toggleFavorite, - sortedSaved, filterSaved, filterHistory, importSaved, deleteSaved, recordHistory, + sortedSaved, filterSaved, filterHistory, deleteSaved, recordHistory, recordScriptHistory, clearHistory, deleteHistory, tabPanel, setTabSpecDraft, patchSpecDraft, tabDirty, - renameLibrary, newLibrary, replaceLibrary, appendLibrary, markLibrarySaved, } from '../../src/state.js'; -import type { StateReader, HistoryResultSnapshot, HistoryEntry, QueryTab, SpecValidationService } from '../../src/state.js'; +import type { + StateReader, HistoryResultSnapshot, HistoryEntry, QueryTab, SpecValidationService, AppState, SavedEntryResult, +} 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 { fakeWorkspaceCommit } from '../helpers/fake-app.js'; afterEach(() => vi.unstubAllGlobals()); @@ -23,6 +25,25 @@ const reader = (over: Record = {}): StateReader => ({ loadJSON: (k, dflt) => (k in over ? over[k] : dflt), }); +// #287 W4: the saved-query CRUD ops now commit through the StoredWorkspaceV1 +// aggregate, whose schema requires a non-empty workspace id — every test +// below that exercises them builds its state through this helper (instead of +// bare `createState(reader())`) so the candidate a real `fakeWorkspaceCommit()` +// validates always has one. +function savedTestState(over: Record = {}): AppState { + const s = createState(reader(over)); + s.workspaceId = 'w1'; + return s; +} + +/** 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. */ +function okEntry(r: SavedEntryResult): SavedQueryV2 { + if (!r.ok) throw new Error('expected an ok SavedEntryResult, got: ' + JSON.stringify(r)); + return r.entry; +} + // tests/helpers/saved-query.js is plain JS with no field annotations; TS can // only infer a parameter type for the fields carrying their own default // value (favorite/name/spec/sql) off the outer `= {}` default, so calling it @@ -87,6 +108,15 @@ describe('createState', () => { expect(s.expanded.value.size).toBe(0); expect(s.libraryName.value).toBe(DEFAULT_LIBRARY_NAME); expect(s.libraryDirty.value).toBe(false); + // #287 W4: no aggregate loaded yet — `dashboard` starts null; + // `loadWorkspaceOnBoot` (app.ts's async boot step) projects the real + // aggregate onto both after this synchronous constructor. `workspaceId` + // is minted synchronously (never blank — the stored-workspace schema + // requires a non-empty id) so a save attempted before boot projection + // completes still succeeds; `loadWorkspaceOnBoot` overwrites it with the + // real committed id once resolved. + expect(s.dashboard).toBeNull(); + expect(s.workspaceId).toMatch(/^ws-/); expect(s.dashLayout).toBe('arrange'); expect(s.dashCols).toBe(3); expect(s.varValues).toEqual({}); @@ -166,35 +196,35 @@ describe('activeTab / allocTabId', () => { }); describe('saved queries', () => { - it('createSavedQuery is a no-op for empty SQL or empty name', () => { - const s = createState(reader()); - const save = vi.fn(); + it('createSavedQuery is a no-op for empty SQL or empty name', async () => { + const s = savedTestState(); + const commit = fakeWorkspaceCommit(); s.tabs.value[0].sqlDraft = ''; - expect(createSavedQuery(s, s.tabs.value[0], 'name', '', save)).toBeNull(); + expect(await createSavedQuery(s, s.tabs.value[0], 'name', '', commit)).toEqual({ ok: false, entry: null }); s.tabs.value[0].sqlDraft = 'SELECT 1'; - expect(createSavedQuery(s, s.tabs.value[0], ' ', '', save)).toBeNull(); - expect(save).not.toHaveBeenCalled(); + expect(await createSavedQuery(s, s.tabs.value[0], ' ', '', commit)).toEqual({ ok: false, entry: null }); + expect(commit).not.toHaveBeenCalled(); }); - it('creates an unsaved query, then atomically commits linked SQL + authoritative Spec', () => { - const s = createState(reader()); - const save = vi.fn(); + it('creates an unsaved query, then atomically commits linked SQL + authoritative Spec', async () => { + const s = savedTestState(); + const commit = fakeWorkspaceCommit(); const tab = s.tabs.value[0]; tab.sqlDraft = 'SELECT 1'; - // `!`: this Save path is guaranteed non-null here (linked-tab creation with - // a non-empty name and sql, asserted just below) — same invariant state.ts - // itself documents at its own `!` sites. - const e1 = createSavedQuery(s, tab, 'My query', '', save, 100)!; + // This Save path is guaranteed `ok: true` here (linked-tab creation with a + // non-empty name and sql, asserted just below) — same invariant state.ts + // itself documents at its own `!` sites, now via `okEntry`'s explicit throw. + const e1 = okEntry(await createSavedQuery(s, tab, 'My query', '', commit, 100)); expect(e1).toEqual(expect.objectContaining({ sql: 'SELECT 1', specVersion: 1 })); expect(e1.spec).toMatchObject({ name: 'My query', favorite: false }); expect(tab.savedId).toBe(e1.id); expect(tab.name).toBe('My query'); expect(s.savedQueries).toHaveLength(1); - expect(save).toHaveBeenLastCalledWith(KEYS.saved, s.savedQueries); + expect(commit).toHaveBeenCalledTimes(1); // Linked Save bypasses popover fields and commits the two drafts directly. tab.sqlDraft = 'SELECT 2'; tab.specParsed!.name = 'My query v2'; tab.dirtySql = true; tab.dirtySpec = true; - const e2 = commitSavedQuery(s, tab, tab.specParsed, save)!; + const e2 = okEntry(await commitSavedQuery(s, tab, tab.specParsed, commit)); expect(e2.id).toBe(e1.id); expect(s.savedQueries).toHaveLength(1); expect(s.savedQueries[0].sql).toBe('SELECT 2'); @@ -202,22 +232,22 @@ describe('saved queries', () => { expect(tab.name).toBe('My query v2'); expect(tabDirty(tab)).toBe(false); }); - it('creation stores a description and linked commits normalize/clear it', () => { - const s = createState(reader()); - const save = vi.fn(); + it('creation stores a description and linked commits normalize/clear it', async () => { + const s = savedTestState(); + const commit = fakeWorkspaceCommit(); const tab = s.tabs.value[0]; tab.sqlDraft = 'SELECT 1'; - const e = createSavedQuery(s, tab, 'Q', ' what it does ', save, 100); // trimmed + const e = okEntry(await createSavedQuery(s, tab, 'Q', ' what it does ', commit, 100)); // trimmed expect(queryDescription(e)).toBe('what it does'); tab.specParsed!.description = ' changed '; - commitSavedQuery(s, tab, tab.specParsed, save); + await commitSavedQuery(s, tab, tab.specParsed, commit); expect(queryDescription(s.savedQueries[0])).toBe('changed'); tab.specParsed!.description = ' '; - commitSavedQuery(s, tab, tab.specParsed, save); + await commitSavedQuery(s, tab, tab.specParsed, commit); expect('description' in s.savedQueries[0].spec).toBe(false); // create with no description arg → no description field const t2 = newTabObj('t2'); t2.sqlDraft = 'SELECT 2'; s.tabs.value.push(t2); - const e2 = createSavedQuery(s, t2, 'Q2', undefined, save, 400)!; + const e2 = okEntry(await createSavedQuery(s, t2, 'Q2', undefined, commit, 400)); expect('description' in e2.spec).toBe(false); }); it('savedForTab resolves the linked entry (or null)', () => { @@ -230,36 +260,36 @@ describe('saved queries', () => { expect(savedForTab(s, s.tabs.value[0])).toBeNull(); expect(savedForTab(s, { savedId: null })).toBeNull(); }); - it('renameSaved updates the entry + any linked tab name', () => { - const s = createState(reader()); + it('renameSaved updates the entry + any linked tab name', async () => { + const s = savedTestState(); s.savedQueries = [savedQuery({ id: 's1', sql: 'x', name: 'old' })]; s.tabs.value[0].savedId = 's1'; - const save = vi.fn(); - renameSaved(s, 's1', ' new ', undefined, save); + const commit = fakeWorkspaceCommit(); + await renameSaved(s, 's1', ' new ', undefined, commit); expect(queryName(s.savedQueries[0])).toBe('new'); expect(s.tabs.value[0].name).toBe('new'); - renameSaved(s, 's1', ' ', undefined, save); // blank ignored + await renameSaved(s, 's1', ' ', undefined, commit); // blank ignored expect(queryName(s.savedQueries[0])).toBe('new'); - renameSaved(s, 'missing', 'x', undefined, save); // unknown id ignored - expect(save).toHaveBeenCalledTimes(1); + await renameSaved(s, 'missing', 'x', undefined, commit); // unknown id ignored + expect(commit).toHaveBeenCalledTimes(1); }); - it('renameSaved sets/clears description when given, leaves it untouched when undefined', () => { - const s = createState(reader()); + it('renameSaved sets/clears description when given, leaves it untouched when undefined', async () => { + const s = savedTestState(); s.savedQueries = [savedQuery({ id: 's1', sql: 'x', name: 'A' })]; - const save = vi.fn(); - renameSaved(s, 's1', 'A', ' a note ', save); // set (trimmed) + const commit = fakeWorkspaceCommit(); + await renameSaved(s, 's1', 'A', ' a note ', commit); // set (trimmed) expect(queryDescription(s.savedQueries[0])).toBe('a note'); - renameSaved(s, 's1', 'A', undefined, save); // name-only → description kept + await renameSaved(s, 's1', 'A', undefined, commit); // name-only → description kept expect(queryDescription(s.savedQueries[0])).toBe('a note'); - renameSaved(s, 's1', 'A', '', save); // explicit empty → cleared + await renameSaved(s, 's1', 'A', '', commit); // explicit empty → cleared expect('description' in s.savedQueries[0].spec).toBe(false); - renameSaved(s, 's1', 'A', ' re ', save); // re-set + await renameSaved(s, 's1', 'A', ' re ', commit); // re-set expect(queryDescription(s.savedQueries[0])).toBe('re'); - renameSaved(s, 's1', 'A', null, save); // null (not undefined) → cleared, not stored as 'null' (#4 review) + await renameSaved(s, 's1', 'A', null, commit); // null (not undefined) → cleared, not stored as 'null' (#4 review) expect('description' in s.savedQueries[0].spec).toBe(false); }); - it('rename/description/favorite patches merge into valid linked drafts and persist once per action', () => { - const s = createState(reader()); + it('rename/description/favorite patches merge into valid linked drafts and persist once per action', async () => { + const s = savedTestState(); const original = savedQuery({ id: 's1', sql: 'x', name: 'Old', favorite: false, panel: { cfg: { type: 'table' }, fieldConfig: { defaults: { color: 'red' } } }, @@ -280,9 +310,9 @@ describe('saved queries', () => { second.savedId = 's1'; setTabSpecDraft(second, { ...original.spec, secondDraftOnly: ['keep'] }); s.tabs.value = [tab, second]; - const save = vi.fn(); - renameSaved(s, 's1', 'New', 'Description', save); - toggleFavorite(s, 's1', save); + const commit = fakeWorkspaceCommit(); + await renameSaved(s, 's1', 'New', 'Description', commit); + await toggleFavorite(s, 's1', commit); for (const spec of [s.savedQueries[0].spec, tab.specParsed]) { expect(spec).toMatchObject({ name: 'New', description: 'Description', favorite: true, @@ -298,26 +328,26 @@ describe('saved queries', () => { name: 'New', description: 'Description', favorite: true, secondDraftOnly: ['keep'], }); expect(second.dirtySpec).toBe(false); - expect(save).toHaveBeenCalledTimes(2); + expect(commit).toHaveBeenCalledTimes(2); expect(original.spec.name).toBe('Old'); expect((original.spec.extension as { nested: { value: number }[] }).nested[0].value).toBe(1); }); - it('toggleFavorite flips the flag; sortedSaved puts favorites first (stable)', () => { - const s = createState(reader()); + it('toggleFavorite flips the flag; sortedSaved puts favorites first (stable)', async () => { + const s = savedTestState(); s.savedQueries = [ savedQuery({ id: 'a', sql: '1', name: 'A' }), savedQuery({ id: 'b', sql: '2', name: 'B' }), savedQuery({ id: 'c', sql: '3', name: 'C' }), ]; - const save = vi.fn(); - toggleFavorite(s, 'c', save); + const commit = fakeWorkspaceCommit(); + await toggleFavorite(s, 'c', commit); expect(queryFavorite(s.savedQueries.find((q) => q.id === 'c'))).toBe(true); - toggleFavorite(s, 'missing', save); // no-op + await toggleFavorite(s, 'missing', commit); // no-op expect(sortedSaved(s).map((q) => q.id)).toEqual(['c', 'a', 'b']); - expect(save).toHaveBeenCalledTimes(1); + expect(commit).toHaveBeenCalledTimes(1); }); - it('invalid JSON blocks pencil/favorite persistence and identifies the affected tab', () => { - const s = createState(reader()); + it('invalid JSON blocks pencil/favorite persistence and identifies the affected tab', async () => { + const s = savedTestState(); const tab = s.tabs.value[0]; const entry = savedQuery({ id: 's1', name: 'Original', favorite: false, sql: 'SELECT 1' }); s.savedQueries = [entry]; @@ -326,24 +356,24 @@ describe('saved queries', () => { tab.specParsed = null; tab.specDiagnostics = [{ severity: 'error', code: 'invalid-json', message: 'invalid JSON' }]; tab.dirtySpec = true; - const save = vi.fn(); - expect(renameSaved(s, 's1', 'Overwrite', undefined, save)).toMatchObject({ ok: false, invalidTab: tab }); - expect(toggleFavorite(s, 's1', save)).toMatchObject({ ok: false, invalidTab: tab }); + 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(queryName(s.savedQueries[0])).toBe('Original'); expect(queryFavorite(s.savedQueries[0])).toBe(false); - expect(save).not.toHaveBeenCalled(); + expect(commit).not.toHaveBeenCalled(); }); - it('external writers validate the persisted entry and every linked draft before mutating', () => { - const s = createState(reader()); + it('external writers validate the persisted entry and every linked draft before mutating', async () => { + const s = savedTestState(); const tab = s.tabs.value[0]; s.savedQueries = [savedQuery({ id: 's1', name: 'Original', favorite: false, sql: 'SELECT 1' })]; tab.savedId = 's1'; setTabSpecDraft(tab, { ...s.savedQueries[0].spec, draftOnly: true }, { dirty: true }); - const save = vi.fn(); + const commit = fakeWorkspaceCommit(); const entryBlocked: SpecValidationService = { validate: () => [{ path: ['favorite'], severity: 'error', code: 'blocked', message: 'blocked' }], }; - expect(toggleFavorite(s, 's1', save, entryBlocked)).toMatchObject({ ok: false, invalidTab: null }); + expect(await toggleFavorite(s, 's1', commit, entryBlocked)).toMatchObject({ ok: false, invalidTab: null }); expect(queryFavorite(s.savedQueries[0])).toBe(false); const draftBlocked: SpecValidationService = { @@ -351,10 +381,46 @@ describe('saved queries', () => { ? [{ path: ['draftOnly'], severity: 'error', code: 'blocked-draft', message: 'blocked draft' }] : [], }; - expect(toggleFavorite(s, 's1', save, draftBlocked)).toMatchObject({ ok: false, invalidTab: tab }); + expect(await toggleFavorite(s, 's1', commit, draftBlocked)).toMatchObject({ ok: false, invalidTab: tab }); expect(queryFavorite(s.savedQueries[0])).toBe(false); expect(tab.specParsed!.favorite).toBe(false); - expect(save).not.toHaveBeenCalled(); + expect(commit).not.toHaveBeenCalled(); + }); + it('a rejected aggregate commit mutates nothing and surfaces diagnostics (#287 W4 strict commit)', async () => { + const s = savedTestState(); + const tab = s.tabs.value[0]; + tab.sqlDraft = 'SELECT 1'; + const diagnostics = [{ path: ['id'], severity: 'error' as const, code: 'test-fail', message: 'boom' }]; + const failingCommit = vi.fn(async () => ({ ok: false as const, diagnostics })); + const result = await createSavedQuery(s, tab, 'Q', '', failingCommit, 100); + expect(result).toEqual({ ok: false, entry: null, diagnostics }); + expect(s.savedQueries).toEqual([]); + expect(tab.savedId).toBeNull(); + expect(s.libraryDirty.value).toBe(false); + expect(failingCommit).toHaveBeenCalledTimes(1); + }); + it('a rejected aggregate commit leaves renameSaved/toggleFavorite/deleteSaved fully untouched', async () => { + const s = savedTestState(); + const tab = s.tabs.value[0]; + const entry = savedQuery({ id: 's1', name: 'Original', favorite: false, sql: 'SELECT 1' }); + s.savedQueries = [entry]; + tab.savedId = 's1'; + const diagnostics = [{ path: [], severity: 'error' as const, code: 'test-fail', message: 'nope' }]; + const failingCommit = async () => ({ ok: false as const, diagnostics }); + + const renamed = await renameSaved(s, 's1', 'New name', undefined, failingCommit); + 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); + expect(favorited).toMatchObject({ ok: false, invalidTab: null, entry: null }); + expect(queryFavorite(s.savedQueries[0])).toBe(false); + + const deleted = await deleteSaved(s, 's1', failingCommit); + expect(deleted).toEqual({ ok: false, diagnostics }); + expect(s.savedQueries).toHaveLength(1); + expect(tab.savedId).toBe('s1'); + expect(s.libraryDirty.value).toBe(false); }); it('patchSpecDraft handles object/function patches and reports a missing or invalid draft', () => { const tab = newTabObj('t1'); @@ -373,28 +439,28 @@ describe('saved queries', () => { expect(patchSpecDraft(tab, { name: 'Recovered' })).toMatchObject({ ok: true }); expect(tab.specParsed).toMatchObject({ name: 'Recovered' }); }); - it('an invalid linked Spec makes atomic Save persist nothing and retain both dirty flags', () => { - const s = createState(reader()); + it('an invalid linked Spec makes atomic Save persist nothing and retain both dirty flags', async () => { + const s = savedTestState(); const tab = s.tabs.value[0]; s.savedQueries = [savedQuery({ id: 's1', name: 'Q', sql: 'SELECT 1' })]; tab.savedId = 's1'; tab.sqlDraft = 'SELECT 2'; tab.dirtySql = true; tab.dirtySpec = true; - const save = vi.fn(); - expect(commitSavedQuery(s, tab, { name: ' ', extension: true }, save)).toBeNull(); + const commit = fakeWorkspaceCommit(); + expect(await commitSavedQuery(s, tab, { name: ' ', extension: true }, commit)).toEqual({ ok: false, entry: null }); expect(s.savedQueries[0].sql).toBe('SELECT 1'); expect(tabDirty(tab)).toBe(true); - expect(save).not.toHaveBeenCalled(); + expect(commit).not.toHaveBeenCalled(); }); - it("linked Save keeps the existing empty-SQL guard except for text panels", () => { - const s = createState(reader()); + it("linked Save keeps the existing empty-SQL guard except for text panels", async () => { + const s = savedTestState(); const tab = s.tabs.value[0]; s.savedQueries = [savedQuery({ id: 's1', name: 'Q', sql: 'SELECT 1' })]; tab.savedId = 's1'; tab.sqlDraft = ''; tab.dirtySql = true; - const save = vi.fn(); - expect(commitSavedQuery(s, tab, { name: 'Q', favorite: false }, save)).toBeNull(); + const commit = fakeWorkspaceCommit(); + expect(await commitSavedQuery(s, tab, { name: 'Q', favorite: false }, commit)).toEqual({ ok: false, entry: null }); expect(s.savedQueries[0].sql).toBe('SELECT 1'); - expect(save).not.toHaveBeenCalled(); + expect(commit).not.toHaveBeenCalled(); const textSpec = { name: 'Q', favorite: false, panel: { cfg: { type: 'text', content: 'note' } } }; - expect(commitSavedQuery(s, tab, textSpec, save)).not.toBeNull(); + expect((await commitSavedQuery(s, tab, textSpec, commit)).ok).toBe(true); expect(s.savedQueries[0].sql).toBe(''); }); it('filterSaved matches name/description/sql case-insensitively; blank → unchanged', () => { @@ -422,23 +488,6 @@ describe('saved queries', () => { expect(filterHistory(list, 'insert').map((h) => h.id)).toEqual(['h2']); expect(filterHistory(list, 'zzz')).toEqual([]); }); - it('importSaved merges (add/skip/update), persists, and uses injected genId', () => { - const s = createState(reader()); - s.savedQueries = [savedQuery({ id: 's1', name: 'A', sql: '1' })]; - const save = vi.fn(); - const r = importSaved(s, [ - { id: 's1', name: 'A', sql: '1' }, // skip (content dup) - { id: 's1', name: 'A2', sql: '1b' }, // update by id - { name: 'B', sql: '2' }, // add (genId) - ], save, () => 'gx'); - expect(r).toEqual({ added: 1, updated: 1, skipped: 1 }); - expect(s.savedQueries.map(queryName)).toEqual(['A2', 'B']); - expect(s.savedQueries.find((q) => queryName(q) === 'B')!.id).toBe('gx'); - expect(save).toHaveBeenCalledWith(KEYS.saved, s.savedQueries); - // default save + genId (no injection) — exercises the default id generator - importSaved(s, [{ name: 'Z', sql: 'zz' }]); - expect(s.savedQueries.find((q) => queryName(q) === 'Z')!.id).toMatch(/^s/); - }); it('tabPanel clones the complete tab-side panel, including future siblings', () => { expect(tabPanel(null)).toBeNull(); // A truthy object with no `specParsed` at all (a bare saved-query entry, @@ -454,42 +503,42 @@ describe('saved queries', () => { expect(panel).toEqual({ cfg, key: 'k', fieldConfig: { defaults: {} } }); expect(panel).not.toBe(tab.specParsed!.panel); }); - it('creation/commit persist the complete panel without a legacy mirror', () => { - const s = createState(reader()); - const save = vi.fn(); + it('creation/commit persist the complete panel without a legacy mirror', async () => { + const s = savedTestState(); + const commit = fakeWorkspaceCommit(); const tab = s.tabs.value[0]; tab.sqlDraft = 'SELECT a, b'; tab.specParsed!.panel = { cfg: { type: 'pie', x: 0, y: [1], series: null }, key: 'a:String|b:UInt64', fieldConfig: { defaults: { color: 'red' } }, }; - const e1 = createSavedQuery(s, tab, 'Chartd', '', save, 100)!; + const e1 = okEntry(await createSavedQuery(s, tab, 'Chartd', '', commit, 100)); expect(queryPanel(e1)).toEqual(tab.specParsed!.panel); expect(queryPanel(e1)).not.toBe(tab.specParsed!.panel); expect('chart' in e1).toBe(false); // re-save with a different cfg; future panel siblings remain. tab.specParsed!.panel!.cfg = { type: 'line', x: 0, y: [1], series: null }; - commitSavedQuery(s, tab, tab.specParsed, save); + await commitSavedQuery(s, tab, tab.specParsed, commit); expect(queryPanel(s.savedQueries[0])!.cfg!.type).toBe('line'); expect(queryPanel(s.savedQueries[0])!.fieldConfig!.defaults!.color).toBe('red'); tab.specParsed!.panel!.cfg = { type: 'logs' }; - commitSavedQuery(s, tab, tab.specParsed, save); + await commitSavedQuery(s, tab, tab.specParsed, commit); expect(queryPanel(s.savedQueries[0])!.cfg).toEqual({ type: 'logs' }); // re-save after the whole panel is cleared. delete tab.specParsed!.panel; - commitSavedQuery(s, tab, tab.specParsed, save); + await commitSavedQuery(s, tab, tab.specParsed, commit); expect(queryPanel(s.savedQueries[0])).toBeUndefined(); }); - it("createSavedQuery allows sql:'' for a text panel only (#166 per-type save guard)", () => { - const s = createState(reader()); - const save = vi.fn(); + it("createSavedQuery allows sql:'' for a text panel only (#166 per-type save guard)", async () => { + const s = savedTestState(); + const commit = fakeWorkspaceCommit(); const tab = s.tabs.value[0]; tab.sqlDraft = ''; - expect(createSavedQuery(s, tab, 'NoSql', '', save, 100)).toBeNull(); // no panel → still blocked + expect(await createSavedQuery(s, tab, 'NoSql', '', commit, 100)).toEqual({ ok: false, entry: null }); // no panel → still blocked tab.specParsed!.panel = { cfg: { type: 'table' } }; - expect(createSavedQuery(s, tab, 'NoSql', '', save, 150)).toBeNull(); // non-text panel → blocked + expect(await createSavedQuery(s, tab, 'NoSql', '', commit, 150)).toEqual({ ok: false, entry: null }); // non-text panel → blocked tab.specParsed!.panel = { cfg: { type: 'text', content: '# hello' } }; - const e = createSavedQuery(s, tab, 'Note', '', save, 200)!; + const e = okEntry(await createSavedQuery(s, tab, 'Note', '', commit, 200)); expect(e).not.toBeNull(); expect(e.sql).toBe(''); expect(queryPanel(e)!.cfg).toEqual({ type: 'text', content: '# hello' }); @@ -498,185 +547,32 @@ describe('saved queries', () => { // not an index-signature bag), hence the local intersection cast. expect((e as SavedQueryV2 & { chart?: unknown }).chart).toBeUndefined(); }); - it('creation captures the result view; linked Spec becomes authoritative afterward', () => { - const s = createState(reader()); - const save = vi.fn(); + it('creation captures the result view; linked Spec becomes authoritative afterward', async () => { + const s = savedTestState(); + const commit = fakeWorkspaceCommit(); const tab = s.tabs.value[0]; tab.sqlDraft = 'SELECT 1'; s.resultView.value = 'panel'; - const e = createSavedQuery(s, tab, 'V', '', save, 100); + const e = okEntry(await createSavedQuery(s, tab, 'V', '', commit, 100)); expect(queryView(e)).toBe('panel'); tab.specParsed!.view = 'json'; - commitSavedQuery(s, tab, tab.specParsed, save); + await commitSavedQuery(s, tab, tab.specParsed, commit); expect(queryView(s.savedQueries[0])).toBe('json'); // raw view (TSV/JSON output) is not a saved view → dropped delete tab.specParsed!.view; - commitSavedQuery(s, tab, tab.specParsed, save); + await commitSavedQuery(s, tab, tab.specParsed, commit); expect(queryView(s.savedQueries[0])).toBeUndefined(); }); - it('deleteSaved removes + clears tab pointers', () => { - const s = createState(reader()); + it('deleteSaved removes + clears tab pointers', async () => { + const s = savedTestState(); s.savedQueries = [savedQuery({ id: 's1', sql: 'x', name: 'n' })]; s.tabs.value[0].savedId = 's1'; - const save = vi.fn(); - deleteSaved(s, 's1', save); + const commit = fakeWorkspaceCommit(); + const result = await deleteSaved(s, 's1', commit); + expect(result).toEqual({ ok: true }); expect(s.savedQueries).toHaveLength(0); expect(s.tabs.value[0].savedId).toBeNull(); expect(s.tabs.value[0].editorMode).toBe('sql'); - expect(save).toHaveBeenCalledWith(KEYS.saved, []); - }); -}); - -describe('library document', () => { - it('dirty flag: saved-query mutations set it; markLibrarySaved clears it', () => { - const s = createState(reader()); - const tab = s.tabs.value[0]; tab.sqlDraft = 'SELECT 1'; - expect(s.libraryDirty.value).toBe(false); - createSavedQuery(s, tab, 'Q', '', vi.fn()); - expect(s.libraryDirty.value).toBe(true); - markLibrarySaved(s); - expect(s.libraryDirty.value).toBe(false); - toggleFavorite(s, tab.savedId!, vi.fn()); // favorite the just-saved entry - expect(s.libraryDirty.value).toBe(true); - markLibrarySaved(s); - renameSaved(s, tab.savedId!, 'Q2', undefined, vi.fn()); - expect(s.libraryDirty.value).toBe(true); - markLibrarySaved(s); - deleteSaved(s, tab.savedId!, vi.fn()); - expect(s.libraryDirty.value).toBe(true); - markLibrarySaved(s); - importSaved(s, [{ name: 'I', sql: 'i' }], vi.fn(), () => 'gi'); - expect(s.libraryDirty.value).toBe(true); - }); - - it('renameLibrary trims + persists + marks dirty; blank falls back to the default', () => { - const s = createState(reader()); - const saveName = vi.fn(); - renameLibrary(s, ' My queries ', saveName); - expect(s.libraryName.value).toBe('My queries'); - expect(s.libraryDirty.value).toBe(true); - expect(saveName).toHaveBeenCalledWith(KEYS.libraryName, 'My queries'); - renameLibrary(s, ' ', saveName); - expect(s.libraryName.value).toBe(DEFAULT_LIBRARY_NAME); - }); - - it('newLibrary clears queries + name, clears dirty, prunes dangling tab links', () => { - const s = createState(reader()); - s.savedQueries = [savedQuery({ id: 's1', name: 'A', sql: '1' })]; - s.libraryName.value = 'Old'; s.libraryDirty.value = true; - s.tabs.value[0].savedId = 's1'; // dangling after clear → pruned - s.tabs.value[0].editorMode = 'spec'; - s.tabs.value.push(newTabObj('t2')); // no savedId → skipped by prune - const save = vi.fn(), saveName = vi.fn(); - newLibrary(s, save, saveName); - expect(s.savedQueries).toEqual([]); - expect(s.libraryName.value).toBe(DEFAULT_LIBRARY_NAME); - expect(s.libraryDirty.value).toBe(false); - expect(s.tabs.value[0].savedId).toBeNull(); - expect(s.tabs.value[0].editorMode).toBe('sql'); - expect(save).toHaveBeenCalledWith(KEYS.saved, []); - expect(saveName).toHaveBeenCalledWith(KEYS.libraryName, DEFAULT_LIBRARY_NAME); - }); - - it('replaceLibrary keeps ids (mints for id-less), carries metadata, adopts the base name, clears dirty, prunes links', () => { - const s = createState(reader()); - s.savedQueries = [savedQuery({ id: 'old', name: 'X', sql: 'x' })]; - s.tabs.value[0].savedId = 'old'; // becomes dangling → pruned - s.tabs.value[0].editorMode = 'spec'; - s.libraryDirty.value = true; - const chart = { cfg: { type: 'bar', x: 0, y: [1], series: null }, key: 'k' }; - const incoming = [ - { id: 'keep', name: 'A', sql: '1', favorite: true, description: 'd', chart, view: 'json' }, - { name: 'B', sql: '2', favorite: false }, // id-less → genId - { id: 'txt', name: 'N', sql: '', favorite: false, panel: { cfg: { type: 'text', content: 'x' } } }, - ]; - let n = 0; - const save = vi.fn(), saveName = vi.fn(); - replaceLibrary(s, incoming, 'My Library.json', save, saveName, () => 'g' + (++n)); - expect(s.savedQueries.map((q) => q.id)).toEqual(['keep', 'g1', 'txt']); - expect(s.savedQueries[0].sql).toBe('1'); - expect(s.savedQueries[0].spec).toEqual({ - name: 'A', favorite: true, description: 'd', - panel: { cfg: chart.cfg, key: 'k' }, view: 'json', - }); - expect(queryPanel(s.savedQueries[2])).toEqual({ cfg: { type: 'text', content: 'x' } }); - expect('chart' in s.savedQueries[2]).toBe(false); - expect(s.libraryName.value).toBe('My Library'); // extension stripped - expect(s.libraryDirty.value).toBe(false); - expect(s.tabs.value[0].savedId).toBeNull(); - expect(s.tabs.value[0].editorMode).toBe('sql'); - expect(save).toHaveBeenCalledWith(KEYS.saved, s.savedQueries); - expect(saveName).toHaveBeenCalledWith(KEYS.libraryName, 'My Library'); - }); - - it('replaceLibrary mints fresh ids for duplicate incoming ids, keeping every id unique', () => { - const s = createState(reader()); - const incoming = [ - { id: 'dup', name: 'A', sql: '1' }, - { id: 'dup', name: 'B', sql: '2' }, // same id → must be reassigned - { id: 'uniq', name: 'C', sql: '3' }, - { name: 'D', sql: '4' }, // id-less → minted - ]; - // first mint collides with an already-seen id → the retry loop must skip it - let n = 0; - const genId = () => { n += 1; return n === 1 ? 'dup' : 'g' + n; }; - replaceLibrary(s, incoming, 'lib.json', vi.fn(), vi.fn(), genId); - const ids = s.savedQueries.map((q) => q.id); - expect(ids[0]).toBe('dup'); // first occurrence keeps its id - expect(ids[2]).toBe('uniq'); // unique id preserved - expect(ids).toHaveLength(4); - expect(new Set(ids).size).toBe(4); // all unique (no duplicate 'dup') - }); - - it('replaceLibrary with no usable file name falls back to the default', () => { - const s = createState(reader()); - replaceLibrary(s, [{ name: 'A', sql: '1' }], '.json', vi.fn(), vi.fn(), () => 'g'); - expect(s.libraryName.value).toBe(DEFAULT_LIBRARY_NAME); - }); - - it('Replace and Append reject invalid Specs before any Library mutation', () => { - const s = createState(reader()); - s.savedQueries = [savedQuery({ id: 'existing', name: 'Existing', sql: 'SELECT 1' })]; - s.libraryName.value = 'Before'; - s.libraryDirty.value = false; - const save = vi.fn(), saveName = vi.fn(); - // `replaceLibrary`/`appendLibrary` take `readonly Record[]` - // (any raw import shape); SavedQueryV2 is a strict interface with no index - // signature of its own, so a same-named-fields-plus-index-signature - // intersection cast (a genuine subtype, not an `unknown` bridge) satisfies it. - const invalid = [savedQuery({ id: 'bad', name: 'Bad', panel: { cfg: { type: 'line', x: 0, y: [] } } })] as - (SavedQueryV2 & Record)[]; - expect(() => replaceLibrary(s, invalid, 'after.json', save, saveName)).toThrow('panel.cfg.y'); - expect(() => appendLibrary(s, invalid, save)).toThrow('panel.cfg.y'); - expect(s.savedQueries.map((query) => query.id)).toEqual(['existing']); - expect(s.libraryName.value).toBe('Before'); - expect(s.libraryDirty.value).toBe(false); - expect(save).not.toHaveBeenCalled(); - expect(saveName).not.toHaveBeenCalled(); - }); - - it('appendLibrary merges via importSaved (dedupe), returns counts, sets dirty', () => { - const s = createState(reader()); - s.savedQueries = [savedQuery({ id: 's1', name: 'A', sql: '1' })]; - const r = appendLibrary(s, [ - { id: 's1', name: 'A', sql: '1' }, // content dup → skip - { name: 'B', sql: '2' }, // add - ], vi.fn(), () => 'gb'); - expect(r).toEqual({ added: 1, updated: 0, skipped: 1 }); - expect(s.savedQueries.map(queryName)).toEqual(['A', 'B']); - expect(s.libraryDirty.value).toBe(true); - }); - - it('library ops default their persistence seams (real storage helpers)', () => { - vi.stubGlobal('localStorage', memStore()); - const s = createState(reader()); - s.tabs.value[0].sqlDraft = 'SELECT 1'; - const e = createSavedQuery(s, s.tabs.value[0], 'Q')!; // default save/now/description - renameLibrary(s, 'Lib'); // default saveName - replaceLibrary(s, [{ id: e.id, name: 'Q', sql: 'SELECT 1' }], 'f.json'); // default seams - newLibrary(s); // default seams - appendLibrary(s, [{ name: 'Z', sql: 'z' }]); // default seam - expect(s.savedQueries.some((q) => queryName(q) === 'Z')).toBe(true); }); }); @@ -751,17 +647,27 @@ describe('history', () => { }); describe('default persistence', () => { - it('createSavedQuery/renameSaved/toggleFavorite/deleteSaved/recordHistory/clearHistory persist via storage by default', () => { + it('recordHistory/clearHistory/deleteHistory persist via storage by default', () => { const s = createState(reader()); - // Exercises the default saveJSON path (writes to happy-dom localStorage). - s.tabs.value[0].sqlDraft = 'SELECT 9'; - const e = createSavedQuery(s, s.tabs.value[0], 'nine')!; - renameSaved(s, e.id, 'nine!'); - toggleFavorite(s, e.id); + // Exercises the default saveJSON path (writes to happy-dom localStorage) — + // recordHistory/deleteHistory/clearHistory are untouched by #287 W4 (only + // the saved-query CRUD ops moved off the flat `save` seam onto the + // aggregate commit, which has no meaningful "real" default — see + // the next test). recordHistory(s, { sqlDraft: 'SELECT 9', result: { rawText: null, rows: [], progress: { elapsed_ns: 0 } } }); - deleteSaved(s, 'nope'); deleteHistory(s, 'nope'); clearHistory(s); expect(s.history).toEqual([]); }); + it('createSavedQuery/renameSaved/toggleFavorite/deleteSaved always require an injected commit (#287 W4 — no flat-storage default)', async () => { + const s = savedTestState(); + const commit = fakeWorkspaceCommit(); + 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 deleteSaved(s, 'nope', commit); + expect(commit).toHaveBeenCalledTimes(4); + expect(s.savedQueries.some((q) => q.id === e.id)).toBe(true); + }); });