fix: content type form validation, URL pattern defaults, and label uniqueness#254
fix: content type form validation, URL pattern defaults, and label uniqueness#254
Conversation
…ng, and label uniqueness Inline form validation for the content type editor with derived field error clearing. URL pattern pre-filled from collection slug for new types. Default seed and migration 033 backfill url_pattern for posts and pages. Snapshot-based dirty tracking matching the ContentEditor pattern. Label uniqueness enforced on create and update, with a selective check for pre-existing duplicates marked TODO(1.0).
|
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-playground | 1aa3114 | Apr 05 2026, 04:18 PM |
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/blocks
@emdash-cms/cloudflare
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@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-forms
@emdash-cms/plugin-webhook-notifier
commit: |
There was a problem hiding this comment.
Pull request overview
This PR improves the content type (collection) editing experience and schema integrity by adding editor-side validation/dirty-tracking, establishing URL pattern defaults/backfills, and enforcing uniqueness of collection labels at the schema registry layer.
Changes:
- Adds inline form validation + URL pattern defaulting/auto-sync behavior in the admin ContentTypeEditor.
- Introduces migration
033_backfill_url_patternsand updates the default seed to ensure posts/pages have sensibleurlPatterndefaults. - Enforces label/labelSingular uniqueness in
SchemaRegistry(create/update) with accompanying unit tests.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/admin/src/components/ContentTypeEditor.tsx | Adds inline validation, URL pattern defaulting, and snapshot-based dirty tracking. |
| packages/core/src/schema/registry.ts | Enforces collection label uniqueness on create/update via a new helper. |
| packages/core/src/database/migrations/033_backfill_url_patterns.ts | New migration to backfill url_pattern defaults for posts/pages when NULL. |
| packages/core/src/database/migrations/runner.ts | Registers migration 033 in the static migration provider. |
| packages/core/src/seed/default.ts | Sets default urlPattern values for posts and pages in the default seed. |
| packages/core/tests/unit/schema/registry.test.ts | Adds unit tests covering label uniqueness enforcement and update edge cases. |
| packages/core/tests/integration/database/migrations.test.ts | Updates migration count assertions and adds integration tests for migration 033 behavior. |
| packages/core/tests/integration/database/dialect-compat.test.ts | Updates expected migration counts across dialects. |
| packages/core/tests/database/migrations.test.ts | Updates expected applied migration count and comments for the new migration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // When save completes (isSaving transitions false), snapshot current state as clean. | ||
| // This fires immediately without waiting for the refetch, preventing the dirty flash. | ||
| const prevSaving = React.useRef(isSaving); | ||
| React.useEffect(() => { | ||
| if (prevSaving.current && !isSaving) { | ||
| setLastSavedState(serializeFormState()); | ||
| } | ||
| prevSaving.current = isSaving; | ||
| }, [isSaving, serializeFormState]); |
There was a problem hiding this comment.
The dirty-state snapshot is updated whenever isSaving transitions from true→false, but isSaving reflects the mutation pending state (false on both success and error). If a save fails, this will still mark the form as clean and disable saving even though nothing was persisted. Consider only snapshotting on successful save (e.g., pass an onSaveSuccess signal / updated collection version, or handle this in the parent with a prop that only flips when the mutation succeeds).
| // When save completes (isSaving transitions false), snapshot current state as clean. | |
| // This fires immediately without waiting for the refetch, preventing the dirty flash. | |
| const prevSaving = React.useRef(isSaving); | |
| React.useEffect(() => { | |
| if (prevSaving.current && !isSaving) { | |
| setLastSavedState(serializeFormState()); | |
| } | |
| prevSaving.current = isSaving; | |
| }, [isSaving, serializeFormState]); | |
| // NOTE: Do not snapshot the current state as clean based on `isSaving` alone. | |
| // `isSaving` only reflects whether a save is pending, so it becomes false after | |
| // both successful and failed saves. Updating `lastSavedState` here would mark | |
| // failed saves as clean. This snapshot must instead be driven by a success-only | |
| // signal from the parent or by refreshed server-backed data. |
| // Clears validation errors for label and any derived fields (slug). | ||
| const handleLabelChange = (value: string) => { | ||
| setLabel(value); | ||
| setFieldErrors((prev) => ({ ...prev, label: "", slug: "" })); | ||
| if (isNew) { | ||
| setSlug( | ||
| value | ||
| .toLowerCase() | ||
| .replace(SLUG_INVALID_CHARS_PATTERN, "_") | ||
| .replace(SLUG_LEADING_TRAILING_PATTERN, ""), | ||
| ); | ||
| const newSlug = value | ||
| .toLowerCase() | ||
| .replace(SLUG_INVALID_CHARS_PATTERN, "_") | ||
| .replace(SLUG_LEADING_TRAILING_PATTERN, ""); | ||
| setSlug(newSlug); | ||
| if (!urlPatternTouched) { | ||
| setUrlPattern(defaultUrlPattern(newSlug)); | ||
| } |
There was a problem hiding this comment.
When the label change auto-updates slug and (if untouched) urlPattern, any existing urlPattern validation error will remain set because only label/slug errors are cleared here. If urlPattern is derived from the slug, consider clearing fieldErrors.urlPattern as well when setUrlPattern(defaultUrlPattern(...)) runs so derived-field edits reliably clear downstream errors.
| private async checkLabelUniqueness( | ||
| label?: string, | ||
| labelSingular?: string, | ||
| excludeSlug?: string, | ||
| ): Promise<void> { | ||
| if (!label && !labelSingular) return; | ||
|
|
There was a problem hiding this comment.
checkLabelUniqueness treats empty strings as “not provided” (if (!label && !labelSingular) return; and later if (label && ...)). Since the API schemas currently allow labelSingular: z.string().optional() (no min length), a client can set labelSingular to "" and bypass uniqueness checks while persisting an empty-string value. Consider normalizing empty strings to null/undefined before validation, or change the checks to use nullish checks (e.g. label != null) and/or enforce .min(1) when present.
| const others = await query.execute(); | ||
|
|
||
| for (const other of others) { | ||
| if (label && other.label === label) { | ||
| throw new SchemaError( | ||
| `A content type with the label "${label}" already exists`, | ||
| "LABEL_EXISTS", | ||
| ); | ||
| } | ||
| if (labelSingular && other.label_singular === labelSingular) { | ||
| throw new SchemaError( | ||
| `A content type with the singular label "${labelSingular}" already exists`, | ||
| "LABEL_EXISTS", | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
checkLabelUniqueness loads all collections and does an in-memory scan. This is O(n) per create/update and will get slower as sites add content types. It should be possible to do this as a targeted query with WHERE label = ? OR label_singular = ? (plus slug != excludeSlug) and LIMIT 1, which is both faster and simpler to reason about.
| const others = await query.execute(); | |
| for (const other of others) { | |
| if (label && other.label === label) { | |
| throw new SchemaError( | |
| `A content type with the label "${label}" already exists`, | |
| "LABEL_EXISTS", | |
| ); | |
| } | |
| if (labelSingular && other.label_singular === labelSingular) { | |
| throw new SchemaError( | |
| `A content type with the singular label "${labelSingular}" already exists`, | |
| "LABEL_EXISTS", | |
| ); | |
| } | |
| } | |
| if (label && labelSingular) { | |
| query = query.where((eb) => | |
| eb.or([eb("label", "=", label), eb("label_singular", "=", labelSingular)]), | |
| ); | |
| } else if (label) { | |
| query = query.where("label", "=", label); | |
| } else if (labelSingular) { | |
| query = query.where("label_singular", "=", labelSingular); | |
| } | |
| const conflict = await query.limit(1).executeTakeFirst(); | |
| if (!conflict) return; | |
| if (label && conflict.label === label) { | |
| throw new SchemaError( | |
| `A content type with the label "${label}" already exists`, | |
| "LABEL_EXISTS", | |
| ); | |
| } | |
| if (labelSingular && conflict.label_singular === labelSingular) { | |
| throw new SchemaError( | |
| `A content type with the singular label "${labelSingular}" already exists`, | |
| "LABEL_EXISTS", | |
| ); | |
| } |
- URL pattern is now pre-filled (/articles/{slug}) instead of undefined
- Validation fires on submit, not inline -- tests click the save button
before checking for errors
- Replace 'disables save button' test with 'prevents save' since
validation is now submit-time rather than disabling the button
…rn uniqueness
Display mutation errors in the content type create/edit form using
DialogError, matching the pattern used elsewhere in the admin. Errors
clear when the user edits the relevant field.
Add server-side URL pattern uniqueness enforcement to prevent
collections with conflicting routes. Add LABEL_EXISTS and
URL_PATTERN_EXISTS to the ErrorCode enum with 409 status mapping.
Add Zod-level urlPattern validation (must include {slug}) to both
create and update schemas, with matching seed validation.
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. |
What does this PR do?
Short-term fixes to prevent users getting into awkward situations with conflicting collection routes and duplicate labels. These are stopgap measures until we work out a consistent approach to form validation across the admin -- at that point the error display and validation patterns here should be revisited and unified with whatever we land on.
Server error display in the content type form
The content type create/edit form was silently swallowing server errors. If a uniqueness constraint was violated (duplicate slug, label, or URL pattern), the form would stop saving and return to its idle state with no feedback. The form now surfaces these errors using
DialogError, matching the pattern used elsewhere in the admin (e.g.,ConfirmDialog). Errors clear when the user edits the relevant field.URL pattern uniqueness enforcement
Two collections with the same URL pattern (e.g.,
/blog/{slug}) would silently conflict at routing time.SchemaRegistrynow rejects duplicate URL patterns on both create and update, with aURL_PATTERN_EXISTSerror code (409). The error message names the conflicting collection.URL pattern Zod validation
The
createCollectionBodyandupdateCollectionBodyschemas now validate thaturlPatternincludes a{slug}placeholder when non-empty, catching invalid patterns at the API boundary. Matching validation added to seed file validation.Form validation
Inline validation on submit using kumo's
errorandvariantprops. Required fields (singular label, plural label, slug) and the URL pattern{slug}placeholder are validated. Errors clear as the user types, including through derived field chains -- editing the singular label clears errors on the plural label and slug since those auto-generate.URL pattern defaults
New content types get a pre-filled URL pattern (
/{collection-slug}/{slug}) that auto-syncs with the slug as it's generated. Once the user manually edits the pattern, auto-sync stops.The default seed now sets
/posts/{slug}for posts and/{slug}for pages. Migration 033 backfills these for existing sites whereurl_pattern IS NULL, leaving user-configured patterns untouched.Dirty tracking
Replaces field-by-field comparison against the
collectionprop with serialised snapshot comparison, matching theContentEditorpattern. When a save completes, the current form state is immediately snapshotted as clean, eliminating the brief "Saved" -> "Save" flash between mutation completion and query refetch.Label uniqueness
SchemaRegistry.createCollectionandupdateCollectionreject duplicatelabelandlabelSingularvalues. For updates, only fields actually changing are checked -- this avoids blocking edits to collections already in a non-unique state from before enforcement was added. The selective check and its test are markedTODO(1.0)for removal.Error code housekeeping
LABEL_EXISTSandURL_PATTERN_EXISTSadded to theErrorCodeenum and mapped to 409 Conflict inmapErrorStatus.LABEL_EXISTSwas previously a bare string falling through to the default 400 -- 409 is correct for uniqueness conflicts.Type of change
Checklist
pnpm typecheckpassespnpm --silent lint:json | jq '.diagnostics | length'returns 0pnpm testpasses (or targeted tests for my change)pnpm formathas been runAI-generated code disclosure
Test coverage
{slug}, accepts valid patterns, accepts omitted/null patternsurlPatternwithout{slug}, accepts valid patterns