From 957890a21614aeb4c735cadadf4302390bc7efcd Mon Sep 17 00:00:00 2001 From: Matt 'TK' Taylor Date: Sun, 5 Apr 2026 01:15:19 +0100 Subject: [PATCH 1/3] fix: content type form validation, URL pattern defaults, dirty tracking, 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). --- .../src/components/ContentTypeEditor.tsx | 187 +++++++++++------- .../migrations/033_backfill_url_patterns.ts | 43 ++++ .../core/src/database/migrations/runner.ts | 2 + packages/core/src/schema/registry.ts | 58 ++++++ packages/core/src/seed/default.ts | 2 + .../core/tests/database/migrations.test.ts | 4 +- .../database/dialect-compat.test.ts | 4 +- .../integration/database/migrations.test.ts | 101 +++++++++- .../core/tests/unit/schema/registry.test.ts | 70 +++++++ 9 files changed, 389 insertions(+), 82 deletions(-) create mode 100644 packages/core/src/database/migrations/033_backfill_url_patterns.ts diff --git a/packages/admin/src/components/ContentTypeEditor.tsx b/packages/admin/src/components/ContentTypeEditor.tsx index 5c65d9a74..aa8556807 100644 --- a/packages/admin/src/components/ContentTypeEditor.tsx +++ b/packages/admin/src/components/ContentTypeEditor.tsx @@ -26,6 +26,10 @@ import { FieldEditor } from "./FieldEditor"; const SLUG_INVALID_CHARS_PATTERN = /[^a-z0-9]+/g; const SLUG_LEADING_TRAILING_PATTERN = /^_|_$/g; +/** Derive a default URL pattern from a collection slug, e.g. "posts" → "/posts/{slug}" */ +const defaultUrlPattern = (collectionSlug: string) => + collectionSlug ? `/${collectionSlug}/{slug}` : ""; + export interface ContentTypeEditorProps { collection?: SchemaCollectionWithFields; isNew?: boolean; @@ -124,6 +128,7 @@ export function ContentTypeEditor({ const [labelSingular, setLabelSingular] = React.useState(collection?.labelSingular ?? ""); const [description, setDescription] = React.useState(collection?.description ?? ""); const [urlPattern, setUrlPattern] = React.useState(collection?.urlPattern ?? ""); + const [urlPatternTouched, setUrlPatternTouched] = React.useState(!isNew); const [supports, setSupports] = React.useState(collection?.supports ?? ["drafts"]); // SEO state @@ -149,57 +154,76 @@ export function ContentTypeEditor({ const [fieldSaving, setFieldSaving] = React.useState(false); const [deleteFieldTarget, setDeleteFieldTarget] = React.useState(null); - const urlPatternValid = !urlPattern || urlPattern.includes("{slug}"); - - // Track whether form has unsaved changes - const hasChanges = React.useMemo(() => { - if (isNew) return slug && label; - if (!collection) return false; - return ( - label !== collection.label || - labelSingular !== (collection.labelSingular ?? "") || - description !== (collection.description ?? "") || - urlPattern !== (collection.urlPattern ?? "") || - JSON.stringify([...supports].toSorted()) !== - JSON.stringify([...collection.supports].toSorted()) || - hasSeo !== collection.hasSeo || - commentsEnabled !== collection.commentsEnabled || - commentsModeration !== collection.commentsModeration || - commentsClosedAfterDays !== collection.commentsClosedAfterDays || - commentsAutoApproveUsers !== collection.commentsAutoApproveUsers - ); - }, [ - isNew, - collection, - slug, - label, - labelSingular, - description, - urlPattern, - supports, - hasSeo, - commentsEnabled, - commentsModeration, - commentsClosedAfterDays, - commentsAutoApproveUsers, - ]); - - // Auto-generate slug from plural label + // Validation state -- errors are shown inline on fields after a submit attempt + const [fieldErrors, setFieldErrors] = React.useState>({}); + + // Dirty tracking via serialized snapshots (same pattern as ContentEditor). + // Comparing serialized strings avoids subtle type mismatches between local + // state and the collection prop (e.g. boolean vs integer from SQLite). + const serializeFormState = React.useCallback( + () => + JSON.stringify({ + label, + labelSingular, + description, + urlPattern, + supports: supports.toSorted(), + hasSeo, + commentsEnabled, + commentsModeration, + commentsClosedAfterDays, + commentsAutoApproveUsers, + }), + [ + label, + labelSingular, + description, + urlPattern, + supports, + hasSeo, + commentsEnabled, + commentsModeration, + commentsClosedAfterDays, + commentsAutoApproveUsers, + ], + ); + + const [lastSavedState, setLastSavedState] = React.useState(serializeFormState); + const currentState = serializeFormState(); + const hasChanges = isNew ? !!(slug && label) : currentState !== lastSavedState; + + // 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]); + + // Auto-generate slug from plural label. + // 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)); + } } }; - // Auto-generate plural label (and slug) from singular label + // Auto-generate plural label (and slug) from singular label. + // Clears validation errors for all three derived fields. const handleSingularLabelChange = (value: string) => { setLabelSingular(value); + setFieldErrors((prev) => ({ ...prev, labelSingular: "" })); if (isNew) { const plural = value ? `${value}s` : ""; handleLabelChange(plural); @@ -212,8 +236,22 @@ export function ContentTypeEditor({ ); }; + const validate = (): Record => { + const errors: Record = {}; + if (!label.trim()) errors.label = "Label is required"; + if (!labelSingular.trim()) errors.labelSingular = "Singular label is required"; + if (isNew && !slug.trim()) errors.slug = "Slug is required"; + if (urlPattern && !urlPattern.includes("{slug}")) { + errors.urlPattern = "Pattern must include a {slug} placeholder"; + } + return errors; + }; + const handleSubmit = (e: React.FormEvent) => { e.preventDefault(); + const errors = validate(); + setFieldErrors(errors); + if (Object.keys(errors).length > 0) return; if (isNew) { onSave({ slug, @@ -317,6 +355,8 @@ export function ContentTypeEditor({ onChange={(e) => handleSingularLabelChange(e.target.value)} placeholder="Post" disabled={isFromCode} + error={fieldErrors.labelSingular} + variant={fieldErrors.labelSingular ? "error" : "default"} /> handleLabelChange(e.target.value)} placeholder="Posts" disabled={isFromCode} + error={fieldErrors.label} + variant={fieldErrors.label ? "error" : "default"} /> {isNew && ( -
- setSlug(e.target.value)} - placeholder="posts" - disabled={!isNew} - /> -

Used in URLs and API endpoints

-
+ { + setSlug(e.target.value); + setFieldErrors((prev) => ({ ...prev, slug: "" })); + }} + placeholder="posts" + description="Used in URLs and API endpoints" + error={fieldErrors.slug} + variant={fieldErrors.slug ? "error" : "default"} + /> )} -
- setUrlPattern(e.target.value)} - placeholder={`/${slug === "pages" ? "" : `${slug}/`}{slug}`} - disabled={isFromCode} - /> - {urlPattern && !urlPattern.includes("{slug}") && ( -

- Pattern must include a {"{slug}"} placeholder -

- )} -

