diff --git a/.changeset/extensible-bylines.md b/.changeset/extensible-bylines.md new file mode 100644 index 000000000..9ee2c87f5 --- /dev/null +++ b/.changeset/extensible-bylines.md @@ -0,0 +1,18 @@ +--- +"emdash": minor +"@emdash-cms/admin": minor +--- + +Adds custom fields to bylines. Sites can define site-specific byline metadata (Twitter handle, pronouns, company, localised job title, etc.) via the new `/byline-schema` admin screen, accessed from the **Byline schema** link button at the top of the Bylines admin page (admin-only). + +Per-field `translatable` flag picks whether values are stored per-locale (one value per locale row in a `translation_group`) or shared across every locale variant of the same byline identity. Schema management is gated by `schema:manage`; value editing by `bylines:manage`. + +Custom-field values can be set at both create and update time. `POST` and `PUT` on `/_emdash/api/admin/bylines` accept the same `customFields` map; the row write and the custom-field writes share a single transaction on Node/PG so a partial failure rolls both back. On D1 (no transactions), a retry POST is treated as completing an abandoned create iff three checks all pass: (a) every fixed column on the existing row matches the new payload (`displayName`, `bio`, `avatarMediaId`, `websiteUrl`, `userId`, `isGuest`, effective locale — null-vs-undefined normalised); (b) the existing row's `translationGroup` matches what a fresh create with the same input would produce (`sourceGroup` when `translationOf` is present, `existing.id` when it isn't); (c) every custom-field value already stored on the row appears in the input payload with an equal value (subset-match, so partial mid-loop crashes can be completed). The recovery branch is conservative on every axis: any fixed-column mismatch, any translation-group mismatch, any overlapping custom-field value mismatch, an input that omits a key the existing row stores, or an input with no custom fields at all → standard `CONFLICT`. Validation runs before any DB write so a bad value (unknown slug, type mismatch, select-choice miss, non-URL or non-http(s) URL for a `url` field) returns 400 `VALIDATION_ERROR` without leaving partial state behind. In the admin, registered fields render inline with Name, Bio, etc. — no separate section header — and are available in the **New byline** dialog as well as edit. + +`BylineSummary` gains an optional `customFields: Record` property. Existing object-literal consumers stay source-compatible because the property is optional and runtime always returns `{}` when no fields are registered. + +Hydration is symmetric with writes: rows are only applied to a byline when they live in the table matching the field's current `translatable` flag, so stale rows from a `translatable` flip can't leak into hydrated output. Schema mutations on `/byline-schema` invalidate the same `byline-fields` query the byline form reads, so newly-registered fields appear in the editor without a page reload. `url` field values are parsed with `new URL(...)` AND restricted to `http:` / `https:` schemes at write time so they can't ship `javascript:` / `data:` / `mailto:` payloads to link rendering. The `BylineFieldEditor` "Save" button stays disabled until a `select` field has at least one option; and select-option lists are accumulated on a null-prototype object so option values that collide with `Object.prototype` keys render correctly. + +The field-definitions cache uses parity on `options.byline_fields_version` as a dirty bit: schema mutations flip the counter to odd before the write lands and to a **new even** value after, with the cache treating any odd version as "bypass the global holder, read fresh from the DB". `markVersionDirty` is parity-aware (ensures odd, no-op if already odd) so a crashed prior attempt's leftover dirty state can't get inverted. `markVersionClean` is **always-advance** (`+2` when starting even, `+1` when starting odd) so two concurrent mutators can't collapse on the same even key and pin the cache on a partial-set snapshot — every committed mutation produces an observable counter change for cache readers. Idempotent-retry exits (`FIELD_EXISTS` on create, `FIELD_NOT_FOUND` on update/delete, no-op input on update) call `markVersionClean` too, which doubles as both the dirty-crash recovery and the false-clean recovery. All version writes use `INSERT … ON CONFLICT DO UPDATE` so a missing options row can't silently turn invalidation into a no-op. + +Implements [#1174](https://github.com/emdash-cms/emdash/discussions/1174). Builds on the bylines-i18n foundation from [#1146](https://github.com/emdash-cms/emdash/pull/1146). diff --git a/e2e/tests/byline-fields.spec.ts b/e2e/tests/byline-fields.spec.ts new file mode 100644 index 000000000..32aedcf20 --- /dev/null +++ b/e2e/tests/byline-fields.spec.ts @@ -0,0 +1,139 @@ +/** + * Byline Custom Fields E2E (Phase 7 of Discussion #1174) + * + * Proves the full round-trip: an admin registers a custom field via + * `/byline-schema`, a byline is given a value through the edit form, + * and the GET response on `/api/admin/bylines/{id}` returns the value + * via `customFields.{slug}` — exercising every layer the PR touches + * (registry, repository hydration, admin API, schema management UI, + * byline edit form). + * + * Workers are pinned to 1 in `playwright.config.ts`, and + * `global-setup.ts` rebuilds the fixture DB per `pnpm test:e2e` + * invocation — so a fixed slug works without conflicting with itself + * across runs. We still suffix with a timestamp because the same + * `pnpm test:e2e` invocation may run this spec alongside others that + * touch the bylines table. + */ + +import { test, expect } from "../fixtures"; + +function apiHeaders(token: string, baseUrl: string): Record { + return { + "Content-Type": "application/json", + Authorization: `Bearer ${token}`, + "X-EmDash-Request": "1", + Origin: baseUrl, + }; +} + +test.describe("Byline custom fields", () => { + test.beforeEach(async ({ admin }) => { + await admin.devBypassAuth(); + }); + + test("custom field value round-trips through the API end to end", async ({ + admin, + page, + serverInfo, + }) => { + const unique = Date.now(); + const fieldSlug = `job_title_${unique}`; + const fieldLabel = `Job title ${unique}`; + const bylineDisplayName = `Jane Custom ${unique}`; + const bylineSlug = `jane-custom-${unique}`; + const fieldValue = "Editor"; + + // --------------------------------------------------------------- + // 1. Register a custom field via /byline-schema + // --------------------------------------------------------------- + + await admin.goto("/byline-schema"); + await admin.waitForLoading(); + + await page.getByRole("button", { name: "New field" }).click(); + // `BylineFieldEditor` auto-fills the slug from the label + // (Phase 5). Filling label first matches the production UX + // flow; overriding slug afterwards proves the input is editable. + await page.getByLabel("Label").fill(fieldLabel); + await page.getByLabel("Slug").fill(fieldSlug); + // Type defaults to `string` — leave it. `translatable` defaults + // to true — leave it. Required stays off. + await page.getByRole("button", { name: "Create field" }).click(); + + // The new field row should show up in the schema table. Scope + // the assertion to the table because the success toast also + // contains the field label ("Created \"{label}\"."), and + // Playwright's strict-mode locator rejects ambiguous matches. + await expect(page.locator("table").getByText(fieldLabel)).toBeVisible({ + timeout: 5000, + }); + + // --------------------------------------------------------------- + // 2. Create a byline via /bylines and select it + // --------------------------------------------------------------- + + await admin.goto("/bylines"); + await admin.waitForLoading(); + + await page.getByRole("button", { name: "New" }).click(); + await page.getByLabel("Display name").fill(bylineDisplayName); + await page.getByLabel("Slug").fill(bylineSlug); + // Guest byline keeps the form simple — no user-link side quest. + await page.getByRole("switch", { name: "Guest byline" }).click(); + await page.getByRole("button", { name: "Create" }).click(); + + // After create, the form moves to edit mode and the sidebar list + // re-renders with the new byline highlighted. Custom-field inputs + // are gated on `selected`, so they appear only after create lands. + await expect(page.getByRole("button", { name: bylineDisplayName })).toBeVisible({ + timeout: 5000, + }); + + // --------------------------------------------------------------- + // 3. Fill the custom field input and save + // --------------------------------------------------------------- + + await expect(page.getByLabel(fieldLabel)).toBeVisible(); + await page.getByLabel(fieldLabel).fill(fieldValue); + await page.getByRole("button", { name: "Save" }).click(); + + // --------------------------------------------------------------- + // 4. Verify the round-trip via the REST API + // --------------------------------------------------------------- + + // Find the byline id via the list endpoint (the sidebar's selected + // row is keyed by id internally; reading via API is easier than + // scraping the DOM). Filter by slug to avoid pagination concerns. + const headers = apiHeaders(serverInfo.token, serverInfo.baseUrl); + const listResponse = await fetch( + `${serverInfo.baseUrl}/_emdash/api/admin/bylines?search=${encodeURIComponent(bylineSlug)}`, + { headers }, + ); + expect(listResponse.ok).toBe(true); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const listBody: any = await listResponse.json(); + const created = (listBody.data?.items ?? []).find( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (b: any) => b.slug === bylineSlug, + ); + expect(created).toBeTruthy(); + expect(created.displayName).toBe(bylineDisplayName); + + const getResponse = await fetch( + `${serverInfo.baseUrl}/_emdash/api/admin/bylines/${created.id}`, + { headers }, + ); + expect(getResponse.ok).toBe(true); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const getBody: any = await getResponse.json(); + const fetched = getBody.data; + + // The core assertion: the value we typed in the form is what + // the API returns for the field slug. Goes through every layer: + // admin UI form state → PATCH body → handler → BylineRepository + // .update → translatable value table → hydration → GET response. + expect(fetched.customFields).toBeTruthy(); + expect(fetched.customFields[fieldSlug]).toBe(fieldValue); + }); +}); diff --git a/packages/admin/src/components/BylineFieldEditor.tsx b/packages/admin/src/components/BylineFieldEditor.tsx new file mode 100644 index 000000000..7310c7d78 --- /dev/null +++ b/packages/admin/src/components/BylineFieldEditor.tsx @@ -0,0 +1,324 @@ +/** + * BylineFieldEditor (Phase 5 of Discussion #1174). + * + * Purpose-built create/edit dialog for byline custom-field definitions. + * Constrains the type select to the five v1 types declared by the + * `BylineFieldType` union — narrower than the content-field universe — + * and exposes the `translatable` toggle (the storage-split flag) that + * the content `FieldEditor` doesn't have. + * + * Single-step form (not the type-picker → config flow used by + * `FieldEditor`): with only five types a ` handleLabelChange(e.target.value)} + placeholder={t`Job title`} + /> +
+ setField("slug", e.target.value)} + // The literal slug is an example identifier, not natural-language + // copy — translators will typically leave it as-is, but the + // AGENTS.md "every user-facing string via Lingui" rule applies + // uniformly, so the catalog extractor sees it. + placeholder={t`job_title`} + disabled={isEdit} + /> + {isEdit && ( +

