fix(#244): restore role-aware initial result views for saved and shared queries

CHANGELOG.md

@@ -55,6 +55,22 @@ auto-generated per-PR notes; this file is the curated, human-readable history.
   a Logs panel, and a Text panel explaining the demo.
 
 ### Fixed
+- **Opening a Filter-role saved or shared query now always starts in the
+  Filter preview** (#244, folds in #249). Library-row activation previously
+  restored whichever result view (Table/JSON/Panel) was already open,
+  independent of the query's Dashboard role; a Filter-role query with SQL
+  that can't auto-run (empty/DDL, reachable via an import or legacy
+  localStorage entry that bypassed SQL-shape validation) fell through to no
+  view change at all. Separately, the share-link/OAuth-handoff bootstrap path
+  only ever restored `resultView` for a *queryless* Panel link — any
+  SQL-bearing shared Filter query, or a SQL-bearing shared Panel query with a
+  persisted `spec.view`, always landed on the default Table view. Both call
+  sites now resolve the same `rolePreviewView(spec) || queryView(query)`
+  precedence (`src/core/result-choice.js`): a role-owned transient preview
+  wins over a persisted view, which wins over the queryless-Panel/default
+  fallback. The preview stays transient — no `spec.view: "filter"` is ever
+  persisted — and ordinary reruns after the query is open continue to
+  preserve the user's current Table/JSON/Filter selection.
 - Review follow-ups on the Dashboard Filter sources work above, found in a
   UI/UX pass on #232 before merge: a curated field's clear (×) button now
   reports the cleared value (not the stale prior selection) to

src/core/result-choice.js

@@ -27,6 +27,18 @@ export function effectiveDashboardRole(spec) {
   return typeof role === 'string' && role ? role : 'panel';
 }
 
+// The transient preview a role owns on initial Library launch — never a
+// persisted `spec.view` (Filter has none to persist; #244). `null` defers to
+// the query's persisted view / the caller's other fallbacks.
+export function rolePreviewView(spec) {
+  switch (effectiveDashboardRole(spec)) {
+    case 'filter':
+      return 'filter';
+    default:
+      return null;
+  }
+}
+
 export function resultChoiceForSpec(spec) {
   if (effectiveDashboardRole(spec) === 'filter') return 'role:filter';
   const type = spec?.panel?.cfg?.type;

src/main.js

@@ -14,6 +14,8 @@ import { exchangeCodeForTokens, bearerFromTokens } from './net/oauth.js';
 import { decodeShare } from './core/share.js';
 import { cloneJson, queryName, queryPanel, queryView, upgradeSavedQuery } from './core/saved-query.js';
 import { isDashboardRoute } from './core/dashboard.js';
+import { rolePreviewView } from './core/result-choice.js';
+import { isQuerylessPanel } from './core/panel-cfg.js';
 import { setTabSpecDraft } from './state.js';
 
 export async function bootstrap(app, env) {
@@ -86,11 +88,17 @@ export async function bootstrap(app, env) {
       t0.name = queryName(shared);
       t0.specVersion = shared.specVersion;
       setTabSpecDraft(t0, cloneJson(shared.spec));
-      if (panel && panel.cfg) {
-        // A panel-only link (no SQL to run) must open the Panel drawer, or
-        // the recipient lands on an empty Table view and sees nothing.
-        if (!shared.sql) app.state.resultView.value = queryView(shared) || 'panel';
-      }
+      // Restore the initial result view with the same role-aware precedence as
+      // Library activation (#244): a role-owned transient preview (Filter)
+      // wins over the persisted view, regardless of whether the share carries
+      // SQL to run — a SQL-bearing share never auto-runs here, so this only
+      // pre-selects the drawer the recipient lands on before they click Run.
+      const launchView = rolePreviewView(shared.spec) || queryView(shared);
+      if (launchView) app.state.resultView.value = launchView === 'chart' ? 'panel' : launchView;
+      // A queryless panel with no role/persisted view (no SQL to run) still
+      // needs the Panel drawer open, or the recipient lands on an empty Table
+      // view and sees nothing.
+      else if (!shared.sql && isQuerylessPanel(panel)) app.state.resultView.value = 'panel';
       hist.replaceState(null, '', loc.pathname + loc.search);
     }
   }

src/ui/saved-history.js

@@ -14,7 +14,7 @@ import {
 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 { effectiveDashboardRole } from '../core/result-choice.js';
+import { effectiveDashboardRole, rolePreviewView } from '../core/result-choice.js';
 import { filterRoleBadge } from './tabs.js';
 
 // Make a Library/History row draggable; dropping it on the editor inserts the
@@ -114,7 +114,12 @@ function renderSaved(app, list) {
     const name = queryName(q);
     const description = queryDescription(q);
     const panel = queryPanel(q);
-    const view = queryView(q);
+    // Library launch precedence (#244): a role-owned transient preview (Filter)
+    // wins over the persisted view, even when dormant Panel state persists a
+    // `spec.view` of its own — the role reflects the query's *current*
+    // intended representation, the dormant view is just preserved for later.
+    const rolePreview = rolePreviewView(q.spec);
+    const launchView = rolePreview || queryView(q);
     const star = h('button', {
       class: 'sv-star' + (favorite ? ' on' : ''), title: favorite ? 'Unfavorite' : 'Favorite',
       onclick: (e) => {
@@ -135,8 +140,13 @@ function renderSaved(app, list) {
     // nothing. `run({view})` handles the auto-runnable path as before.
     const open = () => {
       app.actions.loadIntoNewTab(q);
-      if (isAutoRunnable(q.sql)) app.actions.run({ view });
-      else if (SAVED_VIEWS.has(view)) app.state.resultView.value = view;
+      if (isAutoRunnable(q.sql)) app.actions.run({ view: launchView });
+      // A role-owned preview isn't in SAVED_VIEWS (it's transient, never
+      // persisted — #244) but still wins here: a Filter-role entry that can't
+      // auto-run (e.g. empty/DDL SQL from an import that skipped validation)
+      // still opens the Filter drawer, which renders its own empty state,
+      // rather than falling through to a dormant Table/JSON/Panel view.
+      else if (rolePreview || SAVED_VIEWS.has(launchView)) app.state.resultView.value = launchView;
       // A queryless panel without a remembered view (hand-authored/imported
       // file) still needs the Panel drawer open, or clicking it shows nothing.
       else if (isQuerylessPanel(panel)) app.state.resultView.value = 'panel';

tests/unit/main.test.js

@@ -181,6 +181,84 @@ describe('bootstrap', () => {
     });
   });
 
+  // v2 share hash: { __asb: 2, query: { sql, specVersion, spec } } (src/core/share.js).
+  const v2Hash = (query) => '#' + btoa(unescape(encodeURIComponent(JSON.stringify({
+    __asb: 2, query: { specVersion: 1, spec: { name: 'Shared query', favorite: false }, ...query },
+  }))));
+  const v2Env = (query) => {
+    const hash = v2Hash(query);
+    return fakeEnv({ location: { href: 'https://ch/sql' + hash, origin: 'https://ch', pathname: '/sql', search: '', hash } });
+  };
+
+  it('opens Filter for a v2 share carrying Filter-role SQL, before any run is possible (#244)', async () => {
+    const app = fakeApp();
+    const env = v2Env({ sql: 'SELECT 1', spec: { name: 'Shared query', favorite: false, dashboard: { role: 'filter' } } });
+    await bootstrap(app, env);
+    expect(app.state.tabs.value[0].sqlDraft).toBe('SELECT 1');
+    expect(app.state.resultView.value).toBe('filter');
+  });
+
+  it('Filter role wins over a dormant persisted view:"panel" carried in a share (#244)', async () => {
+    const app = fakeApp();
+    const panelCfg = { cfg: { type: 'kpi' } };
+    const env = v2Env({
+      sql: 'SELECT 1',
+      spec: { name: 'Shared query', favorite: false, view: 'panel', dashboard: { role: 'filter' }, panel: panelCfg },
+    });
+    await bootstrap(app, env);
+    expect(app.state.resultView.value).toBe('filter');
+    // dormant Panel state and the persisted view survive untouched in the tab's Spec.
+    expect(app.state.tabs.value[0].specParsed.view).toBe('panel');
+    expect(app.state.tabs.value[0].specParsed.panel).toEqual(panelCfg);
+  });
+
+  it('restores a SQL-bearing shared Panel query\'s persisted view:"panel" (no role)', async () => {
+    const app = fakeApp();
+    const panelCfg = { cfg: { type: 'kpi' } };
+    const env = v2Env({ sql: 'SELECT 1', spec: { name: 'Shared query', favorite: false, view: 'panel', panel: panelCfg } });
+    await bootstrap(app, env);
+    expect(app.state.resultView.value).toBe('panel');
+  });
+
+  it.each(['table', 'json'])('restores a SQL-bearing shared query\'s persisted %s preference', async (view) => {
+    const app = fakeApp();
+    const env = v2Env({ sql: 'SELECT 1', spec: { name: 'Shared query', favorite: false, view } });
+    await bootstrap(app, env);
+    expect(app.state.resultView.value).toBe(view);
+  });
+
+  it('leaves the default result view alone for a share with no role and no persisted view', async () => {
+    const app = fakeApp();
+    const env = v2Env({ sql: 'SELECT 1' });
+    await bootstrap(app, env);
+    expect(app.state.resultView.value).toBe('table'); // fakeApp()'s untouched default
+  });
+
+  it('restores Filter for a Filter-role share stashed through the OAuth round-trip (#244)', async () => {
+    const app = fakeApp({ token: valid, isSignedIn: () => true });
+    const env = fakeEnv({ location: { href: 'https://ch/sql', origin: 'https://ch', pathname: '/sql', search: '', hash: '' } });
+    env.sessionStorage.setItem('oauth_shared', JSON.stringify({
+      sql: 'SELECT 1', specVersion: 1, spec: { name: 'Shared query', favorite: false, dashboard: { role: 'filter' } },
+    }));
+    await bootstrap(app, env);
+    expect(app.state.resultView.value).toBe('filter');
+  });
+
+  it('maps a legacy persisted view:"chart" through the Panel compatibility path in a share', async () => {
+    const app = fakeApp();
+    const panelCfg = { cfg: { type: 'pie', x: 0, y: [1], series: null } };
+    const env = v2Env({ sql: 'SELECT 1', spec: { name: 'Shared query', favorite: false, view: 'chart', panel: panelCfg } });
+    await bootstrap(app, env);
+    expect(app.state.resultView.value).toBe('panel');
+  });
+
+  it('never persists a transient view:"filter" into the restored tab Spec (#244)', async () => {
+    const app = fakeApp();
+    const env = v2Env({ sql: 'SELECT 1', spec: { name: 'Shared query', favorite: false, dashboard: { role: 'filter' } } });
+    await bootstrap(app, env);
+    expect(app.state.tabs.value[0].specParsed.view).toBeUndefined();
+  });
+
   it('restores a shared query (SQL + chart) from sessionStorage after the OAuth round-trip', async () => {
     // The hash is gone after the IdP redirect; the stash carries it through.
     const app = fakeApp({ token: valid, isSignedIn: () => true });

tests/unit/result-choice.test.js

@@ -1,7 +1,7 @@
 import { describe, expect, it } from 'vitest';
 import {
   applyResultChoice, DASHBOARD_ROLE_RESULT_CHOICES, effectiveDashboardRole,
-  PANEL_RESULT_CHOICES, resultChoiceForSpec,
+  PANEL_RESULT_CHOICES, resultChoiceForSpec, rolePreviewView,
 } from '../../src/core/result-choice.js';
 
 const query = (spec) => ({ id: 'q', sql: 'SELECT 1', specVersion: 1, spec });
@@ -43,4 +43,13 @@ describe('result choices', () => {
     expect(applyResultChoice(source, choice).spec.dashboard).toBeUndefined();
     expect(applyResultChoice(source, null)).toBe(source);
   });
+  it('rolePreviewView: Filter owns the transient launch preview; every other role defers (#244)', () => {
+    expect(rolePreviewView({ dashboard: { role: 'filter' } })).toBe('filter');
+    expect(rolePreviewView({})).toBeNull();
+    expect(rolePreviewView({ dashboard: { role: 'panel' } })).toBeNull();
+    expect(rolePreviewView(undefined)).toBeNull();
+    // dormant Panel state alongside the role has no bearing on the pure helper —
+    // precedence over it is the caller's job (saved-history.js).
+    expect(rolePreviewView({ dashboard: { role: 'filter' }, view: 'panel', panel: { cfg: { type: 'kpi' } } })).toBe('filter');
+  });
 });

tests/unit/saved-history.test.js

@@ -78,6 +78,71 @@ describe('renderSavedHistory', () => {
     expect(app.actions.run).not.toHaveBeenCalled();
   });
 
+  it.each(['table', 'json', 'panel'])(
+    'saved: a Filter-role query always launches into the Filter preview, independent of the current result view (was %s) (#244)',
+    (previousView) => {
+      const app = makeApp();
+      app.state.sidePanel.value = 'saved';
+      app.state.resultView.value = previousView;
+      setSaved(app, [{ id: 'f', name: 'Options', sql: 'SELECT 1', dashboard: { role: 'filter' } }]);
+      renderSavedHistory(app);
+      click(app.dom.savedList.querySelector('.saved-row'));
+      expect(app.actions.run).toHaveBeenCalledWith({ view: 'filter' });
+    },
+  );
+
+  it('saved: Filter role takes precedence over a dormant persisted spec.view and Panel config, without touching either (#244)', () => {
+    const app = makeApp();
+    app.state.sidePanel.value = 'saved';
+    setSaved(app, [
+      { id: 'f1', name: 'No persisted view', sql: 'SELECT 1', dashboard: { role: 'filter' } },
+      {
+        id: 'f2', name: 'Dormant Panel view', sql: 'SELECT 1',
+        dashboard: { role: 'filter' }, view: 'panel', panel: { cfg: { type: 'kpi' } },
+      },
+    ]);
+    renderSavedHistory(app);
+    const rows = app.dom.savedList.querySelectorAll('.saved-row');
+    click(rows[0]);
+    expect(app.actions.run).toHaveBeenLastCalledWith({ view: 'filter' });
+    click(rows[1]);
+    expect(app.actions.run).toHaveBeenLastCalledWith({ view: 'filter' }); // role wins, not 'panel'
+    // launch never mutates the saved entry — dormant view/panel survive untouched
+    const dormant = app.state.savedQueries.find((q) => q.id === 'f2');
+    expect(dormant.spec.view).toBe('panel');
+    expect(dormant.spec.panel).toEqual({ cfg: { type: 'kpi' } });
+    expect(dormant.spec.dashboard).toEqual({ role: 'filter' }); // no spec.view:'filter' persisted
+    expect(app.saveJSON).not.toHaveBeenCalled();
+  });
+
+  it('saved: a Filter-role query that cannot auto-run still opens the Filter drawer instead of a dormant Panel view or nothing (#244)', () => {
+    // A Filter-role entry with empty/DDL/multi-statement SQL can't auto-run
+    // (isAutoRunnable is false) — e.g. one hand-authored, imported, or loaded
+    // from localStorage without the SQL-shape validation the Spec editor
+    // enforces. The role must still win the launch view: `SAVED_VIEWS`
+    // deliberately excludes 'filter' (it's never persisted), so this only
+    // opens correctly if the role bypasses that persisted-view check.
+    const app = makeApp();
+    app.state.sidePanel.value = 'saved';
+    app.state.resultView.value = 'table';
+    setSaved(app, [
+      { id: 'f1', name: 'Empty Filter', sql: '', dashboard: { role: 'filter' } },
+      {
+        id: 'f2', name: 'DDL Filter with dormant Panel', sql: 'CREATE TABLE t (a Int8)',
+        dashboard: { role: 'filter' }, view: 'panel', panel: { cfg: { type: 'kpi' } },
+      },
+    ]);
+    renderSavedHistory(app);
+    const rows = app.dom.savedList.querySelectorAll('.saved-row');
+    click(rows[0]);
+    expect(app.actions.run).not.toHaveBeenCalled(); // not auto-runnable
+    expect(app.state.resultView.value).toBe('filter');
+    app.state.resultView.value = 'table'; // reset before the second row
+    click(rows[1]);
+    expect(app.actions.run).not.toHaveBeenCalled();
+    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', () => {
     const app = makeApp();
     app.state.sidePanel.value = 'saved';