diff --git a/CLAUDE.md b/CLAUDE.md index bce62d7..643331c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -371,6 +371,34 @@ and `last_used` still come back from the route; nothing reads them. --- +## Where a browser lands + +`App.jsx` restores `pf_sourceId` / `pf_versionId` from `localStorage` and +re-checks both against the live list. What it falls back to when there is +nothing stored — a new account, a new machine, a cleared browser — is +**`pf.version.is_default`**, one per source by partial unique index +(`version_one_default`), the way `pf.layout.is_default` works. + +It used to fall back to `versions[0]`, and the list is ordered +`created_at DESC`, so that meant *the newest version* — whatever somebody +created last, a scratch copy as readily as the plan. Accounts that never pick a +version themselves, which is every sales account, therefore followed the +forecasting work around. + +**Admin only**, enforced in `PUT /api/versions/:id`: it decides where everyone +else lands, so it follows the `pf.log` rule rather than being the version's own +setting like its name. The checkbox on the Baseline page's **This forecast** +panel is greyed out for other accounts rather than offering a click that +answers 403. The `StatusBar` picker marks the default `(default)`. + +**A stored selection still wins.** The default is the fallback, not an +override — someone who has been working in another version stays there. The +corollary is that an account which already landed in the wrong scenario keeps +landing there until it switches once. + +Setting the default is two statements — clear the old, set the new — so the +route runs them in one transaction; the index refuses the instant in between. + ## Renaming a version Safe at any time, including on a closed one. Nothing derives from the name: the diff --git a/routes/versions.js b/routes/versions.js index 356f191..b7c62b7 100644 --- a/routes/versions.js +++ b/routes/versions.js @@ -446,17 +446,43 @@ ${colDefs}, // the value alone cannot tell from "not mentioned". router.put('/versions/:id', async (req, res) => { const { name, description, exclude_iters, - adjustment_segment, adjustment_bucket, unlabeled_load } = req.body; + adjustment_segment, adjustment_bucket, unlabeled_load, + is_default } = req.body; const set = (v) => (v === undefined ? null : (String(v).trim() || null)); + + // is_default decides where every account without a saved selection + // lands, so it is not the version's own setting the way its name is: + // it follows the pf.log rule, an admin override and nothing else. The + // UI greys the box out rather than offering a click that answers 403. + if (is_default !== undefined && !req.session?.user?.is_admin) { + return res.status(403).json({ + error: 'Only an admin can set the default forecast' + }); + } + + // The write is two statements when the default moves -- clear the old + // one, set the new -- and the partial unique index refuses the moment + // in between, so they share a transaction. + const client = await pool.connect(); try { - const result = await pool.query(` + await client.query('BEGIN'); + if (is_default === true) { + await client.query(` + UPDATE pf.version SET is_default = false + WHERE source_id = (SELECT source_id FROM pf.version WHERE id = $1) + AND id <> $1 + AND is_default + `, [req.params.id]); + } + const result = await client.query(` UPDATE pf.version SET name = COALESCE($2, name), description = COALESCE($3, description), exclude_iters = COALESCE($4, exclude_iters), adjustment_segment = CASE WHEN $5::bool THEN $6::text ELSE adjustment_segment END, adjustment_bucket = CASE WHEN $7::bool THEN $8::text ELSE adjustment_bucket END, - unlabeled_load = CASE WHEN $9::bool THEN $10::text ELSE unlabeled_load END + unlabeled_load = CASE WHEN $9::bool THEN $10::text ELSE unlabeled_load END, + is_default = CASE WHEN $11::bool THEN $12::bool ELSE is_default END WHERE id = $1 RETURNING * `, [ @@ -466,22 +492,35 @@ ${colDefs}, exclude_iters ? JSON.stringify(exclude_iters) : null, adjustment_segment !== undefined, set(adjustment_segment), adjustment_bucket !== undefined, set(adjustment_bucket), - unlabeled_load !== undefined, set(unlabeled_load) + unlabeled_load !== undefined, set(unlabeled_load), + is_default !== undefined, is_default === true ]); if (result.rows.length === 0) { + await client.query('ROLLBACK'); return res.status(404).json({ error: 'Version not found' }); } + await client.query('COMMIT'); res.json(result.rows[0]); } catch (err) { + await client.query('ROLLBACK').catch(() => {}); console.error(err); // (source_id, name) is unique, so a rename can collide. The POST has // said so for as long as it has existed; this said it with a raw // Postgres message, which reads as a server fault rather than a name // that is already taken. if (err.code === '23505') { - return res.status(409).json({ error: 'A version with that name already exists for this source' }); + return res.status(409).json({ + // Two unique constraints raise 23505 here, and the name is + // only one of them -- version_one_default is the other, + // reachable only if two requests set the default at once. + error: err.constraint === 'version_one_default' + ? 'Another forecast was just made the default — reload and try again' + : 'A version with that name already exists for this source' + }); } res.status(500).json({ error: err.message }); + } finally { + client.release(); } }); diff --git a/setup_sql/01_schema.sql b/setup_sql/01_schema.sql index 9577b39..611b3b9 100644 --- a/setup_sql/01_schema.sql +++ b/setup_sql/01_schema.sql @@ -122,6 +122,22 @@ ALTER TABLE pf.version ADD COLUMN IF NOT EXISTS adjustment_segment text; ALTER TABLE pf.version ADD COLUMN IF NOT EXISTS adjustment_bucket text; ALTER TABLE pf.version ADD COLUMN IF NOT EXISTS unlabeled_load text; +-- Where an account lands when it has no saved selection of its own. +-- +-- Without it the app fell back to the newest version, which is whatever +-- somebody created last -- a scratch scenario as readily as the plan. Sales +-- accounts, which never pick a version, therefore followed whatever the +-- forecasting work happened to be doing that morning. +-- +-- At most one per source, by partial unique index, the way pf.layout does it: +-- two defaults is not a state anything could resolve, so the database refuses +-- it rather than the route picking one. +ALTER TABLE pf.version ADD COLUMN IF NOT EXISTS is_default boolean NOT NULL DEFAULT false; + +CREATE UNIQUE INDEX IF NOT EXISTS version_one_default + ON pf.version (source_id) + WHERE is_default; + -- What the entry did, stamped when it did it. -- -- Not a cache: a log entry's forecast rows never change after it is written. diff --git a/ui/src/App.jsx b/ui/src/App.jsx index 940a101..9f1998f 100644 --- a/ui/src/App.jsx +++ b/ui/src/App.jsx @@ -67,11 +67,18 @@ export default function App() { // localStorage is cleared and then set straight back when the fetch lands. // Forecast's load effect is keyed on that id, so the round trip made every // page load fetch and aggregate the whole version twice. + // + // What it falls back TO is the source's default version, set on the Baseline + // page, and only the newest version when nobody has named one. The list is + // ordered created_at DESC, so the old fallback landed an account on whatever + // scenario was created last -- a scratch copy as readily as the plan. Sales + // accounts never pick a version themselves, so that is where they stayed. useEffect(() => { if (!sourceId || !versionsLoaded) return if (versions.length === 0) { setVersionId(''); return } if (!versionId || !versions.some(v => String(v.id) === String(versionId))) { - setVersionId(String(versions[0].id)) + const landing = versions.find(v => v.is_default) || versions[0] + setVersionId(String(landing.id)) } }, [versions, versionsLoaded, sourceId, versionId]) diff --git a/ui/src/components/StatusBar.jsx b/ui/src/components/StatusBar.jsx index a0243df..b7440dd 100644 --- a/ui/src/components/StatusBar.jsx +++ b/ui/src/components/StatusBar.jsx @@ -66,7 +66,7 @@ export default function StatusBar({ view, sources = [], sourceId, setSourceId, v > {versions.length === 0 ? - : versions.map(v => )} + : versions.map(v => )} {selectedVersion && ( diff --git a/ui/src/views/Baseline.jsx b/ui/src/views/Baseline.jsx index 570a51d..1587161 100644 --- a/ui/src/views/Baseline.jsx +++ b/ui/src/views/Baseline.jsx @@ -536,6 +536,23 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio } catch (err) { flash(err.message, 'error') } } + // The default is the source's, not this version's: setting it here clears it + // wherever it was. Admin only, server-side as well as here. + async function saveDefaultVersion(checked) { + try { + const res = await fetch(`/api/versions/${versionId}`, { + method: 'PUT', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ is_default: checked }), + }) + if (!res.ok) { const d = await res.json(); flash(d.error, 'error'); return } + await refreshVersions(sourceId) + flash(checked + ? 'Default forecast — this is where anyone without a saved selection lands' + : 'No default forecast — the newest one is used instead') + } catch (err) { flash(err.message, 'error') } + } + const selectedVersion = versions.find(v => String(v.id) === versionId) return ( @@ -595,6 +612,19 @@ export default function Baseline({ sources = [], sourceId, versions = [], versio placeholder="optional" className="border border-gray-200 rounded px-2 py-1 text-sm w-full" /> + {/* Where an account with no saved selection of its own lands. One per + source, so ticking it here unticks it wherever it was. */} + )}