fix(core): enforce required and multiple on reference fields - #2503
fix(core): enforce required and multiple on reference fields#2503MA2153 wants to merge 3 commits into
Conversation
A reference field's selections are edges in _emdash_content_references, not a column on the collection's table. The registry skips column DDL for storage-less field types, the schema handlers own the backing relation's lifecycle (created with the field, destroyed with it, target collection immutable), and the previously unregistered relation and reference-edge routes are wired into injectCoreRoutes. A storage-less field never appears in `data` in either direction: it is excluded from the generated Zod shape (so a required reference field is satisfiable at all), rejected with a VALIDATION_ERROR when a caller sends one, and filtered out of reads so a column left behind by an older version cannot round-trip back into a save. That replaces the reference-target existence pass in validateContentData, which validated a column-backed value that no longer exists. Seeds apply a reference field's $ref: value as an edge, so seed files keep working unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reference selections ride in the content create/update body under a `references` key and are written in the same transaction as the entry, so a child that fails to resolve aborts the whole save rather than leaving an entry with taxonomies, bylines and SEO already committed. The editor GET opts into hydrating the first page of each reference field's children. Two paths that used to lose edges now carry them: duplicating an entry copies its outgoing references onto the copy, and purging a row clears the group's edges only once no sibling — trashed ones included, they are still restorable — is left to own them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A reference field's `multiple: false` and `required` settings were UI-only: the content body and the edge endpoint would both happily write several children to a single-reference field, or clear a required one. Both checks now live in `setReferenceChildren`, the one function every edge write passes through, so the content body, the standalone endpoint and the seed engine can't drift from each other. The create path additionally rejects a payload that omits a required reference field altogether, which no edge write would otherwise visit. Updates keep partial semantics: a field the payload doesn't mention is left alone. A relation with no reference field behind it stays unconstrained — the relations API can create one directly, and it carries no field config to enforce. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 8efd08f The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR's core change is the right shape: centralize required/multiple enforcement in setReferenceChildren and add an up-front validateRequiredReferencesPresent check in handleContentCreate for omitted required fields. The tests cover the important paths (create, update, direct edge endpoint, and the unconstrained-relation carve-out).
The diff is large because it currently contains the stacked #2492/#2496 commits; I reviewed the constraint-enforcement surface (and the adjacent code it touches) for correctness.
Two issues:
-
Translation creates are over-validated.
handleContentCreateruns the omitted-required check unconditionally, including whentranslationOfis set. Since reference edges are owned by thetranslation_group, a translation of an entry that already satisfies a required reference inherits those edges and should not be forced to repeat them. As written, a translation create that omitsreferencesis rejected even though the group already has the required edge. -
Type/runtime mismatch for hydrated references.
ContentItem["references"]["children"]is missingtranslationGroup, butresolveEntriespopulates it and the API schema exposes it.
Neither is a security or data-loss issue, so comment rather than request_changes.
|
|
||
| // Selections that are present get checked as they're written; this is the | ||
| // required field the payload leaves out altogether. | ||
| const requiredRefs = await validateRequiredReferencesPresent(db, collection, body.references); |
There was a problem hiding this comment.
[needs fixing] handleContentCreate rejects creates that omit a required reference field before opening the transaction. That is correct for a fresh entry, but it also applies when body.translationOf is set. Translations share the source's translation_group, so reference edges are already satisfied by the source row; requiring the caller to re-send them breaks translation creation for collections with required references.
Skip the omitted-ref check when creating a translation (explicit body.references still flows through setReferenceChildren and is validated there):
| const requiredRefs = await validateRequiredReferencesPresent(db, collection, body.references); | |
| // Selections that are present get checked as they're written; this is the | |
| // required field the payload leaves out altogether. Translations share | |
| // their source's translation_group (and therefore its reference edges), | |
| // so they inherit any required references already satisfied there. | |
| if (!body.translationOf) { | |
| const requiredRefs = await validateRequiredReferencesPresent(db, collection, body.references); | |
| if (!requiredRefs.success) return requiredRefs; | |
| } |
| { | ||
| children: Array<{ | ||
| id: string; | ||
| slug: string | null; | ||
| collection: string; | ||
| title: string | null; | ||
| locale: string | null; | ||
| sortOrder?: number; |
There was a problem hiding this comment.
[suggestion] resolveEntries returns EntryRef objects containing translationGroup, and the API schema adds it to entryRefSchema, but this database-layer type for hydrated references omits it. Add the field so callers don't get a TypeScript error when reading the value that is present at runtime:
| { | |
| children: Array<{ | |
| id: string; | |
| slug: string | null; | |
| collection: string; | |
| title: string | null; | |
| locale: string | null; | |
| sortOrder?: number; | |
| children: Array<{ | |
| id: string; | |
| slug: string | null; | |
| collection: string; | |
| title: string | null; | |
| locale: string | null; | |
| translationGroup: string | null; | |
| sortOrder?: number; | |
| }>; |
Scope checkThis PR changes 3,346 lines across 35 files. Large PRs are harder to review and more likely to be closed without review. If this scope is intentional, no action needed. A maintainer will review it. If not, please consider splitting this into smaller PRs. See CONTRIBUTING.md for contribution guidelines. |
github-actions
Bot
added
the
review/awaiting-author
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/registry-verification
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
Overlapping PRsThis PR modifies files that are also changed by other open PRs:
This may cause merge conflicts or duplicated work. A maintainer will coordinate. |
1 participant
What does this PR do?
Closes the second of the two P1 findings on #1928 (r3757946596): a reference field's
multipleandrequiredsettings were UI-only, so the content and edge APIs would both happily write several children to amultiple: falsefield, or clear a required one.Where the checks live. Both go in
setReferenceChildren— the one function every edge write passes through, whether it came from the content body, the standalonePUT .../childrenendpoint, or the seed engine. Enforcing in the content handler alone would have left the constraint bypassable by calling the endpoint directly, and enforcing in both would have left two copies to drift apart.The one check that can't live there is a create whose payload omits a required reference field altogether: no edge write is attempted for a field nobody mentioned, so there is nothing for the choke point to see.
handleContentCreatechecks that separately, before opening the transaction.Updates keep partial semantics. A reference field an update doesn't mention is left alone, exactly as a required column-backed field is — only an explicit
[]is a violation.A relation with no reference field behind it stays unconstrained. Relations exist independently of fields — the relations API can create one directly, and a plugin may rely on that — so there is no field config to enforce and nothing is rejected. That also means a typo'd relation group still fails, but as
NOT_FOUNDfrom relation resolution, which is the accurate error.A note on required self-references
Writing the tests surfaced this: a required reference field pointing at its own collection makes that collection's first entry impossible to create — there is nothing to reference yet. That's inherent to
requiredon a self-relation, the same way aNOT NULLself-FK behaves, and this PR doesn't try to special-case it. Worth knowing before the admin exposes the toggle; the schema editor may want to warn.Part of #386.
Stack
required/multipleenforcementType of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runmessages.pochanges are included.emdash: minor)AI-generated code disclosure
Screenshots / test output
tests/integration/content/reference-constraints.test.ts, both dialects, 9 cases: