fix(core): apply field defaultValue before create validation (#615)#620
Conversation
A field's defaultValue was only realised as a Prisma @default(...) at DB write time — after the write pipeline's validation phase. That ordering made a required-with-default field (e.g. select({ validation: { isRequired: true }, defaultValue: 'STANDARD' })) fail isRequired validation when the field was omitted on create, diverging from Keystone 6's resolve-then- validate ordering. Introduce a single shared applyCreateDefaults() helper that fills omitted fields with their declared defaultValue during the resolve phase (after resolveInput hooks, before validation), wired into both the top-level Hook Pipeline and the nested-relation create path. Create-only; explicit values (including explicit null) are preserved; the timestamp { kind: 'now' } sentinel is left to the DB-level @default(now()). Prisma @default(...) generation is unchanged. Implements #615 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SigyUcdxbBsdeLVWiFbYtS
🦋 Changeset detectedLatest commit: 37bf2e5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
borisno2
left a comment
There was a problem hiding this comment.
Review — /review (verdict: APPROVE)
I reviewed this strictly from the PR diff and the code it touches (no prior context). The fix is well-scoped, correctly placed in the resolve-then-validate span, and the guard rails hold up under scrutiny. Verdict: APPROVE. (Posting as a Comment review rather than a formal Approve since the change originates from the integration identity; the verdict is an explicit approve.)
What it does
Adds a single shared helper applyCreateDefaults() that fills omitted (undefined) fields with their declared defaultValue during the resolve phase — after resolveInput, before validation — wired into both the top-level Hook Pipeline (Phase 1.75) and the nested-relation create path (processNestedCreate step 3.5). This fixes #615 (a required-with-default field failing isRequired on an omitted create input), matching Keystone's ordering.
Risk-area verification (all pass)
- Placement / create-only. Injection runs after field
resolveInputand before list/fieldvalidateandvalidateFieldRules, in both paths. Top-level is gated byif (operation === 'create'); the nested helper lives insideprocessNestedCreate, which only ever runs with'create'. Update never injects. ConfirmedvalidateFieldRulesvalidatesresolvedData, so injected defaults satisfyisRequired. ✓ - Explicit-value preservation incl.
null.if (resolvedData[fieldKey] !== undefined) continueskips any present key, so an explicitnullis preserved and not overwritten. Falsy literals (false,0,'') are correctly injected since the guard testsundefined, not falsiness. ✓ - Nested create covered. Wired and tested (
profile: { create: {} }resolves the nestedkinddefault). ✓ - Timestamp
{ kind: 'now' }sentinel.isNowSentinelmatches{ kind: 'now' }exactly the shapetimestamp()emits and skips it (left to Prisma@default(now())); a concreteDatedefault is still injected, which is a valid Prisma create value. ✓ - Field skips. Virtual fields are reliably skipped —
virtual()setsvirtual: true(required onVirtualField), soif (fieldConfig.virtual)matches. System (id/createdAt/updatedAt) andrelationshipfields are skipped. ✓ - Prisma
@default(...)generation unchanged. No generator files are touched by this PR. ✓ - House rules. No
anyand no type casting introduced in the new helper;defaultValueis read via'defaultValue' in fieldConfigagainst the typed config and the loneunknownnarrowing (isNowSentinel) is internal. Changeset present (patch,@opensaas/stack-core). ✓ - All field-type defaults safe. Audited every field type:
text/select/calendarDay= string,decimal= string (Prisma coerces),integer= number,checkbox= boolean,json= literal,timestamp= the only object sentinel and it's guarded. No raw-object corruption path exists. ✓
Test coverage
Exercises both the Hook-Pipeline unit surface and full context.db creates: omitted select/text/integer/checkbox resolve to defaults; explicit value and explicit null are preserved; update does not inject; required-without-default still throws; and both top-level and nested-relation creates store the default through the full pipeline. The two sudo.test.ts assertion updates correctly reflect the now-injected views: 0. Good coverage of every flagged case.
Minor observation (non-blocking, no change required)
Injecting a default into resolvedData means a field that was omitted but has a default now appears in resolvedData, so its create-side field-level beforeOperation/afterOperation hooks (gated by fieldKey in resolvedData) will now fire where previously they would not. This is the correct Keystone-aligned behavior (the field genuinely has a resolved value now), but it is a subtle behavioral change for anyone relying on those hooks only firing for explicitly-provided fields. Worth a mention in the changeset/notes, not a code change.
No correctness, regression, or convention issues found.
Generated by Claude Code
…k-firing in changeset
Adds a dedicated unit test for the real `applyCreateDefaults` exercising the
previously-uncovered guard rails (virtual/system/relationship skips, no-default
skip, explicit-value and explicit-null preservation, the timestamp { kind: 'now' }
sentinel skip, and concrete Date injection). This lifts statement coverage for
src/context/apply-defaults.ts from 82.35% to 100%, clearing the 85% per-directory
threshold for src/context/** that failed CI. No runtime behavior changed.
Also appends a note to the changeset documenting that defaulted fields now have
their create-side field-level beforeOperation/afterOperation hooks fire (since the
default is filled into resolvedData before validation).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SigyUcdxbBsdeLVWiFbYtS
Coverage Report for Core Package Coverage (./packages/core)
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||
Coverage Report for UI Package Coverage (./packages/ui)
File CoverageNo changed files found. |
Coverage Report for CLI Package Coverage (./packages/cli)
File CoverageNo changed files found. |
Coverage Report for Auth Package Coverage (./packages/auth)
File CoverageNo changed files found. |
Coverage Report for Storage Package Coverage (./packages/storage)
File CoverageNo changed files found. |
Coverage Report for RAG Package Coverage (./packages/rag)
File CoverageNo changed files found. |
Coverage Report for Storage S3 Package Coverage (./packages/storage-s3)
File CoverageNo changed files found. |
Coverage Report for Storage Vercel Package Coverage (./packages/storage-vercel)
File CoverageNo changed files found. |
Implements #615.
Problem
A field's
defaultValuewas only realised as a Prisma@default(...)applied by the DB at write time — after the write pipeline's validation phase. Because validation never consulteddefaultValue, a create that omitted a required-with-default field (e.g.select({ validation: { isRequired: true }, defaultValue: 'STANDARD' })) failedisRequiredvalidation instead of resolving to its default. This diverges from Keystone 6's resolve-then-validate ordering and affected every field type that supportsdefaultValue, not justselect.Fix (resolve-then-validate, Keystone parity)
A single shared helper
applyCreateDefaults()(packages/core/src/context/apply-defaults.ts) fills omitted fields with their declareddefaultValueduring the resolve phase — afterresolveInputhooks, before validation:hook-pipeline.ts) between fieldresolveInputand listvalidate.nested-operations.ts) at the same point.Guard rails
operation === 'create').undefined(omitted) are filled; an explicitly-provided value, including explicitnull, is left untouched.id/createdAt/updatedAt) and relationship fields.{ kind: 'now' }sentinel is not injected (it is a DB-level@default(now())request, not a literal); a concreteDatedefault is injected.@default(...)generation is unchanged.Tests
tests/default-value-create.test.ts: Hook-Pipeline unit coverage (omittedselect/text/integer/checkboxresolve to defaults; explicit value and explicitnullpreserved; update does not inject; required-without-default still throws) plus fullcontext.dbcreate through a tx-aware mock for top-level and nested-relation creates.tests/sudo.test.tsassertions to reflect the now-correct injection of a field'sdefaultValue: 0into the create payload.@opensaas/stack-core).All 714 core tests pass; build + lint clean.
🤖 Generated with Claude Code
https://claude.ai/code/session_01SigyUcdxbBsdeLVWiFbYtS
Generated by Claude Code