fix: failed validation clears unsaved content#377
Open
adrianboros wants to merge 6 commits into
Open
Conversation
… into beforeCreate/beforeUpdate, validating the raw incoming request data directly
…rs in the new validation flow
✅ Deploy Preview for interledger-org-v5 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
adrianboros
temporarily deployed
to
strapi-staging
GitHub Actions
Inactive
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a Strapi admin UX issue where a failed save (validation error) would revert the editor’s unsaved changes back to the last-saved state. It does this by patching Strapi’s content-manager RTK Query caching behavior on failed mutations and by moving/expanding server-side request-body validation to return field-level errors that the admin can highlight.
Changes:
- Patch
@strapi/content-managerto avoid invalidating/refetching and undoing optimistic cache updates on failed save/publish, preventing form state from being wiped. - Add/expand CMS validation utilities to aggregate multiple field errors and forward
details.errors[].pathso the admin can highlight the specific invalid fields. - Remove lifecycle-hook-based validation that was operating on resolved component references, and register Koa middleware to validate the raw content-manager request body pre-write.
Reviewed changes
Copilot reviewed 16 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-workspace.yaml | Moves workspace-level dependency overrides into the workspace config. |
| pnpm-lock.yaml | Lockfile updates reflecting dependency graph changes from overrides/patch workflow. |
| patches/@strapi__content-manager@5.41.1.patch | Patches Strapi admin document mutations to avoid cache rollback/invalidation on error. |
| package.json | Removes pnpm overrides from root package.json (now centralized elsewhere). |
| cms/src/utils/pageLifecycle.ts | Removes lifecycle validation hook support and associated validation execution. |
| cms/src/utils/navigationLifecycle.ts | Removes navigation validation from lifecycle update path (validation shifts to middleware). |
| cms/src/utils/index.ts | Re-exports newly added validators and merge helper from contentValidation. |
| cms/src/utils/contentValidation.ts | Adds field-error aggregation and validator composition; expands validators to return path-aware errors. |
| cms/src/utils/contentValidation.test.ts | Adds/extends Vitest coverage for validators, including path-aware error details and merging behavior. |
| cms/src/utils/blogLifecycle.ts | Removes blog post validation prior to MDX export from lifecycles (validation shifts to middleware). |
| cms/src/index.ts | Registers new Koa middleware to validate raw content-manager request bodies and forward error details. |
| cms/src/index.test.ts | Adds tests for the body-validation middleware behavior and response shaping. |
| cms/src/api/summit-page/content-types/summit-page/lifecycles.ts | Removes per-content-type lifecycle validation configuration. |
| cms/src/api/grant-page/content-types/grant-page/lifecycles.ts | Removes per-content-type lifecycle validation configuration. |
| cms/src/api/foundation-page/content-types/foundation-page/lifecycles.ts | Removes per-content-type lifecycle validation configuration. |
| cms/pnpm-workspace.yaml | Adds CMS-specific overrides and wires patchedDependencies to the Strapi content-manager patch. |
| cms/pnpm-lock.yaml | CMS lockfile updates reflecting patched dependency and resulting dependency graph changes. |
| cms/package.json | Removes pnpm overrides from cms/package.json (now centralized elsewhere). |
Files not reviewed (2)
- cms/pnpm-lock.yaml: Generated file
- pnpm-lock.yaml: Generated file
Comment on lines
+57
to
+70
| export function mergeValidationErrors( | ||
| ...validationErrors: Array<errors.ValidationError | undefined> | ||
| ): errors.ValidationError | undefined { | ||
| const present = validationErrors.filter( | ||
| (err): err is errors.ValidationError => err != null | ||
| ) | ||
| if (present.length === 0) return undefined | ||
| const allFieldErrors = present.flatMap( | ||
| (err) => (err.details as { errors?: unknown[] } | undefined)?.errors ?? [] | ||
| ) | ||
| return new errors.ValidationError(present[0]!.message, { | ||
| errors: allFieldErrors | ||
| }) | ||
| } |
Comment on lines
437
to
441
| async afterCreate(event: Event) { | ||
| const { result } = event | ||
| if (!result) return | ||
| await runValidation(config, result) | ||
| if (shouldSkipMdxExport()) return | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
In the Strapi admin, saving a page with a validation error would show the error but also wipe the editor's unsaved changes back to the last-saved DB state.
Root causes found (three separate, but related bugs)
@strapi/content-manager—updateDocument's RTK Query mutation optimistically writes edited data into the cache, then rolls it back (patchResult.undo()) on any error, and bothupdateDocument/publishDocumentinvalidated their cache tags even on failure. The admin's<Form>re-syncs to that reverted cache, wiping in-progress edits.beforeCreate/beforeUpdatelifecycle hooks, but those see component fields (primaryCta,hero, etc.) already resolved to{id, __pivot}DB references, not the raw{text, link}shape — causing false-positive rejections on valid saves. Moved validation to Koa middleware reading the raw request body instead (matching the pre-existing, correctvalidateNoNestedJsxpattern).details.errors[].path, and even after fixing that, the middleware's response body construction dropped.detailsentirely, so the admin could only show a generic toast, never highlight the specific field.Fixes added:
patches/@strapi__content-manager@5.41.1.patch(pnpm native patch, scoped incms/pnpm-workspace.yaml— cms has its own standalone lockfile separate from the root workspace, which took a few tries to discover): stops the optimistic-update rollback and skips cache invalidation on error for updateDocument/publishDocument.cms/src/utils/contentValidation.ts: every validator (validateGrantPagePrimaryCta,validateGrantPageFaqSection,validateCtaStrip,validateProfileCta,validateHeroFields,validateBlogFields,validateNavigationLabels) now collects all failing fields per save (not just the first) viacombineFieldErrors, andmergeValidationErrorscombines multiple validators' results for one content type.cms/src/index.ts:registerBodyValidationMiddlewarehelper validates the raw request body pre-write and now correctly forwards details. Added missing validation entirely forgrant-overview-pageandprofile-page(had none), and addedctaStripvalidation togrant-page/grant-overview-page(a required component whose required sub-fields were never enforced on update).beforeCreate/beforeUpdatelifecycle validation additions inpageLifecycle.ts,navigationLifecycle.ts,blogLifecycle.ts.Related Issue
Fixes INTORG-796
Manual Test
Test validation on any page see if the issue is fixed and we don't lose content.
Check valid updates, check mdx file sync logic both ways.
Checks
pnpm run formatpnpm run lintPR Checklist
feat: ...,fix: ...)