Skip to content

fix(admin): name the field by its label when a save fails validation - #3205

Merged
ascorbic merged 3 commits into
emdash-cms:mainfrom
danielmlr:fix/validation-field-labels
Sep 21, 2026
Merged

ascorbic merged 3 commits into
emdash-cms:mainfrom
danielmlr:fix/validation-field-labels

Conversation

@danielmlr

Copy link
Copy Markdown
Contributor

What does this PR do?

Names each rejected field by its editor label and says what it needs, so excerpt: Too big: expected string to have <=160 characters becomes "Summary can have at most 160 characters."

The content validator joined per-field failures into one message, so #1838's per-field formatting never ran for content saves. It now returns them in error.details.issues with path, code and any failed bound; error.message is unchanged. The admin's save toasts turn each issue into a translatable sentence; other errors keep the server message. Like SAVE_REJECTED's details, the issues serve the admin, so the changeset and OpenAPI leave them out; the core test pins each code the admin words, so a Zod upgrade that renames one fails there.

Closes #2426

Part of #2417 (item 2, requested here).

Type of change

  • Bug fix
  • Feature (requires maintainer-approved Discussion)
  • Refactor (no behavior change)
  • Translation
  • Documentation
  • Performance improvement
  • Tests
  • Chore (dependencies, CI, tooling)

Checklist

  • I have read CONTRIBUTING.md
  • pnpm typecheck passes
  • pnpm lint passes
  • pnpm test passes (or targeted tests for my change)
  • pnpm format has been run
  • I have added/updated tests for my changes (if applicable)
  • User-visible strings in the admin UI are wrapped for translation (if applicable). Do not include messages.po changes except in translation PRs — a workflow extracts catalogs on merge to main.
  • I have added and reviewed the user-facing changeset (if this PR changes a published package)
  • New features link to an approved Discussion: https://github.com/emdash-cms/emdash/discussions/... (n/a: bug fix)
  • I have included screenshots below if this PR changes the UI

AI-generated code disclosure

  • This PR includes AI-generated code — model/tool: Claude Opus 5

Screenshots / test output

A Summary field (minLength: 20) and a Reading time field (min: 1, max: 60) rejected by one autosave.

Post editor in English: Summary holds "Too short" and Reading time holds 99, both outlined red. The Autosave failed toast reads "Summary needs at least 20 characters. Reading time must be at most 60."

The same editor in Arabic; the new sentences stay English until the catalog is translated.

The same editor in Arabic with Reading time 0: the toast title is Arabic, its two sentences English.

  • Failing on main: three of the four new core cases (the fourth pins the unchanged error.message) and both new cases in router.test.tsx; content-validation-errors.test.ts covers the wording.
  • pnpm exec playwright test autosave → 3 passed, one of them new; pnpm typecheck clean; pnpm lint:json → 0 diagnostics
  • pnpm --filter @emdash-cms/admin test → 2367 passed; pnpm --filter emdash test → 6897 passed, 1 failed: cron-schedule.test.ts fails in a Europe/Berlin shell and passes with TZ=UTC, outside this diff

A save the content validator rejected reached the editor as one joined
string that named each field by its slug and used the validator's own
wording, e.g. "excerpt: Too big: expected string to have <=160
characters". The validator built those failures per field and then
joined them, so the admin had nothing structured to word differently.

The validator now also returns the failures in error.details.issues,
one per field, with a code and the bound that failed; error.message is
unchanged. The admin turns them into a translatable sentence per field
that uses the label the editor shows, in the toasts for a save, an
autosave, a new entry and a new translation. An error without these
issues keeps the server message.
@changeset-bot

changeset-bot Bot commented Sep 18, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: ac93288

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 12 packages
Name Type
emdash Patch
@emdash-cms/admin Patch
@emdash-cms/cloudflare Patch
@emdash-cms/plugin-test Patch
@emdash-cms/sandbox-workerd Patch
@emdash-cms/auth Patch
@emdash-cms/blocks Patch
create-emdash Patch
@emdash-cms/gutenberg-to-portable-text Patch
@emdash-cms/x402 Patch
@emdash-cms/auth-atproto Patch
@emdash-cms/plugin-embeds Patch

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

@github-actions

Copy link
Copy Markdown
Contributor

Scope check

This PR changes 728 lines across 10 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.

@pkg-pr-new

pkg-pr-new Bot commented Sep 18, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

@emdash-cms/admin

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/admin@3205

@emdash-cms/auth

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/auth@3205

@emdash-cms/auth-atproto

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/auth-atproto@3205

@emdash-cms/blocks

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/blocks@3205

@emdash-cms/cloudflare

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/cloudflare@3205

