mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-23 11:03:41 +00:00
fix(web): schema-driven trigger+scope options in create forms (IDEA-619) (#151)
* fix(web): schema-driven trigger+scope options in create forms (IDEA-619) Follow-up to PLAN-609. Non-software templates (hiring, interviewing) ship their own convention + playbook trigger vocabularies via the Conventions and Playbooks collection schemas, but the web UI's CREATE forms on both pages were still iterating hardcoded software- only constants. Users in a non-software workspace could see seeded items (thanks to the display tolerance added in PR #146) but could not CREATE new items with the workspace's own vocabulary via the web UI — only via the CLI. Both conventions and playbooks pages now: - Load their collection schema alongside items (non-blocking — a failed schema load falls back to the hardcoded software constants so the page stays functional offline or against an older server). - Derive `createTriggers` and `createSurfaces`/`createScopes` from the schema's `trigger`/`scope` field `options`, with the hardcoded lists as the backstop. - Drive the create-form `<select>` dropdowns from the derived lists instead of the hardcoded constants. - Snap `newTrigger` / `newSurface` / `newScope` state into the effective list when the schema changes so the select never shows a phantom value. - Use the schema-derived lists as the "known" baseline for the filter dropdowns (`allSurfaces` / `allTriggers` / `allScopes`), still unioned with any trigger/scope values discovered on loaded items (preserves the display tolerance from PR #146). Net effect: in a hiring workspace, the New Convention form's trigger dropdown shows `on-candidate-advance`, `on-offer-extended`, etc.; the scope dropdown shows `sourcing`, `screening`, `interviewing`, `offers`. Interviewing workspace gets its own vocabulary. Software workspaces are unchanged. Closes IDEA-619. * fix(web): guard schema loads against workspace-switch stale responses Per Codex review on PR #151. When a user navigates between workspaces quickly, an earlier api.collections.get(...) call for workspace A might resolve AFTER the user is on workspace B, overwriting the current schema state with A's schema. The create/filter dropdowns would then reflect the wrong workspace's trigger/scope vocabulary. Fix: capture the workspace slug at call time; skip the state assignment if the current workspace has changed by the time the response resolves. Symmetrical guard on the catch branch so a failed call from the previous workspace doesn't null out the current one. Applied to both conventions and playbooks pages. * fix(web): clear schema state before workspace-schema fetch Per Codex review iteration 2 on PR #151. The previous guard only dropped stale responses AFTER they resolved — but while a new workspace's fetch was in flight, the old workspace's schema was still present in state. In that window the create/filter dropdowns showed the previous workspace's vocabulary on the new page, so a user could submit a convention with stale trigger/scope values. Fix: clear conventionsCollection / playbooksCollection to null at the START of loadXCollection, before awaiting the fetch. During the in-flight window, createTriggers/createSurfaces fall back to the hardcoded software defaults — the correct conservative state for a workspace whose schema we haven't observed yet. The existing resolved-response stale guard remains.
This commit is contained in:
xarmian
@@ -1,8 +1,8 @@
|
||||
<script lang="ts">
|
||||
import { page } from '$app/state';
|
||||
import { api } from '$lib/api/client';
|
||||
import type { Item, ItemConventionMetadata, ItemCreate } from '$lib/types';
|
||||
import { parseFields } from '$lib/types';
|
||||
import type { Collection, Item, ItemConventionMetadata, ItemCreate } from '$lib/types';
|
||||
import { parseFields, parseSchema } from '$lib/types';
|
||||
import { toastStore } from '$lib/stores/toast.svelte';
|
||||
import { SvelteSet, SvelteMap } from 'svelte/reactivity';
|
||||
|
||||
@@ -32,6 +32,7 @@
|
||||
let workspace = $derived(page.params.workspace ?? '');
|
||||
let username = $derived(page.params.username ?? '');
|
||||
let conventions = $state<Item[]>([]);
|
||||
let conventionsCollection = $state<Collection | null>(null);
|
||||
let loading = $state(true);
|
||||
let expandedSlug = $state<string | null>(null);
|
||||
let collapsedGroups = new SvelteSet<string>();
|
||||
@@ -50,14 +51,17 @@
|
||||
// Inline create form state
|
||||
let newTitle = $state('');
|
||||
let newCategory = $state<typeof CATEGORIES[number]>('custom');
|
||||
let newTrigger = $state<Trigger>('always');
|
||||
let newSurface = $state<typeof SURFACES[number]>('all');
|
||||
let newTrigger = $state<string>('always');
|
||||
let newSurface = $state<string>('all');
|
||||
let newEnforcement = $state<typeof ENFORCEMENT_LEVELS[number]>('should');
|
||||
let newCommands = $state('');
|
||||
let newContent = $state('');
|
||||
|
||||
$effect(() => {
|
||||
if (workspace) loadConventions(workspace);
|
||||
if (workspace) {
|
||||
loadConventions(workspace);
|
||||
loadConventionsCollection(workspace);
|
||||
}
|
||||
});
|
||||
|
||||
async function loadConventions(ws: string) {
|
||||
@@ -71,20 +75,76 @@
|
||||
}
|
||||
}
|
||||
|
||||
async function loadConventionsCollection(ws: string) {
|
||||
// Clear any previous workspace's schema before the fetch. Until the new
|
||||
// response lands, createTriggers/createSurfaces fall back to the
|
||||
// hardcoded software defaults — correct for a workspace whose schema
|
||||
// we have not yet observed. This prevents the in-flight window from
|
||||
// rendering the previous workspace's vocabulary on the new page.
|
||||
conventionsCollection = null;
|
||||
try {
|
||||
const coll = await api.collections.get(ws, 'conventions');
|
||||
// Stale-response guard: if the user has since moved to another
|
||||
// workspace, drop the result rather than overwriting state with
|
||||
// schema from a workspace we are no longer on.
|
||||
if (ws !== workspace) return;
|
||||
conventionsCollection = coll;
|
||||
} catch {
|
||||
if (ws !== workspace) return;
|
||||
conventionsCollection = null;
|
||||
}
|
||||
}
|
||||
|
||||
let schemaTriggers = $derived.by<readonly string[]>(() => {
|
||||
if (!conventionsCollection) return [];
|
||||
const schema = parseSchema(conventionsCollection);
|
||||
const field = schema.fields.find((f) => f.key === 'trigger');
|
||||
return field?.options ?? [];
|
||||
});
|
||||
|
||||
let schemaSurfaces = $derived.by<readonly string[]>(() => {
|
||||
if (!conventionsCollection) return [];
|
||||
const schema = parseSchema(conventionsCollection);
|
||||
const field = schema.fields.find((f) => f.key === 'scope');
|
||||
return field?.options ?? [];
|
||||
});
|
||||
|
||||
let createTriggers = $derived<readonly string[]>(
|
||||
schemaTriggers.length > 0 ? schemaTriggers : (TRIGGERS as readonly string[])
|
||||
);
|
||||
let createSurfaces = $derived<readonly string[]>(
|
||||
schemaSurfaces.length > 0 ? schemaSurfaces : (SURFACES as readonly string[])
|
||||
);
|
||||
|
||||
// When schema-driven options load (or change), snap the create-form selections
|
||||
// into the effective list. This prevents the <select> from showing a phantom
|
||||
// value that isn't actually in its <option>s.
|
||||
$effect(() => {
|
||||
if (createTriggers.length > 0 && !createTriggers.includes(newTrigger)) {
|
||||
newTrigger = createTriggers[0];
|
||||
}
|
||||
});
|
||||
$effect(() => {
|
||||
if (createSurfaces.length > 0 && !createSurfaces.includes(newSurface)) {
|
||||
newSurface = createSurfaces[0];
|
||||
}
|
||||
});
|
||||
|
||||
let hasActiveFilters = $derived(searchQuery !== '' || filterScope !== '' || filterPriority !== '');
|
||||
|
||||
// Expose the union of SURFACES plus any scopes discovered on loaded items,
|
||||
// so filter dropdowns show scopes from non-software templates (e.g. hiring's
|
||||
// sourcing/screening/interviewing/offers). Create form still uses narrow SURFACES.
|
||||
// Expose the union of the effective create-form surfaces plus any scopes
|
||||
// discovered on loaded items, so filter dropdowns show scopes from
|
||||
// non-software templates (e.g. hiring's sourcing/screening/interviewing/offers).
|
||||
let allSurfaces = $derived.by(() => {
|
||||
const known = new Set<string>(SURFACES as readonly string[]);
|
||||
const base = createSurfaces;
|
||||
const known = new Set<string>(base);
|
||||
const extra = new Set<string>();
|
||||
for (const c of conventions) {
|
||||
const s = getPrimarySurface(c);
|
||||
if (s && !known.has(s)) extra.add(s);
|
||||
}
|
||||
return [
|
||||
...(SURFACES as readonly string[]),
|
||||
...base,
|
||||
...Array.from(extra).sort(),
|
||||
];
|
||||
});
|
||||
@@ -113,7 +173,7 @@
|
||||
if (!byTrigger.has(t)) byTrigger.set(t, []);
|
||||
byTrigger.get(t)!.push(item);
|
||||
}
|
||||
const knownOrder = TRIGGERS as readonly string[];
|
||||
const knownOrder = createTriggers;
|
||||
const extraTriggers = Array.from(byTrigger.keys())
|
||||
.filter((t) => !knownOrder.includes(t))
|
||||
.sort((a, b) => a.localeCompare(b));
|
||||
@@ -349,15 +409,16 @@
|
||||
<label class="form-field">
|
||||
<span>Trigger</span>
|
||||
<select bind:value={newTrigger}>
|
||||
{#each TRIGGERS as t (t)}
|
||||
<option value={t}>{TRIGGER_META[t].icon} {TRIGGER_META[t].label}</option>
|
||||
{#each createTriggers as t (t)}
|
||||
{@const meta = triggerMeta(t)}
|
||||
<option value={t}>{meta.icon} {meta.label}</option>
|
||||
{/each}
|
||||
</select>
|
||||
</label>
|
||||
<label class="form-field">
|
||||
<span>Surface</span>
|
||||
<select bind:value={newSurface}>
|
||||
{#each SURFACES as s (s)}
|
||||
{#each createSurfaces as s (s)}
|
||||
<option value={s}>{s}</option>
|
||||
{/each}
|
||||
</select>
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
import { page } from '$app/state';
|
||||
import { goto } from '$app/navigation';
|
||||
import { api } from '$lib/api/client';
|
||||
import { parseFields, itemUrlId, type Item } from '$lib/types';
|
||||
import { parseFields, parseSchema, itemUrlId, type Collection, type Item } from '$lib/types';
|
||||
import { toastStore } from '$lib/stores/toast.svelte';
|
||||
|
||||
const TRIGGERS = ['on-implement', 'on-triage', 'on-release', 'on-plan', 'on-review', 'on-deploy', 'manual'] as const;
|
||||
@@ -11,6 +11,7 @@
|
||||
let wsSlug = $derived(page.params.workspace ?? '');
|
||||
let username = $derived(page.params.username ?? '');
|
||||
let playbooks = $state<Item[]>([]);
|
||||
let playbooksCollection = $state<Collection | null>(null);
|
||||
let loading = $state(true);
|
||||
let expandedId = $state<string | null>(null);
|
||||
let showNewForm = $state(false);
|
||||
@@ -26,7 +27,12 @@
|
||||
let newScope = $state<string>('all');
|
||||
let newContent = $state('');
|
||||
|
||||
$effect(() => { if (wsSlug) loadPlaybooks(wsSlug); });
|
||||
$effect(() => {
|
||||
if (wsSlug) {
|
||||
loadPlaybooks(wsSlug);
|
||||
loadPlaybooksCollection(wsSlug);
|
||||
}
|
||||
});
|
||||
async function loadPlaybooks(ws: string) {
|
||||
loading = true;
|
||||
try { playbooks = await api.items.listByCollection(ws, 'playbooks', {}); }
|
||||
@@ -34,24 +40,80 @@
|
||||
finally { loading = false; }
|
||||
}
|
||||
|
||||
async function loadPlaybooksCollection(ws: string) {
|
||||
// Clear any previous workspace's schema before the fetch. Until the
|
||||
// new response lands, createTriggers/createScopes fall back to the
|
||||
// hardcoded software defaults — correct for a workspace whose schema
|
||||
// we have not yet observed. Prevents the in-flight window from
|
||||
// rendering the previous workspace's vocabulary on the new page.
|
||||
playbooksCollection = null;
|
||||
try {
|
||||
const coll = await api.collections.get(ws, 'playbooks');
|
||||
// Stale-response guard: if the user has since moved to another
|
||||
// workspace, drop the result rather than overwriting state with
|
||||
// schema from a workspace we are no longer on.
|
||||
if (ws !== wsSlug) return;
|
||||
playbooksCollection = coll;
|
||||
} catch {
|
||||
if (ws !== wsSlug) return;
|
||||
playbooksCollection = null;
|
||||
}
|
||||
}
|
||||
|
||||
let schemaTriggers = $derived.by<readonly string[]>(() => {
|
||||
if (!playbooksCollection) return [];
|
||||
const schema = parseSchema(playbooksCollection);
|
||||
const field = schema.fields.find((f) => f.key === 'trigger');
|
||||
return field?.options ?? [];
|
||||
});
|
||||
|
||||
let schemaScopes = $derived.by<readonly string[]>(() => {
|
||||
if (!playbooksCollection) return [];
|
||||
const schema = parseSchema(playbooksCollection);
|
||||
const field = schema.fields.find((f) => f.key === 'scope');
|
||||
return field?.options ?? [];
|
||||
});
|
||||
|
||||
let createTriggers = $derived<readonly string[]>(
|
||||
schemaTriggers.length > 0 ? schemaTriggers : (TRIGGERS as readonly string[])
|
||||
);
|
||||
let createScopes = $derived<readonly string[]>(
|
||||
schemaScopes.length > 0 ? schemaScopes : (SCOPES as readonly string[])
|
||||
);
|
||||
|
||||
// Snap the create-form selections into the effective list when it changes,
|
||||
// so the <select> never displays a phantom value that isn't in its <option>s.
|
||||
$effect(() => {
|
||||
if (createTriggers.length > 0 && !createTriggers.includes(newTrigger)) {
|
||||
newTrigger = createTriggers[0];
|
||||
}
|
||||
});
|
||||
$effect(() => {
|
||||
if (createScopes.length > 0 && !createScopes.includes(newScope)) {
|
||||
newScope = createScopes[0];
|
||||
}
|
||||
});
|
||||
|
||||
let hasActiveFilters = $derived(searchQuery !== '' || filterTrigger !== '' || filterScope !== '');
|
||||
|
||||
let allTriggers = $derived.by(() => {
|
||||
const known = TRIGGERS as readonly string[];
|
||||
const known = createTriggers;
|
||||
const knownSet = new Set<string>(known);
|
||||
const discovered = new Set<string>();
|
||||
for (const p of playbooks) {
|
||||
const t = parseFields(p).trigger;
|
||||
if (typeof t === 'string' && t && !known.includes(t)) discovered.add(t);
|
||||
if (typeof t === 'string' && t && !knownSet.has(t)) discovered.add(t);
|
||||
}
|
||||
return [...known, ...Array.from(discovered).sort((a, b) => a.localeCompare(b))];
|
||||
});
|
||||
|
||||
let allScopes = $derived.by(() => {
|
||||
const known = SCOPES as readonly string[];
|
||||
const known = createScopes;
|
||||
const knownSet = new Set<string>(known);
|
||||
const discovered = new Set<string>();
|
||||
for (const p of playbooks) {
|
||||
const s = parseFields(p).scope;
|
||||
if (typeof s === 'string' && s && !known.includes(s)) discovered.add(s);
|
||||
if (typeof s === 'string' && s && !knownSet.has(s)) discovered.add(s);
|
||||
}
|
||||
return [...known, ...Array.from(discovered).sort((a, b) => a.localeCompare(b))];
|
||||
});
|
||||
@@ -184,13 +246,13 @@
|
||||
<div class="form-row">
|
||||
<label class="form-label" for="pb-trigger">Trigger</label>
|
||||
<select id="pb-trigger" bind:value={newTrigger} class="form-select">
|
||||
{#each TRIGGERS as t (t)}<option value={t}>{t}</option>{/each}
|
||||
{#each createTriggers as t (t)}<option value={t}>{t}</option>{/each}
|
||||
</select>
|
||||
</div>
|
||||
<div class="form-row">
|
||||
<label class="form-label" for="pb-scope">Scope</label>
|
||||
<select id="pb-scope" bind:value={newScope} class="form-select">
|
||||
{#each SCOPES as s (s)}<option value={s}>{s}</option>{/each}
|
||||
{#each createScopes as s (s)}<option value={s}>{s}</option>{/each}
|
||||
</select>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
Reference in New Issue
Block a user