diff --git a/CHANGELOG.md b/CHANGELOG.md index 8055bea..8e8ce17 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,9 @@ auto-generated per-PR notes; this file is the curated, human-readable history. - Keep the shared connection host visible after the server-version probe, restore Dashboard filters to their initial default-derived active state, and retain Dashboard time-range announcements when no ordinary filters exist. +- Restore Dashboard tiles for favorited panel queries imported through either + query-only or query-only workspace imports, so the imported report opens + instead of incorrectly offering only “Create dashboard”. ### Added - **Unified SQL Browser and Dashboard chrome.** Both surfaces now share the diff --git a/src/workspace/import-planner.ts b/src/workspace/import-planner.ts index 3e7ba52..fcc8722 100644 --- a/src/workspace/import-planner.ts +++ b/src/workspace/import-planner.ts @@ -23,6 +23,8 @@ 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 { toggleTileMembership } from '../dashboard/application/tile-membership.js'; +import { queryDashboardRole } from '../dashboard/model/workspace-semantics.js'; import { importQueries, replaceWorkspaceContents, } from './workspace-operations.js'; @@ -296,6 +298,44 @@ function replaceIncomingQueries( return out; } +/** Restore the Workbench star's Dashboard-membership contract for imported + * entries. A portable query-only import carries the compatibility + * `spec.favorite` flag, while the live application represents a favorited + * panel query as a Dashboard tile. A replacement also removes obsolete tiles + * when the replacement is no longer a favorited panel. + * + * Only copied/replaced incoming entries participate: an entry skipped by the + * import or resolved to an existing local query must not change the current + * Dashboard merely because the bundle happened to mark it as a favorite. */ +function addImportedFavoriteTiles( + dashboard: DashboardDocumentV1 | null, + incoming: readonly SavedQueryV2[], mapping: IdMapping, + queries: readonly SavedQueryV2[], genId: WorkspaceIdGen, +): DashboardDocumentV1 | null { + const queriesById = new Map(queries.map((query) => [query.id, query] as const)); + const original = dashboard; + let next = dashboard; + for (const source of incoming) { + const resolution = mapping[source.id]; + if (!resolution || resolution.action === 'skip' || resolution.action === 'use-existing') continue; + // Every copy/replace resolution contributes its target to `queries` above. + const imported = queriesById.get(resolution.targetId as string) as SavedQueryV2; + const shouldBeMember = imported.spec.favorite === true && queryDashboardRole(imported) === 'panel'; + const isMember = next?.tiles.some((tile) => tile.queryId === imported.id) === true; + // A newly copied, unfavorited query cannot have a tile. A replacement must + // also reconcile removal, so stale tile/filter references do not survive. + if ((resolution.action === 'replace' || shouldBeMember) && isMember !== shouldBeMember) { + next = toggleTileMembership(next, imported, shouldBeMember, genId); + } + } + // Tile changes are one Dashboard document mutation regardless of how many + // imported queries participate. A newly minted Dashboard starts at revision + // 1; only an existing document advances its revision. + return original && next !== original + ? { ...(next as DashboardDocumentV1), revision: original.revision + 1 } + : next; +} + // --- Plans -------------------------------------------------------------------- export interface PortableBundleImportPlan { @@ -346,8 +386,9 @@ function invalidatedDashboardPlan( ); } -/** Queries-only import (Dashboard untouched): merge the bundle's queries into - * the workspace's query catalog per `decisions`, and validate the result. */ +/** Queries-only import: merge the bundle's queries into the workspace's query + * catalog per `decisions`. Imported favorited panels restore their Dashboard + * tile membership. */ export function planImportQueries( workspace: StoredWorkspaceV2, bundle: PortableBundleV1, decisions: readonly QueryDecision[], genId: WorkspaceIdGen, @@ -355,7 +396,10 @@ export function planImportQueries( ): PortableBundleImportPlan { const mapping = buildQueryIdMapping(bundle.queries, workspace.queries, decisions, genId); const nextQueries = mergeIncomingQueries(bundle.queries, workspace.queries, mapping); - const candidate = importQueries(workspace, nextQueries); + const dashboard = addImportedFavoriteTiles( + workspace.dashboard, bundle.queries, mapping, nextQueries, genId, + ); + const candidate = importQueries({ ...workspace, dashboard }, nextQueries); return validatedPlan(candidate, mapping, options); } @@ -413,6 +457,11 @@ export function planReplaceWorkspace( dashboard = rewritten.dashboard; } - const candidate = replaceWorkspaceContents(workspace, { queries: nextQueries, dashboard }); + const candidate = replaceWorkspaceContents(workspace, { + queries: nextQueries, + dashboard: sourceDashboardId === undefined + ? addImportedFavoriteTiles(dashboard, bundle.queries, mapping, nextQueries, genId) + : dashboard, + }); return validatedPlan(candidate, mapping, options, sourceDashboardId); } diff --git a/tests/unit/app.test.ts b/tests/unit/app.test.ts index 044c2bd..d118a43 100644 --- a/tests/unit/app.test.ts +++ b/tests/unit/app.test.ts @@ -5652,6 +5652,30 @@ describe('unified /sql routing', () => { expect(qs(app.root, '.workspace-loading')).toBeNull(); }); + it('keeps imported favorite queries visible after visiting an empty Dashboard', async () => { + const imported = savedQueryFixture({ + id: 'imported-favorite', name: 'Imported favorite', sql: 'SELECT 1', favorite: true, + }); + const app = createApp(env()); + const workspace: StoredWorkspaceV2 = { + storageVersion: 2, id: 'imported-workspace', key: 'imported_workspace', + name: 'Imported workspace', queries: [imported], dashboard: null, + }; + await seedActiveWorkspace(app, workspace); + app.sqlRoute = { surface: 'workspace', workspaceKey: workspace.key }; + app.renderApp(); + + await app.navigateSqlRoute({ + surface: 'dashboard', workspaceKey: workspace.key, mode: 'edit', + }, 'push'); + expect(qs(app.root, '.dash-create')).not.toBeNull(); + + await app.navigateSqlRoute({ surface: 'workspace', workspaceKey: workspace.key }, 'push'); + + expect(app.state.savedQueries).toEqual([imported]); + expect(qs(app.root, '.saved-row .name').textContent).toBe('Imported favorite'); + }); + it('a Dashboard edit commit that settles after switching to View refreshes View only', async () => { const app = createApp(env()); const workspace = dashboardWorkspace(); diff --git a/tests/unit/import-planner.test.ts b/tests/unit/import-planner.test.ts index 63acba3..d569111 100644 --- a/tests/unit/import-planner.test.ts +++ b/tests/unit/import-planner.test.ts @@ -22,6 +22,11 @@ const filterQuery = (id: string, name = id): SavedQueryV2 => ({ spec: { name, dashboard: { role: 'filter' } }, }); +const setupQuery = (id: string, name = id): SavedQueryV2 => ({ + id, sql: 'SELECT 1', specVersion: 1, + spec: { name, dashboard: { role: 'setup' } }, +}); + const dashboardDoc = (over: Partial = {}): DashboardDocumentV1 => ({ documentVersion: 1, id: 'd1', title: 'D', revision: 1, layout: { type: 'flow', version: 1, preset: 'report', items: {} }, @@ -248,15 +253,48 @@ describe('planImportQueries', () => { expect(plan.sourceDashboardId).toBeUndefined(); }); - it('imports a favorite panel query without inventing Dashboard membership', () => { - const dash = dashboardDoc({ tiles: [] }); - const ws = workspace({ queries: [panelQuery('a')], dashboard: dash }); + it('imports a favorite panel query as a Dashboard tile', () => { + const ws = workspace({ queries: [panelQuery('a')], dashboard: null }); const favorite = panelQuery('b'); favorite.spec.favorite = true; const plan = planImportQueries(ws, bundle({ queries: [favorite] }), [], counter()); expect(plan.candidateWorkspace?.queries.find((query) => query.id === 'b')?.spec.favorite).toBe(true); - expect(plan.candidateWorkspace?.dashboard?.tiles).toEqual([]); + expect(plan.candidateWorkspace?.dashboard).toMatchObject({ + title: 'Dashboard', tiles: [{ queryId: 'b' }], + }); + }); + + it('adds one favorite panel query to an existing Dashboard and increments its revision once', () => { + const dash = dashboardDoc({ revision: 7, tiles: [] }); + const ws = workspace({ queries: [panelQuery('a')], dashboard: dash }); + const favorite = panelQuery('b'); + favorite.spec.favorite = true; + const plan = planImportQueries(ws, bundle({ queries: [favorite] }), [], counter()); + expect(plan.candidateWorkspace?.dashboard?.tiles).toEqual([{ id: 'id-1', queryId: 'b' }]); + expect(plan.candidateWorkspace?.dashboard?.revision).toBe(8); + }); + + it('adds several favorite panel queries but increments an existing Dashboard revision only once', () => { + const dash = dashboardDoc({ revision: 7, tiles: [] }); + const one = panelQuery('one'); + const two = panelQuery('two'); + one.spec.favorite = true; + two.spec.favorite = true; + const plan = planImportQueries(workspace({ dashboard: dash }), bundle({ queries: [one, two] }), [], counter()); + expect(plan.candidateWorkspace?.dashboard?.tiles).toEqual([ + { id: 'id-1', queryId: 'one' }, { id: 'id-2', queryId: 'two' }, + ]); + expect(plan.candidateWorkspace?.dashboard?.revision).toBe(8); + }); + + it('does not advance revision when an imported favorite already has Dashboard membership', () => { + const favorite = panelQuery('p1'); + favorite.spec.favorite = true; + const dash = dashboardDoc({ revision: 7, tiles: [{ id: 't1', queryId: 'p1' }] }); + const decisions: QueryDecision[] = [{ sourceId: 'p1', action: 'replace' }]; + const plan = planImportQueries(workspace({ queries: [favorite], dashboard: dash }), bundle({ queries: [favorite] }), decisions, counter()); expect(plan.candidateWorkspace?.dashboard).toBe(dash); + expect(plan.candidateWorkspace?.dashboard?.revision).toBe(7); }); it('overwrites the existing entry in place on a replace decision', () => { @@ -267,6 +305,35 @@ describe('planImportQueries', () => { expect(plan.candidateWorkspace!.queries[0].spec.name).toBe('new name'); }); + it('removes tile membership and increments revision when a tiled favorite is replaced as unfavorited', () => { + const current = panelQuery('p1'); + current.spec.favorite = true; + const replacement = panelQuery('p1', 'Replacement'); + replacement.spec.favorite = false; + const dash = dashboardDoc({ revision: 7, tiles: [{ id: 't1', queryId: 'p1' }] }); + const plan = planImportQueries( + workspace({ queries: [current], dashboard: dash }), bundle({ queries: [replacement] }), + [{ sourceId: 'p1', action: 'replace' }], counter(), + ); + expect(plan.candidateWorkspace?.dashboard).toMatchObject({ revision: 8, tiles: [] }); + expect(plan.candidateWorkspace?.queries[0].spec.favorite).toBe(false); + }); + + it.each([ + ['filter', filterQuery('p1')], + ['setup', setupQuery('p1')], + ])('removes tile membership when a tiled panel is replaced with a %s query', (_role, replacement) => { + const current = panelQuery('p1'); + current.spec.favorite = true; + replacement.spec.favorite = true; + const dash = dashboardDoc({ revision: 7, tiles: [{ id: 't1', queryId: 'p1' }] }); + const plan = planImportQueries( + workspace({ queries: [current], dashboard: dash }), bundle({ queries: [replacement] }), + [{ sourceId: 'p1', action: 'replace' }], counter(), + ); + expect(plan.candidateWorkspace?.dashboard).toMatchObject({ revision: 8, tiles: [] }); + }); + 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' }]; @@ -400,6 +467,15 @@ describe('planReplaceWorkspace', () => { expect(plan.sourceDashboardId).toBeUndefined(); }); + it('creates Dashboard membership for favorite panel queries in a query-only workspace import', () => { + const favorite = panelQuery('p1'); + favorite.spec.favorite = true; + const plan = planReplaceWorkspace(workspace(), bundle({ queries: [favorite] }), undefined, [], counter()); + expect(plan.candidateWorkspace?.dashboard).toMatchObject({ + title: 'Dashboard', tiles: [{ queryId: 'p1' }], + }); + }); + it('replaces queries AND Dashboard atomically when a source Dashboard is selected, including standalone queries', () => { const ws = workspace(); // t1's query (p1) declares `{p:String}` — see `buildBundle`'s own comment