diff --git a/.changeset/fix-inline-unicode-taxonomies.md b/.changeset/fix-inline-unicode-taxonomies.md new file mode 100644 index 000000000..a8dc3f14a --- /dev/null +++ b/.changeset/fix-inline-unicode-taxonomies.md @@ -0,0 +1,6 @@ +--- +"emdash": patch +"@emdash-cms/admin": patch +--- + +Fixes inline taxonomy term creation for Unicode-only labels and adds numeric suffixes when generated term slugs collide. diff --git a/packages/admin/src/components/TaxonomyManager.tsx b/packages/admin/src/components/TaxonomyManager.tsx index ed2c262a9..ea8085924 100644 --- a/packages/admin/src/components/TaxonomyManager.tsx +++ b/packages/admin/src/components/TaxonomyManager.tsx @@ -434,7 +434,7 @@ function TermFormDialog({ const createMutation = useMutation({ mutationFn: () => createTerm(taxonomyName, { - slug, + ...(autoSlug ? {} : { slug }), label, parentId: parentId || undefined, description: description || undefined, diff --git a/packages/admin/src/components/TaxonomySidebar.tsx b/packages/admin/src/components/TaxonomySidebar.tsx index d8117a49e..c316aee23 100644 --- a/packages/admin/src/components/TaxonomySidebar.tsx +++ b/packages/admin/src/components/TaxonomySidebar.tsx @@ -17,7 +17,7 @@ import * as React from "react"; import { apiFetch, parseApiResponse, throwResponseError } from "../lib/api/client.js"; import { createTerm, withLocale } from "../lib/api/taxonomies.js"; import { rankTermMatches, termExactMatches } from "../lib/taxonomy-match.js"; -import { cn, slugify } from "../lib/utils.js"; +import { cn } from "../lib/utils.js"; interface TaxonomyTerm { id: string; @@ -367,7 +367,6 @@ function TaxonomySection({ const createTermMutation = useMutation({ mutationFn: (label: string) => createTerm(taxonomy.name, { - slug: slugify(label), label, // Create the term in the entry's locale so it resolves on this entry. ...(entryLocale ? { locale: entryLocale } : {}), diff --git a/packages/admin/src/lib/api/taxonomies.ts b/packages/admin/src/lib/api/taxonomies.ts index 6709cf5bc..356992a3a 100644 --- a/packages/admin/src/lib/api/taxonomies.ts +++ b/packages/admin/src/lib/api/taxonomies.ts @@ -71,7 +71,7 @@ export interface CreateTaxonomyInput { } export interface CreateTermInput { - slug: string; + slug?: string; label: string; parentId?: string; description?: string; diff --git a/packages/admin/tests/components/TaxonomyManager.test.tsx b/packages/admin/tests/components/TaxonomyManager.test.tsx index fdea37d63..28dd8b453 100644 --- a/packages/admin/tests/components/TaxonomyManager.test.tsx +++ b/packages/admin/tests/components/TaxonomyManager.test.tsx @@ -2,6 +2,7 @@ import { Toasty } from "@cloudflare/kumo"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import * as React from "react"; import { describe, it, expect, vi, beforeEach } from "vitest"; +import { userEvent } from "vitest/browser"; import { getAvailableParentTerms, @@ -344,6 +345,42 @@ describe("TaxonomyManager", () => { await expect.element(screen.getByText("Description (optional)")).toBeInTheDocument(); }); + it("lets the server derive an auto-generated term slug", async () => { + const screen = await render(, { + wrapper: Wrapper, + }); + await screen.getByRole("button", { name: ADD_CATEGORY_BUTTON_REGEX }).click(); + await screen.getByLabelText("Name").fill("音楽"); + await expect.element(screen.getByLabelText("Slug")).toHaveValue("音楽"); + + await userEvent.keyboard("{Enter}"); + + await vi.waitFor(() => { + const call = vi.mocked(apiFetch).mock.calls.find(([, init]) => init?.method === "POST"); + expect(call).toBeDefined(); + const body = typeof call?.[1]?.body === "string" ? JSON.parse(call[1].body) : undefined; + expect(body).toMatchObject({ label: "音楽" }); + expect(body).not.toHaveProperty("slug"); + }); + }); + + it("sends a manually edited term slug", async () => { + const screen = await render(, { + wrapper: Wrapper, + }); + await screen.getByRole("button", { name: ADD_CATEGORY_BUTTON_REGEX }).click(); + await screen.getByLabelText("Name").fill("Music"); + await screen.getByLabelText("Slug").fill("custom-music"); + + await userEvent.keyboard("{Enter}"); + + await vi.waitFor(() => { + const call = vi.mocked(apiFetch).mock.calls.find(([, init]) => init?.method === "POST"); + const body = typeof call?.[1]?.body === "string" ? JSON.parse(call[1].body) : undefined; + expect(body).toMatchObject({ label: "Music", slug: "custom-music" }); + }); + }); + it("shows parent selector for hierarchical taxonomies", async () => { const screen = await render(, { wrapper: Wrapper, diff --git a/packages/admin/tests/components/TaxonomySidebar.test.tsx b/packages/admin/tests/components/TaxonomySidebar.test.tsx index 4113e4c22..b655cc49e 100644 --- a/packages/admin/tests/components/TaxonomySidebar.test.tsx +++ b/packages/admin/tests/components/TaxonomySidebar.test.tsx @@ -357,13 +357,28 @@ describe("TaxonomySidebar", () => { "/_emdash/api/taxonomies/tags/terms", expect.objectContaining({ method: "POST", - body: JSON.stringify({ slug: "gamma", label: "Gamma" }), + body: JSON.stringify({ label: "Gamma" }), }), ); }); expect(onChange).toHaveBeenCalledWith("tags", ["term_created"]); }); + it("lets the server derive the slug for an inline Unicode term", async () => { + mockApiFetch({ terms: [] }); + const screen = await render(, { wrapper: Wrapper }); + + await screen.getByLabelText("Add Tags").fill("音楽"); + await screen.getByText('Create "音楽"').click(); + + await vi.waitFor(() => { + const call = vi.mocked(apiFetch).mock.calls.find(([, init]) => init?.method === "POST"); + expect(call).toBeDefined(); + const body = typeof call?.[1]?.body === "string" ? JSON.parse(call[1].body) : undefined; + expect(body).toEqual({ label: "音楽" }); + }); + }); + it("continues to render hierarchical taxonomies as a checkbox tree", async () => { mockApiFetch({ taxonomies: [categoriesTaxonomy], terms: [alphaTerm] }); diff --git a/packages/core/src/api/handlers/taxonomies.ts b/packages/core/src/api/handlers/taxonomies.ts index 51db90e4c..0b0d992ab 100644 --- a/packages/core/src/api/handlers/taxonomies.ts +++ b/packages/core/src/api/handlers/taxonomies.ts @@ -19,6 +19,15 @@ import { fetchVisibleTermCounts } from "../../taxonomies/term-counts.js"; import type { ApiResult } from "../types.js"; const NAME_PATTERN = /^[a-z][a-z0-9_]*$/; +const MAX_GENERATED_TERM_SLUG_ATTEMPTS = 16; + +function isTermSlugUniqueViolation(error: unknown): boolean { + const message = error instanceof Error ? error.message.toLowerCase() : ""; + return ( + (message.includes("unique constraint failed") || message.includes("duplicate key")) && + message.includes("slug") + ); +} // --------------------------------------------------------------------------- // Response types @@ -812,7 +821,7 @@ export async function handleTermCreate( db: Kysely, taxonomyName: string, input: { - slug: string; + slug?: string; label: string; parentId?: string | null; description?: string; @@ -820,6 +829,7 @@ export async function handleTermCreate( translationOf?: string; }, ): Promise> { + let attemptedSlug = input.slug; try { const locale = resolveConfiguredLocale(input.locale ?? getI18nConfig()?.defaultLocale ?? "en"); // Taxonomy definitions are per-locale, but terms can exist in any locale @@ -835,7 +845,8 @@ export async function handleTermCreate( input.parentId === "" || input.parentId === undefined ? undefined : input.parentId; // Conflict check is scoped to locale (per-locale slugs are unique). - const existing = await repo.findBySlug(taxonomyName, input.slug, locale); + const existing = + input.slug === undefined ? null : await repo.findBySlug(taxonomyName, input.slug, locale); if (existing) { return { success: false, @@ -873,15 +884,33 @@ export async function handleTermCreate( return { success: false, error: parentError }; } - const term = await repo.create({ - name: taxonomyName, - slug: input.slug, - label: input.label, - parentId: parentId ?? undefined, - data: input.description ? { description: input.description } : undefined, - locale, - translationOf: input.translationOf, - }); + const create = (slug: string) => + repo.create({ + name: taxonomyName, + slug, + label: input.label, + parentId: parentId ?? undefined, + data: input.description ? { description: input.description } : undefined, + locale, + translationOf: input.translationOf, + }); + let term: Awaited> | undefined; + let lastSlugConflict: unknown; + if (input.slug !== undefined) { + term = await create(input.slug); + } else { + for (let attempt = 0; attempt < MAX_GENERATED_TERM_SLUG_ATTEMPTS; attempt++) { + attemptedSlug = await repo.generateUniqueSlug(taxonomyName, input.label, locale); + try { + term = await create(attemptedSlug); + break; + } catch (error) { + if (!isTermSlugUniqueViolation(error)) throw error; + lastSlugConflict = error; + } + } + } + if (!term) throw lastSlugConflict ?? new Error("Failed to create taxonomy term"); invalidateTermCache(); @@ -901,7 +930,16 @@ export async function handleTermCreate( }, }, }; - } catch { + } catch (error) { + if (isTermSlugUniqueViolation(error)) { + return { + success: false, + error: { + code: "CONFLICT", + message: `Term with slug '${attemptedSlug ?? "(generated)"}' already exists in taxonomy '${taxonomyName}'`, + }, + }; + } return { success: false, error: { code: "TERM_CREATE_ERROR", message: "Failed to create term" }, diff --git a/packages/core/src/api/schemas/taxonomies.ts b/packages/core/src/api/schemas/taxonomies.ts index f813c87ba..5ddc1dfd6 100644 --- a/packages/core/src/api/schemas/taxonomies.ts +++ b/packages/core/src/api/schemas/taxonomies.ts @@ -52,7 +52,11 @@ export const updateTaxonomyDefBody = z export const createTermBody = z .object({ - slug: z.string().min(1), + slug: z + .string() + .min(1) + .optional() + .meta({ description: "Term slug. Omit to derive a unique slug from the label." }), label: z.string().min(1), parentId: z.string().nullish(), description: z.string().optional(), diff --git a/packages/core/src/database/repositories/taxonomy.ts b/packages/core/src/database/repositories/taxonomy.ts index 903589691..4c55f74d8 100644 --- a/packages/core/src/database/repositories/taxonomy.ts +++ b/packages/core/src/database/repositories/taxonomy.ts @@ -2,6 +2,7 @@ import { sql, type Kysely, type Selectable } from "kysely"; import { ulid } from "ulidx"; import { invalidateTaxonomyObjectCache } from "../../object-cache/index.js"; +import { slugify } from "../../utils/slugify.js"; import { withTransaction } from "../transaction.js"; import type { Database, TaxonomyTable } from "../types.js"; import { validateIdentifier } from "../validate.js"; @@ -18,6 +19,7 @@ export interface SiblingPosition { * statement inside D1's 100-parameter ceiling. */ const GROUPS_PER_UPDATE = 32; +const NUMERIC_SUFFIX_PATTERN = /^\d+$/; /** Deal the listed groups back out over the slots they hold, in the order given. */ function permuteWithinSlots( @@ -216,6 +218,29 @@ export class TaxonomyRepository { return row ? this.rowToTaxonomy(row) : null; } + /** Generate a locale-scoped term slug, adding a numeric suffix when needed. */ + async generateUniqueSlug(name: string, text: string, locale?: string): Promise { + const baseSlug = slugify(text); + let query = this.db + .selectFrom("taxonomies") + .select("slug") + .where("name", "=", name) + .where((eb) => eb.or([eb("slug", "=", baseSlug), eb("slug", "like", `${baseSlug}-%`)])); + if (locale !== undefined) query = query.where("locale", "=", locale); + const candidates = await query.execute(); + if (!candidates.some((candidate) => candidate.slug === baseSlug)) return baseSlug; + + let maxSuffix = 0; + const prefix = `${baseSlug}-`; + for (const candidate of candidates) { + if (!candidate.slug.startsWith(prefix)) continue; + const suffix = candidate.slug.slice(prefix.length); + if (!NUMERIC_SUFFIX_PATTERN.test(suffix)) continue; + maxSuffix = Math.max(maxSuffix, Number.parseInt(suffix, 10)); + } + return `${baseSlug}-${maxSuffix + 1}`; + } + /** * Get all terms for a taxonomy (e.g., all categories). * diff --git a/packages/core/src/mcp/server.ts b/packages/core/src/mcp/server.ts index 097acec26..b69ec92f2 100644 --- a/packages/core/src/mcp/server.ts +++ b/packages/core/src/mcp/server.ts @@ -2528,7 +2528,11 @@ export function createMcpServer( "new term beneath a chain of 100+ existing ancestors are rejected.", inputSchema: z.object({ taxonomy: z.string().describe("Taxonomy name (e.g. 'categories', 'tags')"), - slug: z.string().describe("URL-safe identifier for the term"), + slug: z + .string() + .min(1) + .optional() + .describe("URL identifier for the term; omit to derive it from the label"), label: z.string().describe("Display name"), parentId: z.string().optional().describe("Parent term ID for hierarchical taxonomies"), description: z.string().optional().describe("Description of the term"), diff --git a/packages/core/tests/integration/mcp/taxonomy.test.ts b/packages/core/tests/integration/mcp/taxonomy.test.ts index 71a72cd9d..9797fcbc2 100644 --- a/packages/core/tests/integration/mcp/taxonomy.test.ts +++ b/packages/core/tests/integration/mcp/taxonomy.test.ts @@ -550,6 +550,18 @@ describe("taxonomy_create_term", () => { expect(term.label).toBe("Tech"); }); + it("derives a Unicode slug when omitted", async () => { + harness = await connectMcpHarness({ db, userId: ADMIN_ID, userRole: Role.ADMIN }); + const result = await harness.client.callTool({ + name: "taxonomy_create_term", + arguments: { taxonomy: "tags", label: "音楽" }, + }); + + expect(result.isError, extractText(result)).toBeFalsy(); + const { term } = extractJson<{ term: { slug: string } }>(result); + expect(term.slug).toBe("音楽"); + }); + it("creates a child term with parentId", async () => { harness = await connectMcpHarness({ db, userId: ADMIN_ID, userRole: Role.ADMIN }); const parent = await harness.client.callTool({ diff --git a/packages/core/tests/unit/taxonomies/term-slug-generation.test.ts b/packages/core/tests/unit/taxonomies/term-slug-generation.test.ts new file mode 100644 index 000000000..f4ad44093 --- /dev/null +++ b/packages/core/tests/unit/taxonomies/term-slug-generation.test.ts @@ -0,0 +1,99 @@ +import { afterEach, beforeEach, expect, it } from "vitest"; + +import { handleTaxonomyCreate, handleTermCreate } from "../../../src/api/handlers/taxonomies.js"; +import { + describeEachDialect, + setupForDialectWithCollections, + teardownForDialect, + type DialectTestContext, +} from "../../utils/test-db.js"; + +describeEachDialect("taxonomy term slug generation", (dialect) => { + let ctx: DialectTestContext; + + beforeEach(async () => { + ctx = await setupForDialectWithCollections(dialect); + const created = await handleTaxonomyCreate(ctx.db, { + name: "tags", + label: "Tags", + collections: ["post"], + }); + expect(created.success).toBe(true); + }); + + afterEach(async () => { + await teardownForDialect(ctx); + }); + + it.each([ + ["مرحبا بالعالم", "مرحبا-بالعالم"], + ["你好世界", "你好世界"], + ["Привет мир", "привет-мир"], + ["שלום עולם", "שלום-עולם"], + ["สวัสดี โลก", "สวัสดี-โลก"], + ["Καλημέρα κόσμε", "καλημέρα-κόσμε"], + ["మేష రాసి", "మేష-రాసి"], + ])("derives a native-script slug for %s", async (label, expected) => { + const result = await handleTermCreate(ctx.db, "tags", { label }); + + expect(result.success).toBe(true); + if (!result.success) return; + expect(result.data.term.slug).toBe(expected); + }); + + it("uses a deterministic fallback for emoji-only labels", async () => { + const result = await handleTermCreate(ctx.db, "tags", { label: "🎵🎵" }); + + expect(result.success).toBe(true); + if (!result.success) return; + expect(result.data.term.slug).toMatch(/^untitled-[a-z0-9]+$/); + }); + + it("adds a numeric suffix for generated slug collisions", async () => { + const first = await handleTermCreate(ctx.db, "tags", { label: "音楽", locale: "en" }); + const second = await handleTermCreate(ctx.db, "tags", { label: "音楽", locale: "en" }); + + expect(first.success).toBe(true); + expect(second.success).toBe(true); + if (!second.success) return; + expect(second.data.term.slug).toBe("音楽-1"); + }); + + it("recovers when concurrent generated slugs collide", async () => { + const results = await Promise.all( + Array.from({ length: 6 }, () => + handleTermCreate(ctx.db, "tags", { label: "同時", locale: "en" }), + ), + ); + + expect(results.every((result) => result.success)).toBe(true); + const slugs = results.flatMap((result) => (result.success ? [result.data.term.slug] : [])); + expect(new Set(slugs)).toEqual( + new Set(["同時", "同時-1", "同時-2", "同時-3", "同時-4", "同時-5"]), + ); + }); + + it("keeps generated slug uniqueness scoped to the locale", async () => { + const english = await handleTermCreate(ctx.db, "tags", { label: "音楽", locale: "en" }); + const japanese = await handleTermCreate(ctx.db, "tags", { label: "音楽", locale: "ja" }); + + expect(english.success).toBe(true); + expect(japanese.success).toBe(true); + if (!english.success || !japanese.success) return; + expect(english.data.term.slug).toBe("音楽"); + expect(japanese.data.term.slug).toBe("音楽"); + }); + + it("still rejects a colliding explicit slug", async () => { + await handleTermCreate(ctx.db, "tags", { slug: "music", label: "Music" }); + + const duplicate = await handleTermCreate(ctx.db, "tags", { + slug: "music", + label: "Other music", + }); + + expect(duplicate.success).toBe(false); + if (duplicate.success) return; + expect(duplicate.error.code).toBe("CONFLICT"); + }); +});