+ {t`Slugs cannot be changed after the field is created.`} +

+ )} +
+ + + {/* Type — disabled on edit because the storage column depends on it */} +
+ onChange(e.target.value === "" ? null : e.target.value)} + /> + ); + case "text": + return ( + onChange(e.target.value === "" ? null : e.target.value)} + rows={3} + /> + ); + case "url": + return ( + onChange(e.target.value === "" ? null : e.target.value)} + /> + ); + case "boolean": + // Booleans are always definite once the field is registered — + // `null` would mean "no row stored", which conceptually maps + // to `false` for a yes/no toggle. The Switch sends a real + // boolean and the storage path persists it verbatim. + return ( + onChange(checked)} + /> + ); + case "select": { + const options = field.validation?.options ?? []; + // Null-prototype object so options that collide with + // `Object.prototype` keys (`__proto__`, `toString`) survive. + const items: Record = Object.create(null); + items[""] = t`-- Select --`; + for (const opt of options) items[opt] = opt; + return ( + `, which accepts `.fill()` directly. + */ + +import { Toast } from "@cloudflare/kumo"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import * as React from "react"; +import { describe, it, expect, vi, beforeEach } from "vitest"; + +import type { BylineFieldDefinition } from "../src/lib/api/byline-fields"; +import type { BylineSummary } from "../src/lib/api/bylines"; +import { render } from "./utils/render.tsx"; + +// TanStack Router pieces consumed by the bylines route. Mocked to +// avoid wiring a full RouterProvider just for one component test. +vi.mock("@tanstack/react-router", async () => { + const actual = + await vi.importActual("@tanstack/react-router"); + return { + ...actual, + useSearch: () => ({ locale: undefined }), + useNavigate: () => vi.fn(), + }; +}); + +// Bylines API surface — every call the page makes is mocked. The +// per-test setup below configures fixtures and tracks `updateByline` +// calls so the final assertion can verify the PATCH body shape. +vi.mock("../src/lib/api/bylines", async () => { + const actual = + await vi.importActual("../src/lib/api/bylines"); + return { + ...actual, + fetchBylines: vi.fn(), + fetchByline: vi.fn(), + fetchBylineTranslations: vi.fn().mockResolvedValue({ items: [] }), + createByline: vi.fn(), + updateByline: vi.fn(), + deleteByline: vi.fn(), + createBylineTranslation: vi.fn(), + }; +}); + +vi.mock("../src/lib/api/users", async () => { + const actual = + await vi.importActual("../src/lib/api/users"); + return { + ...actual, + fetchUsers: vi.fn().mockResolvedValue({ items: [], nextCursor: undefined }), + }; +}); + +vi.mock("../src/lib/api/byline-fields", async () => { + const actual = await vi.importActual( + "../src/lib/api/byline-fields", + ); + return { + ...actual, + listBylineFields: vi.fn(), + }; +}); + +// Manifest used by the locale switcher. Single-locale stub keeps the +// multi-locale UI off — irrelevant to this AC. +vi.mock("../src/lib/api/client", async () => { + const actual = + await vi.importActual("../src/lib/api/client"); + return { + ...actual, + fetchManifest: vi.fn().mockResolvedValue({ + version: "0.0.0", + hash: "test", + collections: {}, + plugins: {}, + authMode: "passkey", + taxonomies: [], + }), + }; +}); + +const { fetchBylines, fetchByline, updateByline } = await import("../src/lib/api/bylines"); +const { listBylineFields } = await import("../src/lib/api/byline-fields"); +const { BylinesPage } = await import("../src/routes/bylines"); + +// --------------------------------------------------------------------------- +// Fixtures +// --------------------------------------------------------------------------- + +function makeByline(overrides: Partial = {}): BylineSummary { + return { + id: "byline_01", + slug: "jane-doe", + displayName: "Jane Doe", + bio: null, + avatarMediaId: null, + websiteUrl: null, + userId: null, + isGuest: true, + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + locale: "en", + translationGroup: null, + customFields: {}, + ...overrides, + }; +} + +function makeField(overrides: Partial = {}): BylineFieldDefinition { + return { + id: "fld_01", + slug: "job_title", + label: "Job title", + type: "string", + required: false, + translatable: true, + validation: null, + sortOrder: 0, + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + ...overrides, + }; +} + +function TestWrapper({ children }: { children: React.ReactNode }) { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false }, mutations: { retry: false } }, + }); + return ( + + {children} + + ); +} + +beforeEach(() => { + vi.clearAllMocks(); +}); + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe("BylinesPage — custom-field inputs (Phase 6 of #1174)", () => { + it("renders the custom-field input after selecting a byline", async () => { + const byline = makeByline(); + vi.mocked(fetchBylines).mockResolvedValue({ items: [byline], nextCursor: undefined }); + vi.mocked(fetchByline).mockResolvedValue(byline); + vi.mocked(listBylineFields).mockResolvedValue({ + items: [makeField({ label: "Job title", slug: "job_title", type: "string" })], + }); + + const screen = await render( + + + , + ); + + // Edit mode: registered field renders inline (no "Custom fields" header). + const bylineButton = screen.getByRole("button", { name: /Jane Doe/ }); + await bylineButton.click(); + + await expect.element(screen.getByLabelText("Job title")).toBeInTheDocument(); + }); + + it("renders the custom-field input in create mode", async () => { + // Phase 6: create-flow parity — POST accepts customFields now. + vi.mocked(fetchBylines).mockResolvedValue({ items: [], nextCursor: undefined }); + vi.mocked(listBylineFields).mockResolvedValue({ + items: [makeField({ label: "Job title", slug: "job_title", type: "string" })], + }); + + const screen = await render( + + + , + ); + + await expect.element(screen.getByText("Create byline")).toBeInTheDocument(); + await expect.element(screen.getByLabelText("Job title")).toBeInTheDocument(); + }); + + it("forwards customFields in the PATCH body on save", async () => { + const byline = makeByline({ + customFields: { job_title: "Senior editor" }, + }); + vi.mocked(fetchBylines).mockResolvedValue({ items: [byline], nextCursor: undefined }); + vi.mocked(fetchByline).mockResolvedValue(byline); + vi.mocked(listBylineFields).mockResolvedValue({ + items: [makeField({ slug: "job_title", type: "string" })], + }); + vi.mocked(updateByline).mockResolvedValue(byline); + + const screen = await render( + + + , + ); + + await screen.getByRole("button", { name: /Jane Doe/ }).click(); + + // Prefilled value from the byline's customFields hydration. + const input = screen.getByLabelText("Job title"); + await expect.element(input).toHaveValue("Senior editor"); + + // Save without modifying — proves the PATCH body still includes + // the customFields map (the form's `toFormState` populated it + // from the response, and `updateMutation` spreads it). + await screen.getByRole("button", { name: "Save" }).click(); + + // Vitest-browser-react resolves the mutation asynchronously; + // give the click a microtask before asserting. + await new Promise((resolve) => setTimeout(resolve, 50)); + + expect(vi.mocked(updateByline)).toHaveBeenCalledTimes(1); + const [bylineId, body] = vi.mocked(updateByline).mock.calls[0]!; + expect(bylineId).toBe(byline.id); + expect(body.customFields).toEqual({ job_title: "Senior editor" }); + }); + + it("omits customFields from the PATCH body when field-defs fail to load", async () => { + // Regression guard for the silent-overwrite scenario flagged in + // the Phase 6 review. When `listBylineFields` errors, the form + // can't render the inputs, so the editor cannot see what + // they'd be saving. The Save button stays enabled so editors + // can still update fixed columns (bio, slug, etc.), but the + // PATCH body must NOT include `customFields` — otherwise the + // hydrated map would round-trip and a "field deleted + // server-side mid-session" race would surface as a 400. + const byline = makeByline({ + customFields: { job_title: "Senior editor" }, + }); + vi.mocked(fetchBylines).mockResolvedValue({ items: [byline], nextCursor: undefined }); + vi.mocked(fetchByline).mockResolvedValue(byline); + vi.mocked(listBylineFields).mockRejectedValue(new Error("network blip")); + vi.mocked(updateByline).mockResolvedValue(byline); + + const screen = await render( + + + , + ); + + await screen.getByRole("button", { name: /Jane Doe/ }).click(); + + // Error surface is shown — editor is told what's happening. + await expect.element(screen.getByText("Couldn't load custom fields.")).toBeInTheDocument(); + + // Save fixed columns; PATCH body must omit `customFields`. + await screen.getByRole("button", { name: "Save" }).click(); + await new Promise((resolve) => setTimeout(resolve, 50)); + + expect(vi.mocked(updateByline)).toHaveBeenCalledTimes(1); + const [, body] = vi.mocked(updateByline).mock.calls[0]!; + expect(body).not.toHaveProperty("customFields"); + // Sanity: the fixed columns ARE in the body — proves the Save + // path actually fired and the omission is targeted. + expect(body.slug).toBe(byline.slug); + expect(body.displayName).toBe(byline.displayName); + }); + + it("renders the right Kumo input for each registered field type", async () => { + const byline = makeByline({ + customFields: { + job_title: "Editor", + bio_long: "A long bio.", + homepage: "https://example.com", + is_staff: true, + tier: "gold", + }, + }); + vi.mocked(fetchBylines).mockResolvedValue({ items: [byline], nextCursor: undefined }); + vi.mocked(fetchByline).mockResolvedValue(byline); + vi.mocked(listBylineFields).mockResolvedValue({ + items: [ + makeField({ id: "f1", slug: "job_title", label: "Job title", type: "string" }), + makeField({ id: "f2", slug: "bio_long", label: "Long bio", type: "text" }), + makeField({ id: "f3", slug: "homepage", label: "Homepage", type: "url" }), + makeField({ id: "f4", slug: "is_staff", label: "Staff", type: "boolean" }), + makeField({ + id: "f5", + slug: "tier", + label: "Tier", + type: "select", + validation: { options: ["bronze", "silver", "gold"] }, + }), + ], + }); + + const screen = await render( + + + , + ); + + await screen.getByRole("button", { name: /Jane Doe/ }).click(); + + // `string` and `url` both render as ``, but their `type` + // attribute differentiates them. `text` renders as `