diff --git a/CHANGELOG.md b/CHANGELOG.md index 88dc52f..6e25f4b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,11 +2,12 @@ ## Unreleased +- Payload plugin: people are recorded by id, never by name (changes the unreleased proposals, before anyone used them). `proposals.nameOf` is gone. `proposals.personId({ req })` gives an opaque id for the signed-in person (default: their user document's id), and that id is all a proposal stores about its author and all an accepted proposal seals into the new snapshot for the author and the manager. An id is 1 to 64 letters, digits, `.`, `_`, `:` and `-`, starting with a letter or digit, so an email address or a name with a space in it is refused: the action stops, nothing is saved, and the refused value isn't written to the log. A one-word handle would pass; the plugin can't tell it from an id. `proposals.displayName({ req, personId })` is optional and gives the name to show: it is looked up each time a proposal is read (once per person per request), returned as `authorName` on the proposal and in the preview, and never stored or sealed, so a name changed or removed at the site changes what is shown at once and leaves every snapshot's checksum alone. Why: a name inside a sealed snapshot can't be taken out when a person leaves. Core is unchanged apart from its comments and one error text: `acceptIntoSnapshot` still seals the `author` and `acceptedBy` strings it is given. Still sealed as written, and not screened: a proposal's title, each move's note and the manager's note. No endpoint turns an id from a snapshot's record back into a name. Tested on Payload 3.93 with an in-memory MongoDB (142 plugin tests, 526 in all) and through the example site (47 checks); eight deliberate breaks of the new code each failed a test. Not tested: Postgres or SQLite, and a host with more than one auth collection. - React: names in the drawing are no longer struck through or covered. In rectangular layouts an inner node's name was drawn 6 units above the node, so when its children's rows were one row apart, a child's branch ran straight through it ("Reptilia" in the amniotes classes view). The name now goes in the first of three places the branches to its children leave clear: on the node's own row, just above it, or just below it; when rows are closer than a name is tall, it takes the place with the most room. A folded branch's name started 8 units out, on top of its 12-unit triangle; it now starts 16 units out, past it. The SVG and the canvas use the same rule (`innerLabelAt`, `leafLabelX`), so a name is in the same place in both. A leaf's name hasn't moved. Limits: a long name can still cross a branch further down the tree, since only the node's own children are looked at, and in a radial drawing a folded branch's triangle still points right whatever the branch's angle. -- Example site: `apps/payload-example`, a small Payload 3.93 and Next.js 16 site that uses the Payload plugin (private, never published). It runs on this machine only, on a throwaway in-memory MongoDB that `pnpm dev` starts and seeds: two made-up accounts with a password made up per run, one published snapshot with two views, and one staged snapshot, all from the repository's illustrative fixtures. Accounts have two roles: a curator manages and decides, a member reads and proposes, only a curator hands out roles, and a member sees no account but their own. `proposals.nameOf` gives a public `handle`, not the email address, because the names in an accepted version can't be taken out later. The public pages are Server Components that read as an anonymous visitor and draw each published view with `TreeView` and `TreeOutline`, with what the view leaves out and its sources; a source's address is a link only when it is http or https. `pnpm smoke` builds the site, starts it on a free port and runs 46 checks over HTTP with sign-in cookies: what a visitor reads and can't do through the pages, REST and GraphQL; cookies that count only from the site's own pages; roles; a proposal made, shared, accepted into a staged, sealed version that names both people by handle and holds no email address. CI runs it, and the CI workflow now cancels a run when a newer push arrives and stops a job after 20 minutes. Passed on `next dev` and on the built site on a Mac; the pages were looked at in headless Chrome in light and dark. Not tested: a person clicking through the admin, a screen reader, Postgres or SQLite, a deployed site. The site has no pages for proposals or imports. +- Example site: `apps/payload-example`, a small Payload 3.93 and Next.js 16 site that uses the Payload plugin (private, never published). It runs on this machine only, on a throwaway in-memory MongoDB that `pnpm dev` starts and seeds: two made-up accounts with a password made up per run, one published snapshot with two views, and one staged snapshot, all from the repository's illustrative fixtures. Accounts have two roles: a curator manages and decides, a member reads and proposes, only a curator hands out roles, and a member sees no account but their own. Proposals record people by account id, and `proposals.displayName` looks up a public `handle`, not the email address, when one is shown. The public pages are Server Components that read as an anonymous visitor and draw each published view with `TreeView` and `TreeOutline`, with what the view leaves out and its sources; a source's address is a link only when it is http or https. `pnpm smoke` builds the site, starts it on a free port and runs 47 checks over HTTP with sign-in cookies: what a visitor reads and can't do through the pages, REST and GraphQL; cookies that count only from the site's own pages; roles; a proposal made, shared, accepted into a staged, sealed version that records both people by id and holds no handle and no email address. CI runs it, and the CI workflow now cancels a run when a newer push arrives and stops a job after 20 minutes. Passed on `next dev` and on the built site on a Mac; the pages were looked at in headless Chrome in light and dark. Not tested: a person clicking through the admin, a screen reader, Postgres or SQLite, a deployed site. The site has no pages for proposals or imports. - React: the package can be imported from a Server Component (Next.js App Router). `CanvasTreeView.tsx`, `Phylo.tsx` and `toolbar.ts` call client hooks and now say `'use client'`; without it, importing anything from the package in a Server Component failed. `layoutFor` moved out of the explorer's client file into its own, so a server can call it, and `TreeView` renders there as SVG with no JavaScript sent. The plain functions still inside client files (`rootedRecord`, `proposeTipLinks`, `linkedPhylogeny`, `supportSummary`, `tipsUnder`, `cameraBox`, the canvas helpers) can only be called in the browser for now; the README lists them. A test pins which files are client modules. Seen working in Next.js 16 through `apps/payload-example`; a tree large enough for the canvas hasn't been tried from a Server Component. - Payload plugin: tests over HTTP. Twelve new tests send real requests through Payload's own REST router (`handleEndpoints`), signed in with a token from `/api/users/login`: every plugin endpoint is routed and reads its parameters from the URL and the body, the standard routes list a reader only their tenant's published snapshots, another tenant and an anonymous or bad-token request get 404 or 403 and no data, a read route takes no write, a new proposal is its sender's whatever the body says, and accept and decline work from a request body. No code changed. Still not run in a real site behind a web server. -- Payload plugin, part 4 of M7: proposals (still not on npm). With the new `proposals` option, a `taxonomy-proposals` collection holds each signed-in person's own proposals over a snapshot. A proposal is private until its author shares it with the people in their tenant who can read; anonymous visitors never see one. Only the author edits it, and only while it is private. A manager accepts or declines a shared proposal through `POST /api/taxonomy-proposals/:id/accept` and `/decline`, and anyone who can read it gets what it would change from `GET …/:id/preview`. Accepting names the new version and the proposal's checksum as last seen, so a proposal that changed in between is refused. It makes a **new staged snapshot version** and never edits the published one; readers see nothing until a manager publishes it. Two managers accepting at once store one version. The author, the snapshot, the tenant and the decision can't be set through an update, by server code either, and an accepted proposal can't be changed or deleted. Deleting the staged version before it is published opens the proposal again. A third access callback, `access.propose`, says who may propose (default: signed-in users). `proposals.nameOf` gives the names an accepted version carries. **Those names are sealed into the snapshot and can't be taken out later**, so a site should give a public handle, not an email address. Limits: each acceptance is made from the published snapshot alone; without database transactions a crash between the two writes can leave a staged version to delete by hand; there is no interface yet (a proposal is a JSON field in the admin). ADR 0012, point 15. Tested on Payload 3.93 with an in-memory MongoDB, with and without transactions (127 plugin tests); not through Payload's HTTP routing, in a real site, or on Postgres or SQLite. +- Payload plugin, part 4 of M7: proposals (still not on npm). With the new `proposals` option, a `taxonomy-proposals` collection holds each signed-in person's own proposals over a snapshot. A proposal is private until its author shares it with the people in their tenant who can read; anonymous visitors never see one. Only the author edits it, and only while it is private. A manager accepts or declines a shared proposal through `POST /api/taxonomy-proposals/:id/accept` and `/decline`, and anyone who can read it gets what it would change from `GET …/:id/preview`. Accepting names the new version and the proposal's checksum as last seen, so a proposal that changed in between is refused. It makes a **new staged snapshot version** and never edits the published one; readers see nothing until a manager publishes it. Two managers accepting at once store one version. The author, the snapshot, the tenant and the decision can't be set through an update, by server code either, and an accepted proposal can't be changed or deleted. Deleting the staged version before it is published opens the proposal again. A third access callback, `access.propose`, says who may propose (default: signed-in users). An accepted version records its author and the manager by person id, not by name (see the entry above). Limits: each acceptance is made from the published snapshot alone; without database transactions a crash between the two writes can leave a staged version to delete by hand; there is no interface yet (a proposal is a JSON field in the admin). ADR 0012, point 15. Tested on Payload 3.93 with an in-memory MongoDB, with and without transactions (127 plugin tests at the time); not through Payload's HTTP routing, in a real site, or on Postgres or SQLite. - Core: `acceptIntoSnapshot(base, proposal, { version, createdAt, acceptedBy, note })` turns a proposal on a classification into a new, unsealed snapshot version. The base isn't changed. The classification's version becomes `+`, each moved placement names the new source record `proposal:` as its provenance with where it was before, that record says who proposed and who accepted, and `derivedFrom` pins the base (id, version, checksum) and the proposal (new `derivedFrom.proposal`, type `AcceptedProposal`, in the JSON Schema too). It refuses a base that isn't sealed or fails its checksum, the base's own version, a proposal that moves nothing or was already accepted into the dataset, anything `applyToClassification` refuses, and every proposal on a phylogeny: a tree rearranged by hand is a result its study never found, so a new analysis is imported as its own hypothesis instead. `proposalSourceId` is exported. - Docs: the root README and the limitations page said the packages weren't on npm. The five `@captiveorigins/onetaxonomy-*` packages are, as alpha versions; the Payload plugin isn't. - OneOrigin brand copy: round joins (owner decision N169, 2026-11-11): the copy in `brand/oneorigin/` is updated from OnePhylogeny's `docs/oneorigin/` at `189c49f`. The guidelines, the brand text and the shared connector icon part now say connectors have round joins. Only the copied brand files and `brand/oneorigin.lock` change; this tool's own icon and drawings are untouched (only OnePhylogeny's icon was redrawn). Checked with `node scripts/oneorigin.mjs check`. diff --git a/README.md b/README.md index 5b7aac6..4b2f2a0 100644 --- a/README.md +++ b/README.md @@ -27,7 +27,7 @@ The full rules are in [scientific invariants](docs/architecture/scientific-invar | [`@captiveorigins/onetaxonomy-education`](packages/education) | Lesson decks (scenes, cameras, annotations, reveals) and worksheets with answers computed from the data | Working | | [`@captiveorigins/payload-plugin-onetaxonomy`](packages/payload) | Payload plugin: sealed snapshots checked by checksum, staged and published, views pinned to them, import jobs that turn source files into staged snapshots, people's own proposals that a curator can accept into a new staged version, and a bounded read API (name search, ancestors, descendants, MRCA, coverage, projected views, exports) behind the host's access rules, with optional tenant scope | Early (M7 parts 1 to 4): tested on Payload with an in-memory MongoDB, not yet in a real site. Not on npm | | [`apps/studio`](apps/studio) | OneTaxonomy for Mac (Tauri 2): open checklists, Darwin Core Archives, Newick, NEXUS and phyloXML trees and snapshots; browse them; collapse weak branches, add a tree to an open checklist and link its tips to taxa (proposed by exact name, confirmed by you), compare two trees, and reroot or midpoint-root as a new tree; save views and sealed snapshots (never over the file that was opened), and save and open `.onetaxonomy` bundles (a documented ZIP with a manifest and checksums) | 0.1.0 on GitHub Releases. Not notarized, and it can't update itself yet | -| [`apps/payload-example`](apps/payload-example) | A small Payload and Next.js site that uses the plugin: two roles, public pages that draw published views on the server, and proposals under a public handle. Runs on your machine with a throwaway in-memory database | An example. Checked over HTTP by its smoke test; nobody has clicked through its admin yet | +| [`apps/payload-example`](apps/payload-example) | A small Payload and Next.js site that uses the plugin: two roles, public pages that draw published views on the server, and proposals that record people by id, with a public handle shown beside it. Runs on your machine with a throwaway in-memory database | An example. Checked over HTTP by its smoke test; nobody has clicked through its admin yet | ## Try it diff --git a/apps/payload-example/README.md b/apps/payload-example/README.md index c7c7246..f7f12af 100644 --- a/apps/payload-example/README.md +++ b/apps/payload-example/README.md @@ -1,14 +1,14 @@ # OneTaxonomy Payload example -A small Payload and Next.js site that uses [`@captiveorigins/payload-plugin-onetaxonomy`](../../packages/payload). It shows the plugin set up the way a real site would: accounts with two roles, public pages that draw published views, and proposals that carry a public handle instead of an email address. +A small Payload and Next.js site that uses [`@captiveorigins/payload-plugin-onetaxonomy`](../../packages/payload). It shows the plugin set up the way a real site would: accounts with two roles, public pages that draw published views, and proposals that record a person by an id, with a public handle shown beside it and no email address. -> **Status: an example, not a product.** It runs on this machine only, on a throwaway in-memory MongoDB, with made-up accounts and the repository's illustrative fixtures. Tested: 46 checks over HTTP with sign-in cookies (`pnpm smoke`), on `next dev` and on the built site, with Payload 3.93 and Next.js 16; the three public pages were looked at in headless Chrome, in light and dark, at desktop width and at 520 px. Not tested: a person clicking through the admin, a screen reader, a phone, Postgres or SQLite, or a deployed site. +> **Status: an example, not a product.** It runs on this machine only, on a throwaway in-memory MongoDB, with made-up accounts and the repository's illustrative fixtures. Tested: 47 checks over HTTP with sign-in cookies (`pnpm smoke`), on `next dev` and on the built site, with Payload 3.93 and Next.js 16; the three public pages were looked at in headless Chrome, in light and dark, at desktop width and at 520 px. Not tested: a person clicking through the admin, a screen reader, a phone, Postgres or SQLite, or a deployed site. ## What it shows - **Public pages.** `/` lists the published views. `/views/` draws one as an SVG tree with its keyboard outline, what the view leaves out, and its sources. Both are Server Components: the tree is drawn on the server and no tree code is sent to the browser. The pages read as an anonymous visitor, so they only ever show what is published. - **Two roles.** A curator manages snapshots, views and import jobs, uses the admin, and decides on proposals. A member reads what is published and makes proposals. Only a curator hands out roles: a member can't raise their own, and sees no account but their own. -- **Proposals under a handle.** `proposals.nameOf` gives each person's `handle`, not their email address. When a curator accepts a proposal, the new snapshot version names the proposer and the curator by handle, inside its checksum. Those names can't be taken out later, which is why the handle field says so where people fill it in. +- **Proposals by id, with a handle shown.** A proposal stores its author's account id, and when a curator accepts one, the new snapshot version records the proposer and the curator by those ids, inside its checksum. No name or email address is sealed. `proposals.displayName` looks up each person's `handle` when a proposal is shown, so a changed handle shows at once and a removed one leaves the id alone. - **Staged, then published.** The seed stores one published snapshot with two views, and one staged snapshot that only the curator sees. An accepted proposal makes a new staged version; visitors see nothing until the curator publishes it. - **Cookies from this site only.** Payload's `csrf` list holds the site's own address, so a sign-in cookie sent from another site's page counts for nothing. @@ -52,8 +52,9 @@ This builds the site, starts it on a free port with its own database, and sends - a visitor can't store a snapshot, make a proposal, or list accounts or proposals; - a sign-in cookie can't be read by scripts, and counts for nothing from another site; - a member can't make themself a curator, sees only their own account, and is sent away from the admin; -- a member's proposal is stored under their handle whatever the request says, with no email address in it, and stays private until shared; -- only the curator can accept it, and the new version is staged, sealed, names both people by handle, holds no email address, and isn't public; +- a member's proposal is stored under their account's id whatever the request says, shows their handle, has no email address in it, and stays private until shared; +- only the curator can accept it, and the new version is staged, sealed, records both people by id, holds no handle and no email address, and isn't public; +- a changed handle shows on the accepted proposal at once, and the sealed version doesn't change; - an accepted proposal can't be deleted. `--dev` runs `next dev` instead of a build, and `--skip-build` reuses the last build. CI runs the smoke test on every pull request. diff --git a/apps/payload-example/payload.config.ts b/apps/payload-example/payload.config.ts index 95a3c06..425f745 100644 --- a/apps/payload-example/payload.config.ts +++ b/apps/payload-example/payload.config.ts @@ -31,8 +31,13 @@ export default buildConfig({ }, locale: { default: 'en' }, proposals: { - // The public handle, never the email address: an accepted proposal keeps this name for good. - nameOf: ({ req }) => String((req.user as { handle?: unknown } | null)?.handle ?? ''), + // `personId` is left at its default, the account's id: that is all a proposal stores and all an accepted one + // seals. The handle is looked up here each time a proposal is shown. A member can't read other accounts, so + // this read is trusted and asks for the handle alone, never the email address. + displayName: async ({ req, personId }) => { + const person = await req.payload.findByID({ collection: 'users', id: personId, depth: 0, select: { handle: true }, overrideAccess: true, disableErrors: true, req }) + return (person as { handle?: string } | null)?.handle + }, }, }), ], diff --git a/apps/payload-example/scripts/smoke.mjs b/apps/payload-example/scripts/smoke.mjs index 9ba71e2..97f7c5d 100644 --- a/apps/payload-example/scripts/smoke.mjs +++ b/apps/payload-example/scripts/smoke.mjs @@ -168,7 +168,7 @@ async function checks() { check('the member\'s cookie sent from another site counts for nothing', elsewhere.status === 403, { status: elsewhere.status }) const made = await call('POST', '/api/taxonomy-proposals?depth=0', { as: member, origin: site, body: { snapshot: snap, proposal: { ...draft, author: 'Somebody Else' } } }) const proposal = made.json?.doc - check('the member\'s proposal is stored private, under their handle', made.status === 201 && proposal?.status === 'private' && proposal?.proposal?.author === 'Morgan Member', made.json?.errors ?? { status: made.status, author: proposal?.proposal?.author }) + check('the member\'s proposal is stored private, under their account\'s id, with their handle shown beside it', made.status === 201 && proposal?.status === 'private' && Boolean(me?.id) && proposal?.proposal?.author === String(me?.id) && proposal?.authorName === 'Morgan Member', made.json?.errors ?? { status: made.status, author: proposal?.proposal?.author, authorName: proposal?.authorName }) check('the stored proposal does not hold the member\'s email address', Boolean(proposal) && !JSON.stringify(proposal).includes('@example.org')) check('a private proposal is not there for the curator', (await get(`/api/taxonomy-proposals/${proposal?.id}/preview`, curator)).status === 404) const shared = await call('PATCH', `/api/taxonomy-proposals/${proposal?.id}?depth=0`, { as: member, origin: site, body: { status: 'shared' } }) @@ -180,13 +180,14 @@ async function checks() { check('the curator adds an account', second.status === 201 && second.json?.doc?.role === 'member', second.json?.errors ?? { status: second.status }) const sam = await signIn('second@example.org') const seenBySam = await get(`/api/taxonomy-proposals/${proposal?.id}`, sam) - check('another member reads the shared proposal by handle, without the author\'s email address', seenBySam.status === 200 && seenBySam.text.includes('Morgan Member') && !seenBySam.text.includes('member@example.org'), { status: seenBySam.status }) + check('another member reads the shared proposal with the author\'s handle, and without their email address', seenBySam.status === 200 && seenBySam.json?.authorName === 'Morgan Member' && !seenBySam.text.includes('member@example.org'), { status: seenBySam.status, authorName: seenBySam.json?.authorName }) const checksum = shared.json?.doc?.checksum for (const [who, as] of [['its author', member], ['another member', sam]]) { check(`${who} cannot accept it`, (await call('POST', `/api/taxonomy-proposals/${proposal?.id}/accept`, { as, origin: site, body: { version: '2', checksum } })).status === 404) } - // The curator previews and accepts: a new staged version, sealed, naming both people by handle. + // The curator previews and accepts: a new staged version, sealed, recording both people by id and neither by name. + const curatorId = (await get('/api/users/me', curator)).json?.user?.id const preview = await get(`/api/taxonomy-proposals/${proposal?.id}/preview`, curator) check('the curator previews it and it can be accepted', preview.status === 200 && preview.json?.acceptable === true, preview.json?.problems ?? { status: preview.status }) const accepted = await call('POST', `/api/taxonomy-proposals/${proposal?.id}/accept`, { as: curator, origin: site, body: { version: '2', checksum } }) @@ -195,8 +196,14 @@ async function checks() { const content = (await get(`/api/taxonomy-snapshots/${next?.doc}/export`, curator)).json check('version 2 is sealed and its checksum holds', Boolean(content) && verifySnapshot(content)) const record = content?.derivedFrom?.proposal - check('version 2 names the proposer and the curator by handle', record?.author === 'Morgan Member' && record?.acceptedBy === 'Cara Curator', record) - check('version 2 holds no email address', Boolean(content) && !JSON.stringify(content).includes('@example.org')) + check('version 2 records the proposer and the curator by id', Boolean(me?.id) && Boolean(curatorId) && record?.author === String(me?.id) && record?.acceptedBy === String(curatorId), record) + const sealed = JSON.stringify(content ?? '') + check('version 2 holds no handle and no email address', Boolean(content) && !sealed.includes('Morgan Member') && !sealed.includes('Cara Curator') && !sealed.includes('@example.org')) + // The handle is looked up when shown: a new one shows at once, and nothing sealed changes. + await call('PATCH', `/api/users/${me?.id}`, { as: curator, origin: site, body: { handle: 'Morgan Renamed' } }) + const renamed = await get(`/api/taxonomy-proposals/${proposal?.id}`, sam) + const again = (await get(`/api/taxonomy-snapshots/${next?.doc}/export`, curator)).json + check('a changed handle shows on the accepted proposal at once, and version 2 is unchanged', renamed.json?.authorName === 'Morgan Renamed' && Boolean(content?.checksum) && again?.checksum === content?.checksum && verifySnapshot(again), { authorName: renamed.json?.authorName }) check('version 2 is not public until the curator publishes it', (await get(`/api/taxonomy-snapshots/${next?.doc}/export`)).status === 404 && ((await get('/api/taxonomy-snapshots?depth=0')).json?.docs ?? []).length === 1) for (const [who, as] of [['its author', member], ['the curator', curator]]) { check(`an accepted proposal can't be deleted by ${who}`, (await call('DELETE', `/api/taxonomy-proposals/${proposal?.id}`, { as, origin: site })).status >= 400) diff --git a/apps/payload-example/src/users.ts b/apps/payload-example/src/users.ts index 28718f8..44b18ca 100644 --- a/apps/payload-example/src/users.ts +++ b/apps/payload-example/src/users.ts @@ -13,7 +13,8 @@ const selfOrCurator = ({ req }: { req: { user?: unknown } }) => { /** * The site's accounts. Two roles: a curator manages the taxonomy and decides on proposals; a member reads and makes - * proposals. `handle` is the public name a person's proposals carry, so it is not their email address. + * proposals. `handle` is the public name shown beside a person's proposals, so it is not their email address. Proposals store the + * account's id; the handle is looked up when one is shown. */ export const users: CollectionConfig = { slug: 'users', @@ -33,7 +34,7 @@ export const users: CollectionConfig = { type: 'text', required: true, unique: true, - admin: { description: 'Shown on your proposals. If a curator accepts one, this name is kept in the new snapshot version and can\'t be taken out later.' }, + admin: { description: 'Shown beside your proposals to people who can read them. Change it and the new name shows everywhere at once. It is not stored in any snapshot.' }, }, { name: 'role', diff --git a/docs/architecture/adr/0012-payload-plugin.md b/docs/architecture/adr/0012-payload-plugin.md index 5ac9760..1a1ce40 100644 --- a/docs/architecture/adr/0012-payload-plugin.md +++ b/docs/architecture/adr/0012-payload-plugin.md @@ -28,16 +28,16 @@ 14. **Accepted by Science (the area lead), 2026-11-11: withdrawing a snapshot doesn't hide its published views.** A published view of a withdrawn snapshot stays readable, and every answer about it carries the flag: `source.status: 'withdrawn'`, `source.withdrawn: true`, and a first-class disclosure line ("Source withdrawn: snapshot … was withdrawn by its publisher after this view was published. …") in projections and exports. The snapshot itself stays hidden from readers, as point 4 says. Why: views are cited and linked; hiding them silently breaks those links and hides that something changed; saved references are never rewritten or dropped silently (I18). A flag keeps the change visible. The alternative, hiding the views, was one access rule away and wasn't chosen. - **Payload's own reads carry it too (part 3).** A view has a `source` field that isn't stored: it is worked out each time the view is read, from the snapshot's head, and is the same object the endpoints answer with (`doc`, `id`, `version`, `checksum`, `status`, `withdrawn`). So a view read over REST, GraphQL or the Local API says its snapshot was withdrawn without a second request. It is null when the snapshot is gone. A `source` sent with a view is ignored. The snapshot's status is read once per request for each snapshot, and read again if the snapshot changes in that request. Populating the view's `snapshot` relationship still gives readers only its id once it is withdrawn, as point 4 says. -15. **Proposals are people's own, and accepting one makes a new staged version (part 4).** With the `proposals` option, a `taxonomy-proposals` collection holds one document per proposal: the snapshot it is drawn over, core's `Proposal` (title, notes, base, moves), a status, and a checksum of the proposal. The plugin sets the proposal's id, author name and date; anything else sent with it is dropped. +15. **Proposals are people's own, and accepting one makes a new staged version (part 4).** With the `proposals` option, a `taxonomy-proposals` collection holds one document per proposal: the snapshot it is drawn over, core's `Proposal` (title, notes, base, moves), a status, and a checksum of the proposal. The plugin sets the proposal's id, author (a person id) and date; anything else sent with it is dropped. - **Whose and who sees.** A proposal belongs to the signed-in person who made it. They need the host's `read` answer and a third callback, `access.propose` (default: signed-in users; only `true` is a yes). Proposals are never shown to anonymous visitors, even on a site whose snapshots are public. A proposal is `private` (seen by its author only), `shared` or `accepted` (seen by everyone in the tenant who may read), or `declined` (seen by its author and managers). Readers see that a proposal was accepted, with the manager's note and the date, but not who decided or the staged version it made: those two fields are for managers. The snapshot is read as the requester, so a snapshot that is missing, hidden or in another tenant gives one answer, and a proposal can't be used to find out that a hidden snapshot exists. - **Sharing locks it.** Only the author edits a proposal, and only while it is private. Sharing needs at least one move and a published snapshot. To change a shared proposal its author takes it back to private first, so what a manager looks at can't change under them without the checksum changing. The author, the snapshot, the tenant and the decision fields can't be set through an update, by anyone, trusted server code included: the checks are in a hook. - **A manager decides, through two actions.** `POST /api//:id/accept` and `/decline` (and `acceptProposal`, `declineProposal`) work for managers in the proposal's tenant; everyone else gets the 404 of a missing proposal. Accepting names the new version and the proposal's checksum as the manager last saw it (`GET …/:id/preview` gives it, with the taxa moved and the groups changed, broken and formed, bounded by `limits.maxResults`). A proposal that changed since is refused. - **Accepting never edits a published snapshot.** Core's `acceptIntoSnapshot` makes an unsealed copy with a new version, in which the classification gets a new version (`+`), every moved placement names the proposal as its provenance, a source record (`proposal:`) says whose proposal it was and who accepted it, and `derivedFrom` pins the base snapshot's id, version and checksum, and the proposal. The plugin seals it, checks the checksum, validates it and stores it **staged**. Readers see nothing new until a manager publishes it, which is the second look. The base must be published at that moment; a staged, withdrawn or deleted base is refused. - **Phylogenies are never accepted.** A proposal on a phylogeny can be made, shared and previewed, as in the explorer, but accepting it is refused in core and in the plugin. A phylogeny is a published hypothesis with a citation; a tree rearranged by hand is a result its study never found, so it can't take that hypothesis's place. A new analysis is imported as its own hypothesis. - **Once, and together.** The new snapshot and the proposal's new status are written in one transaction where the database has them. The proposal is claimed with one conditional update (still shared, same checksum), so two managers accepting at once store one version and the other gets a 409, and an author who takes the proposal back in that moment wins: nothing is accepted. Without transactions the new snapshot is deleted again when the claim fails. Declining is the same conditional update without a snapshot. - - **Names are sealed.** `proposals.nameOf({ req })` gives the name a person's proposals and decisions carry (default `User `); a throw or anything but a non-empty string stops the action, so no record is written without its name. The author's name is taken again each time they save or share. On acceptance the author's and the manager's names go into the new snapshot's source record and `derivedFrom`, inside its checksum. They can't be taken out later without making a new version, and a published version may already be pinned and cited. A host should give a name people are happy to have published: a public handle, not an email address. - - **Kept and deleted.** An author can delete their own private and declined proposals. A shared one must be taken back first. An accepted one can't be changed or deleted, even by trusted code, because a snapshot version names it. If the staged version it made is deleted before publishing, the proposal goes back to shared with a note saying so. A host's own code can take back and delete a departed person's open proposals; their accepted ones stay, under the name they carried. - - **Limits.** Each acceptance is made from the published base alone: two proposals accepted on one snapshot give two versions, not one with both. To combine them, publish the first and propose again on it. Without transactions, a crash between storing the snapshot and claiming the proposal can leave a staged version no proposal points to; a manager deletes it. The note from an earlier decision stays on a proposal that is shared again, until the next decision. The conditional update is one statement on MongoDB; it hasn't been checked on Postgres or SQLite. One person keeps at most `maxPerUser` proposals (default 200, not counting accepted ones) of at most `maxMoves` moves (default 500). In the admin a proposal is a JSON field. + - **People are sealed by id, never by name (changed 2026-11-11, before any release).** The first version of this point sealed a name the host chose (`nameOf`, a public handle). That was wrong for a site whose people can ask to be forgotten: a name inside a checksum can't be taken out without making a new version, and a published version may already be pinned and cited. Now `proposals.personId({ req })` gives an opaque id (default: the signed-in user's document id), and only that id is stored on a proposal and, on acceptance, sealed into the new snapshot's source record and `derivedFrom`. An id is 1 to 64 letters, digits, `.`, `_`, `:` and `-`, starting with a letter or digit, so an email address or a name with a space in it is refused; a throw or any other value also stops the action, nothing is saved, and the log line never repeats the value. The pattern can't tell a one-word handle from an id, so the docs say not to give one. A name is shown through `proposals.displayName({ req, personId })`, called when a proposal is read (once per person per request) and returned as `authorName`: a field that is computed on read, dropped on write and never in the database. When a name is removed at the host, nothing shown or sealed names the person. The title, each move's note and the manager's note are free text and are sealed as written; the plugin doesn't screen them. + - **Kept and deleted.** An author can delete their own private and declined proposals. A shared one must be taken back first. An accepted one can't be changed or deleted, even by trusted code, because a snapshot version names it. If the staged version it made is deleted before publishing, the proposal goes back to shared with a note saying so. A host's own code can take back and delete a departed person's open proposals; their accepted ones stay, under their id, with the relationship to their user document unchanged. + - **Limits.** Each acceptance is made from the published base alone: two proposals accepted on one snapshot give two versions, not one with both. To combine them, publish the first and propose again on it. Without transactions, a crash between storing the snapshot and claiming the proposal can leave a staged version no proposal points to; a manager deletes it. The note from an earlier decision stays on a proposal that is shared again, until the next decision. The conditional update is one statement on MongoDB; it hasn't been checked on Postgres or SQLite. One person keeps at most `maxPerUser` proposals (default 200, not counting accepted ones) of at most `maxMoves` moves (default 500). In the admin a proposal is a JSON field. No endpoint turns an id read from a snapshot's record back into a name; a host that wants that on a snapshot page looks it up itself. **Later parts of M7, not built yet.** @@ -47,4 +47,4 @@ - The atomic claims (import jobs, proposals) on Postgres and SQLite, which need those adapters in the tests. - Very large snapshots. A MongoDB document holds at most 16 MB, so a snapshot past that can't be stored this way (imports refuse past `maxBytes`); the likely answer is the host's upload collection for the JSON, with the checksum kept here. Reading the collection over REST also returns the whole JSON, which is slow for big snapshots. -**Consequences.** One document per snapshot keeps the checksum meaningful and the plugin small, at the cost of loading a whole snapshot to answer the first question about it (later ones use the cache). Locking content in hooks, not access, means even trusted server code can't edit a sealed snapshot; corrections are new versions, which is what views pinning `{ id, version, checksum }` already assume. Hosts must write two callbacks to open anything to anyone, which is deliberate: the defaults show nothing to anonymous visitors and let no one but server code manage. Proposals put two names into a sealed record for good, which is what makes an accepted change traceable, and is why the host chooses what those names are. +**Consequences.** One document per snapshot keeps the checksum meaningful and the plugin small, at the cost of loading a whole snapshot to answer the first question about it (later ones use the cache). Locking content in hooks, not access, means even trusted server code can't edit a sealed snapshot; corrections are new versions, which is what views pinning `{ id, version, checksum }` already assume. Hosts must write two callbacks to open anything to anyone, which is deliberate: the defaults show nothing to anonymous visitors and let no one but server code manage. Proposals put two person ids into a sealed record for good, which is what makes an accepted change traceable; who those ids are is answered by the host, for as long as it chooses to. diff --git a/docs/architecture/backlog.md b/docs/architecture/backlog.md index fe43de4..74b62a3 100644 --- a/docs/architecture/backlog.md +++ b/docs/architecture/backlog.md @@ -76,7 +76,7 @@ Task-level plan per milestone ([`phase-plan.md`](phase-plan.md)). `[x]` done wit - [x] Bounded API: search, ancestors/descendants, MRCA, coverage, projected views, exports; access checks; cross-tenant denial tests (part 1, 2026-11-11: search, ancestors, descendants and MRCA with access checks, limits, 404 for anything unreadable and cross-tenant denial tests; part 2, 2026-11-11: coverage, projected views with their disclosure, and snapshot and view exports, under the same rules and tests; 2026-11-11: every endpoint also answers through Payload's own REST router with a signed-in token, in-process (`test/http.test.ts`). 2026-11-11: run in a real site behind Next.js, with cookies, by `apps/payload-example`'s smoke test, below) - [x] Resumable, idempotent import jobs (part 2, 2026-11-11: a `taxonomy-imports` collection, not Payload's jobs queue (ADR 0012, point 12); staged sealed snapshots, "already imported" on a re-run, failures store nothing, interrupted jobs restart from the file; part 3, 2026-11-11: a job is claimed by one conditional update, so runs started together can't both proceed; tested on an in-memory MongoDB, not in a real site or on Postgres or SQLite) - [x] Withdrawn snapshots' published views stay readable with a "withdrawn source" flag (ADR 0012, point 14, accepted by the area lead 2026-11-11): on the projection and export endpoints (part 2) and as a `source` field on Payload's own reads of a view (part 3) -- [x] `apps/payload-example` (2026-11-11: a Payload 3.93 and Next.js 16 site on a throwaway in-memory MongoDB, on this machine only. Two roles, with a public handle as the name proposals carry; public Server Component pages that draw the published views with `TreeView` and `TreeOutline`; a seed of fixture data. `pnpm smoke` builds it, starts it and runs 46 checks over HTTP with sign-in cookies, in CI too. Not tested: a person clicking through the admin, a screen reader, Postgres or SQLite, a deployed site. It has no pages for proposals or imports: those wait for the admin views) +- [x] `apps/payload-example` (2026-11-11: a Payload 3.93 and Next.js 16 site on a throwaway in-memory MongoDB, on this machine only. Two roles; proposals record people by account id and show a public handle looked up when read (changed 2026-11-11); public Server Component pages that draw the published views with `TreeView` and `TreeOutline`; a seed of fixture data. `pnpm smoke` builds it, starts it and runs 47 checks over HTTP with sign-in cookies, in CI too. Not tested: a person clicking through the admin, a screen reader, Postgres or SQLite, a deployed site. It has no pages for proposals or imports: those wait for the admin views) ## M8 — Science and performance polish diff --git a/docs/test-audit.md b/docs/test-audit.md index e7deba5..9b2c62a 100644 --- a/docs/test-audit.md +++ b/docs/test-audit.md @@ -2,6 +2,37 @@ Each audit lists what this repo promises, the tests that prove it, and the gaps: those closed in the same pass and those left, with why. Audits run after each feature or fix, and at least every 14 days while the repo has commits. Newest first. +## 2026-11-11: people by id in proposals + +Scope: `packages/payload` (`src/proposals.ts`, `src/types.ts`) and `apps/payload-example`. The plugin promised that an accepted proposal sealed a name the host chose. It now promises that only an opaque person id is stored and sealed, and that a name is looked up when shown and never kept. This replaces the lines about handles in the two audits below ("example Payload site" and "Payload plugin part 4"). Plugin tests went from 139 to 142, all unit tests from 523 to 526, the example's smoke checks from 46 to 47. + +### Covered + +- **Only the id is stored** (`proposals.test.ts`, "making a proposal"): a new proposal's author is the person id the site gives, whatever the request sent, including a forged `authorName`. The document read straight from the database has no `authorName` and none of the test's made-up names. +- **A name or an email address is never an id:** fourteen values are refused on create (empty, spaces, a full name, an email address, leading or trailing space, a leading hyphen, a slash, a no-break space, 65 characters, a number, null, undefined, an object), and a throw too. Each stores nothing, and the log line for each doesn't repeat the value. +- **Names are looked up, not kept:** a name changed at the site shows on the next read, for the author and for another reader, with the proposal's checksum unchanged; one list asks once per person per request; a lookup that throws or gives nothing shows the id alone; the preview carries the same name. +- **Accepting seals ids:** the new version's source record and `derivedFrom.proposal` hold the author's and the manager's ids, and the whole stored snapshot document matches none of the made-up names and has no `@`. Accepting is refused, and stores nothing, when the manager's id throws or is a name. +- **When someone leaves:** after the site stops giving a name, an accepted proposal still reads, under the id alone, and nothing stored or sealed names the person; open proposals are taken back and deleted by server code as before. +- **Over HTTP** (`http.test.ts`): a proposal posted with a forged author is stored under the sender's id; the accepted snapshot's record holds the two ids. +- **No tenants, defaults** (`proposals-plain.test.ts`): with no `personId`, the id is the user document's id, and with no `displayName` no name is shown. +- **The example site** (`scripts/smoke.mjs`): a member's proposal is stored under their account id and shows their handle; another member reads the handle and no email address; version 2 records both people by id and holds no handle and no email address; a handle changed by the curator shows on the accepted proposal at once, and version 2's checksum is unchanged and still verifies. Passed on `next dev` and on the built site. + +### Checked by breaking it + +Eight deliberate breaks in `src/proposals.ts`, one at a time, each failed at least one test: the pattern not checked; the refused value added to the log line; `authorName` made a stored field; accept and create each taking the host's value unchecked; a failed name lookup not caught; no per-request cache; the preview without the name. + +### Gaps left + +- **A one-word handle passes the pattern.** "morgan" is a valid id. The plugin can't tell, the README says so, and no test can close it. +- **Free text is sealed as written:** the title, the moves' notes and the manager's note. Nothing screens them for names. The README says so. +- **More than one auth collection:** the default id is the user document's id, which two collections can share. The README says to give your own; not tested. +- **The link to the user document:** an accepted proposal keeps its `author` and `decidedBy` relationships, and no code can change them. What a site does with the user document afterwards is its own; not tested here. +- **Postgres and SQLite:** as before, not run. + +### Left out as overkill + +- Every character class of the pattern: the fourteen refused values cover each rule it has, and the accepted forms are used throughout the tests. + ## 2026-11-11: where names go in the drawing Scope: a defect found while looking at the example site's pages. In rectangular layouts an inner node's name was drawn 6 units above the node, and a child's branch one row apart ran through it ("Reptilia" in the amniotes classes view). A folded branch's name started on top of its triangle. Both are fixed in `packages/react/src/labelPlace.ts`, used by the SVG and the canvas. React tests went from 119 to 132. diff --git a/packages/core/src/model.ts b/packages/core/src/model.ts index 2b8b865..d255446 100644 --- a/packages/core/src/model.ts +++ b/packages/core/src/model.ts @@ -189,8 +189,9 @@ export type DatasetSnapshot = { /** * The proposal a snapshot version was made from: whose it was, who accepted it, the classification version it was - * drawn over and the concepts it moved. Names are written as their owners want them read; they are sealed with the - * snapshot and can't be taken out later. + * drawn over and the concepts it moved. `author` and `acceptedBy` are whatever the caller gave: a name, or an id that + * stands for the person. They are sealed with the snapshot and can't be taken out later, so where people may later ask + * to be forgotten, give an id that means nothing on its own. The title is the author's own words and is sealed too. */ export type AcceptedProposal = { id: Id diff --git a/packages/core/src/proposal.ts b/packages/core/src/proposal.ts index 120d2b0..12d09d1 100644 --- a/packages/core/src/proposal.ts +++ b/packages/core/src/proposal.ts @@ -140,7 +140,10 @@ export type AcceptOptions = { version: string /** ISO date-time of the acceptance, supplied by the caller (core stays pure). It becomes the new snapshot's `createdAt`. */ createdAt: string - /** Who accepted it, written as they want it read. It is sealed with the snapshot. */ + /** + * Who accepted it: a name, or an id that stands for the person. It is sealed with the snapshot and can't be taken + * out later, so where people may later ask to be forgotten, give an id that means nothing on its own. + */ acceptedBy: string note?: string } @@ -170,7 +173,7 @@ export function acceptIntoSnapshot(base: DatasetSnapshot, proposal: Proposal, o: if (!text(o.version)) throw new ProposalError('The new version needs a name') if (o.version === base.version) throw new ProposalError(`Version ${o.version} is the base snapshot’s own: an accepted proposal makes a new version`) if (!text(o.createdAt) || Number.isNaN(Date.parse(o.createdAt))) throw new ProposalError('The acceptance needs its date') - if (!text(o.acceptedBy)) throw new ProposalError('The acceptance needs the name of who accepted it') + if (!text(o.acceptedBy)) throw new ProposalError('The acceptance needs to say who accepted it') if (!text(proposal.author)) throw new ProposalError('A proposal names its author') if (proposal.base.kind !== 'classification') throw new ProposalError('Only a proposal on a classification can be accepted into a dataset. A phylogeny is a published hypothesis: a tree rearranged by hand is a result its study never found. Import the new analysis as its own hypothesis instead') diff --git a/packages/payload/README.md b/packages/payload/README.md index f9107ea..c5fc4fd 100644 --- a/packages/payload/README.md +++ b/packages/payload/README.md @@ -2,7 +2,7 @@ OneTaxonomy for Payload: sealed taxonomy snapshots, views pinned to them, import jobs that turn source files into staged snapshots, people's own proposals that a manager can accept into a new staged version, and a small read API for search, ancestors, descendants, MRCA, coverage, projected views and exports, all behind your own access rules. -> **Status: early, parts 1 to 4 of M7.** Not on npm yet. The collections, publishing, tenant scope, import jobs, proposals and the read endpoints work, and are tested on a real Payload 3.93 with an in-memory MongoDB (139 tests): bad checksums refused, sealed snapshots locked, an import of each format, re-runs that store nothing new, malformed files refused with their line, interrupted jobs resumed, runs started together claimed by one, every endpoint's results, a view's `source` through the Local API and GraphQL, proposals kept private, shared, accepted into a staged version once and declined, access refusals and cross-tenant denial. Twelve of those tests send real requests through Payload's own REST router, signed in with a token from the login route; the rest call the endpoint handlers and the Local API directly. It also runs in a small real site, [`apps/payload-example`](../../apps/payload-example) (Next.js 16, on this machine only), where 46 more checks go over HTTP with sign-in cookies. It hasn't run on a deployed site, or on Postgres or SQLite, and no URL import has fetched over a real network. No admin views yet: a proposal is a JSON field in the admin. See [ADR 0012](../../docs/architecture/adr/0012-payload-plugin.md). +> **Status: early, parts 1 to 4 of M7.** Not on npm yet. The collections, publishing, tenant scope, import jobs, proposals and the read endpoints work, and are tested on a real Payload 3.93 with an in-memory MongoDB (142 tests): bad checksums refused, sealed snapshots locked, an import of each format, re-runs that store nothing new, malformed files refused with their line, interrupted jobs resumed, runs started together claimed by one, every endpoint's results, a view's `source` through the Local API and GraphQL, proposals kept private, shared, accepted into a staged version once and declined, access refusals and cross-tenant denial. Twelve of those tests send real requests through Payload's own REST router, signed in with a token from the login route; the rest call the endpoint handlers and the Local API directly. It also runs in a small real site, [`apps/payload-example`](../../apps/payload-example) (Next.js 16, on this machine only), where 46 more checks go over HTTP with sign-in cookies. It hasn't run on a deployed site, or on Postgres or SQLite, and no URL import has fetched over a real network. No admin views yet: a proposal is a JSON field in the admin. See [ADR 0012](../../docs/architecture/adr/0012-payload-plugin.md). ## What it does @@ -52,7 +52,7 @@ export default buildConfig({ fetch: ({ url }) => fetchFromAllowedHosts(url), // optional; your function decides which URLs are allowed }, proposals: { // optional; `true` takes the defaults; without it there is no proposals collection - nameOf: ({ req }) => req.user.handle, // the name sealed into an accepted version: a public handle, not an email + displayName: ({ personId }) => handleOf(personId), // optional; the name shown beside a person's id, looked up when shown, never stored }, }), ], @@ -96,7 +96,7 @@ Host pages can call `searchNames`, `ancestorsOf`, `descendantsOf`, `mrcaOf`, `co ## Proposals -Turn them on with `proposals: true` or `proposals: { nameOf, maxMoves, maxPerUser }`. +Turn them on with `proposals: true` or `proposals: { personId, displayName, maxMoves, maxPerUser }`. ```js // Anyone signed in who may read and propose. It starts private. @@ -116,7 +116,7 @@ const p = await payload.create({ await payload.update({ collection: 'taxonomy-proposals', id: p.id, data: { status: 'shared' }, user, overrideAccess: false }) ``` -The plugin sets the proposal's id, author name and date, and drops anything else sent with it. A proposal that doesn't fit its snapshot (an unknown node, a move from a parent the node doesn't have, a cycle) is refused with the reason. +The plugin sets the proposal's id, its author (a person id, below) and its date, and drops anything else sent with it. A proposal that doesn't fit its snapshot (an unknown node, a move from a parent the node doesn't have, a cycle) is refused with the reason. | Status | Who sees it | What can happen | | --- | --- | --- | @@ -137,11 +137,17 @@ Anyone else gets the 404 of a missing proposal. A refusal that depends on the pr The new version is core's `acceptIntoSnapshot`: the classification gets a new version, each moved placement names the proposal as its source, a source record `proposal:` says who proposed and who accepted, and `derivedFrom` pins the snapshot it was made from. It is stored staged, so publishing it is a second, separate step. Deleting that staged version before it is published opens the proposal again. -**Names are permanent.** `nameOf` gives the name a person's proposals and decisions carry (default `User `). When a proposal is accepted, its author's name and the manager's are sealed into the new snapshot, inside its checksum. They can't be taken out later. Give a name people are happy to have published. If `nameOf` throws or gives nothing, the action stops and nothing is saved. +**People are recorded by id, never by name.** A proposal's author and the manager who accepts it are stored as a person id. When a proposal is accepted, those two ids are sealed into the new snapshot, inside its checksum, and can't be taken out later. An id that means nothing on its own leaves nothing personal there. -**When someone leaves.** Your server code can take back their shared proposals (`status: 'private'`) and delete their private and declined ones. Their accepted proposals stay, with the name they carried when accepted, and so does that name in the snapshot. +- `personId({ req })` gives the id. The default is the signed-in user's document id. Give your own if you have a steadier one, or more than one auth collection (two collections can hold the same id). +- An id is 1 to 64 characters: letters, digits, `.`, `_`, `:` and `-`, starting with a letter or digit. An email address, or a name with a space in it, is refused: the action stops, nothing is saved, and the value is not written to the log. A one-word handle would pass, so don't give one. The plugin can't tell it from an id. +- `displayName({ req, personId })` gives the name to show for an id. It is looked up each time a proposal is read (once per person per request) and comes back as `authorName` on the proposal and in the preview. It is never stored and never sealed, and anything sent as `authorName` is dropped. If it gives nothing or throws, the id is shown alone. -Options: `slugs` (`snapshots`, `views`, `imports`, `proposals`), `access` (`read`, `manage`, `propose`), `tenant` (`collection`, `field`, `resolve`), `locale`, `limits`, `imports` (`uploads`, `fetch`, `maxBytes`, `staleAfterMinutes`, `runOnCreate`), `proposals` (`nameOf`, `maxMoves` default 500, `maxPerUser` default 200, not counting accepted ones), `admin.group`. +A proposal's title and notes, each move's note and the manager's note are people's own words. On acceptance the title, the moves' notes and the manager's note are sealed into the snapshot, and the proposal itself is kept as it was. Don't put anything personal in them. The plugin doesn't check. + +**When someone leaves.** Your server code can take back their shared proposals (`status: 'private'`) and delete their private and declined ones. Their accepted proposals stay, and so does their id in the snapshot. Stop giving a name for that id from `displayName`, and nothing shown names them. The accepted proposal's document still points to their user document (`author`, and `decidedBy` for a manager); that link can't be changed, so keep or delete the user document as your own rules say. + +Options: `slugs` (`snapshots`, `views`, `imports`, `proposals`), `access` (`read`, `manage`, `propose`), `tenant` (`collection`, `field`, `resolve`), `locale`, `limits`, `imports` (`uploads`, `fetch`, `maxBytes`, `staleAfterMinutes`, `runOnCreate`), `proposals` (`personId`, `displayName`, `maxMoves` default 500, `maxPerUser` default 200, not counting accepted ones), `admin.group`. ## Limits diff --git a/packages/payload/src/index.ts b/packages/payload/src/index.ts index 43596f4..fd1ef7c 100644 --- a/packages/payload/src/index.ts +++ b/packages/payload/src/index.ts @@ -40,6 +40,7 @@ export { export { acceptProposal, declineProposal, + PERSON_ID, previewProposal, proposalChecksum, proposalsCollection, diff --git a/packages/payload/src/proposals.ts b/packages/payload/src/proposals.ts index 0508aad..aff6f3b 100644 --- a/packages/payload/src/proposals.ts +++ b/packages/payload/src/proposals.ts @@ -166,29 +166,64 @@ function hierarchies(index: DatasetIndex, proposal: Proposal): { before: TreeInd return { before: index.phylogeny(b.id), after: indexPhylogeny(applyToPhylogeny(p, proposal)) } } -class NameError extends Error {} +class PersonIdError extends Error {} -/** The requester's name as the host gives it. Anything but a usable string stops the action: a record needs its name. */ -async function nameFor(opts: ResolvedOptions, req: PayloadRequest): Promise { - let name: unknown +/** + * What a person id may look like: 1 to 64 letters, digits, `.`, `_`, `:` and `-`, starting with a letter or digit. + * No space and no `@`, so a full name or an email address never passes. + */ +export const PERSON_ID = /^[A-Za-z0-9][A-Za-z0-9._:-]{0,63}$/ + +/** + * The requester's id for the record, as the host gives it. It is the only thing about a person that an accepted + * proposal seals into a snapshot, so anything that isn't a plain id stops the action. The value is never logged: a + * refused one may be somebody's name. + */ +async function personIdFor(opts: ResolvedOptions, req: PayloadRequest): Promise { + let id: unknown + let threw = false try { - name = await opts.proposals!.nameOf({ req }) + id = await opts.proposals!.personId({ req }) } catch { - name = undefined + threw = true } - const usable = text(name, MAX.name) - if (!usable) throw new NameError('This site couldn\'t give your name for the record, so nothing was saved. Ask the site\'s owner.') - return usable + if (typeof id === 'string' && PERSON_ID.test(id)) return id + req.payload.logger.error( + threw + ? 'OneTaxonomy proposals: proposals.personId threw, so nothing was saved.' + : 'OneTaxonomy proposals: proposals.personId gave something that isn\'t an id (1 to 64 letters, digits, ".", "_", ":" or "-", starting with a letter or digit), so nothing was saved. Give an opaque id, never a name or an email address.', + ) + throw new PersonIdError('This site couldn\'t give your id for the record, so nothing was saved. Ask the site\'s owner.') } -const nameInHook = async (opts: ResolvedOptions, req: PayloadRequest): Promise => { +const personIdInHook = async (opts: ResolvedOptions, req: PayloadRequest): Promise => { try { - return await nameFor(opts, req) + return await personIdFor(opts, req) } catch (error) { throw new APIError((error as Error).message, 500, undefined, true) } } +// One lookup per person per request, however many proposals of theirs a list holds. +const shownNames = new WeakMap>>() + +/** + * The name to show the requester for a person id, from the host's `proposals.displayName`, asked now and never + * stored. Undefined when the host has no name to show, has no lookup, or the lookup fails: the id is shown alone. + */ +function displayNameFor(opts: ResolvedOptions, req: PayloadRequest, personId: unknown): Promise { + const lookup = opts.proposals?.displayName + if (!lookup || typeof personId !== 'string' || personId === '') return Promise.resolve(undefined) + let names = shownNames.get(req) + if (!names) shownNames.set(req, (names = new Map())) + let name = names.get(personId) + if (!name) { + name = (async () => text(await lookup({ req, personId }), MAX.name))().catch(() => undefined) + names.set(personId, name) + } + return name +} + /** * The snapshot a proposal is drawn over, read as the requester, so a proposal can't be used to find out whether a * hidden snapshot exists. Null when it doesn't exist, can't be read, or is in another tenant. @@ -244,7 +279,7 @@ const proposalBeforeChange = (opts: ResolvedOptions): CollectionBeforeChangeHook if (field) kept.push({ [field]: { equals: tenant ?? null } }) const { totalDocs } = await req.payload.count({ collection: opts.slugs.proposals as never, where: { and: kept }, req, overrideAccess: true }) if (totalDocs >= po.maxPerUser) fail(`You have ${totalDocs} proposals, the most this site keeps for one person. Delete one you no longer need first.`) - const proposal = assemble(newId(), await nameInHook(opts, req), new Date().toISOString(), w) + const proposal = assemble(newId(), await personIdInHook(opts, req), new Date().toISOString(), w) mustFit(over!.index, proposal) return { ...data, ...derived(proposal), author: user, status: 'private', decidedBy: null, decidedAt: null, decisionNote: null, acceptedInto: null } } @@ -258,7 +293,7 @@ const proposalBeforeChange = (opts: ResolvedOptions): CollectionBeforeChangeHook for (const f of DECISION) if (f in data && stamp(data[f]) !== stamp(originalDoc[f])) fail(DECIDED_BY_ACTION) if (to !== from && (to === 'accepted' || to === 'declined')) fail(DECIDED_BY_ACTION) - let next = 'proposal' in data ? assemble(original.id, original.author, original.created, written(asObject(data.proposal, 'proposal'), po.maxMoves)) : original + const next = 'proposal' in data ? assemble(original.id, original.author, original.created, written(asObject(data.proposal, 'proposal'), po.maxMoves)) : original const changed = !same(next, original) if (from === 'accepted' && (changed || to !== from)) fail('An accepted proposal is part of a snapshot version\'s record and can\'t change.') if (to !== from && !TRANSITIONS[from]?.includes(to)) fail(`A proposal can't go from ${from} to ${to}.`) @@ -267,8 +302,6 @@ const proposalBeforeChange = (opts: ResolvedOptions): CollectionBeforeChangeHook const sharing = to === 'shared' && from !== 'shared' if (changed || sharing) { - // The author's name is taken again when they save or share, so a proposal carries the name they use now. - if (req.user && idOf(req.user.id) === stamp(originalDoc.author)) next = { ...next, author: await nameInHook(opts, req) } const snapshotDoc = idOf(originalDoc.snapshot) if (changed) { const over = snapshotDoc ? await snapshotFor(req, opts, snapshotDoc, field ? idOf(originalDoc[field]) : undefined) : null @@ -327,7 +360,10 @@ export function proposalsCollection(opts: ResolvedOptions): CollectionConfig { name: 'proposal', type: 'json', required: true, - admin: { description: 'title, notes, base { kind, id, version } and moves [{ node, from, to, note }]. Its id, author and date are set when it is made. It can be edited while it is private.' }, + admin: { + description: + 'title, notes, base { kind, id, version } and moves [{ node, from, to, note }]. Its id, its author (an id, not a name) and its date are set when it is made. It can be edited while it is private. If it is accepted, its title and notes become part of that snapshot version for good, so leave personal details out of them.', + }, }, { name: 'status', @@ -341,6 +377,13 @@ export function proposalsCollection(opts: ResolvedOptions): CollectionConfig { { name: 'proposalId', type: 'text', index: true, admin: ro() }, { name: 'checksum', type: 'text', admin: ro('SHA-256 of the proposal. Accepting names it, so what is accepted is what was looked at.') }, { name: 'author', type: 'text', index: true, admin: { position: 'sidebar', ...ro('The signed-in user who made it.') } }, + { + name: 'authorName', + type: 'text', + virtual: true, + admin: { position: 'sidebar', ...ro('The author\'s name as this site shows it now. Looked up each time and never stored.') }, + hooks: { afterRead: [async ({ siblingData, req }) => (await displayNameFor(opts, req, (siblingData?.proposal as Proposal | undefined)?.author)) ?? null] }, + }, { name: 'decisionNote', type: 'textarea', admin: ro('What the manager said when deciding.') }, { name: 'decidedAt', type: 'date', admin: { position: 'sidebar', ...ro() } }, { name: 'decidedBy', type: 'text', access: { read: manager }, admin: { position: 'sidebar', ...ro('The signed-in user who accepted or declined it.') } }, @@ -357,7 +400,19 @@ export function proposalsCollection(opts: ResolvedOptions): CollectionConfig { type ProposalDoc = { id: string | number; proposal: Proposal; status: ProposalStatus; checksum: string; snapshot?: unknown; author?: string; [tenant: string]: unknown } -export type ProposalRef = { doc: string; id: Id; title: string; author: string; created: string; status: ProposalStatus; checksum: string; base: ProposalBase } +export type ProposalRef = { + doc: string + id: Id + title: string + /** The author's person id, as sealed if the proposal is accepted. */ + author: string + /** The author's name as the host shows it to this requester now (`proposals.displayName`). Left out when there is none. */ + authorName?: string + created: string + status: ProposalStatus + checksum: string + base: ProposalBase +} export type ProposalPreview = { proposal: ProposalRef @@ -430,6 +485,7 @@ export async function previewProposal(req: PayloadRequest, id: string, query: { } const notes = new Map(doc.proposal.moves.map((m) => [m.node, m.note])) const head = await viewSourceOf(req, opts, snapshotDoc) + const authorName = await displayNameFor(opts, req, doc.proposal.author) const why = doc.status === 'accepted' ? 'It was already accepted.' @@ -447,7 +503,7 @@ export async function previewProposal(req: PayloadRequest, id: string, query: { return { ok: true, body: { - proposal: refOf(doc), + proposal: { ...refOf(doc), ...(authorName ? { authorName } : {}) }, snapshot: loaded.ref, acceptable: why === undefined, ...(why ? { notAcceptable: why } : {}), @@ -479,8 +535,8 @@ const writeConflict = (error: unknown) => { * Accepts a shared proposal on a classification, as a manager in its tenant (404 for anyone else). It makes a **new * staged snapshot version** with core's `acceptIntoSnapshot`, seals and checks it, and marks the proposal accepted. * The published snapshot it was made from is not changed, and readers see nothing new until a manager publishes the - * new version. The proposer's and the manager's names, as `proposals.nameOf` gives them, are sealed into the new - * version's source record. + * new version. The proposer's and the manager's person ids, as `proposals.personId` gives them, are sealed into the + * new version's source record. Their names are not: `proposals.displayName` gives those when a proposal is shown. * * `body.version` names the new version. `body.checksum` is the proposal's checksum as the manager last saw it (the * preview gives it): if the proposal has changed since, nothing is accepted. `createdAt` (default now) and `note` @@ -523,7 +579,7 @@ export async function acceptProposal(req: PayloadRequest, id: string, body: unkn let acceptedBy: string try { - acceptedBy = await nameFor(opts, req) + acceptedBy = await personIdFor(opts, req) } catch (error) { return { ok: false, status: 500, error: (error as Error).message } } diff --git a/packages/payload/src/types.ts b/packages/payload/src/types.ts index 6aa190e..805ff01 100644 --- a/packages/payload/src/types.ts +++ b/packages/payload/src/types.ts @@ -64,12 +64,23 @@ export type OneTaxonomyPluginOptions = { export type ProposalOptions = { /** - * The name a person's proposals and decisions carry. An accepted proposal's author and the curator who accepted it - * are sealed into the new snapshot under these names and can't be taken out later, so give a name people are happy - * to have published: a public handle, not an email address. Default `User `. A throw, or anything but a - * non-empty string, stops the action. + * The id a person's proposals and decisions carry: an opaque reference to their account, never their name. An + * accepted proposal's author and the manager who accepted it are sealed into the new snapshot under these ids, + * inside its checksum, and can't be taken out later. An id that means nothing on its own leaves nothing personal + * there when the person goes. Default: the signed-in user's document id. A site with more than one auth collection + * should give its own, since two collections can hold the same id. + * + * It must be 1 to 64 characters: letters, digits, `.`, `_`, `:` and `-`, starting with a letter or digit. So an + * email address or a name with a space in it is refused. The plugin can't tell a handle from an id beyond that: + * don't give one. A throw, or anything else, stops the action and nothing is saved. */ - nameOf?: (args: { req: PayloadRequest }) => string | Promise + personId?: (args: { req: PayloadRequest }) => string | Promise + /** + * The name to show this requester for a person id, looked up each time a proposal is shown. It is never stored and + * never sealed, so changing or removing a name changes what is shown from then on. Give nothing (or throw) when + * there is no name to show: the id is shown alone. Without this option only ids are shown. + */ + displayName?: (args: { req: PayloadRequest; personId: string }) => string | null | undefined | Promise /** Most moves one proposal may hold. Default 500. */ maxMoves?: number /** Most proposals one person may keep at a time, not counting accepted ones. Default 200. */ @@ -108,7 +119,7 @@ export type ResolvedOptions = { limits: { maxResults: number; maxDepth: number; maxMrcaNodes: number; maxNodes: number; maxCoverageIds: number } admin: { group: string } imports: { uploads: ImportOptions['uploads'] | null; fetch: ImportOptions['fetch'] | null; maxBytes: number; staleAfterMs: number; runOnCreate: boolean } | null - proposals: { nameOf: NonNullable; maxMoves: number; maxPerUser: number } | null + proposals: { personId: NonNullable; displayName: ProposalOptions['displayName'] | null; maxMoves: number; maxPerUser: number } | null } export const CUSTOM_KEY = 'onetaxonomy' @@ -139,7 +150,8 @@ export function resolveOptions(o: OneTaxonomyPluginOptions = {}): ResolvedOption : null, proposals: o.proposals ? { - nameOf: (o.proposals === true ? undefined : o.proposals.nameOf) ?? (({ req }) => `User ${String(req.user?.id ?? '')}`), + personId: (o.proposals === true ? undefined : o.proposals.personId) ?? (({ req }) => (req.user ? String(req.user.id) : '')), + displayName: (o.proposals === true ? undefined : o.proposals.displayName) ?? null, maxMoves: positive(o.proposals === true ? undefined : o.proposals.maxMoves, 500), maxPerUser: positive(o.proposals === true ? undefined : o.proposals.maxPerUser, 200), } diff --git a/packages/payload/test/http.test.ts b/packages/payload/test/http.test.ts index 9b0df6f..ef84b7a 100644 --- a/packages/payload/test/http.test.ts +++ b/packages/payload/test/http.test.ts @@ -66,7 +66,10 @@ beforeAll(async () => { tenant: { collection: 'tenants', resolve: ({ req }) => tenantOfUser(req.user) }, // Answered from memory: no network. imports: { fetch: ({ url }) => (url === TREES ? '[&R] ((Alpha:1,Beta:1):1,Gamma:2);' : Promise.reject(new Error('not allowed'))) }, - proposals: { nameOf: ({ req }) => (roleOf(req.user) === 'manager' ? 'Cara Curator' : `Reader ${String(req.user?.id).slice(-4)}`) }, + proposals: { + personId: ({ req }) => `${roleOf(req.user) === 'manager' ? 'm' : 'r'}-${String(req.user?.id).slice(-6)}`, + displayName: ({ personId }) => (personId.startsWith('m-') ? 'Cara Curator' : 'Robin Reader'), + }, }), ], })) @@ -222,7 +225,7 @@ describe('over HTTP: proposals', () => { expect(p).toMatchObject({ status: 'private', author: String(ann.id) }) expect(p.decidedBy ?? null).toBeNull() expect(p.acceptedInto ?? null).toBeNull() - expect(p.proposal.author).toMatch(/^Reader /) + expect(p.proposal.author).toMatch(/^r-[0-9a-f]{6}$/) expect(p.proposal.id).not.toBe('chosen-id') // Private: nobody else gets it, by list, by id or by preview. for (const user of [ben, managerA, managerB, null]) { @@ -267,10 +270,13 @@ describe('over HTTP: proposals', () => { expect((await get(`/${SNAPSHOTS}/${made}/export`, ann)).status).toBe(404) const content = (await get(`/${SNAPSHOTS}/${made}/export`, managerA)).body as DatasetSnapshot expect(verifySnapshot(content)).toBe(true) - expect(content.derivedFrom?.proposal).toMatchObject({ author: p.proposal.author, acceptedBy: 'Cara Curator' }) + expect(content.derivedFrom?.proposal).toMatchObject({ author: `r-${String(ann.id).slice(-6)}`, acceptedBy: `m-${String(managerA.id).slice(-6)}` }) + expect(JSON.stringify(content)).not.toMatch(/Cara Curator|Robin Reader/) // Readers see that it was accepted, without who decided or where it went. const seen = (await get(`/${PROPOSALS}/${p.id}?depth=0`, ben)).body as Doc expect(seen.status).toBe('accepted') + // The author's name is looked up for the response; the proposal itself holds their id. + expect(seen).toMatchObject({ authorName: 'Robin Reader', proposal: { author: `r-${String(ann.id).slice(-6)}` } }) expect(seen).not.toHaveProperty('decidedBy') expect(seen).not.toHaveProperty('acceptedInto') // Kept: no change and no delete, for its author or a manager. diff --git a/packages/payload/test/proposals-plain.test.ts b/packages/payload/test/proposals-plain.test.ts index 7c880c4..2d49f97 100644 --- a/packages/payload/test/proposals-plain.test.ts +++ b/packages/payload/test/proposals-plain.test.ts @@ -34,11 +34,11 @@ beforeAll(async () => { oneTaxonomy({ access, proposals: { - nameOf: async ({ req }) => { + personId: async ({ req }) => { const run = meanwhile meanwhile = null await run?.() - return `Keeper ${String(req.user?.id).slice(-4)}` + return `keeper-${String(req.user?.id).slice(-4)}` }, }, }), @@ -56,9 +56,11 @@ beforeAll(async () => { afterAll(async () => stop?.()) describe('proposals on a site without tenants', () => { - it('names people "User " unless the site says otherwise, and lets any signed-in reader propose', async () => { + it('uses the user\'s document id as the person id unless the site says otherwise, looks up no names, and lets any signed-in reader propose', async () => { const opts = resolveOptions({ proposals: true }) - expect(await opts.proposals!.nameOf({ req: { user: { id: 42 } } } as never)).toBe('User 42') + expect(await opts.proposals!.personId({ req: { user: { id: 42 } } } as never)).toBe('42') + expect(await opts.proposals!.personId({ req: { user: null } } as never)).toBe('') + expect(opts.proposals!.displayName).toBeNull() expect(opts.proposals).toMatchObject({ maxMoves: 500, maxPerUser: 200 }) expect(await opts.access.propose({ req: { user: { id: 42 } } as never, tenant: null })).toBe(true) expect(await opts.access.propose({ req: { user: null } as never, tenant: null })).toBe(false) @@ -80,7 +82,9 @@ describe('proposals on a site without tenants', () => { const done = await accept(p.id, manager, { version: '2', checksum: shared.checksum }) expect(done).toMatchObject({ status: 200, body: { snapshot: { version: '2', status: 'staged' } } }) const made = (await payload.findByID({ collection: SNAPSHOTS as never, id: done.body.snapshot.doc, depth: 0 })) as Doc - expect((made.snapshot as DatasetSnapshot).derivedFrom!.proposal).toMatchObject({ author: `Keeper ${String(ann.id).slice(-4)}`, acceptedBy: `Keeper ${String(manager.id).slice(-4)}` }) + expect((made.snapshot as DatasetSnapshot).derivedFrom!.proposal).toMatchObject({ author: `keeper-${String(ann.id).slice(-4)}`, acceptedBy: `keeper-${String(manager.id).slice(-4)}` }) + // No `displayName` on this site: a proposal shows its author's id and no name. + expect(((await payload.findByID({ collection: PROPOSALS as never, id: p.id, depth: 0, user: ben, overrideAccess: false })) as Doc).authorName ?? null).toBeNull() expect(await seen(SNAPSHOTS, ann)).toEqual([snap]) expect((await payload.findByID({ collection: PROPOSALS as never, id: p.id, depth: 0 })) as Doc).toMatchObject({ status: 'accepted' }) }) diff --git a/packages/payload/test/proposals.test.ts b/packages/payload/test/proposals.test.ts index 22adcb4..58687ef 100644 --- a/packages/payload/test/proposals.test.ts +++ b/packages/payload/test/proposals.test.ts @@ -1,8 +1,8 @@ import { verifySnapshot, type DatasetSnapshot, type Proposal } from '@captiveorigins/onetaxonomy-core' import type { Payload, TypedUser } from 'payload' -import { afterAll, beforeAll, describe, expect, it } from 'vitest' +import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest' -import { oneTaxonomy, proposalChecksum } from '../src' +import { oneTaxonomy, PERSON_ID, proposalChecksum } from '../src' import { access, callEndpoint, makeUser, roleOf, sealed, startPayload, tenantsCollection, usersCollection } from './helpers' const SNAPSHOTS = 'taxonomy-snapshots' @@ -23,10 +23,17 @@ let thrower: TypedUser let snapA: string let snapB: string -// Made-up names. The host gives them; a test can change one, or make the lookup fail. -const names = new Map() +// Made-up people. The host gives each signed-in user a person id, and looks a name up for a person id when one is +// shown. A test can change either, or make either lookup fail. +const ids = new Map() // user document id -> person id +const names = new Map() // person id -> the name shown now +let idFails = false let nameFails = false -// Runs once, inside the next name lookup: in an accept, that is after the proposal was read and before it is claimed. +let nameLookups: string[] = [] +const pid = (user: TypedUser) => ids.get(String(user.id)) as string +// Every made-up name, to check that none of them is ever stored or sealed. +const EVERY_NAME = /Ann K|Ben R|Bo T|Keeper|Cara |Dev Cur|Eli Cur|Forged|@/ +// Runs once, inside the next id lookup: in an accept, that is after the proposal was read and before it is claimed. let meanwhile: (() => Promise) | null = null const tenantOfUser = (user: unknown) => { @@ -69,12 +76,17 @@ beforeAll(async () => { }, tenant: { collection: 'tenants', resolve: ({ req }) => tenantOfUser(req.user) }, proposals: { - nameOf: async ({ req }) => { - if (nameFails) throw new Error('directory down') + personId: async ({ req }) => { + if (idFails) throw new Error('directory down') const run = meanwhile meanwhile = null await run?.() - return names.get(String(req.user?.id)) ?? '' + return ids.get(String(req.user?.id)) as string + }, + displayName: ({ personId }) => { + nameLookups.push(personId) + if (nameFails) throw new Error('directory down') + return names.get(personId) }, maxMoves: 3, maxPerUser: 5, @@ -93,8 +105,11 @@ beforeAll(async () => { readerB = await makeUser(payload, 'reader', { tenant: tenantB }) noTenant = await makeUser(payload, 'manager') thrower = await makeUser(payload, 'thrower', { tenant: tenantA }) - for (const [user, name] of [[managerA, 'Cara Curator'], [managerA2, 'Dev Curator'], [managerB, 'Eli Curator'], [ann, 'Ann K.'], [ben, 'Ben R.'], [readerB, 'Bo T.'], [noTenant, 'No Tenant'], [thrower, 'Thrower'], [viewerA, 'Vi Viewer']] as const) - names.set(String(user.id), name) + const people = [[managerA, 'Cara Curator'], [managerA2, 'Dev Curator'], [managerB, 'Eli Curator'], [ann, 'Ann K.'], [ben, 'Ben R.'], [readerB, 'Bo T.'], [noTenant, 'No Tenant'], [thrower, 'Thrower'], [viewerA, 'Vi Viewer']] as const + people.forEach(([user, name], i) => { + ids.set(String(user.id), `P-${1001 + i}`) + names.set(`P-${1001 + i}`, name) + }) for (const [user, set] of [[managerA, (id: string) => (snapA = id)], [managerB, (id: string) => (snapB = id)]] as const) { const doc = (await payload.create({ collection: SNAPSHOTS as never, data: { snapshot: sealed.amniotes() } as never, user, overrideAccess: false })) as Doc await payload.update({ collection: SNAPSHOTS as never, id: doc.id, data: { status: 'published' } as never, user, overrideAccess: false }) @@ -104,10 +119,11 @@ beforeAll(async () => { afterAll(async () => stop?.()) describe('making a proposal', () => { - it('stores it as its author\'s, private, with the plugin\'s own id, name, date and checksum', async () => { + it('stores it as its author\'s, private, with the plugin\'s own id, date and checksum, under their person id and not their name', async () => { const before = Date.now() const doc = await propose(ann, { author: String(managerA.id), + authorName: 'Forged Name', proposalId: 'forged', checksum: 'sha256:forged', title: 'Forged title', @@ -118,8 +134,15 @@ describe('making a proposal', () => { expect(idOf(doc.tenant)).toBe(tenantA) expect(String(idOf(doc.snapshot))).toBe(snapA) expect(p.id).toMatch(/^p-[0-9a-f]{12}$/) - expect(p).toEqual({ id: p.id, title: 'Birds are reptiles', author: 'Ann K.', created: p.created, notes: 'Two lines of evidence.', base: { kind: 'classification', id: 'traditional', version: '1' }, moves: [birds] }) + expect(p).toEqual({ id: p.id, title: 'Birds are reptiles', author: 'P-1004', created: p.created, notes: 'Two lines of evidence.', base: { kind: 'classification', id: 'traditional', version: '1' }, moves: [birds] }) + expect(pid(ann)).toBe('P-1004') expect(Date.parse(p.created)).toBeGreaterThanOrEqual(before) + // The name is looked up for whoever reads it, and isn't in what is stored. + expect(doc.authorName).toBe('Ann K.') + const kept = await payload.db.findOne({ collection: PROPOSALS as never, where: { id: { equals: doc.id } } }) + expect(kept).toBeTruthy() + expect(kept).not.toHaveProperty('authorName') + expect(JSON.stringify(kept)).not.toMatch(EVERY_NAME) for (const f of ['decidedBy', 'decidedAt', 'decisionNote', 'acceptedInto']) expect((await stored(doc.id))[f] ?? null).toBeNull() await remove(doc.id, ann) }) @@ -171,18 +194,29 @@ describe('making a proposal', () => { await payload.delete({ collection: SNAPSHOTS as never, id: staged.id }) }) - it('stops when the site can\'t give the person\'s name', async () => { + it('stops when the site can\'t give the person\'s id, or gives a name or an email address instead', async () => { const before = (await payload.count({ collection: PROPOSALS as never })).totalDocs - nameFails = true + const logged = vi.spyOn(payload.logger, 'error').mockImplementation(() => undefined) try { - await expect(propose(ann)).rejects.toMatchObject({ status: 500, message: expect.stringMatching(/couldn't give your name/) }) + idFails = true + await expect(propose(ann)).rejects.toMatchObject({ status: 500, message: expect.stringMatching(/couldn't give your id/) }) + idFails = false + const refused = ['', ' ', 'Ann Keeper', 'ann.keeper@example.org', ' P-1004', 'P-1004 ', '-P-1004', 'P/1004', 'Ann\u00a0K', 'x'.repeat(65), 1004, null, undefined, { id: 'P-1004' }] + for (const bad of refused) { + ids.set(String(ann.id), bad) + await expect(propose(ann), JSON.stringify(bad)).rejects.toMatchObject({ status: 500, message: expect.stringMatching(/couldn't give your id/) }) + } + // The log says what went wrong each time, and never repeats the value: it may be somebody's name. + expect(logged).toHaveBeenCalledTimes(refused.length + 1) + expect(JSON.stringify(logged.mock.calls)).not.toMatch(/Ann|example\.org|1004/) + expect(JSON.stringify(logged.mock.calls)).toMatch(/never a name or an email address/) } finally { - nameFails = false + idFails = false + ids.set(String(ann.id), 'P-1004') + logged.mockRestore() } - names.set(String(ann.id), ' ') - await expect(propose(ann)).rejects.toThrow(/couldn't give your name/) - names.set(String(ann.id), 'Ann K.') expect((await payload.count({ collection: PROPOSALS as never })).totalDocs).toBe(before) + for (const ok of ['P-1004', '42', '6730ab12cd34ef56ab78cd90', 'users:6730ab12', 'a1b2c3d4-0000-4000-8000-000000000000', 'CO_1.2', 'x'.repeat(64)]) expect(PERSON_ID.test(ok), ok).toBe(true) }) it('keeps at most maxPerUser proposals per person', async () => { @@ -262,13 +296,12 @@ describe('private and shared', () => { expect(await stored(p.id)).toMatchObject({ status: 'shared', checksum: p.checksum }) }) - it('lets its author take it back, revise it under their current name, and share it again', async () => { + it('lets its author take it back, revise it and share it again, under the id it was made with', async () => { await update(p.id, { status: 'private' }, ann) expect(await seen(ben)).toEqual([]) - names.set(String(ann.id), 'Ann Keeper') const revised = await update(p.id, { proposal: draft({ title: 'Birds and mammals are reptiles', moves: [birds, mammals] }) }, ann) const before = p.proposal as Proposal - expect(revised.proposal).toMatchObject({ id: before.id, created: before.created, author: 'Ann Keeper', title: 'Birds and mammals are reptiles' }) + expect(revised.proposal).toMatchObject({ id: before.id, created: before.created, author: 'P-1004', title: 'Birds and mammals are reptiles' }) expect(revised).toMatchObject({ title: 'Birds and mammals are reptiles', proposalId: before.id, checksum: proposalChecksum(revised.proposal as Proposal) }) expect(revised.checksum).not.toBe(p.checksum) // An edit that doesn't fit is refused, and the stored proposal stays as it was. @@ -278,6 +311,47 @@ describe('private and shared', () => { expect(p).toMatchObject({ status: 'shared', title: 'Birds are reptiles' }) }) + it('shows a new name at once, with nothing stored to change', async () => { + expect((await readAs(p.id, ben)).authorName).toBe('Ann K.') + names.set(pid(ann), 'Ann Keeper') + const now = await readAs(p.id, ben) + expect(now).toMatchObject({ authorName: 'Ann Keeper', checksum: p.checksum }) + expect((now.proposal as Proposal).author).toBe('P-1004') + // A name can't be written through the proposal: the field is only ever looked up. + await update(p.id, { authorName: 'Forged Name' }, ann) + expect((await readAs(p.id, ben)).authorName).toBe('Ann Keeper') + expect(JSON.stringify(await payload.db.findOne({ collection: PROPOSALS as never, where: { id: { equals: p.id } } }))).not.toMatch(/Forged|Keeper/) + }) + + it('asks for each person\'s name once per request, and shows the id alone when the lookup fails or has no name', async () => { + const more = [await shared(ann, { proposal: draft({ title: 'Second' }) }), await shared(ann, { proposal: draft({ title: 'Third' }) })] + nameLookups = [] + const listed = (await payload.find({ collection: PROPOSALS as never, user: ben, overrideAccess: false, depth: 0, limit: 100 })).docs as Doc[] + expect(listed.filter((d) => (d.proposal as Proposal).author === 'P-1004').length).toBeGreaterThanOrEqual(3) + expect(nameLookups.filter((id) => id === 'P-1004')).toHaveLength(1) + nameFails = true + try { + const doc = await readAs(p.id, ben) + expect(doc.authorName ?? null).toBeNull() + expect((doc.proposal as Proposal).author).toBe('P-1004') + const shown = await preview(p.id, ben) + expect(shown.status).toBe(200) + expect(shown.body.proposal).toMatchObject({ author: 'P-1004' }) + expect(shown.body.proposal).not.toHaveProperty('authorName') + } finally { + nameFails = false + } + for (const empty of ['', ' ', 'x'.repeat(201)]) { + names.set(pid(ann), empty) + expect((await readAs(p.id, ben)).authorName ?? null).toBeNull() + } + names.set(pid(ann), 'Ann Keeper') + for (const doc of more) { + await update(doc.id, { status: 'private' }, ann) + await remove(doc.id, ann) + } + }) + it('never lets a decision, the author, the snapshot or the tenant be set through an update', async () => { const other = (await payload.create({ collection: SNAPSHOTS as never, data: { snapshot: sealed.structures() } as never, user: managerA, overrideAccess: false })) as Doc const tries: [Record, RegExp][] = [ @@ -307,7 +381,7 @@ describe('preview', () => { const [p] = (await payload.find({ collection: PROPOSALS as never, user: ben, overrideAccess: false, depth: 0 })).docs as Doc[] const { status, body } = await preview(p!.id, ben) expect(status).toBe(200) - expect(body.proposal).toMatchObject({ doc: String(p!.id), id: p!.proposalId, title: 'Birds are reptiles', author: 'Ann Keeper', status: 'shared', checksum: p!.checksum, base: { kind: 'classification', id: 'traditional', version: '1' } }) + expect(body.proposal).toMatchObject({ doc: String(p!.id), id: p!.proposalId, title: 'Birds are reptiles', author: 'P-1004', authorName: 'Ann Keeper', status: 'shared', checksum: p!.checksum, base: { kind: 'classification', id: 'traditional', version: '1' } }) expect(body.snapshot).toMatchObject({ doc: snapA, id: 'fixture-amniotes', version: '1' }) expect(body).toMatchObject({ acceptable: true, truncated: false, counts: { moves: 1 } }) expect(body).not.toHaveProperty('notAcceptable') @@ -349,17 +423,24 @@ describe('accepting', () => { [undefined, 400, /Name the new snapshot version/], ] for (const [body, status, error] of tries) expect(await accept(p.id, managerA, body), JSON.stringify(body)).toMatchObject({ status, body: { error: expect.stringMatching(error) } }) - nameFails = true + const logged = vi.spyOn(payload.logger, 'error').mockImplementation(() => undefined) try { - expect(await accept(p.id, managerA, { version: '2', checksum: p.checksum })).toMatchObject({ status: 500, body: { error: expect.stringMatching(/couldn't give your name/) } }) + idFails = true + expect(await accept(p.id, managerA, { version: '2', checksum: p.checksum })).toMatchObject({ status: 500, body: { error: expect.stringMatching(/couldn't give your id/) } }) + idFails = false + // A manager whose id comes back as a name accepts nothing: the name would be sealed. + ids.set(String(managerA.id), 'Cara Curator') + expect(await accept(p.id, managerA, { version: '2', checksum: p.checksum })).toMatchObject({ status: 500, body: { error: expect.stringMatching(/couldn't give your id/) } }) } finally { - nameFails = false + idFails = false + ids.set(String(managerA.id), 'P-1001') + logged.mockRestore() } expect(await snapshotCount()).toBe(count) expect(await stored(p.id)).toMatchObject({ status: 'shared', checksum: p.checksum }) }) - it('makes a new staged version, with both names in its record, and leaves the published one alone', async () => { + it('makes a new staged version, with both people\'s ids and neither name in its record, and leaves the published one alone', async () => { const base = await snapshot(snapA) const { status, body } = await accept(p.id, managerA, { version: '2', checksum: p.checksum, createdAt: '2026-11-11T12:00:00.000Z', note: 'Agreed at the review' }) expect(status).toBe(200) @@ -375,10 +456,12 @@ describe('accepting', () => { expect(verifySnapshot(content)).toBe(true) expect(content.derivedFrom).toEqual({ snapshot: 'fixture-amniotes', version: '1', checksum: base.checksum, - proposal: { id: p.proposalId, title: 'Birds are reptiles', author: 'Ann Keeper', acceptedBy: 'Cara Curator', classification: { id: 'traditional', version: '1' }, moved: ['c:aves'] }, + proposal: { id: p.proposalId, title: 'Birds are reptiles', author: 'P-1004', acceptedBy: 'P-1001', classification: { id: 'traditional', version: '1' }, moved: ['c:aves'] }, }) expect(content.classifications[0]!.placements.find((x) => x.concept === 'c:aves')!.parent).toBe('c:reptilia') - expect(content.sources.find((s) => s.id === `proposal:${p.proposalId}`)).toBeTruthy() + expect(content.sources.find((s) => s.id === `proposal:${p.proposalId}`)!.provenance).toMatch(/^Proposed by P-1004 \(.+\)\. Accepted by P-1001 \(/) + // Nothing sealed names a person: the whole stored document, checksum included, holds ids only. + expect(JSON.stringify(next)).not.toMatch(EVERY_NAME) // The published snapshot is what it was, and readers see nothing new until the new version is published. const after = await snapshot(snapA) @@ -440,7 +523,8 @@ describe('accepting', () => { const again = await accept(p.id, managerA2, { version: '2', checksum: p.checksum }) expect(again.status).toBe(200) const content = (await snapshot(again.body.snapshot.doc)).snapshot as DatasetSnapshot - expect(content.derivedFrom!.proposal).toMatchObject({ author: 'Ann Keeper', acceptedBy: 'Dev Curator' }) + expect(content.derivedFrom!.proposal).toMatchObject({ author: 'P-1004', acceptedBy: 'P-1002' }) + expect(JSON.stringify(content)).not.toMatch(EVERY_NAME) expect((await stored(p.id)).status).toBe('accepted') }) @@ -545,22 +629,42 @@ describe('declining', () => { }) describe('when someone leaves', () => { - it('lets the site take back and delete their open proposals, under the name they last used', async () => { + it('lets the site take back and delete their open proposals', async () => { const open = await shared(ben, { proposal: draft({ title: 'Mammals are reptiles', moves: [mammals] }) }) const drafted = await propose(ben, { proposal: draft({ title: 'A thought' }) }) - // Server code, with no user. The name isn't asked for again: nobody is signed in to ask about. - nameFails = true + // Server code, with no user. The id isn't asked for again: nobody is signed in to ask about. + idFails = true try { const back = (await payload.update({ collection: PROPOSALS as never, id: open.id as string, data: { status: 'private' } as never, depth: 0 })) as Doc expect(back).toMatchObject({ status: 'private', checksum: open.checksum, author: String(ben.id) }) - expect((back.proposal as Proposal).author).toBe('Ben R.') + expect((back.proposal as Proposal).author).toBe('P-1005') } finally { - nameFails = false + idFails = false } expect(await seen(ann)).not.toContain(open.id) for (const doc of [open, drafted]) await payload.delete({ collection: PROPOSALS as never, id: doc.id as string }) for (const doc of [open, drafted]) await expect(stored(doc.id)).rejects.toThrow() - // Accepted ones stay ("accepts once" above): a snapshot version names them. + }) + + it('shows their accepted proposals without a name once the site stops giving one, with nothing to take out of the snapshot', async () => { + const [accepted] = (await payload.find({ collection: PROPOSALS as never, where: { and: [{ status: { equals: 'accepted' } }, { author: { equals: String(ann.id) } }] }, depth: 0, limit: 1 })).docs as Doc[] + expect(accepted).toBeTruthy() + expect((await readAs(accepted!.id, ben)).authorName).toBe('Ann Keeper') + const version = await snapshot(accepted!.acceptedInto) + names.delete(pid(ann)) + try { + const doc = await readAs(accepted!.id, ben) + expect(doc.authorName ?? null).toBeNull() + expect(doc).toMatchObject({ status: 'accepted', checksum: accepted!.checksum }) + expect((doc.proposal as Proposal).author).toBe('P-1004') + // The sealed version is what it was, still verifies, and never held the name. + const after = await snapshot(accepted!.acceptedInto) + expect(after.checksum).toBe(version.checksum) + expect(verifySnapshot(after.snapshot as DatasetSnapshot)).toBe(true) + expect(JSON.stringify(after)).not.toMatch(EVERY_NAME) + } finally { + names.set(pid(ann), 'Ann Keeper') + } }) })