@emdash-cms/contentful-to-portable-text

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/contentful-to-portable-text@3205

emdash

npm i https://pkg.pr.new/emdash-cms/emdash@3205

create-emdash

npm i https://pkg.pr.new/emdash-cms/emdash/create-emdash@3205

@emdash-cms/gutenberg-to-portable-text

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/gutenberg-to-portable-text@3205

@emdash-cms/plugin-cli

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/plugin-cli@3205

@emdash-cms/plugin-test

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/plugin-test@3205

@emdash-cms/plugin-types

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/plugin-types@3205

@emdash-cms/registry-client

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/registry-client@3205

@emdash-cms/registry-lexicons

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/registry-lexicons@3205

@emdash-cms/registry-loader

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/registry-loader@3205

@emdash-cms/registry-moderation

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/registry-moderation@3205

@emdash-cms/registry-verification

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/registry-verification@3205

@emdash-cms/sandbox-workerd

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/sandbox-workerd@3205

@emdash-cms/x402

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/x402@3205

@emdash-cms/plugin-ai-moderation

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/plugin-ai-moderation@3205

@emdash-cms/plugin-atproto

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/plugin-atproto@3205

@emdash-cms/plugin-audit-log

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/plugin-audit-log@3205

@emdash-cms/plugin-color

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/plugin-color@3205

@emdash-cms/plugin-embeds

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/plugin-embeds@3205

@emdash-cms/plugin-field-kit

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/plugin-field-kit@3205

@emdash-cms/plugin-forms

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/plugin-forms@3205

@emdash-cms/plugin-webhook-notifier

npm i https://pkg.pr.new/emdash-cms/emdash/@emdash-cms/plugin-webhook-notifier@3205

commit: ac93288

@emdashbot emdashbot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the right fix for the right problem. The previous validation error message joined every failing field into one opaque string, so the admin had no per-field context to produce a human-readable toast. The change keeps the joined server message intact for backward compatibility but surfaces structured per-field issues in error.details.issues, then maps those issues to localized sentences using the editor-visible field labels. That fits cleanly into EmDash's API-envelope shape, Lingui-based admin, and the existing VALIDATION_ERROR / ApiResponseError flow.

I checked:

  • Approach / architecture: The content validator now returns details.issues; the admin's new describeContentValidationError turns each issue into a translatable sentence and falls back to the server message for non-validation errors. This is additive and backward-compatible.
  • Core validation changes: validateContentData is called from handleContentCreate and handleContentUpdate in packages/core/src/emdash-runtime.ts with partial: false / partial: true respectively. The new issue objects are sanitized (no raw issue.input is returned) and SQL identifiers remain parameterized / validated.
  • Admin error wiring: ContentNewPage, ContentEditPage save, ContentEditPage autosave, and translation creation all use the new helper. Other content mutations (publish, unpublish, schedule, discard, delete) do not carry user-edited data, so they are correctly out of scope.
  • Localization: All new user-facing strings go through Lingui macros (msg, plural) and the global i18n._() call is already a repo-wide pattern. No messages.po files are touched.
  • RTL / Tailwind / comments: No new layout classes; no new logged-out queries; no new queries against content tables. Comments added are explanatory JSDoc and non-obvious "why" notes, not reviewer-facing narrative.
  • Tests: Core integration tests pin every issue code the admin words; admin unit tests cover sentence construction, label fallback, deduplication of required+type errors, and the fallback to server messages; router tests and a Playwright autosave test exercise the full save/autosave paths.
  • Changeset: The changeset names the user-facing behavior, the affected packages are correct (emdash and @emdash-cms/admin), and the bump type is patch.

The code is clean and I found no blocking issues.

@emdashbot emdashbot Bot added review/approved Approved; no new commits since and removed review/needs-review No maintainer or bot review yet labels Sep 19, 2026
@ascorbic
ascorbic enabled auto-merge (squash) September 21, 2026 05:52
@ascorbic
ascorbic disabled auto-merge September 21, 2026 06:40
…d-labels

# Conflicts:
#	packages/admin/src/components/ContentEditor.tsx
@danielmlr danielmlr added the ci:run Approve and re-run CI on the PR's latest commit label Sep 21, 2026
@github-actions github-actions Bot removed the ci:run Approve and re-run CI on the PR's latest commit label Sep 21, 2026

@ascorbic ascorbic left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@ascorbic
ascorbic merged commit 93df4e8 into emdash-cms:main Sep 21, 2026
90 of 91 checks passed
@danielmlr
danielmlr deleted the fix/validation-field-labels branch September 21, 2026 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Admin's per-field validation messages never fire for content saves — core's content validator cannot supply error.details.issues

2 participants