- Pattern for generating URLs, e.g. /blog/{"{slug}"} -

-
+ { + setUrlPattern(e.target.value); + setUrlPatternTouched(true); + setFieldErrors((prev) => ({ ...prev, urlPattern: "" })); + }} + placeholder="/{slug}" + disabled={isFromCode} + description={`Pattern for generating URLs, e.g. /blog/{"{slug}"}`} + error={fieldErrors.urlPattern} + variant={fieldErrors.urlPattern ? "error" : "default"} + />
@@ -502,11 +543,7 @@ export function ContentTypeEditor({ )} {!isFromCode && ( - )} diff --git a/packages/core/src/database/migrations/033_backfill_url_patterns.ts b/packages/core/src/database/migrations/033_backfill_url_patterns.ts new file mode 100644 index 000000000..8a0d03833 --- /dev/null +++ b/packages/core/src/database/migrations/033_backfill_url_patterns.ts @@ -0,0 +1,43 @@ +import type { Kysely } from "kysely"; +import { sql } from "kysely"; + +/** + * Migration: Backfill default URL patterns for posts and pages + * + * Existing sites created before URL patterns were added to the default seed + * have NULL url_pattern on their posts and pages collections. This migration + * sets sensible defaults so resolveEmDashPath works out of the box. + * + * Only updates rows where url_pattern IS NULL -- user-configured patterns + * are left untouched. + */ +export async function up(db: Kysely): Promise { + await sql` + UPDATE _emdash_collections + SET url_pattern = '/posts/{slug}' + WHERE slug = 'posts' AND url_pattern IS NULL + `.execute(db); + + await sql` + UPDATE _emdash_collections + SET url_pattern = '/{slug}' + WHERE slug = 'pages' AND url_pattern IS NULL + `.execute(db); +} + +export async function down(db: Kysely): Promise { + // Reverting to NULL is safe -- it restores the pre-migration state. + // We can't distinguish "was NULL before" from "user set it to this exact value", + // but the values we set are the obvious defaults, so clearing them is reasonable. + await sql` + UPDATE _emdash_collections + SET url_pattern = NULL + WHERE slug = 'posts' AND url_pattern = '/posts/{slug}' + `.execute(db); + + await sql` + UPDATE _emdash_collections + SET url_pattern = NULL + WHERE slug = 'pages' AND url_pattern = '/{slug}' + `.execute(db); +} diff --git a/packages/core/src/database/migrations/runner.ts b/packages/core/src/database/migrations/runner.ts index 05b6ea6e7..9fd585b09 100644 --- a/packages/core/src/database/migrations/runner.ts +++ b/packages/core/src/database/migrations/runner.ts @@ -33,6 +33,7 @@ import * as m029 from "./029_redirects.js"; import * as m030 from "./030_widen_scheduled_index.js"; import * as m031 from "./031_bylines.js"; import * as m032 from "./032_rate_limits.js"; +import * as m033 from "./033_backfill_url_patterns.js"; /** * Migration provider that uses statically imported migrations. @@ -72,6 +73,7 @@ class StaticMigrationProvider implements MigrationProvider { "030_widen_scheduled_index": m030, "031_bylines": m031, "032_rate_limits": m032, + "033_backfill_url_patterns": m033, }; } } diff --git a/packages/core/src/schema/registry.ts b/packages/core/src/schema/registry.ts index 6d112600b..019d0436e 100644 --- a/packages/core/src/schema/registry.ts +++ b/packages/core/src/schema/registry.ts @@ -129,6 +129,9 @@ export class SchemaRegistry { throw new SchemaError(`Collection "${input.slug}" already exists`, "COLLECTION_EXISTS"); } + // Check label uniqueness + await this.checkLabelUniqueness(input.label, input.labelSingular); + const id = ulid(); // Insert collection record and create content table in a transaction @@ -176,6 +179,24 @@ export class SchemaRegistry { throw new SchemaError(`Collection "${slug}" not found`, "COLLECTION_NOT_FOUND"); } + // Only check label uniqueness for fields that are actually changing. + // This avoids blocking updates to collections that are already in a + // non-unique state (e.g., two collections with the same label created + // before uniqueness was enforced). + // TODO(1.0): Remove this conditional -- check all labels unconditionally. + // By 1.0 all sites should have unique labels (enforced on create since + // this release). The selective check only exists for the migration window. + const labelChanged = input.label !== undefined && input.label !== existing.label; + const labelSingularChanged = + input.labelSingular !== undefined && input.labelSingular !== existing.labelSingular; + if (labelChanged || labelSingularChanged) { + await this.checkLabelUniqueness( + labelChanged ? input.label : undefined, + labelSingularChanged ? input.labelSingular : undefined, + slug, + ); + } + const now = new Date().toISOString(); // Derive hasSeo from supports array if supports is being updated and hasSeo not explicitly set @@ -236,6 +257,43 @@ export class SchemaRegistry { return updated; } + /** + * Check that a collection's labels are unique across all collections. + * Pass `excludeSlug` to skip the collection being updated. + */ + private async checkLabelUniqueness( + label?: string, + labelSingular?: string, + excludeSlug?: string, + ): Promise { + if (!label && !labelSingular) return; + + let query = this.db + .selectFrom("_emdash_collections") + .select(["slug", "label", "label_singular"]); + + if (excludeSlug) { + query = query.where("slug", "!=", excludeSlug); + } + + 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", + ); + } + } + } + /** * Delete a collection */ diff --git a/packages/core/src/seed/default.ts b/packages/core/src/seed/default.ts index f33fdb45b..4fc6485ea 100644 --- a/packages/core/src/seed/default.ts +++ b/packages/core/src/seed/default.ts @@ -18,6 +18,7 @@ export const defaultSeed: SeedFile = { slug: "posts", label: "Posts", labelSingular: "Post", + urlPattern: "/posts/{slug}", supports: ["drafts", "revisions", "search"], fields: [ { @@ -49,6 +50,7 @@ export const defaultSeed: SeedFile = { slug: "pages", label: "Pages", labelSingular: "Page", + urlPattern: "/{slug}", supports: ["drafts", "revisions", "search"], fields: [ { diff --git a/packages/core/tests/database/migrations.test.ts b/packages/core/tests/database/migrations.test.ts index 606e97ce6..2e4da1a17 100644 --- a/packages/core/tests/database/migrations.test.ts +++ b/packages/core/tests/database/migrations.test.ts @@ -70,7 +70,7 @@ describe("Database Migrations", () => { await expect(runMigrations(db)).resolves.not.toThrow(); const status = await getMigrationStatus(db); - expect(status.applied).toHaveLength(31); // 001_initial through 032_rate_limits (no 010) + expect(status.applied).toHaveLength(32); // 001_initial through 033_backfill_url_patterns (no 010) }); it("should record migration in tracking table", async () => { @@ -78,7 +78,7 @@ describe("Database Migrations", () => { const records = await db.selectFrom("_emdash_migrations").selectAll().execute(); - expect(records).toHaveLength(31); + expect(records).toHaveLength(32); expect(records[0].name).toBe("001_initial"); expect(records[0].timestamp).toBeDefined(); expect(records[1].name).toBe("002_media_status"); diff --git a/packages/core/tests/integration/database/dialect-compat.test.ts b/packages/core/tests/integration/database/dialect-compat.test.ts index 8bbd0507a..496dba59a 100644 --- a/packages/core/tests/integration/database/dialect-compat.test.ts +++ b/packages/core/tests/integration/database/dialect-compat.test.ts @@ -75,7 +75,7 @@ describeEachDialect("Migrations", (dialect) => { const migrations = await ctx.db.selectFrom("_emdash_migrations").selectAll().execute(); - expect(migrations).toHaveLength(31); + expect(migrations).toHaveLength(32); expect(migrations[0]?.name).toBe("001_initial"); }); @@ -85,7 +85,7 @@ describeEachDialect("Migrations", (dialect) => { const migrations = await ctx.db.selectFrom("_emdash_migrations").selectAll().execute(); - expect(migrations).toHaveLength(31); + expect(migrations).toHaveLength(32); }); it("reports correct migration status", async () => { diff --git a/packages/core/tests/integration/database/migrations.test.ts b/packages/core/tests/integration/database/migrations.test.ts index 3fe469ede..a0801e095 100644 --- a/packages/core/tests/integration/database/migrations.test.ts +++ b/packages/core/tests/integration/database/migrations.test.ts @@ -57,7 +57,7 @@ describe("Database Migrations (Integration)", () => { const migrations = await db.selectFrom("_emdash_migrations").selectAll().execute(); - expect(migrations).toHaveLength(31); + expect(migrations).toHaveLength(32); expect(migrations[0]?.name).toBe("001_initial"); expect(migrations[0]?.timestamp).toBeDefined(); expect(migrations[1]?.name).toBe("002_media_status"); @@ -98,8 +98,8 @@ describe("Database Migrations (Integration)", () => { const migrations = await db.selectFrom("_emdash_migrations").selectAll().execute(); - // Should still only have thirty-one migration records - expect(migrations).toHaveLength(31); + // Should still only have thirty-two migration records + expect(migrations).toHaveLength(32); }); it("should report correct migration status", async () => { @@ -409,4 +409,99 @@ describe("Database Migrations (Integration)", () => { expect(logs).toHaveLength(1); expect(logs[0]?.action).toBe("content:create"); }); + + describe("033_backfill_url_patterns", () => { + it("should set url_pattern for posts and pages with NULL values", async () => { + await runMigrations(db); + + // Insert test collections with NULL url_pattern (simulating pre-migration state) + await db + .insertInto("_emdash_collections") + .values({ id: "bp-posts", slug: "test_posts", label: "Posts", has_seo: 0 } as any) + .execute(); + await db + .insertInto("_emdash_collections") + .values({ id: "bp-pages", slug: "test_pages", label: "Pages", has_seo: 0 } as any) + .execute(); + + // Verify they start with NULL + const before = await db + .selectFrom("_emdash_collections") + .select(["slug", "url_pattern"]) + .where("slug", "in", ["test_posts", "test_pages"]) + .execute(); + expect(before.every((c) => c.url_pattern === null)).toBe(true); + + // Run the migration up function directly with the known slugs + // (the real migration targets "posts" and "pages", so test with those) + await db + .insertInto("_emdash_collections") + .values({ id: "bp-real-posts", slug: "posts", label: "Real Posts", has_seo: 0 } as any) + .onConflict((oc) => oc.column("slug").doNothing()) + .execute(); + await db + .updateTable("_emdash_collections" as any) + .set({ url_pattern: null }) + .where("slug", "=", "posts") + .execute(); + + await db + .insertInto("_emdash_collections") + .values({ id: "bp-real-pages", slug: "pages", label: "Real Pages", has_seo: 0 } as any) + .onConflict((oc) => oc.column("slug").doNothing()) + .execute(); + await db + .updateTable("_emdash_collections" as any) + .set({ url_pattern: null }) + .where("slug", "=", "pages") + .execute(); + + // Run migration 033 directly + const { up } = await import("../../../src/database/migrations/033_backfill_url_patterns.js"); + await up(db); + + const posts = await db + .selectFrom("_emdash_collections") + .select("url_pattern") + .where("slug", "=", "posts") + .executeTakeFirst(); + + const pages = await db + .selectFrom("_emdash_collections") + .select("url_pattern") + .where("slug", "=", "pages") + .executeTakeFirst(); + + expect(posts?.url_pattern).toBe("/posts/{slug}"); + expect(pages?.url_pattern).toBe("/{slug}"); + }); + + it("should not overwrite user-configured url_pattern values", async () => { + await runMigrations(db); + + // Set a custom url_pattern on posts + await db + .insertInto("_emdash_collections") + .values({ id: "bp-custom", slug: "posts", label: "Posts", has_seo: 0 } as any) + .onConflict((oc) => oc.column("slug").doNothing()) + .execute(); + await db + .updateTable("_emdash_collections" as any) + .set({ url_pattern: "/blog/{slug}" }) + .where("slug", "=", "posts") + .execute(); + + // Run migration 033 directly + const { up } = await import("../../../src/database/migrations/033_backfill_url_patterns.js"); + await up(db); + + const posts = await db + .selectFrom("_emdash_collections") + .select("url_pattern") + .where("slug", "=", "posts") + .executeTakeFirst(); + + expect(posts?.url_pattern).toBe("/blog/{slug}"); + }); + }); }); diff --git a/packages/core/tests/unit/schema/registry.test.ts b/packages/core/tests/unit/schema/registry.test.ts index 218bcfd38..44aec5456 100644 --- a/packages/core/tests/unit/schema/registry.test.ts +++ b/packages/core/tests/unit/schema/registry.test.ts @@ -372,4 +372,74 @@ describe("SchemaRegistry", () => { expect(field).toBeNull(); }); }); + + describe("Label Uniqueness", () => { + it("should reject creating a collection with a duplicate label", async () => { + await registry.createCollection({ slug: "posts", label: "Articles" }); + + await expect( + registry.createCollection({ slug: "news", label: "Articles" }), + ).rejects.toThrow(); + }); + + it("should reject creating a collection with a duplicate singular label", async () => { + await registry.createCollection({ + slug: "posts", + label: "Posts", + labelSingular: "Post", + }); + + await expect( + registry.createCollection({ + slug: "news", + label: "News", + labelSingular: "Post", + }), + ).rejects.toThrow(); + }); + + it("should reject updating a collection label to one already in use", async () => { + await registry.createCollection({ slug: "posts", label: "Posts" }); + await registry.createCollection({ slug: "pages", label: "Pages" }); + + await expect(registry.updateCollection("pages", { label: "Posts" })).rejects.toThrow(); + }); + + it("should allow updating a collection to keep its own label", async () => { + await registry.createCollection({ slug: "posts", label: "Posts" }); + + // Updating other fields while keeping the same label should not throw + const updated = await registry.updateCollection("posts", { + label: "Posts", + description: "Blog posts", + }); + + expect(updated.description).toBe("Blog posts"); + }); + + // TODO(1.0): Remove this test. Once all sites have unique labels, the + // selective check it covers should be replaced with unconditional validation. + it("should allow updating non-label fields when labels are already duplicated", async () => { + // Simulate a pre-existing non-unique state by inserting directly + await registry.createCollection({ slug: "posts", label: "Articles" }); + await db + .updateTable("_emdash_collections") + .set({ label: "Articles" }) + .where("slug", "=", "posts") + .execute(); + await registry.createCollection({ slug: "news", label: "News" }); + await db + .updateTable("_emdash_collections") + .set({ label: "Articles" }) + .where("slug", "=", "news") + .execute(); + + // Updating description (not labels) should succeed despite the duplicate + const updated = await registry.updateCollection("posts", { + description: "Some articles", + }); + + expect(updated.description).toBe("Some articles"); + }); + }); }); From 6dfffc7f133e003d06622d9f78178cf74a07a0a1 Mon Sep 17 00:00:00 2001 From: Matt 'TK' Taylor Date: Sun, 5 Apr 2026 09:40:38 +0100 Subject: [PATCH 2/3] test(admin): update ContentTypeEditor tests for new validation behaviour - 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 --- .../components/ContentTypeEditor.test.tsx | 23 +++++++++++++++---- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/packages/admin/tests/components/ContentTypeEditor.test.tsx b/packages/admin/tests/components/ContentTypeEditor.test.tsx index d25cbd5a6..a8698d407 100644 --- a/packages/admin/tests/components/ContentTypeEditor.test.tsx +++ b/packages/admin/tests/components/ContentTypeEditor.test.tsx @@ -195,7 +195,7 @@ describe("ContentTypeEditor", () => { label: "Articles", labelSingular: "Article", description: undefined, - urlPattern: undefined, + urlPattern: "/articles/{slug}", supports: ["drafts"], // default hasSeo: false, }); @@ -446,22 +446,35 @@ describe("ContentTypeEditor", () => { }); it("shows validation error when pattern lacks {slug}", async () => { + const onSave = vi.fn(); const collection = makeCollection(); - const screen = await render(); + const screen = await render( + , + ); await screen.getByLabelText("URL Pattern").fill("/blog/broken"); + // Validation fires on submit, not inline + const saveButton = screen.getByRole("button", { name: SAVE_CHANGES_BUTTON_REGEX }); + await saveButton.click(); + await expect.element(screen.getByText(URL_PATTERN_SLUG_RE)).toBeInTheDocument(); + expect(onSave).not.toHaveBeenCalled(); }); - it("disables save button when pattern lacks {slug}", async () => { + it("prevents save when pattern lacks {slug}", async () => { + const onSave = vi.fn(); const collection = makeCollection(); - const screen = await render(); + const screen = await render( + , + ); await screen.getByLabelText("URL Pattern").fill("/blog/broken"); const saveButton = screen.getByRole("button", { name: SAVE_CHANGES_BUTTON_REGEX }); - await expect.element(saveButton).toBeDisabled(); + await saveButton.click(); + + expect(onSave).not.toHaveBeenCalled(); }); it("enables save button when pattern includes {slug}", async () => { From 1aa3114f3f4fb8895182d6b66ac5e627704432e7 Mon Sep 17 00:00:00 2001 From: Matt 'TK' Taylor Date: Sun, 5 Apr 2026 17:16:21 +0100 Subject: [PATCH 3/3] fix: surface server errors in content type form and enforce URL pattern 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. --- .../src/components/ContentTypeEditor.tsx | 13 +++++ packages/admin/src/router.tsx | 4 ++ packages/core/src/api/errors.ts | 4 ++ packages/core/src/api/schemas/schema.ts | 9 +++- packages/core/src/schema/registry.ts | 38 ++++++++++++++ packages/core/src/seed/validate.ts | 5 ++ packages/core/tests/unit/api/schemas.test.ts | 51 ++++++++++++++++++- .../core/tests/unit/seed/validate.test.ts | 33 ++++++++++++ templates/marketing/emdash-env.d.ts | 15 ------ .../portfolio-cloudflare/emdash-env.d.ts | 15 ------ templates/portfolio/emdash-env.d.ts | 15 ------ 11 files changed, 154 insertions(+), 48 deletions(-) diff --git a/packages/admin/src/components/ContentTypeEditor.tsx b/packages/admin/src/components/ContentTypeEditor.tsx index aa8556807..790505c87 100644 --- a/packages/admin/src/components/ContentTypeEditor.tsx +++ b/packages/admin/src/components/ContentTypeEditor.tsx @@ -20,6 +20,7 @@ import type { } from "../lib/api"; import { cn } from "../lib/utils"; import { ConfirmDialog } from "./ConfirmDialog"; +import { DialogError, getMutationError } from "./DialogError"; import { FieldEditor } from "./FieldEditor"; // Regex patterns for slug generation @@ -34,6 +35,10 @@ export interface ContentTypeEditorProps { collection?: SchemaCollectionWithFields; isNew?: boolean; isSaving?: boolean; + /** Mutation error from the last save attempt. */ + error?: Error | null; + /** Called when the user edits form fields, to reset the mutation error. */ + onErrorClear?: () => void; onSave: (input: CreateCollectionInput | UpdateCollectionInput) => void; onAddField?: (input: CreateFieldInput) => void; onUpdateField?: (fieldSlug: string, input: CreateFieldInput) => void; @@ -114,6 +119,8 @@ export function ContentTypeEditor({ collection, isNew, isSaving, + error, + onErrorClear, onSave, onAddField, onUpdateField, @@ -207,6 +214,7 @@ export function ContentTypeEditor({ const handleLabelChange = (value: string) => { setLabel(value); setFieldErrors((prev) => ({ ...prev, label: "", slug: "" })); + onErrorClear?.(); if (isNew) { const newSlug = value .toLowerCase() @@ -224,6 +232,7 @@ export function ContentTypeEditor({ const handleSingularLabelChange = (value: string) => { setLabelSingular(value); setFieldErrors((prev) => ({ ...prev, labelSingular: "" })); + onErrorClear?.(); if (isNew) { const plural = value ? `${value}s` : ""; handleLabelChange(plural); @@ -376,6 +385,7 @@ export function ContentTypeEditor({ onChange={(e) => { setSlug(e.target.value); setFieldErrors((prev) => ({ ...prev, slug: "" })); + onErrorClear?.(); }} placeholder="posts" description="Used in URLs and API endpoints" @@ -400,6 +410,7 @@ export function ContentTypeEditor({ setUrlPattern(e.target.value); setUrlPatternTouched(true); setFieldErrors((prev) => ({ ...prev, urlPattern: "" })); + onErrorClear?.(); }} placeholder="/{slug}" disabled={isFromCode} @@ -542,6 +553,8 @@ export function ContentTypeEditor({
)} + + {!isFromCode && (