fix(#370): keep Dashboard membership and stars consistent

CHANGELOG.md

@@ -39,6 +39,16 @@ auto-generated per-PR notes; this file is the curated, human-readable history.
   involved (#343 non-goals; membership semantics stay with #370).
 
 ### Fixed
+- **Dashboard tile deletion now keeps Workbench star membership consistent**
+  (#370). `dashboard.tiles[]` is the canonical favorite state for panel-role
+  queries: deleting the final tile clears the compatibility
+  `spec.favorite` flag, while deleting one of several instances keeps it set.
+  The same atomic workspace transform removes the selected tile from explicit
+  filter targets, normalizes the active layout/fallback, and advances the
+  Dashboard revision once. Legacy or imported `favorite: true` panel queries
+  without a tile now render unstarred and one click creates membership while
+  repairing the mirror flag; filter/setup favorites keep their independent
+  compatibility behavior.
 - **Migrated the development test stack to Vitest 4** (#372). Vitest and its
   V8 coverage provider now use the supported 4.x line, removed pool options
   have been migrated, stricter mock typings are explicit, and the more accurate

src/dashboard/application/dashboard-authoring-session.ts

@@ -32,6 +32,7 @@ import { buildDashboardExportBundle } from '../model/dashboard-export.js';
 import { defaultLayoutRegistry } from '../layouts/layout-registry.js';
 import { applyCommand } from './dashboard-commands.js';
 import type { DashboardCommand, DashboardCommandResult } from './dashboard-commands.js';
+import { removeTileMembership } from './tile-membership.js';
 import { createEmptyDashboard } from './empty-dashboard.js';
 import { createQueryResolver } from './dashboard-query-resolver.js';
 import { validateStoredWorkspaceDocument } from '../../workspace/stored-workspace.js';
@@ -108,9 +109,11 @@ export function createDashboardAuthoringSession(
   /** Validate a candidate dashboard as part of a complete candidate workspace:
    *  structure + references/roles/limits (stored-workspace pipeline), then —
    *  only when that passes — resolve and validate every panel presentation. */
-  function validateCandidate(dashboard: DashboardDocumentV1): WorkspaceDiagnostic[] {
+  function validateCandidate(
+    dashboard: DashboardDocumentV1, candidateQueries: SavedQueryV2[] = queries,
+  ): WorkspaceDiagnostic[] {
     const candidate: StoredWorkspaceV1 = {
-      storageVersion: 1, id: workspaceId, name: workspaceName, queries, dashboard,
+      storageVersion: 1, id: workspaceId, name: workspaceName, queries: candidateQueries, dashboard,
     };
     const structural = validateStoredWorkspaceDocument(candidate, codecOptions);
     if (structural.length) return structural;
@@ -151,11 +154,16 @@ export function createDashboardAuthoringSession(
     const applied = applyCommand(current.document, command, { resolver, genTileId: genId, plugin: resolved.plugin });
     if (!applied.ok) return returnFail<T>(applied.diagnostics, baseVersion);
 
-    const normalized = resolved.plugin.normalize(applied.dashboard);
-    const diagnostics = validateCandidate(normalized);
+    const membership = command.type === 'remove-tile'
+      ? removeTileMembership(current.document, queries, command.tileId)
+      : null;
+    const candidateQueries = membership?.queries ?? queries;
+    const normalized = resolved.plugin.normalize(membership?.dashboard ?? applied.dashboard);
+    const diagnostics = validateCandidate(normalized, candidateQueries);
     if (diagnostics.length) return returnFail<T>(diagnostics, baseVersion);
 
     const draftVersion = baseVersion + 1;
+    queries = candidateQueries;
     batch(() => {
       stateSignal.value = {
         document: normalized, draftVersion, dirty: true,

src/dashboard/application/tile-membership.ts

@@ -21,6 +21,12 @@ import { regenerateGridFallback } from '../layouts/grafana-grid-layout.js';
 import { createEmptyDashboard } from './empty-dashboard.js';
 import type { DashboardDocumentV1, SavedQueryV2 } from '../../generated/json-schema.types.js';
 
+export interface TileRemovalResult {
+  dashboard: DashboardDocumentV1;
+  queries: SavedQueryV2[];
+  queryId: string;
+}
+
 /** Remove every tile referencing `queryId`, and scrub those tile ids out of
  *  every filter's `targets` — the typed counterpart of saved-query-mutation.ts's
  *  `removeAffectedTiles` (that one operates on unknown/untyped documents). */
@@ -37,6 +43,45 @@ function removeTilesForQuery(dashboard: DashboardDocumentV1, queryId: string): D
   return { ...dashboard, tiles, filters };
 }
 
+/** The Workbench star is canonical tile membership for panel-role queries.
+ * Filter/setup favorites retain their independent compatibility semantics. */
+export function queryMembershipFavorite(
+  dashboard: DashboardDocumentV1 | null,
+  query: SavedQueryV2,
+): boolean {
+  if (queryDashboardRole(query) !== 'panel') return query.spec.favorite === true;
+  return !!dashboard?.tiles.some((tile) => tile.queryId === query.id);
+}
+
+/** Remove ONE Dashboard tile and synchronize the affected panel query's
+ * compatibility favorite flag with its post-delete membership. Filter target
+ * cleanup, layout normalization and grid fallback regeneration are part of
+ * the same pure transform; revision ownership remains with the commit caller. */
+export function removeTileMembership(
+  dashboard: DashboardDocumentV1,
+  queries: SavedQueryV2[],
+  tileId: string,
+): TileRemovalResult | null {
+  const removedTile = dashboard.tiles.find((tile) => tile.id === tileId);
+  if (!removedTile) return null;
+  const tiles = dashboard.tiles.filter((tile) => tile.id !== tileId);
+  const filters = dashboard.filters.map((filter) => (
+    filter.targets
+      ? { ...filter, targets: filter.targets.filter((target) => target !== tileId) }
+      : filter
+  ));
+  const next = { ...dashboard, tiles, filters };
+  const normalized = resolveLayoutPluginSync(next.layout).normalize(next);
+  regenerateGridFallback(normalized.layout, normalized.tiles);
+  const member = normalized.tiles.some((tile) => tile.queryId === removedTile.queryId);
+  const nextQueries = queries.map((query) => (
+    query.id === removedTile.queryId && queryDashboardRole(query) === 'panel'
+      ? { ...query, spec: { ...query.spec, favorite: member } }
+      : query
+  ));
+  return { dashboard: normalized, queries: nextQueries, queryId: removedTile.queryId };
+}
+
 /**
  * Reflect a Workbench favorite flip onto Dashboard tile membership (#299).
  *

src/state.ts

@@ -15,7 +15,9 @@ import {
   loadStr as loadStrUntyped,
 } from './core/storage.js';
 import { emptyRecentMap as emptyRecentMapUntyped } from './core/recent-values.js';
-import { toggleTileMembership } from './dashboard/application/tile-membership.js';
+import {
+  queryMembershipFavorite, toggleTileMembership,
+} from './dashboard/application/tile-membership.js';
 import type { ResultSort } from './core/sort.js';
 import {
   defaultSpecValidationService as defaultSpecValidationServiceUntyped,
@@ -1123,7 +1125,7 @@ export async function toggleFavorite(
   // the desired boolean from the query it displays, and the transform re-checks
   // applicability against `latest` — tile membership is derived from
   // `latest.dashboard` (passed as `dashboard` below), never stale `state.dashboard`.
-  const favorite = !queryFavorite(entry);
+  const favorite = !queryMembershipFavorite(state.dashboard, entry);
   return patchSavedSpec(state, id, { favorite }, mutate, validationService,
     (dashboard, patchedEntry) => toggleTileMembership(dashboard, patchedEntry, favorite, genId));
 }
@@ -1132,7 +1134,8 @@ export async function toggleFavorite(
 export function sortedSaved(state: AppState): SavedQueryV2[] {
   return state.savedQueries
     .map((q, i): [SavedQueryV2, number] => [q, i])
-    .sort((a, b) => (queryFavorite(b[0]) ? 1 : 0) - (queryFavorite(a[0]) ? 1 : 0) || a[1] - b[1])
+    .sort((a, b) => (queryMembershipFavorite(state.dashboard, b[0]) ? 1 : 0)
+      - (queryMembershipFavorite(state.dashboard, a[0]) ? 1 : 0) || a[1] - b[1])
     .map(([q]) => q);
 }
 

src/ui/dashboard.ts

@@ -70,6 +70,7 @@ import {
 import type { GrafanaGridLayoutModel, GridRenderMode } from '../dashboard/layouts/grafana-grid-layout.js';
 import { applyCommand } from '../dashboard/application/dashboard-commands.js';
 import type { DashboardCommand } from '../dashboard/application/dashboard-commands.js';
+import { removeTileMembership } from '../dashboard/application/tile-membership.js';
 import { createQueryResolver } from '../dashboard/application/dashboard-query-resolver.js';
 import { resolveDashboardMode } from '../dashboard/application/session-bundle.js';
 import {
@@ -875,6 +876,21 @@ export async function renderDashboard(app: DashboardApp): Promise<void> {
     };
   }
 
+  /** Apply a route command plus its workspace-level membership semantics. A
+   * raw remove-tile is first command-validated, then replaced by the shared
+   * transform that also cleans targets and synchronizes spec.favorite. */
+  function applyRouteCommand(
+    baseDoc: DashboardDocumentV1, command: DashboardCommand, queriesForResolver: SavedQueryV2[],
+  ) {
+    const applied = applyCommand(baseDoc, command, ctxFor(baseDoc, queriesForResolver));
+    if (!applied.ok) return applied;
+    if (command.type !== 'remove-tile') return { ...applied, queries: queriesForResolver };
+    const membership = removeTileMembership(baseDoc, queriesForResolver, command.tileId);
+    return membership
+      ? { ...applied, dashboard: membership.dashboard, queries: membership.queries }
+      : { ...applied, queries: queriesForResolver };
+  }
+
   // ── Structural commands (reorder via drag, preset) ────────────────────────
   // move-tile / update-placement / change-layout are the phase-3 authoring
   // commands; the dashboard UI drives only move-tile (drag) and change-layout
@@ -899,7 +915,7 @@ export async function renderDashboard(app: DashboardApp): Promise<void> {
     // RESULTING document's own engine, so a post-switch grid document is
     // pruned by the grid plugin (its own `items`), not flow's (which would
     // only ever see its own fallback surface).
-    const applied = applyCommand(currentDoc, command, ctxFor(currentDoc, queries));
+    const applied = applyRouteCommand(currentDoc, command, queries);
     // A UI-driven command (drag move-tile, preset change-layout, grid
     // resize/delete) is always valid; a rejected candidate is simply ignored
     // (no draft change).
@@ -936,12 +952,12 @@ export async function renderDashboard(app: DashboardApp): Promise<void> {
       observed = latest;
       if (!latest || !latest.dashboard) return null;
       const base = latest.dashboard;
-      const reapplied = applyCommand(base, command, ctxFor(base, latest.queries));
+      const reapplied = applyRouteCommand(base, command, latest.queries);
       if (!reapplied.ok) return null;
       const committedDoc = resolveLayoutPluginSync(reapplied.dashboard.layout).normalize(reapplied.dashboard);
       return {
         candidate: {
-          storageVersion: 1, id: latest.id, name: latest.name, queries: latest.queries,
+          storageVersion: 1, id: latest.id, name: latest.name, queries: reapplied.queries,
           dashboard: { ...committedDoc, revision: base.revision + 1 },
         },
       };
@@ -1051,7 +1067,7 @@ export async function renderDashboard(app: DashboardApp): Promise<void> {
     if (!rebased) return;
     const rebaseQueries = committedWorkspace!.queries;
     for (const pending of pendingCommands) {
-      const r = applyCommand(rebased, pending, ctxFor(rebased, rebaseQueries));
+      const r = applyRouteCommand(rebased, pending, rebaseQueries);
       // A replay that no longer applies is simply skipped here — its own
       // queued `mutateWorkspace` call will independently null-abort and
       // toast when its turn comes.

src/ui/saved-history.ts

@@ -15,7 +15,8 @@ import type { AppState, HistoryEntry } from '../state.js';
 import { flashToast } from './toast.js';
 import { isAutoRunnable } from '../core/sql-split.js';
 import { isQuerylessPanel } from '../core/panel-cfg.js';
-import { queryDescription, queryFavorite, queryName, queryPanel, queryView } from '../core/saved-query.js';
+import { queryDescription, queryName, queryPanel, queryView } from '../core/saved-query.js';
+import { queryMembershipFavorite } from '../dashboard/application/tile-membership.js';
 import { effectiveDashboardRole, rolePreviewView } from '../core/result-choice.js';
 import { filterRoleBadge } from './tabs.js';
 import type { App } from './app.types.js';
@@ -122,7 +123,7 @@ function renderSaved(app: App, list: HTMLElement): void {
   }
   for (const q of items) {
     if (app.state.editingSavedId.value === q.id) { list.appendChild(savedEditForm(app, q)); continue; }
-    const favorite = queryFavorite(q);
+    const favorite = queryMembershipFavorite(state.dashboard, q);
     const name = queryName(q);
     const description = queryDescription(q);
     const panel = queryPanel(q);

tests/e2e/dashboard-membership.html

@@ -0,0 +1,86 @@
+<!doctype html>
+<html>
+<head>
+  <meta charset="utf-8" />
+  <title>Dashboard membership workflow</title>
+  <link rel="stylesheet" href="/src/styles.css" />
+</head>
+<body data-theme="dark">
+  <main id="surface"></main>
+  <script type="importmap">
+    { "imports": {
+      "@preact/signals-core": "/node_modules/@preact/signals-core/dist/signals-core.mjs",
+      "marked": "/node_modules/marked/lib/marked.esm.js",
+      "@codemirror/state": "/node_modules/@codemirror/state/dist/index.js",
+      "@codemirror/view": "/node_modules/@codemirror/view/dist/index.js",
+      "@codemirror/commands": "/node_modules/@codemirror/commands/dist/index.js",
+      "@codemirror/lang-json": "/node_modules/@codemirror/lang-json/dist/index.js",
+      "@codemirror/language": "/node_modules/@codemirror/language/dist/index.js",
+      "@codemirror/lang-sql": "/node_modules/@codemirror/lang-sql/dist/index.js",
+      "@codemirror/lang-xml": "/node_modules/@codemirror/lang-xml/dist/index.js",
+      "@codemirror/autocomplete": "/node_modules/@codemirror/autocomplete/dist/index.js",
+      "@codemirror/search": "/node_modules/@codemirror/search/dist/index.js",
+      "@lezer/common": "/node_modules/@lezer/common/dist/index.js",
+      "@lezer/highlight": "/node_modules/@lezer/highlight/dist/index.js",
+      "@lezer/lr": "/node_modules/@lezer/lr/dist/index.js",
+      "@lezer/json": "/node_modules/@lezer/json/dist/index.js",
+      "@lezer/xml": "/node_modules/@lezer/xml/dist/index.js",
+      "style-mod": "/node_modules/style-mod/src/style-mod.js",
+      "w3c-keyname": "/node_modules/w3c-keyname/index.js",
+      "crelt": "/node_modules/crelt/index.js",
+      "@marijn/find-cluster-break": "/node_modules/@marijn/find-cluster-break/src/index.js"
+    } }
+  </script>
+  <script type="module">
+    import { createApp } from '/src/ui/app.js';
+    import { renderSavedHistory } from '/src/ui/saved-history.js';
+    import { renderDashboard } from '/src/ui/dashboard.js';
+
+    const seed = {
+      storageVersion: 1, id: 'workspace', name: 'Workspace', dashboard: null,
+      queries: [{
+        id: 'q1', sql: 'SELECT 1', specVersion: 1,
+        spec: { name: 'Revenue', favorite: false },
+      }],
+    };
+    const root = document.querySelector('#surface');
+    const app = createApp({ root, broadcastChannel: () => null });
+    app.conn.ensureFreshToken = async () => true;
+    app.conn.chCtx.onSignedOut = () => {};
+    app.exec.executeRead = async (result) => {
+      result.columns = [{ name: 'value', type: 'UInt8' }];
+      result.rows = [[1]];
+      result.progress = { rows: 1, bytes: 1 };
+      result.error = null;
+      return result;
+    };
+
+    function renderWorkbench() {
+      const tabs = document.createElement('div');
+      const search = document.createElement('div');
+      const list = document.createElement('div');
+      const open = document.createElement('button');
+      open.textContent = 'Open Dashboard';
+      open.onclick = () => { void renderDashboard(app); };
+      root.replaceChildren(tabs, search, list, open);
+      app.dom.savedTabsRow = tabs;
+      app.dom.savedSearch = search;
+      app.dom.savedList = list;
+      renderSavedHistory(app);
+    }
+
+    let workspace = await app.workspace.loadCurrent();
+    if (!sessionStorage.getItem('membership-seeded')) {
+      await app.workspace.clearCurrent();
+      const committed = await app.workspace.commit(seed);
+      if (!committed.ok) throw new Error(committed.diagnostics[0]?.message || 'seed failed');
+      workspace = committed.workspace;
+      sessionStorage.setItem('membership-seeded', '1');
+    }
+    app.applyCommittedWorkspace(workspace);
+    renderWorkbench();
+    window.__workspace = () => app.workspace.loadCurrent();
+    window.__ready = true;
+  </script>
+</body>
+</html>

tests/e2e/dashboard-membership.spec.js

@@ -0,0 +1,31 @@
+import { test, expect } from '@playwright/test';
+
+test('Workbench star → Dashboard delete → Workbench reload keeps membership consistent', async ({ page }) => {
+  const pageErrors = [];
+  page.on('pageerror', (error) => pageErrors.push(error.message));
+  await page.goto('/tests/e2e/dashboard-membership.html');
+  await page.waitForTimeout(250);
+  expect(pageErrors).toEqual([]);
+  await page.waitForFunction(() => window.__ready === true);
+
+  await page.locator('.sv-star[title="Favorite"]').click();
+  await page.waitForFunction(async () => (await window.__workspace()).queries[0].spec.favorite === true);
+  let workspace = await page.evaluate(() => window.__workspace());
+  expect(workspace.queries[0].spec.favorite).toBe(true);
+  expect(workspace.dashboard.tiles).toHaveLength(1);
+
+  await page.getByRole('button', { name: 'Open Dashboard' }).click();
+  expect(pageErrors).toEqual([]);
+  await expect(page.locator('.dash-tile-body')).toContainText('1');
+  await page.getByRole('button', { name: 'Remove Revenue from the dashboard' }).click();
+  await page.waitForFunction(async () => (await window.__workspace()).queries[0].spec.favorite === false);
+  workspace = await page.evaluate(() => window.__workspace());
+  expect(workspace.queries[0].spec.favorite).toBe(false);
+  expect(workspace.dashboard.tiles).toEqual([]);
+  expect(workspace.dashboard.revision).toBe(2);
+
+  await page.reload();
+  await page.waitForFunction(() => window.__ready === true);
+  await expect(page.locator('.sv-star[title="Favorite"]')).toBeVisible();
+  await expect(page.locator('.sv-star')).not.toHaveClass(/\bon\b/);
+});