diff --git a/api/databases.ts b/api/databases.ts index f11cbd5..0cedab0 100644 --- a/api/databases.ts +++ b/api/databases.ts @@ -141,7 +141,25 @@ export async function saveDatabases(production: string, development: string): Pr * a name that failed for a different reason, and reporting "created" about a database that is not * there is the one outcome worth engineering against. */ -export async function createDatabases(): Promise<{ created: string[]; status: DatabasesStatus }> { +export async function createDatabases(names?: { + production?: string; + development?: string; +}): Promise<{ created: string[]; status: DatabasesStatus }> { + /* + Names may arrive with the request, and then they are SAVED before anything is created. + + Because otherwise Create acts on stored names while the person is looking at typed ones, and the + two can differ. Making them press Save first was the version of this that shipped, and it left a + screen with two filled fields, no Create button and nothing explaining why — the button keyed off + the saved names, which were still empty. + + Recording the name you just used to create a database is not a side effect worth hiding behind a + second click. + */ + if (names?.production !== undefined || names?.development !== undefined) { + await saveDatabases(names.production ?? '', names.development ?? ''); + } + const status = await readDatabases(); const wanted = [status.production.name, status.development.name].filter((name): name is string => Boolean(name)); if (!wanted.length) throw new Error('Name the databases before creating them.'); diff --git a/api/router.ts b/api/router.ts index 74722ce..d02e21a 100644 --- a/api/router.ts +++ b/api/router.ts @@ -8,7 +8,7 @@ import { readMediaStatus, createMediaDir } from './media'; import { readSync, startSync } from './sync'; import { readRuns } from './ledger'; import { reconcile } from './derive'; -import { createDatabases, readDatabases, saveDatabases } from './databases'; +import { createDatabases, readDatabases } from './databases'; import { fofosRequest, FofosError, probe, type FofosResponse } from './fofos-client'; /** @@ -287,24 +287,18 @@ router.get('/_config/media/sync', async (ctx) => { /** * The two databases the site reads. Owner-only, like everything else under `/_config`. * - * Saving and creating are separate routes because they are separate decisions: a name can be stored - * for a database that already exists and needs no creating, and pressing Create must never be the only - * way to record what you typed. + * ONE write route, not two. There was a `PUT` that only stored the names, on the reasoning that naming + * and creating are separate decisions — but the panel has one button, so nothing called it, and a + * second way to write the same two columns is a second thing that can drift from the first. */ router.get('/_config/databases', async (ctx) => ctx.json(await readDatabases())); -router.put('/_config/databases', async (ctx) => { +router.post('/_config/databases/create', async (ctx) => { + // The names come with the request, so what gets created is what is on screen rather than what was + // stored at some earlier point. They are saved first, by the same validation as the PUT above. const { production, development } = (ctx.get('body') ?? {}) as { production?: string; development?: string }; try { - return ctx.json(await saveDatabases(production ?? '', development ?? '')); - } catch (error) { - return ctx.json({ error: (error as Error).message }, 400); - } -}); - -router.post('/_config/databases/create', async (ctx) => { - try { - return ctx.json(await createDatabases()); + return ctx.json(await createDatabases({ production, development })); } catch (error) { return ctx.json({ error: (error as Error).message }, 400); } diff --git a/web/DatabaseBlock.tsx b/web/DatabaseBlock.tsx index c353da9..f74e882 100644 --- a/web/DatabaseBlock.tsx +++ b/web/DatabaseBlock.tsx @@ -10,22 +10,23 @@ import { useFlash } from './useFlash'; /** * Where the fofos site's own data lives — two databases, named here. * - * ── Naming and creating are separate ── + * ── ONE button ── * - * Save stores what you typed; Create makes whichever of the two is not there yet. They are different - * decisions and were worth two buttons: a database that already exists still needs its name recorded, - * and making Create the only way to save would mean nobody could point the site at something that was - * already set up. + * It shipped with two — Save, then Create — on the reasoning that naming and creating are different + * decisions. They are not, from the outside: you type two names because you want two databases. Worse, + * Create keyed off the SAVED names, so on a fresh screen with both fields filled it did not appear at + * all, and the hint explaining why was behind the same condition. Two filled fields, one Save button, + * and no way to tell what came next. * - * ── It reports what is THERE, per name ── + * So one button that records the names and creates whatever is not there, labelled with whichever of + * those it is actually about to do. Recording the name you just used to make a database is not a side + * effect worth a second click. * - * A tick or a dashed circle beside each field, from `pg_database` rather than from what was saved. The - * useful question about a database name is not whether somebody typed it — it is whether the thing it - * names exists, and those two came apart the moment saving stopped creating. + * ── It reports what is THERE, against what is TYPED ── * - * ── Create only offers itself when there is something to create ── - * - * Both present and the button goes. A button that does nothing is a thing to wonder about later. + * The tick beside each field asks `pg_database` whether a database of that name exists, and it asks + * about the text in the box rather than the text that was last saved. That is the question somebody + * typing a name actually has, and answering it about a different name would be worse than silence. */ type Named = { name: string | null; exists: boolean | null }; @@ -60,13 +61,16 @@ export const DatabaseBlock = () => { void load(); }, [load]); - const save = async () => { + /** Records the names and creates whichever is missing. The server does both, in that order. */ + const apply = async () => { setBusy(true); setError(null); try { - setStatus( - await clientRef.current.put('/fofos-admin/_config/databases', { production, development }), + const result = await clientRef.current.post<{ created: string[]; status: Status }>( + '/fofos-admin/_config/databases/create', + { production, development }, ); + setStatus(result.status); flash(); } catch (failure) { setError(readableError(failure)); @@ -75,33 +79,11 @@ export const DatabaseBlock = () => { } }; - const create = async () => { - setBusy(true); - setError(null); - try { - const result = await clientRef.current.post<{ created: string[]; status: Status }>( - '/fofos-admin/_config/databases/create', - {}, - ); - setStatus(result.status); - } catch (failure) { - setError(readableError(failure)); - } finally { - setBusy(false); - } - }; - - const field = ( - label: string, - hint: string, - value: string, - onChange: (next: string) => void, - state: Named | undefined, - ) => { - // Only about what is SAVED. A tick next to a name somebody is still typing would be about a - // different database from the one on screen. - const settled = state?.name === value.trim() ? state : undefined; - const Icon = settled?.exists ? CircleCheck : CircleDashed; + const field = (label: string, hint: string, value: string, onChange: (next: string) => void) => { + // Against the typed text and the real list from `pg_database`. Nothing to say about an empty box. + const typed = value.trim(); + const exists = typed ? (status?.available.includes(typed) ?? false) : null; + const Icon = exists ? CircleCheck : CircleDashed; return (
@@ -119,12 +101,12 @@ export const DatabaseBlock = () => { spellCheck={false} /> - {settled ? ( + {exists === null ? null : ( <> - - {settled.exists ? 'exists' : 'not yet'} + + {exists ? 'exists' : 'not yet'} - ) : null} + )}
@@ -134,8 +116,12 @@ export const DatabaseBlock = () => { const dirty = production.trim() !== (status?.production.name ?? '') || development.trim() !== (status?.development.name ?? ''); - // From the SAVED names, not the typed ones — Create acts on what is stored. - const missing = [status?.production, status?.development].filter((entry) => entry?.name && entry.exists === false); + // Typed, non-empty, and not on the instance. What the one button is about to make. + const missing = [production.trim(), development.trim()].filter( + (name) => name && !(status?.available.includes(name) ?? false), + ); + + const label = missing.length ? `Create ${missing.length === 1 ? missing[0] : 'databases'}` : 'Save names'; return ( { > {!status ? null : ( <> - {field('Production', 'what the live site reads', production, setProduction, status.production)} - {field('Development', 'what you work against', development, setDevelopment, status.development)} + {field('Production', 'what the live site reads', production, setProduction)} + {field('Development', 'what you work against', development, setDevelopment)} {error &&

{error}

} -
- - - {/* Only when a saved name has nothing behind it. Nothing to create, no button. */} - {missing.length > 0 && ( - - )} - - {/* Save first. Create acts on what is stored, and a button that quietly used a different - value from the one in the field would be the worst kind of surprise here. */} - {missing.length > 0 && dirty && ( - Save the names first. - )} + ) : ( + + )} + {label} +
)}