From 7fd1e906a912dd91f6852ab4c5b854774cbb66b7 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 13:18:37 +0000 Subject: [PATCH 01/47] Kill the api-keys and attributes mutation survivors The branch mutation gate found the moved suites never pinned the form labels and hints, the flash wordings, the redirect targets, the activity-log entries, or the ordered-collection append. New direct tests assert each of those exactly: the api-keys name box and its bound, the key-only flash still showing the copy panel, the delete flash and target, the active nav entry, and a new attributes CRUD suite covering create, reorder, option add, edit, and both deletes with their logged messages. Two provably-equivalent mutants are recorded: the entity load's `?? null` after find(), and the option insert's placeholder sortOrder that append always overwrites. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7 --- scripts/mutation/equivalent-mutants.txt | 2 + test/features/admin/api-keys.test.ts | 40 ++- test/features/admin/attributes/crud.test.ts | 255 ++++++++++++++++++++ 3 files changed, 295 insertions(+), 2 deletions(-) create mode 100644 test/features/admin/attributes/crud.test.ts diff --git a/scripts/mutation/equivalent-mutants.txt b/scripts/mutation/equivalent-mutants.txt index b6c3b09c16..69c73f85a8 100644 --- a/scripts/mutation/equivalent-mutants.txt +++ b/scripts/mutation/equivalent-mutants.txt @@ -994,3 +994,5 @@ src/features/admin/listing-page-data.ts:282:61 ?? → || # Map.get returns Li # The news card query names a single table, so its columns need no alias to # resolve. src/shared/db/news-posts.ts:148:47 news_post → "" # dropping the alias leaves the column names unqualified, and the read selects from one table (news_posts AS news_post) plus scalar subqueries that carry their own qualified id, so every name still resolves to the same column +src/features/admin/api-keys.ts:147:75 ?? → || # entity-page load: find() returns a key row object or undefined — a row object is always truthy, so `?? null` and `|| null` agree on every input +src/features/admin/attributes.ts:188:20 0 → 1 # option insert's sortOrder is a placeholder: attributeOptionsOrder.append immediately overwrites it with MAX(sibling)+1 (or the collection's start), and nothing reads the row in between — no input can see the inserted value diff --git a/test/features/admin/api-keys.test.ts b/test/features/admin/api-keys.test.ts index 8c94717179..4aaf097b2c 100644 --- a/test/features/admin/api-keys.test.ts +++ b/test/features/admin/api-keys.test.ts @@ -196,6 +196,36 @@ describeWithEnv("API Keys", { db: true }, () => { }); describe("admin UI", () => { + test("serves the name box with its label, hint, bound, and required flag", async () => { + // The expected attributes are written out here so a changed form + // definition fails this test instead of moving the expectation along. + const { apiKeyForm } = await import("#routes/admin/api-keys.ts"); + const html = apiKeyForm.render(); + expect(html).toContain("Name"); + const input = html.slice(html.indexOf('name="name"') - 200); + expect(input).toContain('maxlength="100"'); + expect(input).toContain('placeholder="e.g. CI Pipeline"'); + expect(input).toContain("required"); + }); + + test("a flash that is only a key still shows the copy panel, not a message", async () => { + // The key sits after the newline; a flash that STARTS with the newline + // has an empty message and only the key. The copy panel must still + // appear — the split runs on position 0, which is a found position. + const cookie = await testCookie(); + const response = await handleRequest( + mockRequest(`/admin/api-keys?flash=${FLASH_TEST_ID}`, { + headers: { + cookie: `${cookie}; ${flashCookieHeader("\nBARE-KEY-123")}`, + }, + }), + ); + + const html = await response.text(); + expect(html).toContain("Copy your API key now"); + expect(html).toContain("BARE-KEY-123"); + }); + test("GET /admin/api-keys shows the page", async () => { const cookie = await testCookie(); const response = await handleRequest( @@ -348,6 +378,8 @@ describeWithEnv("API Keys", { db: true }, () => { expect(firstHtml).toContain(`href="/admin/api-keys/${id}/actions"`); expect(firstHtml).not.toContain(`/admin/api-keys/${id}/delete`); expect(firstHtml).toContain("Never"); + // The manage page lights up the API keys entry in the admin nav. + expect(firstHtml).toContain(''); // Once used, the summary renders the formatted last-used date. await touchApiKeyLastUsed(id); @@ -451,7 +483,11 @@ describeWithEnv("API Keys", { db: true }, () => { confirm_identifier: "Doomed Key", }); - expect(response.status).toBe(302); + await expectFlashRedirect( + "/admin/api-keys", + "API key deleted", + true, + )(response); expect(await getApiKeysForUser(1)).toEqual([]); }); @@ -466,7 +502,7 @@ describeWithEnv("API Keys", { db: true }, () => { // backstop renders the mismatch error, so the operator actually sees it. await expectFlashRedirect( `/admin/api-keys/${id}/delete`, - expect.stringContaining("does not match"), + expect.stringContaining("API key name does not match"), false, )(response); expect(await getApiKeysForUser(1)).toHaveLength(1); diff --git a/test/features/admin/attributes/crud.test.ts b/test/features/admin/attributes/crud.test.ts new file mode 100644 index 0000000000..e868d17bd2 --- /dev/null +++ b/test/features/admin/attributes/crud.test.ts @@ -0,0 +1,255 @@ +import { expect } from "@std/expect"; +import { describe, it as test } from "@std/testing/bdd"; +import { + attributeNameForm, + attributeOptionForm, +} from "#routes/admin/attributes.ts"; +import { getAllActivityLog } from "#test-utils/activity-log.ts"; +import { + expectFlashRedirect, + expectHtmlResponse, + expectRedirect, + expectStatus, + inputNamed, +} from "#test-utils/assertions.ts"; +import { describeWithEnv } from "#test-utils/db.ts"; +import { createTestAttributeWithOptions } from "#test-utils/db-helpers/attributes.ts"; +import { adminFormPost, adminGet } from "#test-utils/session.ts"; + +/** Create an attribute through the real POST and return its new id. */ +const createAttribute = async (name: string): Promise => { + const { response } = await adminFormPost("/admin/attributes", { name }); + const location = expectRedirect(response, /^\/admin\/attributes\/\d+/); + return Number(new URL(location, "http://localhost").pathname.split("/")[3]); +}; + +describeWithEnv( + "server (admin attribute create and edit)", + { db: true }, + () => { + describe("the two attribute forms", () => { + // The expected label and hint are written out here so a changed form + // definition fails this test instead of moving the expectation with it. + test("serves the name box with its label and hint", () => { + const html = attributeNameForm.render(); + expect(html).toContain("Attribute name"); + expect(inputNamed(html, "name")).toContain( + 'placeholder="e.g. Difficulty"', + ); + }); + + test("serves the option box with its label and hint", () => { + const html = attributeOptionForm.render(); + expect(html).toContain("Option text"); + expect(inputNamed(html, "text")).toContain( + 'placeholder="e.g. Beginner"', + ); + }); + }); + + describe("POST /admin/attributes", () => { + test("creating an attribute stores it, logs it, and says so", async () => { + const { response } = await adminFormPost("/admin/attributes", { + name: "Terrain", + }); + + const location = expectRedirect(response, /^\/admin\/attributes\/\d+/); + await expectFlashRedirect( + new URL(location, "http://localhost").pathname, + "Attribute created", + true, + )(response); + await expectHtmlResponse(await adminGet(location), 200, "Terrain"); + const messages = (await getAllActivityLog()).map( + (entry) => entry.message, + ); + expect(messages).toContain("Attribute 'Terrain' created"); + }); + + test("a new attribute joins the order, so it can be moved", async () => { + await createAttribute("Alpha"); + const betaId = await createAttribute("Beta"); + + const before = await expectHtmlResponse( + await adminGet("/admin/attributes"), + 200, + ); + expect(before.indexOf(">Alpha<")).toBeGreaterThanOrEqual(0); + expect(before.indexOf(">Alpha<")).toBeLessThan( + before.indexOf(">Beta<"), + ); + + await adminFormPost(`/admin/attributes/${betaId}/move-up`, {}); + + const after = await expectHtmlResponse( + await adminGet("/admin/attributes"), + 200, + ); + expect(after.indexOf(">Beta<")).toBeGreaterThanOrEqual(0); + expect(after.indexOf(">Beta<")).toBeLessThan(after.indexOf(">Alpha<")); + }); + + test("an empty name is refused back to the attributes page", async () => { + const { response } = await adminFormPost("/admin/attributes", { + name: "", + }); + + await expectFlashRedirect( + "/admin/attributes", + "Attribute name is required", + false, + )(response); + }); + }); + + describe("POST /admin/attributes/:id/options", () => { + test("adding an option stores it, logs it, and says so", async () => { + const attribute = await createTestAttributeWithOptions( + "Difficulty", + [], + ); + + const { response } = await adminFormPost( + `/admin/attributes/${attribute.id}/options`, + { text: "Easy" }, + ); + + await expectFlashRedirect( + `/admin/attributes/${attribute.id}`, + "Option added", + true, + )(response); + await expectHtmlResponse( + await adminGet(`/admin/attributes/${attribute.id}`), + 200, + "Easy", + ); + const messages = (await getAllActivityLog()).map( + (entry) => entry.message, + ); + expect(messages).toContain( + "Attribute option 'Easy' added to Difficulty", + ); + }); + }); + + describe("POST /admin/attributes/:id/delete", () => { + test("deleting needs the exact attribute name", async () => { + const attribute = await createTestAttributeWithOptions("Doomed", [ + "Only", + ]); + + const { response } = await adminFormPost( + `/admin/attributes/${attribute.id}/delete`, + { confirm_identifier: "Wrong" }, + ); + + await expectFlashRedirect( + `/admin/attributes/${attribute.id}/delete`, + expect.stringContaining("Attribute name does not match"), + false, + )(response); + }); + + test("deleting removes the attribute, logs it, and says so", async () => { + const attribute = await createTestAttributeWithOptions("Doomed", [ + "Only", + ]); + + const { response } = await adminFormPost( + `/admin/attributes/${attribute.id}/delete`, + { confirm_identifier: "Doomed" }, + ); + + await expectFlashRedirect( + "/admin/attributes", + "Attribute deleted", + true, + )(response); + expectStatus(404)(await adminGet(`/admin/attributes/${attribute.id}`)); + const messages = (await getAllActivityLog()).map( + (entry) => entry.message, + ); + expect(messages).toContain("Attribute 'Doomed' deleted"); + }); + }); + + describe("POST /admin/attributes/:id/options/:optionId/delete", () => { + test("deleting an option needs the exact option text", async () => { + const attribute = await createTestAttributeWithOptions("Difficulty", [ + "Easy", + ]); + const easy = attribute.options[0]!; + + const { response } = await adminFormPost( + `/admin/attributes/${attribute.id}/options/${easy.id}/delete`, + { confirm_identifier: "Nope" }, + ); + + await expectFlashRedirect( + `/admin/attributes/${attribute.id}/options/${easy.id}/delete`, + "Option text does not match. Please type the exact option text to confirm deletion.", + false, + )(response); + }); + + test("deleting an option removes it, logs it, and says so", async () => { + const attribute = await createTestAttributeWithOptions("Difficulty", [ + "Easy", + ]); + const easy = attribute.options[0]!; + + const { response } = await adminFormPost( + `/admin/attributes/${attribute.id}/options/${easy.id}/delete`, + { confirm_identifier: "Easy" }, + ); + + await expectFlashRedirect( + `/admin/attributes/${attribute.id}`, + "Option deleted", + true, + )(response); + const html = await expectHtmlResponse( + await adminGet(`/admin/attributes/${attribute.id}`), + 200, + ); + expect(html).not.toContain(">Easy<"); + const messages = (await getAllActivityLog()).map( + (entry) => entry.message, + ); + expect(messages).toContain( + "Attribute option 'Easy' deleted from Difficulty", + ); + }); + }); + + describe("POST /admin/attributes/:id/edit", () => { + test("editing saves the new name, logs it, and says so", async () => { + const attribute = await createTestAttributeWithOptions("Old name", [ + "Only", + ]); + + const { response } = await adminFormPost( + `/admin/attributes/${attribute.id}/edit`, + { name: "New name" }, + ); + + await expectFlashRedirect( + `/admin/attributes/${attribute.id}`, + "Attribute updated", + true, + )(response); + const html = await expectHtmlResponse( + await adminGet(`/admin/attributes/${attribute.id}`), + 200, + "New name", + ); + expect(html).not.toContain("Old name"); + const messages = (await getAllActivityLog()).map( + (entry) => entry.message, + ); + expect(messages).toContain("Attribute 'New name' updated"); + }); + }); + }, +); From 6a6fe106f6954d37f1ab5925dd8b9288750a9aa9 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 13:47:24 +0000 Subject: [PATCH 02/47] Answer the review: move the CRUD cases, split the suite, make inserts atomic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review points, all done: The attribute CRUD journeys now live only in the mirror suites — the new crud and options files under test/features/admin/attributes/ merge the killer assertions with the moved integration cases, and the integration file keeps just the pages that are not CRUD. The api-keys suite is now a folder of four focused files (records, ui, manage, auth), each under the size target, sharing adminFormPost and a page helper instead of local scaffolds. The option and attribute creates now run their insert, order append, and activity log in one transaction. The "equivalent" record for the placeholder sortOrder was wrong — a failed append used to leave the half-made row behind — so it is removed, and two regression tests force the order write to fail and expect nothing to be stored. Both fail against the old non-atomic code. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7 --- scripts/mutation/equivalent-mutants.txt | 1 - src/features/admin/attributes.ts | 53 +- test/features/admin/api-keys.test.ts | 790 ------------------ test/features/admin/api-keys/auth.test.ts | 221 +++++ test/features/admin/api-keys/manage.test.ts | 178 ++++ test/features/admin/api-keys/records.test.ts | 143 ++++ test/features/admin/api-keys/ui.test.ts | 245 ++++++ test/features/admin/attributes/crud.test.ts | 447 +++++----- .../features/admin/attributes/options.test.ts | 215 +++++ test/integration/server/attributes.test.ts | 287 +------ test/test-utils/activity-log.ts | 4 + test/test-utils/db-helpers/attributes.ts | 11 + 12 files changed, 1288 insertions(+), 1307 deletions(-) delete mode 100644 test/features/admin/api-keys.test.ts create mode 100644 test/features/admin/api-keys/auth.test.ts create mode 100644 test/features/admin/api-keys/manage.test.ts create mode 100644 test/features/admin/api-keys/records.test.ts create mode 100644 test/features/admin/api-keys/ui.test.ts create mode 100644 test/features/admin/attributes/options.test.ts diff --git a/scripts/mutation/equivalent-mutants.txt b/scripts/mutation/equivalent-mutants.txt index 69c73f85a8..7eefd2bb09 100644 --- a/scripts/mutation/equivalent-mutants.txt +++ b/scripts/mutation/equivalent-mutants.txt @@ -995,4 +995,3 @@ src/features/admin/listing-page-data.ts:282:61 ?? → || # Map.get returns Li # resolve. src/shared/db/news-posts.ts:148:47 news_post → "" # dropping the alias leaves the column names unqualified, and the read selects from one table (news_posts AS news_post) plus scalar subqueries that carry their own qualified id, so every name still resolves to the same column src/features/admin/api-keys.ts:147:75 ?? → || # entity-page load: find() returns a key row object or undefined — a row object is always truthy, so `?? null` and `|| null` agree on every input -src/features/admin/attributes.ts:188:20 0 → 1 # option insert's sortOrder is a placeholder: attributeOptionsOrder.append immediately overwrites it with MAX(sibling)+1 (or the collection's start), and nothing reads the row in between — no input can see the inserted value diff --git a/src/features/admin/attributes.ts b/src/features/admin/attributes.ts index 4cfd6b9041..ae892997de 100644 --- a/src/features/admin/attributes.ts +++ b/src/features/admin/attributes.ts @@ -44,10 +44,12 @@ import { listingAttributeOptions, pruneInvalidAttributeOptionIds, } from "#shared/db/attributes.ts"; +import { type TxScope, withTransaction } from "#shared/db/client.ts"; import { flatCollectionSwap, scopedCollectionSwap, } from "#shared/db/ordered-collection.ts"; +import { writeTableRow } from "#shared/db/table.ts"; import { getFlash } from "#shared/flash-context.ts"; import { defineTextForm } from "#shared/forms/definition.ts"; import { @@ -92,9 +94,22 @@ const handleAttributesPost = createAuthedFormRoute({ form: attributeNameForm, onInvalid: ({ error }) => errorRedirect("/admin/attributes", error), onValid: async ({ values: { name } }) => { - const attribute = await attributesTable.insert({ name }); - await attributesOrder.append({ key: attribute.id }); - await logActivity(`Attribute '${name}' created`); + // One transaction: an attribute must never exist without its order entry + // or its log line. + const attribute = await withTransaction(async (transaction) => { + const created = await writeTableRow(transaction, attributesTable, { + input: { name }, + kind: "insert", + }); + await attributesOrder.append({ key: created.id, transaction }); + await logActivity( + `Attribute '${name}' created`, + undefined, + undefined, + transaction, + ); + return created; + }); return redirect( `/admin/attributes/${attribute.id}`, "Attribute created", @@ -148,11 +163,15 @@ const logAttributeOptionActivity = ( optionText: string, action: string, attribute: AttributeWithOptions, + transaction?: TxScope, ) => logActivity( `Attribute option '${optionText}' ${action} ${attributeNameFlat( attribute.name, )}`, + undefined, + undefined, + transaction, ); const handleAttributeEdit = createAuthedFormRoute< @@ -183,16 +202,26 @@ const handleAddOption = createAuthedFormRoute< onInvalid: redirectToAttribute, onValid: ({ params, values: { text } }) => orNotFound(getAttributeWithOptions(params.id), async (attribute) => { - const option = await attributeOptionsTable.insert({ - attributeId: params.id, - sortOrder: 0, - text, - }); - await attributeOptionsOrder.append({ - key: option.id, - scope: params.id, + // One transaction: an option must never exist without its place in the + // order or its log line. The inserted sortOrder is a placeholder the + // append overwrites before anything can read it. + await withTransaction(async (transaction) => { + const option = await writeTableRow(transaction, attributeOptionsTable, { + input: { attributeId: params.id, sortOrder: 0, text }, + kind: "insert", + }); + await attributeOptionsOrder.append({ + key: option.id, + scope: params.id, + transaction, + }); + await logAttributeOptionActivity( + text, + "added to", + attribute, + transaction, + ); }); - await logAttributeOptionActivity(text, "added to", attribute); return redirect(`/admin/attributes/${params.id}`, "Option added", true); }), }); diff --git a/test/features/admin/api-keys.test.ts b/test/features/admin/api-keys.test.ts deleted file mode 100644 index 4aaf097b2c..0000000000 --- a/test/features/admin/api-keys.test.ts +++ /dev/null @@ -1,790 +0,0 @@ -import { expect } from "@std/expect"; -import { describe, it as test } from "@std/testing/bdd"; -import { handleRequest } from "#routes"; -import { buildSessionCookie } from "#shared/cookies.ts"; -import { hmacHash } from "#shared/crypto/hashing.ts"; -import { unwrapKeyWithToken } from "#shared/crypto/keys.ts"; -import { generateSecureToken } from "#shared/crypto/utils.ts"; -import { signCsrfToken } from "#shared/csrf.ts"; -import { setAdminFeatureEnabled } from "#shared/db/admin-features.ts"; -import { apiKeyLimiter } from "#shared/db/api-key-attempts.ts"; -import { - createApiKey, - deleteApiKey, - getApiKeyByToken, - getApiKeyForUser, - getApiKeysForUser, - touchApiKeyLastUsed, -} from "#shared/db/api-keys.ts"; -import { getDb, insert } from "#shared/db/client.ts"; -import { - enableQueryLog, - getQueryLog, - runWithQueryLogContext, -} from "#shared/db/query-log.ts"; -import { createSession } from "#shared/db/sessions.ts"; -import { MAX_APIKEY_ATTEMPTS } from "#shared/limits.ts"; -import { - assertJson, - expectFlash, - expectFlashRedirect, - expectRedirect, - FLASH_TEST_ID, - flashCookieHeader, -} from "#test-utils/assertions.ts"; -import { extractCsrfToken } from "#test-utils/csrf.ts"; -import { describeWithEnv } from "#test-utils/db.ts"; -import { createTestListing } from "#test-utils/db-helpers/listings.ts"; -import { withEnv } from "#test-utils/env.ts"; -import { mockRequest } from "#test-utils/mocks.ts"; -import { - createTestApiKeyFull, - requestAsApiKey, - requestAsSession, - testCookie, - testCsrfToken, -} from "#test-utils/session.ts"; -import { - storedFeatureEnabled, - withFeatureWriteFailure, -} from "#test-utils/settings.ts"; - -describeWithEnv("API Keys", { db: true }, () => { - /** POST `fields` to an `/admin/api-keys…` path with a fresh CSRF token + - * the owner test cookie, returning the response. Collapses the repeated - * `URLSearchParams` + `mockRequest` + form-encoded-headers scaffold that - * every admin POST test in this file spells out. When `fields` already - * includes a `csrf_token` (e.g. extracted from a GET), it's kept as-is. */ - const postApiKeyForm = async ( - path: string, - fields: Record, - ): Promise => { - const cookie = await testCookie(); - const csrfToken = fields.csrf_token ?? (await testCsrfToken()); - const body = new URLSearchParams({ ...fields, csrf_token: csrfToken }); - return handleRequest( - mockRequest(path, { - body: body.toString(), - headers: { - "content-type": "application/x-www-form-urlencoded", - cookie, - }, - method: "POST", - }), - ); - }; - describe("database operations", () => { - test("creates and retrieves an API key", async () => { - const { apiKey, id } = await createTestApiKeyFull(); - - expect(id).toBeGreaterThan(0); - expect(apiKey).toBeTruthy(); - - const found = await getApiKeyByToken(apiKey); - expect(found).not.toBeNull(); - expect(found!.user_id).toBe(1); - expect(found!.id).toBe(id); - }); - - test("unwraps DATA_KEY from API key", async () => { - const { apiKey } = await createTestApiKeyFull(); - - const found = await getApiKeyByToken(apiKey); - const unwrapped = await unwrapKeyWithToken( - found!.wrapped_data_key, - apiKey, - ); - expect(unwrapped).not.toBeNull(); - }); - - test("returns null for unknown token", async () => { - const found = await getApiKeyByToken("nonexistent-token"); - expect(found).toBeNull(); - }); - - test("throws for wrong token unwrap", async () => { - const { apiKey } = await createTestApiKeyFull(); - - const found = await getApiKeyByToken(apiKey); - await expect( - unwrapKeyWithToken(found!.wrapped_data_key, "wrong-token"), - ).rejects.toThrow(); - }); - - test("lists API keys for a user", async () => { - const { dataKey } = await createTestApiKeyFull("Key A"); - await createApiKey(1, "Key B", dataKey, generateSecureToken); - - const keys = await getApiKeysForUser(1); - expect(keys).toHaveLength(2); - expect(keys[0]!.name).toBe("Key A"); - expect(keys[1]!.name).toBe("Key B"); - }); - - test("deletes an API key", async () => { - const { id } = await createTestApiKeyFull(); - - const deleted = await deleteApiKey(id, 1); - expect(deleted).toBe(true); - expect(await getApiKeysForUser(1)).toEqual([]); - }); - - test("delete fails for wrong user", async () => { - const { id } = await createTestApiKeyFull(); - - const deleted = await deleteApiKey(id, 999); - expect(deleted).toBe(false); - expect(await getApiKeysForUser(1)).toHaveLength(1); - }); - - test("touchApiKeyLastUsed stamps last_used on the stored row only", async () => { - const { dataKey, id } = await createTestApiKeyFull("Touch Test"); - const other = await createApiKey( - 1, - "Untouched", - dataKey, - generateSecureToken, - ); - - // Fresh keys start with an empty last_used. - const before = await getApiKeysForUser(1); - expect(before.find((k) => k.id === id)!.lastUsed).toBe(""); - - await touchApiKeyLastUsed(id); - - const after = await getApiKeysForUser(1); - const touched = after.find((k) => k.id === id)!; - // The stored value changed to a parseable timestamp… - expect(touched.lastUsed).not.toBe(""); - expect(Number.isNaN(Date.parse(touched.lastUsed))).toBe(false); - // …and only the touched key's row was updated. - expect(after.find((k) => k.id === other.id)!.lastUsed).toBe(""); - }); - - test("gets a single API key by ID and user", async () => { - const { id } = await createTestApiKeyFull("Lookup Key"); - - const found = await getApiKeyForUser(id, 1); - expect(found).not.toBeNull(); - expect(found!.name).toBe("Lookup Key"); - }); - - test("display reads select only the API key fields they return", async () => { - const { id } = await createTestApiKeyFull("Narrow read"); - - await runWithQueryLogContext(async () => { - enableQueryLog(); - await getApiKeysForUser(1); - await getApiKeyForUser(id, 1); - expect(getQueryLog().map((entry) => entry.sql)).toEqual([ - "SELECT id, name, created, last_used FROM api_keys WHERE user_id = ? ORDER BY id ASC", - "SELECT id, name FROM api_keys WHERE id = ? AND user_id = ? LIMIT ?", - ]); - }); - }); - - test("getApiKeyForUser throws for wrong user", async () => { - const { id } = await createTestApiKeyFull("Wrong User"); - - await expect(getApiKeyForUser(id, 999)).rejects.toThrow(); - }); - - test("lists empty array for user with no keys", async () => { - const keys = await getApiKeysForUser(999); - expect(keys).toHaveLength(0); - }); - }); - - describe("admin UI", () => { - test("serves the name box with its label, hint, bound, and required flag", async () => { - // The expected attributes are written out here so a changed form - // definition fails this test instead of moving the expectation along. - const { apiKeyForm } = await import("#routes/admin/api-keys.ts"); - const html = apiKeyForm.render(); - expect(html).toContain("Name"); - const input = html.slice(html.indexOf('name="name"') - 200); - expect(input).toContain('maxlength="100"'); - expect(input).toContain('placeholder="e.g. CI Pipeline"'); - expect(input).toContain("required"); - }); - - test("a flash that is only a key still shows the copy panel, not a message", async () => { - // The key sits after the newline; a flash that STARTS with the newline - // has an empty message and only the key. The copy panel must still - // appear — the split runs on position 0, which is a found position. - const cookie = await testCookie(); - const response = await handleRequest( - mockRequest(`/admin/api-keys?flash=${FLASH_TEST_ID}`, { - headers: { - cookie: `${cookie}; ${flashCookieHeader("\nBARE-KEY-123")}`, - }, - }), - ); - - const html = await response.text(); - expect(html).toContain("Copy your API key now"); - expect(html).toContain("BARE-KEY-123"); - }); - - test("GET /admin/api-keys shows the page", async () => { - const cookie = await testCookie(); - const response = await handleRequest( - mockRequest("/admin/api-keys", { - headers: { cookie }, - }), - ); - - expect(response.status).toBe(200); - const html = await response.text(); - expect(html).toContain("API Keys"); - expect(html).toContain("Create API key"); - }); - - test("POST /admin/api-keys creates a key and redirects with it", async () => { - const cookie = await testCookie(); - - // GET the page to get CSRF token - const getResponse = await handleRequest( - mockRequest("/admin/api-keys", { headers: { cookie } }), - ); - const pageHtml = await getResponse.text(); - const csrfToken = extractCsrfToken(pageHtml); - - const body = new URLSearchParams({ - csrf_token: csrfToken!, - name: "My Test Key", - }); - const response = await handleRequest( - mockRequest("/admin/api-keys", { - body: body.toString(), - headers: { - "content-type": "application/x-www-form-urlencoded", - cookie, - }, - method: "POST", - }), - ); - - const location = expectRedirect(response); - const locationUrl = new URL(location, "http://localhost"); - locationUrl.searchParams.delete("flash"); - expect(locationUrl.pathname).toBe("/admin/api-keys"); - expectFlash(response, expect.stringContaining("API key created\n")); - - // Follow the redirect and verify the key is shown - const flashCookie = response.headers - .getSetCookie() - .find((c) => c.startsWith("flash_"))!; - const redirectResponse = await handleRequest( - mockRequest(location, { - headers: { cookie: `${cookie}; ${flashCookie.split(";")[0]}` }, - }), - ); - const html = await redirectResponse.text(); - expect(html).toContain("API key created"); - expect(html).toContain("Copy your API key now"); - expect(await storedFeatureEnabled("apiKeys")).toBe(true); - }); - - test("does not create a key when enabling the feature fails", async () => { - await setAdminFeatureEnabled("apiKeys", false); - await withFeatureWriteFailure(async () => { - const response = await postApiKeyForm("/admin/api-keys", { - name: "Unrecoverable key", - }); - expectFlash( - response, - "SQLITE_CONSTRAINT: feature enable failed", - false, - ); - expect(await getApiKeysForUser(1)).toEqual([]); - expect(await storedFeatureEnabled("apiKeys")).toBe(false); - }); - }); - - test("POST /admin/api-keys rejects empty name", async () => { - const response = await postApiKeyForm("/admin/api-keys", { name: "" }); - - // Should redirect with error - expect(response.status).toBe(302); - expectFlash(response, "Name is required", false); - }); - - test("POST /admin/api-keys rejects missing name field", async () => { - const response = await postApiKeyForm("/admin/api-keys", {}); - - expect(response.status).toBe(302); - expectFlash(response, "Name is required", false); - }); - - test("POST /admin/api-keys rejects name over 100 characters", async () => { - const response = await postApiKeyForm("/admin/api-keys", { - name: "x".repeat(101), - }); - - expect(response.status).toBe(302); - expectFlash(response, "Name must be under 100 characters", false); - }); - - test("POST /admin/api-keys/:id/delete returns 404 for nonexistent key", async () => { - const response = await postApiKeyForm("/admin/api-keys/99999/delete", { - confirm_identifier: "anything", - }); - - expect(response.status).toBe(404); - }); - - test("GET /admin/api-keys shows success message without newline (no new key)", async () => { - const cookie = await testCookie(); - const response = await handleRequest( - mockRequest(`/admin/api-keys?flash=${FLASH_TEST_ID}`, { - headers: { cookie: `${cookie}; ${flashCookieHeader("Key updated")}` }, - }), - ); - - const html = await response.text(); - expect(html).toContain("Key updated"); - expect(html).not.toContain("Copy your API key now"); - }); - - test("GET /admin/api-keys shows existing keys with last used date", async () => { - const { id } = await createTestApiKeyFull("Visible Key"); - await touchApiKeyLastUsed(id); - - const cookie = await testCookie(); - const response = await handleRequest( - mockRequest("/admin/api-keys", { headers: { cookie } }), - ); - - const html = await response.text(); - expect(html).toContain("Visible Key"); - expect(html).not.toContain("Never"); - // The name links to the per-key manage page, not an inline delete link. - expect(html).toContain(`href="/admin/api-keys/${id}"`); - }); - - test("GET /admin/api-keys/:id shows the summary and canonical tab links", async () => { - const { id } = await createTestApiKeyFull("Managed Key"); - const cookie = await testCookie(); - - // A never-used key shows the "Never" placeholder for last used. - const first = await handleRequest( - mockRequest(`/admin/api-keys/${id}`, { headers: { cookie } }), - ); - expect(first.status).toBe(200); - const firstHtml = await first.text(); - expect(firstHtml).toContain("Managed Key"); - expect(firstHtml).toContain(`href="/admin/api-keys/${id}"`); - expect(firstHtml).toContain(`href="/admin/api-keys/${id}/actions"`); - expect(firstHtml).not.toContain(`/admin/api-keys/${id}/delete`); - expect(firstHtml).toContain("Never"); - // The manage page lights up the API keys entry in the admin nav. - expect(firstHtml).toContain(''); - - // Once used, the summary renders the formatted last-used date. - await touchApiKeyLastUsed(id); - const second = await handleRequest( - mockRequest(`/admin/api-keys/${id}`, { headers: { cookie } }), - ); - const secondHtml = await second.text(); - expect(secondHtml).not.toContain("Never"); - }); - - test("GET /admin/api-keys/:id/actions shows the delete action", async () => { - const { id } = await createTestApiKeyFull("Managed Key"); - const cookie = await testCookie(); - - const response = await handleRequest( - mockRequest(`/admin/api-keys/${id}/actions`, { headers: { cookie } }), - ); - - expect(response.status).toBe(200); - const html = await response.text(); - expect(html).toContain(`href="/admin/api-keys/${id}/delete"`); - expect(html).toContain(`href="/admin/api-keys/${id}"`); - expect(html).toContain(`href="/admin/api-keys/${id}/actions"`); - }); - - test("GET /admin/api-keys/:id rejects an unknown tab", async () => { - const { id } = await createTestApiKeyFull("Managed Key"); - const cookie = await testCookie(); - - const response = await handleRequest( - mockRequest(`/admin/api-keys/${id}/unknown`, { headers: { cookie } }), - ); - - expect(response.status).toBe(404); - }); - - test("read-only mode keeps the summary but hides the actions tab", async () => { - using _env = withEnv({ - READ_ONLY_FROM: "2020-01-01T00:00:00.000Z", - }); - const { id } = await createTestApiKeyFull("Read-only Key"); - const cookie = await testCookie(); - - const summary = await handleRequest( - mockRequest(`/admin/api-keys/${id}`, { headers: { cookie } }), - ); - const actions = await handleRequest( - mockRequest(`/admin/api-keys/${id}/actions`, { headers: { cookie } }), - ); - - expect(summary.status).toBe(200); - expect(await summary.text()).not.toContain( - `href="/admin/api-keys/${id}/actions"`, - ); - expect(actions.status).toBe(404); - }); - - test("GET /admin/api-keys/:id returns 404 for a nonexistent key", async () => { - const cookie = await testCookie(); - const response = await handleRequest( - mockRequest("/admin/api-keys/99999", { headers: { cookie } }), - ); - - expect(response.status).toBe(404); - }); - - test("GET /admin/api-keys shows success message from flash cookie", async () => { - const cookie = await testCookie(); - const response = await handleRequest( - mockRequest(`/admin/api-keys?flash=${FLASH_TEST_ID}`, { - headers: { cookie: `${cookie}; ${flashCookieHeader("done")}` }, - }), - ); - - const html = await response.text(); - expect(html).toContain("done"); - }); - - test("GET /admin/api-keys shows error message from flash cookie", async () => { - const cookie = await testCookie(); - const response = await handleRequest( - mockRequest(`/admin/api-keys?flash=${FLASH_TEST_ID}`, { - headers: { - cookie: `${cookie}; ${flashCookieHeader("key failed", false)}`, - }, - }), - ); - - const html = await response.text(); - expect(html).toContain("key failed"); - expect(html).toContain("error"); - }); - - // Sits beside the story `@story:servicing.letting-another-system-in`, - // which walks the same journey. This keeps the delete route's own line - // covered: a Cucumber run does not feed the coverage gate. - test("POST /admin/api-keys/:id/delete removes a key with name confirmation", async () => { - const { id } = await createTestApiKeyFull("Doomed Key"); - - const response = await postApiKeyForm(`/admin/api-keys/${id}/delete`, { - confirm_identifier: "Doomed Key", - }); - - await expectFlashRedirect( - "/admin/api-keys", - "API key deleted", - true, - )(response); - expect(await getApiKeysForUser(1)).toEqual([]); - }); - - test("POST /admin/api-keys/:id/delete rejects wrong name", async () => { - const { id } = await createTestApiKeyFull("My Key"); - - const response = await postApiKeyForm(`/admin/api-keys/${id}/delete`, { - confirm_identifier: "Wrong Name", - }); - - // The delete-confirmation page has no error slot of its own; the Layout - // backstop renders the mismatch error, so the operator actually sees it. - await expectFlashRedirect( - `/admin/api-keys/${id}/delete`, - expect.stringContaining("API key name does not match"), - false, - )(response); - expect(await getApiKeysForUser(1)).toHaveLength(1); - }); - - test("GET /admin/api-keys/:id/delete shows confirmation page", async () => { - const { id } = await createTestApiKeyFull("Confirm Key"); - - const cookie = await testCookie(); - const response = await handleRequest( - mockRequest(`/admin/api-keys/${id}/delete`, { - headers: { cookie }, - }), - ); - - expect(response.status).toBe(200); - const html = await response.text(); - expect(html).toContain("Confirm Key"); - expect(html).toContain("confirm_identifier"); - }); - - test("GET /admin/api-keys/:id/delete returns 404 for nonexistent key", async () => { - const cookie = await testCookie(); - const response = await handleRequest( - mockRequest("/admin/api-keys/99999/delete", { - headers: { cookie }, - }), - ); - - expect(response.status).toBe(404); - }); - }); - - describe("Bearer token authentication", () => { - test("authenticates /api/admin/* request with Bearer token", async () => { - await createTestListing({ name: "Bearer Test" }); - - const { apiKey } = await createTestApiKeyFull("Auth Test"); - - await assertJson( - handleRequest(requestAsApiKey("/api/admin/listings", apiKey)), - 200, - (body) => { - expect(body.listings).toBeDefined(); - }, - ); - }); - - test("rejects invalid Bearer token", async () => { - const response = await handleRequest( - requestAsApiKey("/api/admin/listings", "invalid-token"), - ); - - expect(response.status).toBe(401); - }); - - test("locks out Bearer auth after too many failed attempts", async () => { - const { apiKey } = await createTestApiKeyFull("Rate Limited"); - // Saturate the failed-attempt limit for the test's "direct" IP. Once - // locked, even a valid key is rejected until the lockout expires. - for (let i = 0; i < MAX_APIKEY_ATTEMPTS; i++) { - await apiKeyLimiter.record("direct"); - } - const response = await handleRequest( - requestAsApiKey("/api/admin/listings", apiKey), - ); - expect(response.status).toBe(401); - }); - - test("rejects request without auth", async () => { - const response = await handleRequest(mockRequest("/admin/api-keys/docs")); - - expect(response.status).toBe(302); - }); - }); - - describe("admin UI edge cases", () => { - test("GET /admin/api-keys without success or error params shows no messages", async () => { - const cookie = await testCookie(); - const response = await handleRequest( - mockRequest("/admin/api-keys", { headers: { cookie } }), - ); - - const html = await response.text(); - expect(html).not.toContain('class="success"'); - expect(html).not.toContain('class="error"'); - }); - - test("POST /admin/api-keys redirects when session has no wrapped data key", async () => { - // Create a session without wrapped_data_key - const token = generateSecureToken(); - const csrfToken = await signCsrfToken(); - const expires = Date.now() + 86400000; - await createSession(token, csrfToken, expires, null, 1); - const cookie = buildSessionCookie(token); - - const body = new URLSearchParams({ - csrf_token: csrfToken, - name: "No Key Session", - }); - const response = await handleRequest( - mockRequest("/admin/api-keys", { - body: body.toString(), - headers: { - "content-type": "application/x-www-form-urlencoded", - cookie, - }, - method: "POST", - }), - ); - - expect(response.status).toBe(302); - expectFlash(response, "Session key unavailable", false); - }); - - test("GET /admin/api-keys/docs returns HTML docs via cookie auth", async () => { - const cookie = await testCookie(); - const response = await handleRequest( - mockRequest("/admin/api-keys/docs", { headers: { cookie } }), - ); - - expect(response.status).toBe(200); - const html = await response.text(); - expect(html).toContain("API Documentation"); - expect(html).toContain("Bearer"); - }); - }); - - describe("admin JSON API", () => { - test("GET /api/admin/listings returns listings via API key", async () => { - await createTestListing({ name: "Test Listing" }); - - const { apiKey } = await createTestApiKeyFull("Listings API"); - - const body = await assertJson( - handleRequest(requestAsApiKey("/api/admin/listings", apiKey)), - 200, - (body) => { - expect(body.listings).toBeDefined(); - expect(body.listings.length).toBeGreaterThan(0); - expect(body.admin_level).toBe("owner"); - }, - ); - - // Verify snake_case keys and no internal fields - const listing = body.listings[0]; - expect(listing.name).toBe("Test Listing"); - expect(listing.max_attendees).toBeDefined(); - expect(listing.attendee_count).toBeDefined(); - expect(listing.listing_type).toBeDefined(); - expect(listing.slug_index).toBeUndefined(); - }); - - test("GET /api/admin/listings returns listings via cookie+CSRF", async () => { - await createTestListing({ name: "Cookie Listing" }); - - const cookie = await testCookie(); - const csrfToken = await testCsrfToken(); - - await assertJson( - handleRequest( - requestAsSession("/api/admin/listings", { cookie, csrfToken }), - ), - 200, - (body) => { - expect(body.listings).toBeDefined(); - }, - ); - }); - - test("GET /api/admin/listings returns 401 for invalid API key", async () => { - const response = await handleRequest( - requestAsApiKey("/api/admin/listings", "bad-key"), - ); - - expect(response.status).toBe(401); - }); - - test("GET /api/admin/listings returns 401 without auth", async () => { - const response = await handleRequest(mockRequest("/api/admin/listings")); - - expect(response.status).toBe(401); - }); - - test("returns 401 when API key user no longer exists", async () => { - const token = generateSecureToken(); - const keyIndex = await hmacHash(token); - - // Disable FK checks to insert an orphaned API key row - await getDb().execute({ args: [], sql: "PRAGMA foreign_keys = OFF" }); - await getDb().execute( - insert("api_keys", { - created: new Date().toISOString(), - key_index: keyIndex, - last_used: "", - name: "Ghost", - user_id: 9999, - wrapped_data_key: "dummy", - }), - ); - await getDb().execute({ args: [], sql: "PRAGMA foreign_keys = ON" }); - - const response = await handleRequest( - requestAsApiKey("/api/admin/listings", token), - ); - - expect(response.status).toBe(401); - }); - - test("returns 401 when API key wrapped data key is corrupted", async () => { - const { apiKey, id } = await createTestApiKeyFull("Corrupt Key"); - - // Corrupt the wrapped_data_key in the DB - await getDb().execute({ - args: ["corrupted-data", id], - sql: "UPDATE api_keys SET wrapped_data_key = ? WHERE id = ?", - }); - - const response = await handleRequest( - requestAsApiKey("/api/admin/listings", apiKey), - ); - - expect(response.status).toBe(401); - }); - - test("GET /api/admin/listings serves a cookie without a CSRF header", async () => { - await createTestListing({ name: "CSRF Listing" }); - const cookie = await testCookie(); - - // A safe GET carries no body and can't mutate state, so a cookie session - // need not (and a feed/browser client often cannot) send an x-csrf-token - // header to read a JSON endpoint. - await assertJson( - handleRequest( - mockRequest("/api/admin/listings", { - headers: { cookie }, - }), - ), - 200, - (body) => { - expect(body.listings).toBeDefined(); - }, - ); - }); - - test("touchApiKeyLastUsed surfaces the test-override error to its caller", async () => { - // The fire-and-forget swallowing happens at the request layer; the - // function itself must still throw so that layer has something to catch. - const { setTouchOverride } = await import("#shared/test-overrides.ts"); - const { touchApiKeyLastUsed } = await import("#shared/db/api-keys.ts"); - setTouchOverride(new Error("touch failed")); - try { - await expect(touchApiKeyLastUsed(1)).rejects.toThrow("touch failed"); - } finally { - setTouchOverride(null); - } - }); - - test("getApiKeyForUser throws a not-found error for an unknown key", async () => { - const { getApiKeyForUser } = await import("#shared/db/api-keys.ts"); - await expect(getApiKeyForUser(999_999, 1)).rejects.toThrow( - "API key 999999 not found for user 1", - ); - }); - - test("request succeeds when touchApiKeyLastUsed fails (fire-and-forget)", async () => { - await createTestListing({ name: "Touch Test" }); - const { apiKey } = await createTestApiKeyFull("Touch Test Key"); - - // Make touchApiKeyLastUsed throw via test hook - const { setTouchOverride } = await import("#shared/test-overrides.ts"); - setTouchOverride(new Error("touch failed")); - - try { - const response = await handleRequest( - requestAsApiKey("/api/admin/listings", apiKey), - ); - // Request should succeed despite touchApiKeyLastUsed throwing - expect(response.status).toBe(200); - } finally { - setTouchOverride(null); - } - }); - }); -}); diff --git a/test/features/admin/api-keys/auth.test.ts b/test/features/admin/api-keys/auth.test.ts new file mode 100644 index 0000000000..1266c371e7 --- /dev/null +++ b/test/features/admin/api-keys/auth.test.ts @@ -0,0 +1,221 @@ +import { expect } from "@std/expect"; +import { describe, test } from "@std/testing/bdd"; +import { handleRequest } from "#routes"; +import { hmacHash } from "#shared/crypto/hashing.ts"; +import { generateSecureToken } from "#shared/crypto/utils.ts"; +import { apiKeyLimiter } from "#shared/db/api-key-attempts.ts"; +import { getDb, insert } from "#shared/db/client.ts"; +import { MAX_APIKEY_ATTEMPTS } from "#shared/limits.ts"; +import { assertJson } from "#test-utils/assertions.ts"; +import { describeWithEnv } from "#test-utils/db.ts"; +import { createTestListing } from "#test-utils/db-helpers/listings.ts"; +import { mockRequest } from "#test-utils/mocks.ts"; +import { + createTestApiKeyFull, + requestAsApiKey, + requestAsSession, + testCookie, + testCsrfToken, +} from "#test-utils/session.ts"; + +describeWithEnv("API key authentication", { db: true }, () => { + describe("Bearer token authentication", () => { + test("authenticates /api/admin/* request with Bearer token", async () => { + await createTestListing({ name: "Bearer Test" }); + + const { apiKey } = await createTestApiKeyFull("Auth Test"); + + await assertJson( + handleRequest(requestAsApiKey("/api/admin/listings", apiKey)), + 200, + (body) => { + expect(body.listings).toBeDefined(); + }, + ); + }); + + test("rejects invalid Bearer token", async () => { + const response = await handleRequest( + requestAsApiKey("/api/admin/listings", "invalid-token"), + ); + + expect(response.status).toBe(401); + }); + + test("locks out Bearer auth after too many failed attempts", async () => { + const { apiKey } = await createTestApiKeyFull("Rate Limited"); + // Saturate the failed-attempt limit for the test's "direct" IP. Once + // locked, even a valid key is rejected until the lockout expires. + for (let i = 0; i < MAX_APIKEY_ATTEMPTS; i++) { + await apiKeyLimiter.record("direct"); + } + const response = await handleRequest( + requestAsApiKey("/api/admin/listings", apiKey), + ); + expect(response.status).toBe(401); + }); + + test("rejects request without auth", async () => { + const response = await handleRequest(mockRequest("/admin/api-keys/docs")); + + expect(response.status).toBe(302); + }); + }); + + describe("admin JSON API", () => { + test("GET /api/admin/listings returns listings via API key", async () => { + await createTestListing({ name: "Test Listing" }); + + const { apiKey } = await createTestApiKeyFull("Listings API"); + + const body = await assertJson( + handleRequest(requestAsApiKey("/api/admin/listings", apiKey)), + 200, + (body) => { + expect(body.listings).toBeDefined(); + expect(body.listings.length).toBeGreaterThan(0); + expect(body.admin_level).toBe("owner"); + }, + ); + + // Verify snake_case keys and no internal fields + const listing = body.listings[0]; + expect(listing.name).toBe("Test Listing"); + expect(listing.max_attendees).toBeDefined(); + expect(listing.attendee_count).toBeDefined(); + expect(listing.listing_type).toBeDefined(); + expect(listing.slug_index).toBeUndefined(); + }); + + test("GET /api/admin/listings returns listings via cookie+CSRF", async () => { + await createTestListing({ name: "Cookie Listing" }); + + const cookie = await testCookie(); + const csrfToken = await testCsrfToken(); + + await assertJson( + handleRequest( + requestAsSession("/api/admin/listings", { cookie, csrfToken }), + ), + 200, + (body) => { + expect(body.listings).toBeDefined(); + }, + ); + }); + + test("GET /api/admin/listings returns 401 for invalid API key", async () => { + const response = await handleRequest( + requestAsApiKey("/api/admin/listings", "bad-key"), + ); + + expect(response.status).toBe(401); + }); + + test("GET /api/admin/listings returns 401 without auth", async () => { + const response = await handleRequest(mockRequest("/api/admin/listings")); + + expect(response.status).toBe(401); + }); + + test("returns 401 when API key user no longer exists", async () => { + const token = generateSecureToken(); + const keyIndex = await hmacHash(token); + + // Disable FK checks to insert an orphaned API key row + await getDb().execute({ args: [], sql: "PRAGMA foreign_keys = OFF" }); + await getDb().execute( + insert("api_keys", { + created: new Date().toISOString(), + key_index: keyIndex, + last_used: "", + name: "Ghost", + user_id: 9999, + wrapped_data_key: "dummy", + }), + ); + await getDb().execute({ args: [], sql: "PRAGMA foreign_keys = ON" }); + + const response = await handleRequest( + requestAsApiKey("/api/admin/listings", token), + ); + + expect(response.status).toBe(401); + }); + + test("returns 401 when API key wrapped data key is corrupted", async () => { + const { apiKey, id } = await createTestApiKeyFull("Corrupt Key"); + + // Corrupt the wrapped_data_key in the DB + await getDb().execute({ + args: ["corrupted-data", id], + sql: "UPDATE api_keys SET wrapped_data_key = ? WHERE id = ?", + }); + + const response = await handleRequest( + requestAsApiKey("/api/admin/listings", apiKey), + ); + + expect(response.status).toBe(401); + }); + + test("GET /api/admin/listings serves a cookie without a CSRF header", async () => { + await createTestListing({ name: "CSRF Listing" }); + const cookie = await testCookie(); + + // A safe GET carries no body and can't mutate state, so a cookie session + // need not (and a feed/browser client often cannot) send an x-csrf-token + // header to read a JSON endpoint. + await assertJson( + handleRequest( + mockRequest("/api/admin/listings", { + headers: { cookie }, + }), + ), + 200, + (body) => { + expect(body.listings).toBeDefined(); + }, + ); + }); + + test("touchApiKeyLastUsed surfaces the test-override error to its caller", async () => { + // The fire-and-forget swallowing happens at the request layer; the + // function itself must still throw so that layer has something to catch. + const { setTouchOverride } = await import("#shared/test-overrides.ts"); + const { touchApiKeyLastUsed } = await import("#shared/db/api-keys.ts"); + setTouchOverride(new Error("touch failed")); + try { + await expect(touchApiKeyLastUsed(1)).rejects.toThrow("touch failed"); + } finally { + setTouchOverride(null); + } + }); + + test("getApiKeyForUser throws a not-found error for an unknown key", async () => { + const { getApiKeyForUser } = await import("#shared/db/api-keys.ts"); + await expect(getApiKeyForUser(999_999, 1)).rejects.toThrow( + "API key 999999 not found for user 1", + ); + }); + + test("request succeeds when touchApiKeyLastUsed fails (fire-and-forget)", async () => { + await createTestListing({ name: "Touch Test" }); + const { apiKey } = await createTestApiKeyFull("Touch Test Key"); + + // Make touchApiKeyLastUsed throw via test hook + const { setTouchOverride } = await import("#shared/test-overrides.ts"); + setTouchOverride(new Error("touch failed")); + + try { + const response = await handleRequest( + requestAsApiKey("/api/admin/listings", apiKey), + ); + // Request should succeed despite touchApiKeyLastUsed throwing + expect(response.status).toBe(200); + } finally { + setTouchOverride(null); + } + }); + }); +}); diff --git a/test/features/admin/api-keys/manage.test.ts b/test/features/admin/api-keys/manage.test.ts new file mode 100644 index 0000000000..6bb0a47ed9 --- /dev/null +++ b/test/features/admin/api-keys/manage.test.ts @@ -0,0 +1,178 @@ +import { expect } from "@std/expect"; +import { describe, it as test } from "@std/testing/bdd"; +import { handleRequest } from "#routes"; +import { getApiKeysForUser, touchApiKeyLastUsed } from "#shared/db/api-keys.ts"; +import { expectFlashRedirect } from "#test-utils/assertions.ts"; +import { describeWithEnv } from "#test-utils/db.ts"; +import { withEnv } from "#test-utils/env.ts"; +import { mockRequest } from "#test-utils/mocks.ts"; +import { + adminFormPost, + createTestApiKeyFull, + testCookie, +} from "#test-utils/session.ts"; + +describeWithEnv("API key manage page", { db: true }, () => { + describe("per-key pages", () => { + test("GET /admin/api-keys/:id shows the summary and canonical tab links", async () => { + const { id } = await createTestApiKeyFull("Managed Key"); + const cookie = await testCookie(); + + // A never-used key shows the "Never" placeholder for last used. + const first = await handleRequest( + mockRequest(`/admin/api-keys/${id}`, { headers: { cookie } }), + ); + expect(first.status).toBe(200); + const firstHtml = await first.text(); + expect(firstHtml).toContain("Managed Key"); + expect(firstHtml).toContain(`href="/admin/api-keys/${id}"`); + expect(firstHtml).toContain(`href="/admin/api-keys/${id}/actions"`); + expect(firstHtml).not.toContain(`/admin/api-keys/${id}/delete`); + expect(firstHtml).toContain("Never"); + // The manage page lights up the API keys entry in the admin nav. + expect(firstHtml).toContain(''); + + // Once used, the summary renders the formatted last-used date. + await touchApiKeyLastUsed(id); + const second = await handleRequest( + mockRequest(`/admin/api-keys/${id}`, { headers: { cookie } }), + ); + const secondHtml = await second.text(); + expect(secondHtml).not.toContain("Never"); + }); + + test("GET /admin/api-keys/:id/actions shows the delete action", async () => { + const { id } = await createTestApiKeyFull("Managed Key"); + const cookie = await testCookie(); + + const response = await handleRequest( + mockRequest(`/admin/api-keys/${id}/actions`, { headers: { cookie } }), + ); + + expect(response.status).toBe(200); + const html = await response.text(); + expect(html).toContain(`href="/admin/api-keys/${id}/delete"`); + expect(html).toContain(`href="/admin/api-keys/${id}"`); + expect(html).toContain(`href="/admin/api-keys/${id}/actions"`); + }); + + test("GET /admin/api-keys/:id rejects an unknown tab", async () => { + const { id } = await createTestApiKeyFull("Managed Key"); + const cookie = await testCookie(); + + const response = await handleRequest( + mockRequest(`/admin/api-keys/${id}/unknown`, { headers: { cookie } }), + ); + + expect(response.status).toBe(404); + }); + + test("read-only mode keeps the summary but hides the actions tab", async () => { + using _env = withEnv({ + READ_ONLY_FROM: "2020-01-01T00:00:00.000Z", + }); + const { id } = await createTestApiKeyFull("Read-only Key"); + const cookie = await testCookie(); + + const summary = await handleRequest( + mockRequest(`/admin/api-keys/${id}`, { headers: { cookie } }), + ); + const actions = await handleRequest( + mockRequest(`/admin/api-keys/${id}/actions`, { headers: { cookie } }), + ); + + expect(summary.status).toBe(200); + expect(await summary.text()).not.toContain( + `href="/admin/api-keys/${id}/actions"`, + ); + expect(actions.status).toBe(404); + }); + + test("GET /admin/api-keys/:id returns 404 for a nonexistent key", async () => { + const cookie = await testCookie(); + const response = await handleRequest( + mockRequest("/admin/api-keys/99999", { headers: { cookie } }), + ); + + expect(response.status).toBe(404); + }); + }); + + describe("delete flow", () => { + // Sits beside the story `@story:servicing.letting-another-system-in`, + // which walks the same journey. This keeps the delete route's own line + // covered: a Cucumber run does not feed the coverage gate. + test("POST /admin/api-keys/:id/delete removes a key with name confirmation", async () => { + const { id } = await createTestApiKeyFull("Doomed Key"); + + const response = ( + await adminFormPost(`/admin/api-keys/${id}/delete`, { + confirm_identifier: "Doomed Key", + }) + ).response; + + await expectFlashRedirect( + "/admin/api-keys", + "API key deleted", + true, + )(response); + expect(await getApiKeysForUser(1)).toEqual([]); + }); + + test("POST /admin/api-keys/:id/delete rejects wrong name", async () => { + const { id } = await createTestApiKeyFull("My Key"); + + const response = ( + await adminFormPost(`/admin/api-keys/${id}/delete`, { + confirm_identifier: "Wrong Name", + }) + ).response; + + // The delete-confirmation page has no error slot of its own; the Layout + // backstop renders the mismatch error, so the operator actually sees it. + await expectFlashRedirect( + `/admin/api-keys/${id}/delete`, + expect.stringContaining("API key name does not match"), + false, + )(response); + expect(await getApiKeysForUser(1)).toHaveLength(1); + }); + + test("POST /admin/api-keys/:id/delete returns 404 for nonexistent key", async () => { + const response = ( + await adminFormPost("/admin/api-keys/99999/delete", { + confirm_identifier: "anything", + }) + ).response; + + expect(response.status).toBe(404); + }); + + test("GET /admin/api-keys/:id/delete shows confirmation page", async () => { + const { id } = await createTestApiKeyFull("Confirm Key"); + + const cookie = await testCookie(); + const response = await handleRequest( + mockRequest(`/admin/api-keys/${id}/delete`, { + headers: { cookie }, + }), + ); + + expect(response.status).toBe(200); + const html = await response.text(); + expect(html).toContain("Confirm Key"); + expect(html).toContain("confirm_identifier"); + }); + + test("GET /admin/api-keys/:id/delete returns 404 for nonexistent key", async () => { + const cookie = await testCookie(); + const response = await handleRequest( + mockRequest("/admin/api-keys/99999/delete", { + headers: { cookie }, + }), + ); + + expect(response.status).toBe(404); + }); + }); +}); diff --git a/test/features/admin/api-keys/records.test.ts b/test/features/admin/api-keys/records.test.ts new file mode 100644 index 0000000000..44e0875170 --- /dev/null +++ b/test/features/admin/api-keys/records.test.ts @@ -0,0 +1,143 @@ +import { expect } from "@std/expect"; +import { describe, test } from "@std/testing/bdd"; +import { unwrapKeyWithToken } from "#shared/crypto/keys.ts"; +import { generateSecureToken } from "#shared/crypto/utils.ts"; +import { + createApiKey, + deleteApiKey, + getApiKeyByToken, + getApiKeyForUser, + getApiKeysForUser, + touchApiKeyLastUsed, +} from "#shared/db/api-keys.ts"; +import { + enableQueryLog, + getQueryLog, + runWithQueryLogContext, +} from "#shared/db/query-log.ts"; +import { describeWithEnv } from "#test-utils/db.ts"; +import { createTestApiKeyFull } from "#test-utils/session.ts"; + +describeWithEnv("API key records", { db: true }, () => { + describe("database operations", () => { + test("creates and retrieves an API key", async () => { + const { apiKey, id } = await createTestApiKeyFull(); + + expect(id).toBeGreaterThan(0); + expect(apiKey).toBeTruthy(); + + const found = await getApiKeyByToken(apiKey); + expect(found).not.toBeNull(); + expect(found!.user_id).toBe(1); + expect(found!.id).toBe(id); + }); + + test("unwraps DATA_KEY from API key", async () => { + const { apiKey } = await createTestApiKeyFull(); + + const found = await getApiKeyByToken(apiKey); + const unwrapped = await unwrapKeyWithToken( + found!.wrapped_data_key, + apiKey, + ); + expect(unwrapped).not.toBeNull(); + }); + + test("returns null for unknown token", async () => { + const found = await getApiKeyByToken("nonexistent-token"); + expect(found).toBeNull(); + }); + + test("throws for wrong token unwrap", async () => { + const { apiKey } = await createTestApiKeyFull(); + + const found = await getApiKeyByToken(apiKey); + await expect( + unwrapKeyWithToken(found!.wrapped_data_key, "wrong-token"), + ).rejects.toThrow(); + }); + + test("lists API keys for a user", async () => { + const { dataKey } = await createTestApiKeyFull("Key A"); + await createApiKey(1, "Key B", dataKey, generateSecureToken); + + const keys = await getApiKeysForUser(1); + expect(keys).toHaveLength(2); + expect(keys[0]!.name).toBe("Key A"); + expect(keys[1]!.name).toBe("Key B"); + }); + + test("deletes an API key", async () => { + const { id } = await createTestApiKeyFull(); + + const deleted = await deleteApiKey(id, 1); + expect(deleted).toBe(true); + expect(await getApiKeysForUser(1)).toEqual([]); + }); + + test("delete fails for wrong user", async () => { + const { id } = await createTestApiKeyFull(); + + const deleted = await deleteApiKey(id, 999); + expect(deleted).toBe(false); + expect(await getApiKeysForUser(1)).toHaveLength(1); + }); + + test("touchApiKeyLastUsed stamps last_used on the stored row only", async () => { + const { dataKey, id } = await createTestApiKeyFull("Touch Test"); + const other = await createApiKey( + 1, + "Untouched", + dataKey, + generateSecureToken, + ); + + // Fresh keys start with an empty last_used. + const before = await getApiKeysForUser(1); + expect(before.find((k) => k.id === id)!.lastUsed).toBe(""); + + await touchApiKeyLastUsed(id); + + const after = await getApiKeysForUser(1); + const touched = after.find((k) => k.id === id)!; + // The stored value changed to a parseable timestamp… + expect(touched.lastUsed).not.toBe(""); + expect(Number.isNaN(Date.parse(touched.lastUsed))).toBe(false); + // …and only the touched key's row was updated. + expect(after.find((k) => k.id === other.id)!.lastUsed).toBe(""); + }); + + test("gets a single API key by ID and user", async () => { + const { id } = await createTestApiKeyFull("Lookup Key"); + + const found = await getApiKeyForUser(id, 1); + expect(found).not.toBeNull(); + expect(found!.name).toBe("Lookup Key"); + }); + + test("display reads select only the API key fields they return", async () => { + const { id } = await createTestApiKeyFull("Narrow read"); + + await runWithQueryLogContext(async () => { + enableQueryLog(); + await getApiKeysForUser(1); + await getApiKeyForUser(id, 1); + expect(getQueryLog().map((entry) => entry.sql)).toEqual([ + "SELECT id, name, created, last_used FROM api_keys WHERE user_id = ? ORDER BY id ASC", + "SELECT id, name FROM api_keys WHERE id = ? AND user_id = ? LIMIT ?", + ]); + }); + }); + + test("getApiKeyForUser throws for wrong user", async () => { + const { id } = await createTestApiKeyFull("Wrong User"); + + await expect(getApiKeyForUser(id, 999)).rejects.toThrow(); + }); + + test("lists empty array for user with no keys", async () => { + const keys = await getApiKeysForUser(999); + expect(keys).toHaveLength(0); + }); + }); +}); diff --git a/test/features/admin/api-keys/ui.test.ts b/test/features/admin/api-keys/ui.test.ts new file mode 100644 index 0000000000..bf100c6b3c --- /dev/null +++ b/test/features/admin/api-keys/ui.test.ts @@ -0,0 +1,245 @@ +import { expect } from "@std/expect"; +import { describe, it as test } from "@std/testing/bdd"; +import { handleRequest } from "#routes"; +import { buildSessionCookie } from "#shared/cookies.ts"; +import { generateSecureToken } from "#shared/crypto/utils.ts"; +import { signCsrfToken } from "#shared/csrf.ts"; +import { setAdminFeatureEnabled } from "#shared/db/admin-features.ts"; +import { getApiKeysForUser, touchApiKeyLastUsed } from "#shared/db/api-keys.ts"; +import { createSession } from "#shared/db/sessions.ts"; +import { + expectFlash, + expectRedirect, + FLASH_TEST_ID, + flashCookieHeader, +} from "#test-utils/assertions.ts"; +import { extractCsrfToken } from "#test-utils/csrf.ts"; +import { describeWithEnv } from "#test-utils/db.ts"; +import { mockRequest } from "#test-utils/mocks.ts"; +import { + adminFormPost, + createTestApiKeyFull, + testCookie, +} from "#test-utils/session.ts"; +import { + storedFeatureEnabled, + withFeatureWriteFailure, +} from "#test-utils/settings.ts"; + +describeWithEnv("API keys admin UI", { db: true }, () => { + /** GET the API keys page as the owner — optionally with a flash cookie — + * and return its HTML. */ + const getApiKeysHtml = async (flashCookie?: string): Promise => { + const cookie = await testCookie(); + const response = await handleRequest( + mockRequest( + flashCookie + ? `/admin/api-keys?flash=${FLASH_TEST_ID}` + : "/admin/api-keys", + { + headers: { + cookie: flashCookie ? `${cookie}; ${flashCookie}` : cookie, + }, + }, + ), + ); + expect(response.status).toBe(200); + return response.text(); + }; + + describe("the create form and page", () => { + test("serves the name box with its label, hint, bound, and required flag", async () => { + // The expected attributes are written out here so a changed form + // definition fails this test instead of moving the expectation along. + const { apiKeyForm } = await import("#routes/admin/api-keys.ts"); + const html = apiKeyForm.render(); + expect(html).toContain("Name"); + const input = html.slice(html.indexOf('name="name"') - 200); + expect(input).toContain('maxlength="100"'); + expect(input).toContain('placeholder="e.g. CI Pipeline"'); + expect(input).toContain("required"); + }); + + test("GET /admin/api-keys shows the page", async () => { + const html = await getApiKeysHtml(); + expect(html).toContain("API Keys"); + expect(html).toContain("Create API key"); + }); + + test("GET /admin/api-keys shows existing keys with last used date", async () => { + const { id } = await createTestApiKeyFull("Visible Key"); + await touchApiKeyLastUsed(id); + + const html = await getApiKeysHtml(); + expect(html).toContain("Visible Key"); + expect(html).not.toContain("Never"); + // The name links to the per-key manage page, not an inline delete link. + expect(html).toContain(`href="/admin/api-keys/${id}"`); + }); + + test("GET /admin/api-keys without success or error params shows no messages", async () => { + const html = await getApiKeysHtml(); + expect(html).not.toContain('class="success"'); + expect(html).not.toContain('class="error"'); + }); + + test("GET /admin/api-keys/docs returns HTML docs via cookie auth", async () => { + const cookie = await testCookie(); + const response = await handleRequest( + mockRequest("/admin/api-keys/docs", { headers: { cookie } }), + ); + + expect(response.status).toBe(200); + const html = await response.text(); + expect(html).toContain("API Documentation"); + expect(html).toContain("Bearer"); + }); + }); + + describe("flash messages", () => { + test("shows a success message from the flash cookie", async () => { + const html = await getApiKeysHtml(flashCookieHeader("done")); + expect(html).toContain("done"); + }); + + test("shows an error message from the flash cookie", async () => { + const html = await getApiKeysHtml(flashCookieHeader("key failed", false)); + expect(html).toContain("key failed"); + expect(html).toContain("error"); + }); + + test("shows a success message without a newline (no new key)", async () => { + const html = await getApiKeysHtml(flashCookieHeader("Key updated")); + expect(html).toContain("Key updated"); + expect(html).not.toContain("Copy your API key now"); + }); + + test("a flash that is only a key still shows the copy panel, not a message", async () => { + // The key sits after the newline; a flash that STARTS with the newline + // has an empty message and only the key. The copy panel must still + // appear — the split runs on position 0, which is a found position. + const html = await getApiKeysHtml(flashCookieHeader("\nBARE-KEY-123")); + expect(html).toContain("Copy your API key now"); + expect(html).toContain("BARE-KEY-123"); + }); + }); + + describe("creating keys", () => { + test("POST /admin/api-keys creates a key and redirects with it", async () => { + const cookie = await testCookie(); + + // GET the page to get CSRF token + const getResponse = await handleRequest( + mockRequest("/admin/api-keys", { headers: { cookie } }), + ); + const pageHtml = await getResponse.text(); + const csrfToken = extractCsrfToken(pageHtml); + + const body = new URLSearchParams({ + csrf_token: csrfToken!, + name: "My Test Key", + }); + const response = await handleRequest( + mockRequest("/admin/api-keys", { + body: body.toString(), + headers: { + "content-type": "application/x-www-form-urlencoded", + cookie, + }, + method: "POST", + }), + ); + + const location = expectRedirect(response); + const locationUrl = new URL(location, "http://localhost"); + locationUrl.searchParams.delete("flash"); + expect(locationUrl.pathname).toBe("/admin/api-keys"); + expectFlash(response, expect.stringContaining("API key created\n")); + + // Follow the redirect and verify the key is shown + const flashCookie = response.headers + .getSetCookie() + .find((c) => c.startsWith("flash_"))!; + const redirectResponse = await handleRequest( + mockRequest(location, { + headers: { cookie: `${cookie}; ${flashCookie.split(";")[0]}` }, + }), + ); + const html = await redirectResponse.text(); + expect(html).toContain("API key created"); + expect(html).toContain("Copy your API key now"); + expect(await storedFeatureEnabled("apiKeys")).toBe(true); + }); + + test("does not create a key when enabling the feature fails", async () => { + await setAdminFeatureEnabled("apiKeys", false); + await withFeatureWriteFailure(async () => { + const response = ( + await adminFormPost("/admin/api-keys", { + name: "Unrecoverable key", + }) + ).response; + expectFlash( + response, + "SQLITE_CONSTRAINT: feature enable failed", + false, + ); + expect(await getApiKeysForUser(1)).toEqual([]); + expect(await storedFeatureEnabled("apiKeys")).toBe(false); + }); + }); + + test("POST /admin/api-keys rejects empty name", async () => { + const response = (await adminFormPost("/admin/api-keys", { name: "" })) + .response; + + expect(response.status).toBe(302); + expectFlash(response, "Name is required", false); + }); + + test("POST /admin/api-keys rejects missing name field", async () => { + const response = (await adminFormPost("/admin/api-keys", {})).response; + + expect(response.status).toBe(302); + expectFlash(response, "Name is required", false); + }); + + test("POST /admin/api-keys rejects name over 100 characters", async () => { + const response = ( + await adminFormPost("/admin/api-keys", { + name: "x".repeat(101), + }) + ).response; + + expect(response.status).toBe(302); + expectFlash(response, "Name must be under 100 characters", false); + }); + + test("POST /admin/api-keys redirects when session has no wrapped data key", async () => { + // Create a session without wrapped_data_key + const token = generateSecureToken(); + const csrfToken = await signCsrfToken(); + const expires = Date.now() + 86400000; + await createSession(token, csrfToken, expires, null, 1); + const cookie = buildSessionCookie(token); + + const body = new URLSearchParams({ + csrf_token: csrfToken, + name: "No Key Session", + }); + const response = await handleRequest( + mockRequest("/admin/api-keys", { + body: body.toString(), + headers: { + "content-type": "application/x-www-form-urlencoded", + cookie, + }, + method: "POST", + }), + ); + + expect(response.status).toBe(302); + expectFlash(response, "Session key unavailable", false); + }); + }); +}); diff --git a/test/features/admin/attributes/crud.test.ts b/test/features/admin/attributes/crud.test.ts index e868d17bd2..44ce655d27 100644 --- a/test/features/admin/attributes/crud.test.ts +++ b/test/features/admin/attributes/crud.test.ts @@ -4,252 +4,259 @@ import { attributeNameForm, attributeOptionForm, } from "#routes/admin/attributes.ts"; -import { getAllActivityLog } from "#test-utils/activity-log.ts"; +import { setAdminFeatureEnabled } from "#shared/db/admin-features.ts"; +import { + getAllAttributesWithOptions, + getAttributeWithOptions, +} from "#shared/db/attributes.ts"; +import { execute } from "#shared/db/client.ts"; +import { activityMessages } from "#test-utils/activity-log.ts"; import { expectFlashRedirect, expectHtmlResponse, expectRedirect, expectStatus, inputNamed, + testRequiresAuth, } from "#test-utils/assertions.ts"; import { describeWithEnv } from "#test-utils/db.ts"; -import { createTestAttributeWithOptions } from "#test-utils/db-helpers/attributes.ts"; +import { + createAttributeViaRoute, + createTestAttributeWithOptions, +} from "#test-utils/db-helpers/attributes.ts"; import { adminFormPost, adminGet } from "#test-utils/session.ts"; +import { + enableFeature, + storedFeatureEnabled, + withFeatureWriteFailure, +} from "#test-utils/settings.ts"; -/** Create an attribute through the real POST and return its new id. */ -const createAttribute = async (name: string): Promise => { - const { response } = await adminFormPost("/admin/attributes", { name }); - const location = expectRedirect(response, /^\/admin\/attributes\/\d+/); - return Number(new URL(location, "http://localhost").pathname.split("/")[3]); -}; - -describeWithEnv( - "server (admin attribute create and edit)", - { db: true }, - () => { - describe("the two attribute forms", () => { - // The expected label and hint are written out here so a changed form - // definition fails this test instead of moving the expectation with it. - test("serves the name box with its label and hint", () => { - const html = attributeNameForm.render(); - expect(html).toContain("Attribute name"); - expect(inputNamed(html, "name")).toContain( - 'placeholder="e.g. Difficulty"', - ); - }); +describeWithEnv("server (admin attribute CRUD)", { db: true }, () => { + describe("the two attribute forms", () => { + // The expected label and hint are written out here so a changed form + // definition fails this test instead of moving the expectation with it. + test("serves the name box with its label and hint", () => { + const html = attributeNameForm.render(); + expect(html).toContain("Attribute name"); + expect(inputNamed(html, "name")).toContain( + 'placeholder="e.g. Difficulty"', + ); + }); - test("serves the option box with its label and hint", () => { - const html = attributeOptionForm.render(); - expect(html).toContain("Option text"); - expect(inputNamed(html, "text")).toContain( - 'placeholder="e.g. Beginner"', - ); - }); + test("serves the option box with its label and hint", () => { + const html = attributeOptionForm.render(); + expect(html).toContain("Option text"); + expect(inputNamed(html, "text")).toContain('placeholder="e.g. Beginner"'); }); + }); - describe("POST /admin/attributes", () => { - test("creating an attribute stores it, logs it, and says so", async () => { - const { response } = await adminFormPost("/admin/attributes", { - name: "Terrain", - }); - - const location = expectRedirect(response, /^\/admin\/attributes\/\d+/); - await expectFlashRedirect( - new URL(location, "http://localhost").pathname, - "Attribute created", - true, - )(response); - await expectHtmlResponse(await adminGet(location), 200, "Terrain"); - const messages = (await getAllActivityLog()).map( - (entry) => entry.message, - ); - expect(messages).toContain("Attribute 'Terrain' created"); - }); + describe("POST /admin/attributes", () => { + testRequiresAuth("/admin/attributes", { + body: { name: "Auth attribute" }, + method: "POST", + }); - test("a new attribute joins the order, so it can be moved", async () => { - await createAttribute("Alpha"); - const betaId = await createAttribute("Beta"); - - const before = await expectHtmlResponse( - await adminGet("/admin/attributes"), - 200, - ); - expect(before.indexOf(">Alpha<")).toBeGreaterThanOrEqual(0); - expect(before.indexOf(">Alpha<")).toBeLessThan( - before.indexOf(">Beta<"), - ); - - await adminFormPost(`/admin/attributes/${betaId}/move-up`, {}); - - const after = await expectHtmlResponse( - await adminGet("/admin/attributes"), - 200, - ); - expect(after.indexOf(">Beta<")).toBeGreaterThanOrEqual(0); - expect(after.indexOf(">Beta<")).toBeLessThan(after.indexOf(">Alpha<")); + test("creating an attribute stores it, logs it, and says so", async () => { + const { response } = await adminFormPost("/admin/attributes", { + name: "Terrain", }); - test("an empty name is refused back to the attributes page", async () => { - const { response } = await adminFormPost("/admin/attributes", { - name: "", - }); + const location = expectRedirect(response, /^\/admin\/attributes\/\d+/); + await expectFlashRedirect( + new URL(location, "http://localhost").pathname, + "Attribute created", + true, + )(response); + await expectHtmlResponse( + await adminGet(location), + 200, + "Terrain", + "No options yet.", + ); + expect(await storedFeatureEnabled("attributes")).toBe(true); + expect(await activityMessages()).toContain("Attribute 'Terrain' created"); + }); - await expectFlashRedirect( - "/admin/attributes", - "Attribute name is required", - false, - )(response); - }); + test("a new attribute joins the order, so it can be moved", async () => { + const firstId = await createAttributeViaRoute("Alpha"); + const secondId = await createAttributeViaRoute("Beta"); + + // Moving the top attribute up is harmless. + await expectFlashRedirect( + "/admin/attributes", + "Attribute moved", + )((await adminFormPost(`/admin/attributes/${firstId}/move-up`)).response); + expect( + (await getAllAttributesWithOptions()).map((item) => item.name), + ).toEqual(["Alpha", "Beta"]); + + await adminFormPost(`/admin/attributes/${secondId}/move-up`); + expect( + (await getAllAttributesWithOptions()).map((item) => item.name), + ).toEqual(["Beta", "Alpha"]); }); - describe("POST /admin/attributes/:id/options", () => { - test("adding an option stores it, logs it, and says so", async () => { - const attribute = await createTestAttributeWithOptions( - "Difficulty", - [], - ); - - const { response } = await adminFormPost( - `/admin/attributes/${attribute.id}/options`, - { text: "Easy" }, - ); - - await expectFlashRedirect( - `/admin/attributes/${attribute.id}`, - "Option added", - true, - )(response); - await expectHtmlResponse( - await adminGet(`/admin/attributes/${attribute.id}`), - 200, - "Easy", - ); - const messages = (await getAllActivityLog()).map( - (entry) => entry.message, - ); - expect(messages).toContain( - "Attribute option 'Easy' added to Difficulty", - ); + test("an empty name is refused back to the attributes page", async () => { + await setAdminFeatureEnabled("attributes", false); + const { response } = await adminFormPost("/admin/attributes", { + name: "", }); + + await expectFlashRedirect( + "/admin/attributes", + "Attribute name is required", + false, + )(response); + expect(await storedFeatureEnabled("attributes")).toBe(false); }); - describe("POST /admin/attributes/:id/delete", () => { - test("deleting needs the exact attribute name", async () => { - const attribute = await createTestAttributeWithOptions("Doomed", [ - "Only", - ]); - - const { response } = await adminFormPost( - `/admin/attributes/${attribute.id}/delete`, - { confirm_identifier: "Wrong" }, - ); - - await expectFlashRedirect( - `/admin/attributes/${attribute.id}/delete`, - expect.stringContaining("Attribute name does not match"), - false, - )(response); - }); + test("a failed order write leaves no half-made attribute behind", async () => { + // Fail the append's sort_order write: the insert and the append run in + // one transaction, so the attribute must roll back with it. + await execute(` + CREATE TRIGGER fail_attribute_order + BEFORE UPDATE OF sort_order ON attributes + BEGIN + SELECT RAISE(ABORT, 'order write failed'); + END + `); + try { + await expect( + adminFormPost("/admin/attributes", { name: "Ghost" }), + ).rejects.toThrow("order write failed"); + } finally { + await execute("DROP TRIGGER IF EXISTS fail_attribute_order"); + } + expect(await getAllAttributesWithOptions()).toEqual([]); + }); - test("deleting removes the attribute, logs it, and says so", async () => { - const attribute = await createTestAttributeWithOptions("Doomed", [ - "Only", - ]); - - const { response } = await adminFormPost( - `/admin/attributes/${attribute.id}/delete`, - { confirm_identifier: "Doomed" }, - ); - - await expectFlashRedirect( - "/admin/attributes", - "Attribute deleted", - true, - )(response); - expectStatus(404)(await adminGet(`/admin/attributes/${attribute.id}`)); - const messages = (await getAllActivityLog()).map( - (entry) => entry.message, - ); - expect(messages).toContain("Attribute 'Doomed' deleted"); - }); + test("does not create an attribute when enabling the feature fails", async () => { + await enableFeature("attributes"); + await expect( + withFeatureWriteFailure(async () => { + await adminFormPost("/admin/attributes", { name: "Hidden" }); + }), + ).rejects.toThrow("feature enable failed"); + expect(await getAllAttributesWithOptions()).toEqual([]); }); + }); - describe("POST /admin/attributes/:id/options/:optionId/delete", () => { - test("deleting an option needs the exact option text", async () => { - const attribute = await createTestAttributeWithOptions("Difficulty", [ - "Easy", - ]); - const easy = attribute.options[0]!; - - const { response } = await adminFormPost( - `/admin/attributes/${attribute.id}/options/${easy.id}/delete`, - { confirm_identifier: "Nope" }, - ); - - await expectFlashRedirect( - `/admin/attributes/${attribute.id}/options/${easy.id}/delete`, - "Option text does not match. Please type the exact option text to confirm deletion.", - false, - )(response); - }); + describe("POST /admin/attributes/:id/edit", () => { + test("editing saves the new name, logs it, and says so", async () => { + const attribute = await createTestAttributeWithOptions("Old name", [ + "Only", + ]); - test("deleting an option removes it, logs it, and says so", async () => { - const attribute = await createTestAttributeWithOptions("Difficulty", [ - "Easy", - ]); - const easy = attribute.options[0]!; - - const { response } = await adminFormPost( - `/admin/attributes/${attribute.id}/options/${easy.id}/delete`, - { confirm_identifier: "Easy" }, - ); - - await expectFlashRedirect( - `/admin/attributes/${attribute.id}`, - "Option deleted", - true, - )(response); - const html = await expectHtmlResponse( - await adminGet(`/admin/attributes/${attribute.id}`), - 200, - ); - expect(html).not.toContain(">Easy<"); - const messages = (await getAllActivityLog()).map( - (entry) => entry.message, - ); - expect(messages).toContain( - "Attribute option 'Easy' deleted from Difficulty", - ); - }); + const { response } = await adminFormPost( + `/admin/attributes/${attribute.id}/edit`, + { name: "New name" }, + ); + + await expectFlashRedirect( + `/admin/attributes/${attribute.id}`, + "Attribute updated", + true, + )(response); + expect((await getAttributeWithOptions(attribute.id))?.name).toBe( + "New name", + ); + expect(await activityMessages()).toContain( + "Attribute 'New name' updated", + ); }); - describe("POST /admin/attributes/:id/edit", () => { - test("editing saves the new name, logs it, and says so", async () => { - const attribute = await createTestAttributeWithOptions("Old name", [ - "Only", - ]); - - const { response } = await adminFormPost( - `/admin/attributes/${attribute.id}/edit`, - { name: "New name" }, - ); - - await expectFlashRedirect( - `/admin/attributes/${attribute.id}`, - "Attribute updated", - true, - )(response); - const html = await expectHtmlResponse( - await adminGet(`/admin/attributes/${attribute.id}`), - 200, - "New name", - ); - expect(html).not.toContain("Old name"); - const messages = (await getAllActivityLog()).map( - (entry) => entry.message, - ); - expect(messages).toContain("Attribute 'New name' updated"); - }); + test("an empty name is refused back to the attribute page", async () => { + const id = await createAttributeViaRoute("Required fields"); + + await expectFlashRedirect( + `/admin/attributes/${id}`, + "Attribute name is required", + false, + )((await adminFormPost(`/admin/attributes/${id}/edit`)).response); + }); + + test("does not update an attribute when enabling the feature fails", async () => { + const attribute = await createTestAttributeWithOptions("Before", []); + await enableFeature("attributes"); + await expect( + withFeatureWriteFailure(async () => { + await adminFormPost(`/admin/attributes/${attribute.id}/edit`, { + name: "After", + }); + }), + ).rejects.toThrow("feature enable failed"); + expect((await getAttributeWithOptions(attribute.id))?.name).toBe( + "Before", + ); + }); + + test("returns 404 when changing a missing attribute", async () => { + expectStatus(404)( + ( + await adminFormPost("/admin/attributes/999999/edit", { + name: "Missing", + }) + ).response, + ); + expectStatus(404)( + ( + await adminFormPost("/admin/attributes/999999/options", { + text: "Missing", + }) + ).response, + ); + expectStatus(404)( + (await adminFormPost("/admin/attributes/999999/move-up")).response, + ); + }); + }); + + describe("POST /admin/attributes/:id/delete", () => { + test("deleting needs the exact attribute name", async () => { + const attribute = await createTestAttributeWithOptions("Doomed", [ + "Only", + ]); + + const { response } = await adminFormPost( + `/admin/attributes/${attribute.id}/delete`, + { confirm_identifier: "Wrong" }, + ); + + await expectFlashRedirect( + `/admin/attributes/${attribute.id}/delete`, + expect.stringContaining("Attribute name does not match"), + false, + )(response); + expect(await getAttributeWithOptions(attribute.id)).not.toBeNull(); + }); + + test("deleting removes the attribute, logs it, and says so", async () => { + const attribute = await createTestAttributeWithOptions("Audience", [ + "Adults", + ]); + + await expectHtmlResponse( + await adminGet(`/admin/attributes/${attribute.id}/delete`), + 200, + "Delete Attribute", + "Audience", + ); + + const { response } = await adminFormPost( + `/admin/attributes/${attribute.id}/delete`, + { confirm_identifier: "Audience" }, + ); + + await expectFlashRedirect( + "/admin/attributes", + "Attribute deleted", + true, + )(response); + expect(await getAttributeWithOptions(attribute.id)).toBeNull(); + expectStatus(404)(await adminGet(`/admin/attributes/${attribute.id}`)); + expect(await activityMessages()).toContain( + "Attribute 'Audience' deleted", + ); }); - }, -); + }); +}); diff --git a/test/features/admin/attributes/options.test.ts b/test/features/admin/attributes/options.test.ts new file mode 100644 index 0000000000..3e99fba252 --- /dev/null +++ b/test/features/admin/attributes/options.test.ts @@ -0,0 +1,215 @@ +import { expect } from "@std/expect"; +import { describe, it as test } from "@std/testing/bdd"; +import { getAttributeWithOptions } from "#shared/db/attributes.ts"; +import { execute } from "#shared/db/client.ts"; +import { activityMessages } from "#test-utils/activity-log.ts"; +import { + expectFlashRedirect, + expectHtmlResponse, + expectStatus, +} from "#test-utils/assertions.ts"; +import { describeWithEnv } from "#test-utils/db.ts"; +import { + createAttributeViaRoute, + createTestAttributeWithOptions, +} from "#test-utils/db-helpers/attributes.ts"; +import { adminFormPost, adminGet } from "#test-utils/session.ts"; + +/** Ask the delete route to remove Difficulty's only option, "Easy". */ +const postEasyOptionDelete = async ( + confirmIdentifier: string, +): Promise<{ attributeId: number; optionId: number; response: Response }> => { + const attribute = await createTestAttributeWithOptions("Difficulty", [ + "Easy", + ]); + const optionId = attribute.options[0]!.id; + const { response } = await adminFormPost( + `/admin/attributes/${attribute.id}/options/${optionId}/delete`, + { confirm_identifier: confirmIdentifier }, + ); + return { attributeId: attribute.id, optionId, response }; +}; + +describeWithEnv("server (admin attribute options)", { db: true }, () => { + describe("POST /admin/attributes/:id/options", () => { + test("adding an option stores it, logs it, and says so", async () => { + const attribute = await createTestAttributeWithOptions("Difficulty", []); + + const { response } = await adminFormPost( + `/admin/attributes/${attribute.id}/options`, + { text: "Easy" }, + ); + + await expectFlashRedirect( + `/admin/attributes/${attribute.id}`, + "Option added", + true, + )(response); + await expectHtmlResponse( + await adminGet(`/admin/attributes/${attribute.id}`), + 200, + "Easy", + ); + expect(await activityMessages()).toContain( + "Attribute option 'Easy' added to Difficulty", + ); + }); + + test("a failed order write leaves no half-made option behind", async () => { + // Fail the append's sort_order write: the insert and the append run in + // one transaction, so the option must roll back with it instead of + // surviving with its placeholder order. + const attribute = await createTestAttributeWithOptions("Fragile", []); + await execute(` + CREATE TRIGGER fail_option_order + BEFORE UPDATE OF sort_order ON attribute_options + BEGIN + SELECT RAISE(ABORT, 'order write failed'); + END + `); + try { + await expect( + adminFormPost(`/admin/attributes/${attribute.id}/options`, { + text: "Ghost", + }), + ).rejects.toThrow("order write failed"); + } finally { + await execute("DROP TRIGGER IF EXISTS fail_option_order"); + } + expect((await getAttributeWithOptions(attribute.id))?.options).toEqual( + [], + ); + }); + + test("an empty option is refused back to the attribute page", async () => { + const id = await createAttributeViaRoute("Fields"); + + await expectFlashRedirect( + `/admin/attributes/${id}`, + "Option text is required", + false, + )((await adminFormPost(`/admin/attributes/${id}/options`)).response); + }); + }); + + describe("POST /admin/attributes/:id/options/:optionId/edit and moves", () => { + test("adds, edits, and reorders options", async () => { + const id = await createAttributeViaRoute("Format"); + await adminFormPost(`/admin/attributes/${id}/options`, { + text: "Online", + }); + await adminFormPost(`/admin/attributes/${id}/options`, { + text: "In person", + }); + const before = (await getAttributeWithOptions(id))!; + const second = before.options[1]!; + + const edited = await adminFormPost( + `/admin/attributes/${id}/options/${second.id}/edit`, + { text: "In-person" }, + ); + await expectFlashRedirect( + `/admin/attributes/${id}`, + "Option updated", + )(edited.response); + await adminFormPost( + `/admin/attributes/${id}/options/${second.id}/move-up`, + ); + + const after = (await getAttributeWithOptions(id))!; + expect(after.options.map((option) => option.text)).toEqual([ + "In-person", + "Online", + ]); + + // Moving it back down restores the original order. + await adminFormPost( + `/admin/attributes/${id}/options/${second.id}/move-down`, + ); + const restored = (await getAttributeWithOptions(id))!; + expect(restored.options.map((option) => option.text)).toEqual([ + "Online", + "In-person", + ]); + }); + + test("returns 404 or redirects when changing a missing or invalid option", async () => { + const attribute = await createTestAttributeWithOptions("Missing option", [ + "Only", + ]); + const option = attribute.options[0]!; + + expectStatus(404)( + await adminGet( + `/admin/attributes/${attribute.id}/options/999999/delete`, + ), + ); + expectStatus(404)( + ( + await adminFormPost( + `/admin/attributes/${attribute.id}/options/999999/edit`, + { text: "Missing" }, + ) + ).response, + ); + await expectFlashRedirect( + `/admin/attributes/${attribute.id}/options/${option.id}/edit`, + "Option text is required", + false, + )( + ( + await adminFormPost( + `/admin/attributes/${attribute.id}/options/${option.id}/edit`, + ) + ).response, + ); + }); + }); + + describe("POST /admin/attributes/:id/options/:optionId/delete", () => { + test("deleting an option needs the exact option text", async () => { + const { attributeId, optionId, response } = + await postEasyOptionDelete("Nope"); + + await expectFlashRedirect( + `/admin/attributes/${attributeId}/options/${optionId}/delete`, + "Option text does not match. Please type the exact option text to confirm deletion.", + false, + )(response); + expect( + (await getAttributeWithOptions(attributeId))?.options, + ).toHaveLength(1); + }); + + test("deleting an option removes it, logs it, and says so", async () => { + const { attributeId, response } = await postEasyOptionDelete("Easy"); + + await expectFlashRedirect( + `/admin/attributes/${attributeId}`, + "Option deleted", + true, + )(response); + expect((await getAttributeWithOptions(attributeId))?.options).toEqual([]); + expect(await activityMessages()).toContain( + "Attribute option 'Easy' deleted from Difficulty", + ); + }); + + test("shows the confirmation page before deleting", async () => { + const attribute = await createTestAttributeWithOptions("Season", [ + "Spring", + ]); + + await expectHtmlResponse( + await adminGet( + `/admin/attributes/${attribute.id}/options/${ + attribute.options[0]!.id + }/delete`, + ), + 200, + "Delete Option", + "Spring", + ); + }); + }); +}); diff --git a/test/integration/server/attributes.test.ts b/test/integration/server/attributes.test.ts index e941923f8d..a593e4c213 100644 --- a/test/integration/server/attributes.test.ts +++ b/test/integration/server/attributes.test.ts @@ -2,13 +2,8 @@ import { expect } from "@std/expect"; import { beforeEach, describe, it as test } from "@std/testing/bdd"; import { handleRequest } from "#routes"; import { setAdminFeatureEnabled } from "#shared/db/admin-features.ts"; +import { listingAttributeOptions } from "#shared/db/attributes.ts"; import { - getAllAttributesWithOptions, - getAttributeWithOptions, - listingAttributeOptions, -} from "#shared/db/attributes.ts"; -import { - expectFlash, expectFlashRedirect, expectHtmlResponse, expectStatus, @@ -23,16 +18,8 @@ import { createTestListing, duplicateTestListing, } from "#test-utils/db-helpers/listings.ts"; -import { - adminFormPost, - adminGet, - getTestSession, -} from "#test-utils/session.ts"; -import { - enableFeature, - storedFeatureEnabled, - withFeatureWriteFailure, -} from "#test-utils/settings.ts"; +import { adminGet, getTestSession } from "#test-utils/session.ts"; +import { enableFeature } from "#test-utils/settings.ts"; const postRepeatedOptions = async ( path: string, @@ -54,17 +41,6 @@ const postRepeatedOptions = async ( ); }; -const createAttributeViaRoute = async (name: string): Promise => { - const { response } = await adminFormPost("/admin/attributes", { name }); - expect(response.status).toBe(302); - expectFlash(response, "Attribute created"); - const attribute = (await getAllAttributesWithOptions()).find( - (item) => item.name === name, - ); - expect(attribute).toBeTruthy(); - return attribute!.id; -}; - describeWithEnv("server (admin attributes)", { db: true }, () => { describe("GET /admin/attributes", () => { testRequiresAuth("/admin/attributes"); @@ -91,263 +67,6 @@ describeWithEnv("server (admin attributes)", { db: true }, () => { }); }); - describe("attribute CRUD", () => { - testRequiresAuth("/admin/attributes", { - body: { name: "Auth attribute" }, - method: "POST", - }); - - test("creates an attribute and redirects to its detail page", async () => { - const id = await createAttributeViaRoute("Difficulty"); - expect(await storedFeatureEnabled("attributes")).toBe(true); - - await expectHtmlResponse( - await adminGet(`/admin/attributes/${id}`), - 200, - "Difficulty", - "No options yet.", - ); - }); - - test("does not enable Attributes for an invalid create", async () => { - await setAdminFeatureEnabled("attributes", false); - const { response } = await adminFormPost("/admin/attributes"); - response.body?.cancel(); - expect(await storedFeatureEnabled("attributes")).toBe(false); - }); - - test("does not create an attribute when enabling the feature fails", async () => { - await enableFeature("attributes"); - await expect( - withFeatureWriteFailure(async () => { - await adminFormPost("/admin/attributes", { name: "Hidden" }); - }), - ).rejects.toThrow("feature enable failed"); - expect(await getAllAttributesWithOptions()).toEqual([]); - }); - - test("redirects invalid attribute forms back to the right page", async () => { - const id = await createAttributeViaRoute("Required fields"); - - await expectFlashRedirect( - "/admin/attributes", - expect.any(String), - false, - )((await adminFormPost("/admin/attributes")).response); - await expectFlashRedirect( - `/admin/attributes/${id}`, - expect.any(String), - false, - )((await adminFormPost(`/admin/attributes/${id}/edit`)).response); - await expectFlashRedirect( - `/admin/attributes/${id}`, - expect.any(String), - false, - )((await adminFormPost(`/admin/attributes/${id}/options`)).response); - }); - - test("updates an attribute name", async () => { - const id = await createAttributeViaRoute("Old name"); - - const { response } = await adminFormPost(`/admin/attributes/${id}/edit`, { - name: "New name", - }); - - await expectFlashRedirect( - `/admin/attributes/${id}`, - "Attribute updated", - )(response); - expect((await getAttributeWithOptions(id))?.name).toBe("New name"); - }); - - test("does not update an attribute when enabling the feature fails", async () => { - const attribute = await createTestAttributeWithOptions("Before", []); - await enableFeature("attributes"); - await expect( - withFeatureWriteFailure(async () => { - await adminFormPost(`/admin/attributes/${attribute.id}/edit`, { - name: "After", - }); - }), - ).rejects.toThrow("feature enable failed"); - expect((await getAttributeWithOptions(attribute.id))?.name).toBe( - "Before", - ); - }); - - test("returns 404 when changing a missing attribute", async () => { - expectStatus(404)( - ( - await adminFormPost("/admin/attributes/999999/edit", { - name: "Missing", - }) - ).response, - ); - expectStatus(404)( - ( - await adminFormPost("/admin/attributes/999999/options", { - text: "Missing", - }) - ).response, - ); - expectStatus(404)( - (await adminFormPost("/admin/attributes/999999/move-up")).response, - ); - }); - - test("adds, edits, and reorders options", async () => { - const id = await createAttributeViaRoute("Format"); - await adminFormPost(`/admin/attributes/${id}/options`, { - text: "Online", - }); - await adminFormPost(`/admin/attributes/${id}/options`, { - text: "In person", - }); - const before = (await getAttributeWithOptions(id))!; - const second = before.options[1]!; - - const edited = await adminFormPost( - `/admin/attributes/${id}/options/${second.id}/edit`, - { text: "In-person" }, - ); - await expectFlashRedirect( - `/admin/attributes/${id}`, - "Option updated", - )(edited.response); - await adminFormPost( - `/admin/attributes/${id}/options/${second.id}/move-up`, - ); - - const after = (await getAttributeWithOptions(id))!; - expect(after.options.map((option) => option.text)).toEqual([ - "In-person", - "Online", - ]); - - // Moving it back down restores the original order. - await adminFormPost( - `/admin/attributes/${id}/options/${second.id}/move-down`, - ); - const restored = (await getAttributeWithOptions(id))!; - expect(restored.options.map((option) => option.text)).toEqual([ - "Online", - "In-person", - ]); - }); - - test("returns 404 or redirects when changing a missing or invalid option", async () => { - const attribute = await createTestAttributeWithOptions("Missing option", [ - "Only", - ]); - const option = attribute.options[0]!; - - expectStatus(404)( - await adminGet( - `/admin/attributes/${attribute.id}/options/999999/delete`, - ), - ); - expectStatus(404)( - ( - await adminFormPost( - `/admin/attributes/${attribute.id}/options/999999/edit`, - { text: "Missing" }, - ) - ).response, - ); - await expectFlashRedirect( - `/admin/attributes/${attribute.id}/options/${option.id}/edit`, - expect.any(String), - false, - )( - ( - await adminFormPost( - `/admin/attributes/${attribute.id}/options/${option.id}/edit`, - ) - ).response, - ); - }); - - test("reorders attributes and keeps edge moves harmless", async () => { - const first = await createTestAttributeWithOptions("First", []); - const second = await createTestAttributeWithOptions("Second", []); - - await expectFlashRedirect( - "/admin/attributes", - "Attribute moved", - )( - (await adminFormPost(`/admin/attributes/${first.id}/move-up`)).response, - ); - expect( - (await getAllAttributesWithOptions()).map((item) => item.name), - ).toEqual(["First", "Second"]); - - await expectFlashRedirect( - "/admin/attributes", - "Attribute moved", - )( - (await adminFormPost(`/admin/attributes/${second.id}/move-up`)) - .response, - ); - expect( - (await getAllAttributesWithOptions()).map((item) => item.name), - ).toEqual(["Second", "First"]); - }); - - test("deletes an option after confirmation", async () => { - const attribute = await createTestAttributeWithOptions("Season", [ - "Spring", - ]); - - await expectHtmlResponse( - await adminGet( - `/admin/attributes/${attribute.id}/options/${ - attribute.options[0]!.id - }/delete`, - ), - 200, - "Delete Option", - "Spring", - ); - - const { response } = await adminFormPost( - `/admin/attributes/${attribute.id}/options/${ - attribute.options[0]!.id - }/delete`, - { confirm_identifier: "Spring" }, - ); - await expectFlashRedirect( - `/admin/attributes/${attribute.id}`, - "Option deleted", - )(response); - expect((await getAttributeWithOptions(attribute.id))?.options).toEqual( - [], - ); - }); - - test("deletes an attribute after confirmation", async () => { - const attribute = await createTestAttributeWithOptions("Audience", [ - "Adults", - ]); - - await expectHtmlResponse( - await adminGet(`/admin/attributes/${attribute.id}/delete`), - 200, - "Delete Attribute", - "Audience", - ); - - const { response } = await adminFormPost( - `/admin/attributes/${attribute.id}/delete`, - { confirm_identifier: "Audience" }, - ); - await expectFlashRedirect( - "/admin/attributes", - "Attribute deleted", - )(response); - expect(await getAttributeWithOptions(attribute.id)).toBeNull(); - }); - }); - describe("listing attributes tab", () => { beforeEach(() => enableFeature("attributes")); diff --git a/test/test-utils/activity-log.ts b/test/test-utils/activity-log.ts index 77ee4c41dd..634b450cb2 100644 --- a/test/test-utils/activity-log.ts +++ b/test/test-utils/activity-log.ts @@ -80,3 +80,7 @@ export const getListingWithActivityLogOrNull = ( /** True when the activity log holds an entry whose message equals `message`. */ export const wasActivityLogged = async (message: string): Promise => (await getAllActivityLog()).some((entry) => entry.message === message); + +/** The decrypted messages currently in the activity log, for "was it logged" asserts. */ +export const activityMessages = async (): Promise => + (await getAllActivityLog()).map((entry) => entry.message); diff --git a/test/test-utils/db-helpers/attributes.ts b/test/test-utils/db-helpers/attributes.ts index 92cdcc1c5a..a4b4ea460c 100644 --- a/test/test-utils/db-helpers/attributes.ts +++ b/test/test-utils/db-helpers/attributes.ts @@ -47,3 +47,14 @@ export const assignTestAttributeOptions = ( listingId, options.map((option) => option.id), ); + +/** Create an attribute through the real admin POST and return its new id. */ +export const createAttributeViaRoute = async ( + name: string, +): Promise => { + const { adminFormPost } = await import("#test-utils/session.ts"); + const { expectRedirect } = await import("#test-utils/assertions.ts"); + const { response } = await adminFormPost("/admin/attributes", { name }); + const location = expectRedirect(response, /^\/admin\/attributes\/\d+/); + return Number(new URL(location, "http://localhost").pathname.split("/")[3]); +}; From 707a48e6c27c781c4aa9bdb953d4df37932c3f3d Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 14:08:52 +0000 Subject: [PATCH 03/47] Kill the join, i18n, rebrand, and remaining api-keys survivors The join form's labels, hints, bounds, and autocomplete are now pinned by a render test. The i18n loading layer gets direct tests for its guards and bookkeeping: a non-string message and a key claimed by two groups both refuse to load, a group loads once even when ensured twice or concurrently, a failed load can be tried again, and withMessageGroups loads copy before the work runs. The rebrand scanner gets three more stories: the closing bracket of a split tag is markup, and copy after a plural adopts the first branch's exit state. Four provably-equivalent scanner mutants are recorded (unread residual tagIsCode writes, and the inert "<" scan start). The api-keys page pins its input name, the exact flash shape of a created key, and which flash slot renders a key-only message. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7 --- scripts/mutation/equivalent-mutants.txt | 12 +++ test/features/admin/api-keys/ui.test.ts | 17 +++- test/features/join.test.ts | 19 ++++ test/shared/i18n.test.ts | 115 ++++++++++++++++++++++++ test/shared/rebrand.test.ts | 32 +++++++ 5 files changed, 193 insertions(+), 2 deletions(-) diff --git a/scripts/mutation/equivalent-mutants.txt b/scripts/mutation/equivalent-mutants.txt index 7eefd2bb09..4dfb93594f 100644 --- a/scripts/mutation/equivalent-mutants.txt +++ b/scripts/mutation/equivalent-mutants.txt @@ -995,3 +995,15 @@ src/features/admin/listing-page-data.ts:282:61 ?? → || # Map.get returns Li # resolve. src/shared/db/news-posts.ts:148:47 news_post → "" # dropping the alias leaves the column names unqualified, and the read selects from one table (news_posts AS news_post) plus scalar subqueries that carry their own qualified id, so every name still resolves to the same column src/features/admin/api-keys.ts:147:75 ?? → || # entity-page load: find() returns a key row object or undefined — a row object is always truthy, so `?? null` and `|| null` agree on every input + +# rebrand scanner — four provably-equivalent survivors. +# tagIsCode is read in exactly one place: stepTag's close, reachable only while +# inTag is true — and every assignment of inTag=true (stepProse's unfinished +# tag) also assigns tagIsCode. So a stale tagIsCode value can never be read. +src/shared/rebrand.ts:66:14 false → true # freshScan's tagIsCode: inTag starts false, so stepTag cannot run before stepProse has overwritten tagIsCode — the initial value is never read +src/shared/rebrand.ts:138:26 false → true # stepTag's tagIsCode reset after closing: inTag is false from here, and the next inTag=true assignment rewrites tagIsCode first — the residual value is never read +# scanTagEnd treats "<" as an inert character (it is neither a quote nor ">"), +# so starting the scan at the "<" itself instead of one past it visits one +# extra no-op character and returns the same index with the same quote state. +src/shared/rebrand.ts:152:36 + → * # `open + 1` → `open * 1` = open: the scan starts on the inert "<" and behaves identically +src/shared/rebrand.ts:152:39 1 → 0 # `open + 0` = open: same inert-"<" start, identical result diff --git a/test/features/admin/api-keys/ui.test.ts b/test/features/admin/api-keys/ui.test.ts index bf100c6b3c..f189e7f098 100644 --- a/test/features/admin/api-keys/ui.test.ts +++ b/test/features/admin/api-keys/ui.test.ts @@ -53,6 +53,7 @@ describeWithEnv("API keys admin UI", { db: true }, () => { // definition fails this test instead of moving the expectation along. const { apiKeyForm } = await import("#routes/admin/api-keys.ts"); const html = apiKeyForm.render(); + expect(html).toContain('name="name"'); expect(html).toContain("Name"); const input = html.slice(html.indexOf('name="name"') - 200); expect(input).toContain('maxlength="100"'); @@ -120,7 +121,14 @@ describeWithEnv("API keys admin UI", { db: true }, () => { // appear — the split runs on position 0, which is a found position. const html = await getApiKeysHtml(flashCookieHeader("\nBARE-KEY-123")); expect(html).toContain("Copy your API key now"); - expect(html).toContain("BARE-KEY-123"); + // The key sits in its code block, exactly, with no stray newline. + expect(html).toContain("BARE-KEY-123"); + // The page's own message is empty, so the only flash on the page is + // the layout backstop's — which sits first inside
. A non-empty + // page message would render inside the page body instead. + expect(html).toContain( + '
{ const locationUrl = new URL(location, "http://localhost"); locationUrl.searchParams.delete("flash"); expect(locationUrl.pathname).toBe("/admin/api-keys"); - expectFlash(response, expect.stringContaining("API key created\n")); + // The whole flash is the fixed sentence plus the bare key — a corrupted + // key (e.g. an "undefined" prefix) fails the exact shape. + expectFlash( + response, + expect.stringMatching(/^API key created\n[A-Za-z0-9_-]+$/), + ); // Follow the redirect and verify the key is shown const flashCookie = response.headers diff --git a/test/features/join.test.ts b/test/features/join.test.ts index 424570357b..25a5a53739 100644 --- a/test/features/join.test.ts +++ b/test/features/join.test.ts @@ -23,6 +23,25 @@ import { } from "#test-utils/mocks.ts"; describeWithEnv("server (multi-user admin)", { db: true }, () => { + describe("the join form", () => { + // The expected labels, hints, and bounds are written out here so a + // changed form definition fails this test instead of moving along. + test("serves both password boxes with their labels, hints, and bounds", async () => { + const { joinForm } = await import("#routes/join.ts"); + const html = joinForm.render(); + expect(html).toContain("Password"); + expect(html).toContain("Confirm Password"); + expect(html).toContain("Minimum 8 characters"); + const password = html.slice(0, html.indexOf('name="password_confirm"')); + expect(password).toContain('minlength="8"'); + expect(password).toContain('autocomplete="new-password"'); + expect(password).toContain("required"); + const confirm = html.slice(html.indexOf('name="password_confirm"') - 300); + expect(confirm).toContain('autocomplete="new-password"'); + expect(confirm).toContain("required"); + }); + }); + describe("login flow", () => { test("login with username and password", async () => { const response = await handleRequest( diff --git a/test/shared/i18n.test.ts b/test/shared/i18n.test.ts index 5bafea0530..2e84839744 100644 --- a/test/shared/i18n.test.ts +++ b/test/shared/i18n.test.ts @@ -2,13 +2,19 @@ import { expect } from "@std/expect"; import { describe, it as test } from "@std/testing/bdd"; import { IntlMessageFormat } from "intl-messageformat"; import { + ensureMessageGroups, getLocale, getRegisteredLocales, parseAcceptLanguage, resetI18nForTest, runWithLocale, t, + withMessageGroups, } from "#i18n"; +import { + ENGLISH_MESSAGE_LOADERS, + type MessageLoader, +} from "#locales/manifest.ts"; import { withEnv } from "#test-utils/env.ts"; import { allEnglishMessages } from "#test-utils/i18n.ts"; @@ -278,4 +284,113 @@ describe("i18n", () => { expect(parseAcceptLanguage("xx-YY", REG)).toBe("en"); }); }); + + describe("message group loading", () => { + const GROUP = "seed-data" as const; + const KEY = "admin.seeds.title"; + + /** Swap the group's loader for the test, then restore it and the state. */ + const withLoader = async ( + loader: MessageLoader, + fn: () => Promise, + ): Promise => { + const original = ENGLISH_MESSAGE_LOADERS[GROUP]; + ENGLISH_MESSAGE_LOADERS[GROUP] = loader; + resetI18nForTest(true); + try { + await fn(); + } finally { + ENGLISH_MESSAGE_LOADERS[GROUP] = original; + resetI18nForTest(true); + } + }; + + test("refuses a message that is not a string", async () => { + const broken = { bad: 5 } as unknown as Readonly>; + await withLoader( + () => Promise.resolve(broken), + async () => { + await expect(ensureMessageGroups([GROUP])).rejects.toThrow( + 'message "bad" is not a string', + ); + }, + ); + }); + + test("refuses a key that two groups both claim", async () => { + // The system group already owns this key; a second group offering it + // must fail loudly instead of silently overwriting the copy. + await withLoader( + () => Promise.resolve({ "common.demo_mode_notice": "clash" }), + async () => { + await expect(ensureMessageGroups([GROUP])).rejects.toThrow( + "belongs to both", + ); + }, + ); + }); + + test("asks the loader once, even when ensured twice", async () => { + let calls = 0; + await withLoader( + () => { + calls += 1; + return Promise.resolve({ [KEY]: "Example data" }); + }, + async () => { + await ensureMessageGroups([GROUP]); + await ensureMessageGroups([GROUP]); + expect(calls).toBe(1); + }, + ); + }); + + test("asks the loader once for two overlapping requests", async () => { + let calls = 0; + let release = (): void => {}; + const gate = new Promise((resolve) => { + release = resolve; + }); + await withLoader( + async () => { + calls += 1; + await gate; + return { [KEY]: "Example data" }; + }, + async () => { + const first = ensureMessageGroups([GROUP]); + const second = ensureMessageGroups([GROUP]); + release(); + await Promise.all([first, second]); + expect(calls).toBe(1); + }, + ); + }); + + test("a failed load can be tried again", async () => { + let calls = 0; + await withLoader( + () => { + calls += 1; + return calls === 1 + ? Promise.reject(new Error("load failed")) + : Promise.resolve({ [KEY]: "Example data" }); + }, + async () => { + await expect(ensureMessageGroups([GROUP])).rejects.toThrow( + "load failed", + ); + await ensureMessageGroups([GROUP]); + expect(calls).toBe(2); + }, + ); + }); + + test("withMessageGroups loads the group before running the work", async () => { + resetI18nForTest(true); + const message = await withMessageGroups([GROUP], () => t(KEY)); + expect(message).toBe("Example data"); + resetI18nForTest(true); + }); + }); }); diff --git a/test/shared/rebrand.test.ts b/test/shared/rebrand.test.ts index 1bc922b00a..33282be0e6 100644 --- a/test/shared/rebrand.test.ts +++ b/test/shared/rebrand.test.ts @@ -248,4 +248,36 @@ describe("buildReplacer", () => { "ls -a lists all events", ); }); + + test("the closing bracket of a split tag is markup, never copy", () => { + // With a replacement term of ">" the bracket would be rewritten if it + // ever passed through the prose path; it must stay part of the tag. + const rebranded = buildReplacer(">|]")('Text bold'); + expect(format(rebranded, { n: 1 })).toBe('Text bold'); + }); + + test("copy after a plural whose branches close a span uses the first branch's exit", () => { + // Every branch closes the span, so the copy after the plural is + // prose again and must rebrand. + const rebranded = buildReplacer("listing|event")( + "{kind, select, ls {ls} other {dir}} lists listings", + ); + expect(format(rebranded, { kind: "ls" })).toBe( + "ls lists events", + ); + expect(format(rebranded, { kind: "x" })).toBe( + "dir lists events", + ); + }); + + test("branches that disagree resolve to the first branch's exit state", () => { + // Branch "a" opens a code span it never closes; "other" opens nothing. + // The first branch's exit carries forward, so the trailing copy stays + // protected whichever branch the value picks. + const rebranded = buildReplacer("listing|event")( + "{kind, select, a {x} other {y}} listings", + ); + expect(format(rebranded, { kind: "a" })).toBe("x listings"); + expect(format(rebranded, { kind: "other" })).toBe("y listings"); + }); }); From 1b2d2a1bc73862fa45928087b16300ab112c9a7f Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 14:14:59 +0000 Subject: [PATCH 04/47] Kill the attributes tab, option edit/move, and database-reset survivors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The listing attributes tab save gets its own direct suite: the chosen options are stored, the activity line and flash say so, and the route 404s while the feature is off. The factory now hands readIds the configured field name, so option_ids has one source of truth instead of a dead config field beside a hard-coded copy. Option edits assert their logged message and moves their flash. The database-reset form pins its label and autocomplete, and a wrong phrase goes back to the reset page itself. The transactional option insert's placeholder sortOrder is recorded as provably equivalent — the same transaction always overwrites or removes it. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7 --- scripts/mutation/equivalent-mutants.txt | 1 + src/features/admin/attributes.ts | 4 +- src/features/admin/listing-choice-post.ts | 7 ++- .../admin/attributes/listing-tab.test.ts | 53 +++++++++++++++++++ .../features/admin/attributes/options.test.ts | 10 +++- test/features/admin/database-reset.test.ts | 4 ++ 6 files changed, 74 insertions(+), 5 deletions(-) create mode 100644 test/features/admin/attributes/listing-tab.test.ts diff --git a/scripts/mutation/equivalent-mutants.txt b/scripts/mutation/equivalent-mutants.txt index 4dfb93594f..6612da7ed0 100644 --- a/scripts/mutation/equivalent-mutants.txt +++ b/scripts/mutation/equivalent-mutants.txt @@ -1007,3 +1007,4 @@ src/shared/rebrand.ts:138:26 false → true # stepTag's tagIsCode reset after # extra no-op character and returns the same index with the same quote state. src/shared/rebrand.ts:152:36 + → * # `open + 1` → `open * 1` = open: the scan starts on the inert "<" and behaves identically src/shared/rebrand.ts:152:39 1 → 0 # `open + 0` = open: same inert-"<" start, identical result +src/features/admin/attributes.ts:210:55 0 → 1 # option insert's sortOrder is a placeholder: the same transaction's append overwrites it before commit, and a failed append rolls the row back — no committed state can carry the inserted value diff --git a/src/features/admin/attributes.ts b/src/features/admin/attributes.ts index ae892997de..103ca94f40 100644 --- a/src/features/admin/attributes.ts +++ b/src/features/admin/attributes.ts @@ -363,10 +363,10 @@ const handleListingAttributesPost = createListingChoicePost({ fieldName: "option_ids", label: "Attributes", noun: "option", - readIds: async (form) => + readIds: async (form, fieldName) => pruneInvalidAttributeOptionIds( await getAllAttributeOptionIds(), - form.getNumberArray("option_ids"), + form.getNumberArray(fieldName), ), saveIds: listingAttributeOptions.setIds, tab: "attributes", diff --git a/src/features/admin/listing-choice-post.ts b/src/features/admin/listing-choice-post.ts index 0e8e759434..27159e8ad5 100644 --- a/src/features/admin/listing-choice-post.ts +++ b/src/features/admin/listing-choice-post.ts @@ -12,7 +12,10 @@ type ListingChoicePostConfig = { fieldName: string; label: string; noun: string; - readIds?: (form: FormParams) => number[] | Promise; + readIds?: ( + form: FormParams, + fieldName: string, + ) => number[] | Promise; saveIds: (listingId: number, ids: number[]) => Promise; tab: string; }; @@ -35,7 +38,7 @@ export const createListingChoicePost = ({ async (listing, _session, form, _request, { id }) => { if (!settings.features[feature]) return notFoundResponse(); const ids = readIds - ? await readIds(form) + ? await readIds(form, fieldName) : form.getNumberArray(fieldName); await saveIds(id, ids); await logActivity( diff --git a/test/features/admin/attributes/listing-tab.test.ts b/test/features/admin/attributes/listing-tab.test.ts new file mode 100644 index 0000000000..4a0c64d6c0 --- /dev/null +++ b/test/features/admin/attributes/listing-tab.test.ts @@ -0,0 +1,53 @@ +import { expect } from "@std/expect"; +import { describe, it as test } from "@std/testing/bdd"; +import { getSelectedAttributesForListings } from "#shared/db/attributes.ts"; +import { activityMessages } from "#test-utils/activity-log.ts"; +import { expectFlashRedirect, expectStatus } from "#test-utils/assertions.ts"; +import { describeWithEnv } from "#test-utils/db.ts"; +import { createTestAttributeWithOptions } from "#test-utils/db-helpers/attributes.ts"; +import { createTestListing } from "#test-utils/db-helpers/listings.ts"; +import { adminFormPost } from "#test-utils/session.ts"; +import { enableFeature } from "#test-utils/settings.ts"; + +describeWithEnv("server (listing attributes tab save)", { db: true }, () => { + describe("POST /admin/listing/:id/attributes", () => { + test("saving a listing's options stores them, logs them, and says so", async () => { + const attribute = await createTestAttributeWithOptions("Difficulty", [ + "Easy", + "Hard", + ]); + await enableFeature("attributes"); + const listing = await createTestListing({ name: "Climbing" }); + const easy = attribute.options[0]!; + + const { response } = await adminFormPost( + `/admin/listing/${listing.id}/attributes`, + { option_ids: String(easy.id) }, + ); + + await expectFlashRedirect( + `/admin/listing/${listing.id}/attributes`, + "Attributes updated", + true, + )(response); + const selected = await getSelectedAttributesForListings([listing.id]); + expect( + selected.get(listing.id)![0]!.options.map((option) => option.text), + ).toEqual(["Easy"]); + expect(await activityMessages()).toContain( + "Attributes updated for 'Climbing' (1 option)", + ); + }); + + test("the save is refused while the attributes feature is off", async () => { + const listing = await createTestListing({ name: "No feature" }); + + const { response } = await adminFormPost( + `/admin/listing/${listing.id}/attributes`, + {}, + ); + + expectStatus(404)(response); + }); + }); +}); diff --git a/test/features/admin/attributes/options.test.ts b/test/features/admin/attributes/options.test.ts index 3e99fba252..05558360aa 100644 --- a/test/features/admin/attributes/options.test.ts +++ b/test/features/admin/attributes/options.test.ts @@ -112,9 +112,17 @@ describeWithEnv("server (admin attribute options)", { db: true }, () => { `/admin/attributes/${id}`, "Option updated", )(edited.response); - await adminFormPost( + expect(await activityMessages()).toContain( + "Attribute option 'In-person' updated in Format", + ); + const moved = await adminFormPost( `/admin/attributes/${id}/options/${second.id}/move-up`, ); + await expectFlashRedirect( + `/admin/attributes/${id}`, + "Option moved", + true, + )(moved.response); const after = (await getAttributeWithOptions(id))!; expect(after.options.map((option) => option.text)).toEqual([ diff --git a/test/features/admin/database-reset.test.ts b/test/features/admin/database-reset.test.ts index 0a008d5888..911489a5db 100644 --- a/test/features/admin/database-reset.test.ts +++ b/test/features/admin/database-reset.test.ts @@ -57,6 +57,8 @@ describeWithEnv("server (demo reset)", { db: true }, () => { "Reset Database", "confirm_phrase", RESET_DATABASE_PHRASE, + "Confirmation phrase", + 'autocomplete="off"', ); }); @@ -138,7 +140,9 @@ describeWithEnv("server (demo reset)", { db: true }, () => { const response = await submitDemoResetForm({ confirm_phrase: "wrong phrase", }); + // Back to the reset page itself, with the mismatch error. expect(response.status).toBe(302); + expect(response.headers.get("location")).toContain("/demo/reset"); expectFlash( response, expect.stringContaining(RESET_PHRASE_MISMATCH_ERROR), From b57c9b463760ced4faf97c3244ef24c0c3377a8f Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 14:16:02 +0000 Subject: [PATCH 05/47] Pin the site forms' labels, hints, and markdown affordance The home and contact form definitions were unasserted: the boxes now pin their names, labels, hints, autocomplete, placeholder, and the markdown preview wiring. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7 --- test/features/admin/site.test.ts | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test/features/admin/site.test.ts b/test/features/admin/site.test.ts index e70320b1eb..a0d4d5615d 100644 --- a/test/features/admin/site.test.ts +++ b/test/features/admin/site.test.ts @@ -26,6 +26,33 @@ const expectRedirectContaining = (response: Response, text: string) => { }; describeWithEnv("server (admin site)", { db: true }, () => { + describe("the site forms", () => { + // The expected labels, hints, and names are written out here so a changed + // form definition fails this test instead of moving the expectation along. + test("serves the home form boxes with their labels and hints", async () => { + const { siteHomeForm } = await import("#routes/admin/site.ts"); + const html = siteHomeForm.render(); + expect(html).toContain('name="website_title"'); + expect(html).toContain("Website Title"); + expect(html).toContain('autocomplete="off"'); + expect(html).toContain( + "Displayed as the main heading on all public pages (max 128 characters).", + ); + expect(html).toContain('name="homepage_text"'); + expect(html).toContain("Homepage Text"); + expect(html).toContain('placeholder="Welcome to our site..."'); + // markdown: true wires the preview affordance to the textarea. + expect(html).toContain("data-markdown-preview"); + }); + + test("serves the contact form box with its label", async () => { + const { siteContactForm } = await import("#routes/admin/site.ts"); + const html = siteContactForm.render(); + expect(html).toContain('name="contact_page_text"'); + expect(html).toContain("Contact Page Text"); + }); + }); + describe("GET /admin/site", () => { testRequiresAuth("/admin/site"); From 21ea8a851a0df09e9a294f1cd0ceef12d346c741 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 14:42:51 +0000 Subject: [PATCH 06/47] Answer the review, unify the ordered inserts, kill the last survivors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit's nine points, all done: the record-layer tests moved into records with static imports, the manage-page test split with statuses asserted, the wrong-user error pinned exactly, form assertions scoped to the extracted control (inputNamed now matches textareas and selects too), the rollback boilerplate folded into withFailingOrderTrigger, the attribute mismatch message asserted in full, the reset redirect compared by pathname, and the fixture helper's imports made static. The three insert-and-order flows (attribute, option, answer) now share one mechanism: insertScopedOrderedRow in ordered-collection.ts rides writeRowInTransaction, so a failed order write or log line rolls the insert back — the question and answer creates were still non-atomic. The stale question ignore-list records (the deleted form id) are gone, and new tests pin the question and answer forms, the display-as choices, the delete flow's exact messages, and the ledger entry form's required flags and zero floor. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7 --- scripts/mutation/equivalent-mutants.txt | 2 - src/features/admin/attributes.ts | 62 ++++++-------- src/features/admin/questions.ts | 50 +++++++---- src/shared/db/ordered-collection.ts | 37 ++++++++ test/features/admin/api-keys/auth.test.ts | 20 ----- test/features/admin/api-keys/manage.test.ts | 43 ++++++---- test/features/admin/api-keys/records.test.ts | 22 ++++- test/features/admin/api-keys/ui.test.ts | 4 +- test/features/admin/attributes/crud.test.ts | 17 +--- .../features/admin/attributes/options.test.ts | 15 +--- test/features/admin/database-reset.test.ts | 4 +- test/features/admin/questions/delete.test.ts | 85 +++++++++++++++++++ test/features/admin/site.test.ts | 12 +-- test/features/join.test.ts | 5 +- test/test-utils/assertions.ts | 11 ++- test/test-utils/db-helpers/attributes.ts | 24 +++++- .../templates/admin/ledger/entry-form.test.ts | 13 +++ 17 files changed, 292 insertions(+), 134 deletions(-) create mode 100644 test/features/admin/questions/delete.test.ts diff --git a/scripts/mutation/equivalent-mutants.txt b/scripts/mutation/equivalent-mutants.txt index 6612da7ed0..9dda49712e 100644 --- a/scripts/mutation/equivalent-mutants.txt +++ b/scripts/mutation/equivalent-mutants.txt @@ -443,8 +443,6 @@ src/features/admin/group-page-data.ts:194:49 ?? → || # dayPrices.get(): Day # questions.ts form `id`s feed only CSRF/flash-form coordination, never the # rendered CsrfForm id, which is "new-question". -src/features/admin/questions.ts:133:7 questionText → "" # defineForm config.id: only feeds CSRF/flash-form coordination; the page's CsrfForm uses id="new-question", so this value is never rendered observably -src/features/admin/questions.ts:146:7 answerText → "" # answerTextForm config.id: same as questionText — unobservable form-coordination id # buildAssetPathsModule embed-script gate `pathConst === "EMBED_JS_PATH"`: # pathConst is the string 4th element of an AssetDef tuple compared against a diff --git a/src/features/admin/attributes.ts b/src/features/admin/attributes.ts index 103ca94f40..adda77368e 100644 --- a/src/features/admin/attributes.ts +++ b/src/features/admin/attributes.ts @@ -44,12 +44,12 @@ import { listingAttributeOptions, pruneInvalidAttributeOptionIds, } from "#shared/db/attributes.ts"; -import { type TxScope, withTransaction } from "#shared/db/client.ts"; +import { type TxScope, writeRowInTransaction } from "#shared/db/client.ts"; import { flatCollectionSwap, + insertScopedOrderedRow, scopedCollectionSwap, } from "#shared/db/ordered-collection.ts"; -import { writeTableRow } from "#shared/db/table.ts"; import { getFlash } from "#shared/flash-context.ts"; import { defineTextForm } from "#shared/forms/definition.ts"; import { @@ -96,22 +96,21 @@ const handleAttributesPost = createAuthedFormRoute({ onValid: async ({ values: { name } }) => { // One transaction: an attribute must never exist without its order entry // or its log line. - const attribute = await withTransaction(async (transaction) => { - const created = await writeTableRow(transaction, attributesTable, { - input: { name }, - kind: "insert", - }); - await attributesOrder.append({ key: created.id, transaction }); - await logActivity( - `Attribute '${name}' created`, - undefined, - undefined, - transaction, - ); - return created; - }); + const attributeId = await writeRowInTransaction( + await attributesTable.insertStatement!({ name }), + null, + async (transaction, id) => { + await attributesOrder.append({ key: id, transaction }); + await logActivity( + `Attribute '${name}' created`, + undefined, + undefined, + transaction, + ); + }, + ); return redirect( - `/admin/attributes/${attribute.id}`, + `/admin/attributes/${attributeId}`, "Attribute created", true, ); @@ -193,6 +192,12 @@ const handleAttributeEdit = createAuthedFormRoute< }), }); +/** Insert an option and place it in its attribute's order, atomically. */ +const addOptionRow = insertScopedOrderedRow( + attributeOptionsTable, + attributeOptionsOrder, +); + const handleAddOption = createAuthedFormRoute< { text: string }, AttributeParams @@ -205,23 +210,12 @@ const handleAddOption = createAuthedFormRoute< // One transaction: an option must never exist without its place in the // order or its log line. The inserted sortOrder is a placeholder the // append overwrites before anything can read it. - await withTransaction(async (transaction) => { - const option = await writeTableRow(transaction, attributeOptionsTable, { - input: { attributeId: params.id, sortOrder: 0, text }, - kind: "insert", - }); - await attributeOptionsOrder.append({ - key: option.id, - scope: params.id, - transaction, - }); - await logAttributeOptionActivity( - text, - "added to", - attribute, - transaction, - ); - }); + await addOptionRow( + params.id, + { attributeId: params.id, sortOrder: 0, text }, + (transaction) => + logAttributeOptionActivity(text, "added to", attribute, transaction), + ); return redirect(`/admin/attributes/${params.id}`, "Option added", true); }), }); diff --git a/src/features/admin/questions.ts b/src/features/admin/questions.ts index a435574ca4..17e5911e5a 100644 --- a/src/features/admin/questions.ts +++ b/src/features/admin/questions.ts @@ -43,6 +43,7 @@ import { getAllListings } from "#shared/db/listings/records.ts"; import { getAllModifiers } from "#shared/db/modifiers.ts"; import { flatCollectionSwap, + insertScopedOrderedRow, scopedCollectionSwap, } from "#shared/db/ordered-collection.ts"; import { @@ -174,17 +175,22 @@ const handleQuestionsPost = createAuthedFormRoute({ onInvalid: ({ error }) => errorRedirect("/admin/questions", error), onValid: async ({ values: { display_type, text } }) => { const displayType = requireQuestionDisplayType(display_type); - const question = await questionsTable.insert({ - displayType, - text, - }); - await questionsOrder.append({ key: question.id }); - await logActivity(`Question '${text}' created`); - return redirect( - `/admin/questions/${question.id}`, - "Question created", - true, + // One transaction: a question must never exist without its order entry + // or its log line. + const questionId = await writeRowInTransaction( + await questionsTable.insertStatement!({ displayType, text }), + null, + async (transaction, id) => { + await questionsOrder.append({ key: id, transaction }); + await logActivity( + `Question '${text}' created`, + undefined, + undefined, + transaction, + ); + }, ); + return redirect(`/admin/questions/${questionId}`, "Question created", true); }, }); @@ -284,17 +290,27 @@ const handleAddAnswer = createAuthedFormRoute< "Free-text questions don't have answer options", ); } - const answer = await answersTable.insert({ - questionId: params.id, - sortOrder: 0, - text, - }); - await answersOrder.append({ key: answer.id, scope: params.id }); - await logActivity(`Answer '${text}' added to question ${params.id}`); + // One transaction: an answer must never exist without its place in the + // order or its log line. The inserted sortOrder is a placeholder the + // append overwrites before anything can read it. + await addAnswerRow( + params.id, + { questionId: params.id, sortOrder: 0, text }, + (transaction) => + logActivity( + `Answer '${text}' added to question ${params.id}`, + undefined, + undefined, + transaction, + ), + ); return redirect(`/admin/questions/${params.id}`, "Answer added", true); }, }); +/** Insert an answer and place it in its question's order, atomically. */ +const addAnswerRow = insertScopedOrderedRow(answersTable, answersOrder); + /** Confirmed-delete handlers for questions */ const questionDelete = createConfirmedHandlers({ // The confirmation page shows the flattened text (newlines → " / "), and a diff --git a/src/shared/db/ordered-collection.ts b/src/shared/db/ordered-collection.ts index 9db9966679..60254199dc 100644 --- a/src/shared/db/ordered-collection.ts +++ b/src/shared/db/ordered-collection.ts @@ -4,6 +4,7 @@ import { resultRows, type TxScope, useTransaction, + writeRowInTransaction, } from "#shared/db/client.ts"; type Columns = string | readonly [string, ...string[]]; @@ -194,3 +195,39 @@ export const defineOrderedCollection = < return { append, next, nextMany, swap }; }; + +/** + * Insert a child row and give it the next place in its scoped order — one + * transaction, with any extra writes (like the log line) riding along. A + * failed order write or log rolls the insert back with it. + */ +export const insertScopedOrderedRow = + ( + table: { + insertStatement?: (input: Input) => Promise<{ + args: InValue[]; + sql: string; + }>; + }, + order: { + append: (operation: { + key: number; + scope: number; + transaction: TxScope; + }) => Promise; + }, + ) => + ( + scope: number, + input: Input, + log: (transaction: TxScope) => Promise, + ): Promise => + (async () => + writeRowInTransaction( + await table.insertStatement!(input), + null, + async (transaction, id) => { + await order.append({ key: id, scope, transaction }); + await log(transaction); + }, + ))(); diff --git a/test/features/admin/api-keys/auth.test.ts b/test/features/admin/api-keys/auth.test.ts index 1266c371e7..e08b4430f1 100644 --- a/test/features/admin/api-keys/auth.test.ts +++ b/test/features/admin/api-keys/auth.test.ts @@ -179,26 +179,6 @@ describeWithEnv("API key authentication", { db: true }, () => { ); }); - test("touchApiKeyLastUsed surfaces the test-override error to its caller", async () => { - // The fire-and-forget swallowing happens at the request layer; the - // function itself must still throw so that layer has something to catch. - const { setTouchOverride } = await import("#shared/test-overrides.ts"); - const { touchApiKeyLastUsed } = await import("#shared/db/api-keys.ts"); - setTouchOverride(new Error("touch failed")); - try { - await expect(touchApiKeyLastUsed(1)).rejects.toThrow("touch failed"); - } finally { - setTouchOverride(null); - } - }); - - test("getApiKeyForUser throws a not-found error for an unknown key", async () => { - const { getApiKeyForUser } = await import("#shared/db/api-keys.ts"); - await expect(getApiKeyForUser(999_999, 1)).rejects.toThrow( - "API key 999999 not found for user 1", - ); - }); - test("request succeeds when touchApiKeyLastUsed fails (fire-and-forget)", async () => { await createTestListing({ name: "Touch Test" }); const { apiKey } = await createTestApiKeyFull("Touch Test Key"); diff --git a/test/features/admin/api-keys/manage.test.ts b/test/features/admin/api-keys/manage.test.ts index 6bb0a47ed9..701dcd172a 100644 --- a/test/features/admin/api-keys/manage.test.ts +++ b/test/features/admin/api-keys/manage.test.ts @@ -13,32 +13,39 @@ import { } from "#test-utils/session.ts"; describeWithEnv("API key manage page", { db: true }, () => { + /** GET the key's manage page as the owner, assert 200, return its HTML. */ + const getManagePage = async (id: number): Promise => { + const cookie = await testCookie(); + const response = await handleRequest( + mockRequest(`/admin/api-keys/${id}`, { headers: { cookie } }), + ); + expect(response.status).toBe(200); + return response.text(); + }; + describe("per-key pages", () => { test("GET /admin/api-keys/:id shows the summary and canonical tab links", async () => { const { id } = await createTestApiKeyFull("Managed Key"); - const cookie = await testCookie(); // A never-used key shows the "Never" placeholder for last used. - const first = await handleRequest( - mockRequest(`/admin/api-keys/${id}`, { headers: { cookie } }), - ); - expect(first.status).toBe(200); - const firstHtml = await first.text(); - expect(firstHtml).toContain("Managed Key"); - expect(firstHtml).toContain(`href="/admin/api-keys/${id}"`); - expect(firstHtml).toContain(`href="/admin/api-keys/${id}/actions"`); - expect(firstHtml).not.toContain(`/admin/api-keys/${id}/delete`); - expect(firstHtml).toContain("Never"); + const html = await getManagePage(id); + expect(html).toContain("Managed Key"); + expect(html).toContain(`href="/admin/api-keys/${id}"`); + expect(html).toContain(`href="/admin/api-keys/${id}/actions"`); + expect(html).not.toContain(`/admin/api-keys/${id}/delete`); + expect(html).toContain("Never"); // The manage page lights up the API keys entry in the admin nav. - expect(firstHtml).toContain(''); + expect(html).toContain(''); + }); + + test("GET /admin/api-keys/:id shows the last-used date once the key is used", async () => { + const { id } = await createTestApiKeyFull("Managed Key"); - // Once used, the summary renders the formatted last-used date. await touchApiKeyLastUsed(id); - const second = await handleRequest( - mockRequest(`/admin/api-keys/${id}`, { headers: { cookie } }), - ); - const secondHtml = await second.text(); - expect(secondHtml).not.toContain("Never"); + const html = await getManagePage(id); + expect(html).not.toContain("Never"); + // The touch just happened, so the rendered date carries this year. + expect(html).toContain(String(new Date().getFullYear())); }); test("GET /admin/api-keys/:id/actions shows the delete action", async () => { diff --git a/test/features/admin/api-keys/records.test.ts b/test/features/admin/api-keys/records.test.ts index 44e0875170..c7602910c1 100644 --- a/test/features/admin/api-keys/records.test.ts +++ b/test/features/admin/api-keys/records.test.ts @@ -15,6 +15,7 @@ import { getQueryLog, runWithQueryLogContext, } from "#shared/db/query-log.ts"; +import { setTouchOverride } from "#shared/test-overrides.ts"; import { describeWithEnv } from "#test-utils/db.ts"; import { createTestApiKeyFull } from "#test-utils/session.ts"; @@ -132,7 +133,9 @@ describeWithEnv("API key records", { db: true }, () => { test("getApiKeyForUser throws for wrong user", async () => { const { id } = await createTestApiKeyFull("Wrong User"); - await expect(getApiKeyForUser(id, 999)).rejects.toThrow(); + await expect(getApiKeyForUser(id, 999)).rejects.toThrow( + `API key ${id} not found for user 999`, + ); }); test("lists empty array for user with no keys", async () => { @@ -140,4 +143,21 @@ describeWithEnv("API key records", { db: true }, () => { expect(keys).toHaveLength(0); }); }); + + test("touchApiKeyLastUsed surfaces the test-override error to its caller", async () => { + // The fire-and-forget swallowing happens at the request layer; the + // function itself must still throw so that layer has something to catch. + setTouchOverride(new Error("touch failed")); + try { + await expect(touchApiKeyLastUsed(1)).rejects.toThrow("touch failed"); + } finally { + setTouchOverride(null); + } + }); + + test("getApiKeyForUser throws a not-found error for an unknown key", async () => { + await expect(getApiKeyForUser(999_999, 1)).rejects.toThrow( + "API key 999999 not found for user 1", + ); + }); }); diff --git a/test/features/admin/api-keys/ui.test.ts b/test/features/admin/api-keys/ui.test.ts index f189e7f098..8608d17e6e 100644 --- a/test/features/admin/api-keys/ui.test.ts +++ b/test/features/admin/api-keys/ui.test.ts @@ -12,6 +12,7 @@ import { expectRedirect, FLASH_TEST_ID, flashCookieHeader, + inputNamed, } from "#test-utils/assertions.ts"; import { extractCsrfToken } from "#test-utils/csrf.ts"; import { describeWithEnv } from "#test-utils/db.ts"; @@ -53,9 +54,8 @@ describeWithEnv("API keys admin UI", { db: true }, () => { // definition fails this test instead of moving the expectation along. const { apiKeyForm } = await import("#routes/admin/api-keys.ts"); const html = apiKeyForm.render(); - expect(html).toContain('name="name"'); expect(html).toContain("Name"); - const input = html.slice(html.indexOf('name="name"') - 200); + const input = inputNamed(html, "name"); expect(input).toContain('maxlength="100"'); expect(input).toContain('placeholder="e.g. CI Pipeline"'); expect(input).toContain("required"); diff --git a/test/features/admin/attributes/crud.test.ts b/test/features/admin/attributes/crud.test.ts index 44ce655d27..f8eaeb3e10 100644 --- a/test/features/admin/attributes/crud.test.ts +++ b/test/features/admin/attributes/crud.test.ts @@ -9,7 +9,6 @@ import { getAllAttributesWithOptions, getAttributeWithOptions, } from "#shared/db/attributes.ts"; -import { execute } from "#shared/db/client.ts"; import { activityMessages } from "#test-utils/activity-log.ts"; import { expectFlashRedirect, @@ -23,6 +22,7 @@ import { describeWithEnv } from "#test-utils/db.ts"; import { createAttributeViaRoute, createTestAttributeWithOptions, + withFailingOrderTrigger, } from "#test-utils/db-helpers/attributes.ts"; import { adminFormPost, adminGet } from "#test-utils/session.ts"; import { @@ -113,20 +113,11 @@ describeWithEnv("server (admin attribute CRUD)", { db: true }, () => { test("a failed order write leaves no half-made attribute behind", async () => { // Fail the append's sort_order write: the insert and the append run in // one transaction, so the attribute must roll back with it. - await execute(` - CREATE TRIGGER fail_attribute_order - BEFORE UPDATE OF sort_order ON attributes - BEGIN - SELECT RAISE(ABORT, 'order write failed'); - END - `); - try { + await withFailingOrderTrigger("attributes", async () => { await expect( adminFormPost("/admin/attributes", { name: "Ghost" }), ).rejects.toThrow("order write failed"); - } finally { - await execute("DROP TRIGGER IF EXISTS fail_attribute_order"); - } + }); expect(await getAllAttributesWithOptions()).toEqual([]); }); @@ -224,7 +215,7 @@ describeWithEnv("server (admin attribute CRUD)", { db: true }, () => { await expectFlashRedirect( `/admin/attributes/${attribute.id}/delete`, - expect.stringContaining("Attribute name does not match"), + "Attribute name does not match. Please type the exact attribute name to confirm deletion.", false, )(response); expect(await getAttributeWithOptions(attribute.id)).not.toBeNull(); diff --git a/test/features/admin/attributes/options.test.ts b/test/features/admin/attributes/options.test.ts index 05558360aa..e907243808 100644 --- a/test/features/admin/attributes/options.test.ts +++ b/test/features/admin/attributes/options.test.ts @@ -1,7 +1,6 @@ import { expect } from "@std/expect"; import { describe, it as test } from "@std/testing/bdd"; import { getAttributeWithOptions } from "#shared/db/attributes.ts"; -import { execute } from "#shared/db/client.ts"; import { activityMessages } from "#test-utils/activity-log.ts"; import { expectFlashRedirect, @@ -12,6 +11,7 @@ import { describeWithEnv } from "#test-utils/db.ts"; import { createAttributeViaRoute, createTestAttributeWithOptions, + withFailingOrderTrigger, } from "#test-utils/db-helpers/attributes.ts"; import { adminFormPost, adminGet } from "#test-utils/session.ts"; @@ -60,22 +60,13 @@ describeWithEnv("server (admin attribute options)", { db: true }, () => { // one transaction, so the option must roll back with it instead of // surviving with its placeholder order. const attribute = await createTestAttributeWithOptions("Fragile", []); - await execute(` - CREATE TRIGGER fail_option_order - BEFORE UPDATE OF sort_order ON attribute_options - BEGIN - SELECT RAISE(ABORT, 'order write failed'); - END - `); - try { + await withFailingOrderTrigger("attribute_options", async () => { await expect( adminFormPost(`/admin/attributes/${attribute.id}/options`, { text: "Ghost", }), ).rejects.toThrow("order write failed"); - } finally { - await execute("DROP TRIGGER IF EXISTS fail_option_order"); - } + }); expect((await getAttributeWithOptions(attribute.id))?.options).toEqual( [], ); diff --git a/test/features/admin/database-reset.test.ts b/test/features/admin/database-reset.test.ts index 911489a5db..37caf94524 100644 --- a/test/features/admin/database-reset.test.ts +++ b/test/features/admin/database-reset.test.ts @@ -142,7 +142,9 @@ describeWithEnv("server (demo reset)", { db: true }, () => { }); // Back to the reset page itself, with the mismatch error. expect(response.status).toBe(302); - expect(response.headers.get("location")).toContain("/demo/reset"); + expect( + new URL(response.headers.get("location")!, "http://localhost").pathname, + ).toBe("/demo/reset"); expectFlash( response, expect.stringContaining(RESET_PHRASE_MISMATCH_ERROR), diff --git a/test/features/admin/questions/delete.test.ts b/test/features/admin/questions/delete.test.ts new file mode 100644 index 0000000000..825d4fb8f4 --- /dev/null +++ b/test/features/admin/questions/delete.test.ts @@ -0,0 +1,85 @@ +import { expect } from "@std/expect"; +import { describe, it as test } from "@std/testing/bdd"; +import { answerTextForm, questionTextForm } from "#routes/admin/questions.ts"; +import { createQuestion } from "#test/test-utils/questions/helpers.ts"; +import { activityMessages } from "#test-utils/activity-log.ts"; +import { + expectFlashRedirect, + expectStatus, + inputNamed, +} from "#test-utils/assertions.ts"; +import { describeWithEnv } from "#test-utils/db.ts"; +import { adminFormPost, adminGet } from "#test-utils/session.ts"; + +describeWithEnv( + "server (admin question forms and delete)", + { db: true }, + () => { + describe("the two question forms", () => { + // The expected labels, hints, and choices are written out here so a + // changed form definition fails this test instead of moving along. + test("serves the question box with its label, hint, and required flag", () => { + const html = questionTextForm.render(); + expect(html).toContain("Question text"); + const text = inputNamed(html, "text"); + expect(text).toContain('placeholder="e.g. What is your T-shirt size?"'); + expect(text).toContain("required"); + }); + + test("serves the display-as choice with all three ways to show it", () => { + const html = questionTextForm.render(); + expect(html).toContain("Display as"); + expect(html).toContain(">Radio buttons<"); + expect(html).toContain(">Select box<"); + expect(html).toContain(">Free text<"); + // The choices carry their stored values in order. + expect(html).toContain('value="radio"'); + expect(html).toContain('value="select"'); + expect(html).toContain('value="free_text"'); + }); + + test("serves the answer box with its label and hint", () => { + const html = answerTextForm.render(); + expect(html).toContain("Answer text"); + expect(inputNamed(html, "text")).toContain('placeholder="e.g. Medium"'); + }); + }); + + describe("POST /admin/questions/:id/delete", () => { + test("deleting needs the exact question text", async () => { + const id = await createQuestion("Doomed question?"); + + const { response } = await adminFormPost( + `/admin/questions/${id}/delete`, + { confirm_identifier: "Wrong" }, + ); + + await expectFlashRedirect( + `/admin/questions/${id}/delete`, + expect.stringContaining("Question text does not match"), + false, + )(response); + expectStatus(200)(await adminGet(`/admin/questions/${id}`)); + }); + + test("deleting removes the question, logs it, and says so", async () => { + const id = await createQuestion("Doomed question?"); + + const { response } = await adminFormPost( + `/admin/questions/${id}/delete`, + { confirm_identifier: "Doomed question?" }, + ); + + await expectFlashRedirect( + "/admin/questions", + "Question deleted", + true, + )(response); + expectStatus(404)(await adminGet(`/admin/questions/${id}`)); + expect(await activityMessages()).toContain( + "Question 'Doomed question?' deleted", + ); + }); + }); + }, +); diff --git a/test/features/admin/site.test.ts b/test/features/admin/site.test.ts index a0d4d5615d..e86b35ba0a 100644 --- a/test/features/admin/site.test.ts +++ b/test/features/admin/site.test.ts @@ -11,6 +11,7 @@ import { expectRedirect, FLASH_TEST_ID, flashCookieHeader, + inputNamed, testRequiresAuth, } from "#test-utils/assertions.ts"; import { hasCheckedInput } from "#test-utils/csrf.ts"; @@ -32,23 +33,22 @@ describeWithEnv("server (admin site)", { db: true }, () => { test("serves the home form boxes with their labels and hints", async () => { const { siteHomeForm } = await import("#routes/admin/site.ts"); const html = siteHomeForm.render(); - expect(html).toContain('name="website_title"'); expect(html).toContain("Website Title"); - expect(html).toContain('autocomplete="off"'); expect(html).toContain( "Displayed as the main heading on all public pages (max 128 characters).", ); - expect(html).toContain('name="homepage_text"'); + expect(inputNamed(html, "website_title")).toContain('autocomplete="off"'); expect(html).toContain("Homepage Text"); - expect(html).toContain('placeholder="Welcome to our site..."'); + const homepage = inputNamed(html, "homepage_text"); + expect(homepage).toContain('placeholder="Welcome to our site..."'); // markdown: true wires the preview affordance to the textarea. - expect(html).toContain("data-markdown-preview"); + expect(homepage).toContain("data-markdown-preview"); }); test("serves the contact form box with its label", async () => { const { siteContactForm } = await import("#routes/admin/site.ts"); const html = siteContactForm.render(); - expect(html).toContain('name="contact_page_text"'); + inputNamed(html, "contact_page_text"); expect(html).toContain("Contact Page Text"); }); }); diff --git a/test/features/join.test.ts b/test/features/join.test.ts index 25a5a53739..6657f38852 100644 --- a/test/features/join.test.ts +++ b/test/features/join.test.ts @@ -8,6 +8,7 @@ import { expectFlashRedirect, expectHtmlResponse, expectRedirectWithFlash, + inputNamed, } from "#test-utils/assertions.ts"; import { requireJoinCsrfToken, submitJoinForm } from "#test-utils/csrf.ts"; import { describeWithEnv } from "#test-utils/db.ts"; @@ -32,11 +33,11 @@ describeWithEnv("server (multi-user admin)", { db: true }, () => { expect(html).toContain("Password"); expect(html).toContain("Confirm Password"); expect(html).toContain("Minimum 8 characters"); - const password = html.slice(0, html.indexOf('name="password_confirm"')); + const password = inputNamed(html, "password"); expect(password).toContain('minlength="8"'); expect(password).toContain('autocomplete="new-password"'); expect(password).toContain("required"); - const confirm = html.slice(html.indexOf('name="password_confirm"') - 300); + const confirm = inputNamed(html, "password_confirm"); expect(confirm).toContain('autocomplete="new-password"'); expect(confirm).toContain("required"); }); diff --git a/test/test-utils/assertions.ts b/test/test-utils/assertions.ts index 55a94610d6..3ae0e86bd0 100644 --- a/test/test-utils/assertions.ts +++ b/test/test-utils/assertions.ts @@ -204,11 +204,14 @@ export const expectHtmlContains = ( return html; }; -/** The one input tag on a page with the given name, so a test can check the - * exact attributes a form serves (default, bounds, required). */ +/** The one form control tag on a page with the given name — input, textarea, + * or select — so a test can check the exact attributes a form serves + * (default, bounds, required). */ export const inputNamed = (html: string, name: string): string => { - const tag = html.match(new RegExp(`]*name="${name}"[^>]*>`))?.[0]; - if (!tag) throw new Error(`No input named ${name} on the page`); + const tag = html.match( + new RegExp(`<(?:input|textarea|select)[^>]*name="${name}"[^>]*>`), + )?.[0]; + if (!tag) throw new Error(`No control named ${name} on the page`); return tag; }; diff --git a/test/test-utils/db-helpers/attributes.ts b/test/test-utils/db-helpers/attributes.ts index a4b4ea460c..f3048ab684 100644 --- a/test/test-utils/db-helpers/attributes.ts +++ b/test/test-utils/db-helpers/attributes.ts @@ -6,6 +6,8 @@ import { attributesTable, listingAttributeOptions, } from "#shared/db/attributes.ts"; +import { expectRedirect } from "#test-utils/assertions.ts"; +import { adminFormPost } from "#test-utils/session.ts"; export const createTestAttribute = async ( name = "Test attribute", @@ -52,9 +54,27 @@ export const assignTestAttributeOptions = ( export const createAttributeViaRoute = async ( name: string, ): Promise => { - const { adminFormPost } = await import("#test-utils/session.ts"); - const { expectRedirect } = await import("#test-utils/assertions.ts"); const { response } = await adminFormPost("/admin/attributes", { name }); const location = expectRedirect(response, /^\/admin\/attributes\/\d+/); return Number(new URL(location, "http://localhost").pathname.split("/")[3]); }; + +/** Make every sort_order write on the table fail, run the work, then clean up. */ +export const withFailingOrderTrigger = async ( + table: "attribute_options" | "attributes", + run: () => Promise, +): Promise => { + const { execute } = await import("#shared/db/client.ts"); + await execute(` + CREATE TRIGGER fail_${table}_order + BEFORE UPDATE OF sort_order ON ${table} + BEGIN + SELECT RAISE(ABORT, 'order write failed'); + END + `); + try { + await run(); + } finally { + await execute(`DROP TRIGGER IF EXISTS fail_${table}_order`); + } +}; diff --git a/test/ui/templates/admin/ledger/entry-form.test.ts b/test/ui/templates/admin/ledger/entry-form.test.ts index ce54a77e72..0909a352cb 100644 --- a/test/ui/templates/admin/ledger/entry-form.test.ts +++ b/test/ui/templates/admin/ledger/entry-form.test.ts @@ -11,9 +11,20 @@ import { type LedgerEntryAddOption, ledgerEntryForm, } from "#templates/admin/ledger/entry-form.ts"; +import { inputNamed } from "#test-utils/assertions.ts"; import { testWithSetting } from "#test-utils/settings.ts"; describe("ledger entry forms", () => { + // The expected bounds and flags are written out here so a changed form + // definition fails this test instead of moving the expectation along. + test("serves the amount and date boxes as required, with a zero floor", () => { + const html = ledgerEntryForm.render(); + const amount = inputNamed(html, "amount"); + expect(amount).toContain('min="0"'); + expect(amount).toContain("required"); + expect(inputNamed(html, "occurred_at")).toContain("required"); + }); + test("preselects the posted entry type when redisplaying the add form", () => { const options: LedgerEntryAddOption[] = [ { @@ -43,6 +54,8 @@ describe("ledger entry forms", () => { entry_type: MANUAL_ATTENDEE_CHARGE, occurred_at: "2026-06-22T09:30", }); + // The type choice itself is a required control. + expect(inputNamed(html, "entry_type")).toContain("required"); expect(html).toContain( '', ); From b5c2565ea048db7c84cea5dea355bc1a3100cce0 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 14:44:42 +0000 Subject: [PATCH 07/47] Import the client statically in the trigger helper Same rule the review applied next door: no cycle exists, so the function-scoped import was only indirection. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7 --- test/test-utils/db-helpers/attributes.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test-utils/db-helpers/attributes.ts b/test/test-utils/db-helpers/attributes.ts index f3048ab684..e4292acd59 100644 --- a/test/test-utils/db-helpers/attributes.ts +++ b/test/test-utils/db-helpers/attributes.ts @@ -6,6 +6,7 @@ import { attributesTable, listingAttributeOptions, } from "#shared/db/attributes.ts"; +import { execute } from "#shared/db/client.ts"; import { expectRedirect } from "#test-utils/assertions.ts"; import { adminFormPost } from "#test-utils/session.ts"; @@ -64,7 +65,6 @@ export const withFailingOrderTrigger = async ( table: "attribute_options" | "attributes", run: () => Promise, ): Promise => { - const { execute } = await import("#shared/db/client.ts"); await execute(` CREATE TRIGGER fail_${table}_order BEFORE UPDATE OF sort_order ON ${table} From dbd10257f16ab5c41884f16ba87bec540505654f Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 14:47:44 +0000 Subject: [PATCH 08/47] Follow inputNamed's wider contract in its own test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The helper now hands back any named control, so its miss message says "control" — and the test asserts the textarea and select cases too. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7 --- test/test-utils/assertions.test.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/test/test-utils/assertions.test.ts b/test/test-utils/assertions.test.ts index bb3cd6924c..d24fdef447 100644 --- a/test/test-utils/assertions.test.ts +++ b/test/test-utils/assertions.test.ts @@ -23,9 +23,18 @@ describe("inputNamed", () => { ); }); - test("rejects a page with no input of that name", () => { + test("rejects a page with no control of that name", () => { expect(() => inputNamed("

No boxes here

", "a")).toThrow( - "No input named a on the page", + "No control named a on the page", + ); + }); + + test("hands back a textarea or select with the given name too", () => { + expect(inputNamed(' + + + `; + + await browser.submitForm({ intro: "Welcome" }, "Save"); + + expect(postedPath()).toBe("/words"); + }); + + it("keeps the first matching form when nothing renders a sent field", async () => { + const { browser, postedPath } = postedPathBrowser(); + // A field no form renders is a plain override, so form choice falls back + // to the first form carrying the button, as it always did. + browser.currentHtml = ` +
+ +
+
+ +
+ `; + + await browser.submitForm({ extra: "override" }, "Save"); + + expect(postedPath()).toBe("/first"); }); it("selects a form by body text even when no button text matches", async () => { From 730c254568fd5aec0b38b383bdf6b4ca7d320d70 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 19:51:47 +0000 Subject: [PATCH 39/47] Refuse a hand-passed --registry in the audit parent The flag is the parent-to-child manifest. Accepted from the command line, the extra file would be audited in the snapshot but never copied back, so a --write run could silently discard its pruning. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7 --- scripts/audit-equivalent-mutants.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/scripts/audit-equivalent-mutants.ts b/scripts/audit-equivalent-mutants.ts index 2dad2fb22d..4afb44d775 100644 --- a/scripts/audit-equivalent-mutants.ts +++ b/scripts/audit-equivalent-mutants.ts @@ -86,6 +86,11 @@ if (import.meta.main) { if (isSnapshotChild()) { Deno.exit(await runSnapshotChild(() => runAudit(options))); } + // The manifest is the parent's to build: a hand-passed file would be + // audited in the snapshot yet never copied back, so refuse it outright. + if (options.registry.length > 0) { + throw new Error(`--registry is internal to the audit\n\n${usage}`); + } // One shard list serves the audit and the copy-back alike, so a shard added // while the snapshot is being made can never be pruned and then lost. const registry = await registryCopyBackPaths(); From d8a843bf338540254c1ab18858c95afca251f61b Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 21:27:36 +0000 Subject: [PATCH 40/47] Kill the gate's nineteen survivors: tighter pins and five equivalents MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gate surfaced two substring traps: the £10 example price also matches inside £10.20, and the short business-email warning also matches the settings nag. Both assertions now pin the full phrase. New tests cover the question move flash and redirect, the answer-delete activity log line, both required question fields, the answer form's label, and the title field's id. Five genuinely unobservable mutants — the overwritten sortOrder placeholder, a throw-only options label, an ?? over always-truthy arrays, and three never-read column keys — are recorded with proofs. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7 --- .../mutation/equivalent-mutants/features.txt | 5 +++++ .../equivalent-mutants/ui-templates.txt | 7 +++++++ test/features/admin/questions/answers.test.ts | 4 ++++ test/features/admin/questions/forms.test.ts | 9 +++++++++ .../admin/questions/questions.test.ts | 19 +++++++++++++++++++ test/features/admin/site/contact.test.ts | 9 +++++++-- test/features/admin/site/home.test.ts | 2 ++ .../templates/admin/guide/payments.test.tsx | 14 ++++++++++---- 8 files changed, 63 insertions(+), 6 deletions(-) diff --git a/scripts/mutation/equivalent-mutants/features.txt b/scripts/mutation/equivalent-mutants/features.txt index 2db4b8113c..43288e6427 100644 --- a/scripts/mutation/equivalent-mutants/features.txt +++ b/scripts/mutation/equivalent-mutants/features.txt @@ -159,3 +159,8 @@ src/features/api/folded-booking.ts:118:31 ?? → || # qtyByChild accumulates src/features/api/folded-booking.ts:176:53 1 → 0 # the thankYouUrl guard checks fold.listings.length > 1 only when parentThankYouUrl is truthy (parent flow), which always folds parent + >= 1 child (length >= 2); package callers pass no parentThankYouUrl, so the && short-circuits and the check never runs src/features/api/folded-booking.ts:301:70 0 → 1 # feeSubtotal flows into priceCheckout → owedOrderForLedger, which drops all extras and sets total to 0; bookingFactsFromOrder reads extras (empty) → bookingFee: 0 regardless. The feeSubtotal value never reaches any observable output. src/features/api/folded-booking.ts:381:17 1 → 0 # base.dayCount is read only by parentResolvedDuration → childDaysFromParent(parent, dayCount, 1), which uses its first argument (dayCount) only in the parent.customisable_days branch; processParentApiBooking rejects customisable parents at line 335 before reaching the fold, so that branch is unreachable. For daily parents the function returns clampDurationDays(parent.duration_days), and for standard parents it returns 1 (the standardValue) — neither reads dayCount, so 1 and 0 produce the same duration and thus the same fold output for every reachable input. + +# question answers (features/admin/questions/answers.ts) — the ordered-insert +# placeholder and a throw-only label that can never throw. +src/features/admin/questions/answers.ts:88:45 0 → 1 # the inserted sortOrder is a placeholder: insertScopedOrderedRow runs order.append in the same transaction, and append sets sort_order from MAX over siblings that explicitly exclude the row itself, then overwrites it unconditionally before commit — the placeholder value is never read by anything +src/features/admin/questions/forms.ts:32:9 Display as → "" # requireChoiceOptions reads its label only in its two throw paths: an empty options list, or a value failing an invalid-rule. QUESTION_DISPLAY_TYPES is a non-empty picklist constant and requireChoiceOptions carries no invalid rule, so neither throw is reachable and the label is never read diff --git a/scripts/mutation/equivalent-mutants/ui-templates.txt b/scripts/mutation/equivalent-mutants/ui-templates.txt index 1967803162..e66881734c 100644 --- a/scripts/mutation/equivalent-mutants/ui-templates.txt +++ b/scripts/mutation/equivalent-mutants/ui-templates.txt @@ -23,3 +23,10 @@ src/ui/templates/admin/attributes.tsx:173:61 ?? → || # `data.listingCounts. src/ui/templates/admin/questions.tsx:270:50 ?? → || # `answerCounts?.get(a.id) ?? 0`: answerCounts is Map|undefined, so the left side is a number or undefined. The only falsy-non-null number is 0, and 0 ?? 0 === 0 || 0; undefined maps to 0 under both — no input distinguishes them (same class as attributes.tsx:173:61 and checkout-pricing.ts:85:35) # cache-registry table sets: invalidatorsByTable.get() is Set|undefined, and a # Set object is always truthy, so `?? new Set()` and `|| new Set()` agree. + +# questions list page (ui/templates/admin/questions.tsx) — column keys the page +# never reads back, and a Map.get fallback over always-truthy arrays. +src/ui/templates/admin/questions.tsx:87:50 ?? → || # `listingNames.get(question.id) ?? []`: listingNames is Map, so the left side is an array (always truthy) or undefined — no falsy-non-null value exists, and ?? and || agree on every input (same class as questions.tsx:270:50) +src/ui/templates/admin/questions.tsx:170:14 question → "" # a column `key` is read only by renderTable's columnKeys/hiddenKeys lookups and saved-filter value rendering; reorderableListPage renders the questions table with only reorder options — no columnKeys, hiddenKeys, or filter — so this key is never read +src/ui/templates/admin/questions.tsx:176:14 answers → "" # same as questions.tsx:170:14 — the questions list table is rendered without columnKeys, hiddenKeys, or filters, so its column keys are never read +src/ui/templates/admin/questions.tsx:187:14 listings → "" # same as questions.tsx:170:14 — the questions list table is rendered without columnKeys, hiddenKeys, or filters, so its column keys are never read diff --git a/test/features/admin/questions/answers.test.ts b/test/features/admin/questions/answers.test.ts index da7686a5f8..42f4410bf5 100644 --- a/test/features/admin/questions/answers.test.ts +++ b/test/features/admin/questions/answers.test.ts @@ -6,6 +6,7 @@ import { addAnswer, createQuestion, } from "#test/test-utils/questions/helpers.ts"; +import { activityMessages } from "#test-utils/activity-log.ts"; import { expectFlash, expectFlashRedirect, @@ -212,6 +213,9 @@ describeWithEnv("server (admin questions)", { db: true }, () => { ); const question = await getQuestionWithAnswers(qId); expect(question!.answers.find((a) => a.id === aId)).toBeUndefined(); + expect(await activityMessages()).toContain( + `Answer 'Goodbye Answer' deleted from question ${qId}`, + ); }); test("rejects deletion with wrong text", async () => { diff --git a/test/features/admin/questions/forms.test.ts b/test/features/admin/questions/forms.test.ts index 462a3e8835..ed49c4129f 100644 --- a/test/features/admin/questions/forms.test.ts +++ b/test/features/admin/questions/forms.test.ts @@ -23,6 +23,14 @@ describe("question/answer form field rendering", () => { expect(html).toContain("data-markdown-preview"); }); + test("questionTextForm requires both of its fields", () => { + const html = questionTextForm.render(); + // A question without text, or without a display type, must be refused by + // the browser itself — dropping either `required` would let one through. + expect(html).toMatch(/]* required>/); + expect(html).toMatch(/]* required>/); + }); + test("questionTextForm renders the display-type select with mapped labels", () => { const html = questionTextForm.render(); expect(html).toContain("Display as"); @@ -36,6 +44,7 @@ describe("question/answer form field rendering", () => { test("answerTextForm renders a text input with its placeholder", () => { const html = answerTextForm.render(); + expect(html).toContain("Answer text"); expect(html).toContain('type="text"'); expect(html).toContain('placeholder="e.g. Medium"'); }); diff --git a/test/features/admin/questions/questions.test.ts b/test/features/admin/questions/questions.test.ts index ee87360899..f15b0d3d53 100644 --- a/test/features/admin/questions/questions.test.ts +++ b/test/features/admin/questions/questions.test.ts @@ -215,6 +215,25 @@ describeWithEnv("server (admin questions)", { db: true }, () => { }); }); + describe("POST /admin/questions/:id/move-down", () => { + test("moves the question and returns to the list saying so", async () => { + const firstId = await createQuestion("First question?"); + await createQuestion("Second question?"); + + const { response } = await adminFormPost( + `/admin/questions/${firstId}/move-down`, + ); + + await expectFlashRedirect("/admin/questions", "Question moved")(response); + const { getAllQuestionsWithAnswers } = await import( + "#shared/db/questions/queries.ts" + ); + expect( + (await getAllQuestionsWithAnswers()).map((question) => question.text), + ).toEqual(["Second question?", "First question?"]); + }); + }); + describe("GET /admin/questions/:id", () => { testRequiresAuth("/admin/questions/1", { setup: async () => { diff --git a/test/features/admin/site/contact.test.ts b/test/features/admin/site/contact.test.ts index ddbfa16859..21b4a01d40 100644 --- a/test/features/admin/site/contact.test.ts +++ b/test/features/admin/site/contact.test.ts @@ -164,17 +164,22 @@ describeWithEnv( expect(html).toContain("Botpoison is active"); }); + // The full note, not just "Set a business email": the settings nag says + // that much on its own, so the short phrase can't tell the two apart. + const businessEmailNote = + "Set a business email on the Settings page to receive contact form"; + test("warns when no business email is set", async () => { const response = await adminGet("/admin/site/contact"); const html = await response.text(); - expect(html).toContain("Set a business email"); + expect(html).toContain(businessEmailNote); }); test("hides the business-email warning once one is set", async () => { await settings.update.businessEmail("owner@example.com"); const response = await adminGet("/admin/site/contact"); const html = await response.text(); - expect(html).not.toContain("Set a business email"); + expect(html).not.toContain(businessEmailNote); }); test("reflects the enabled state in the checkbox", async () => { diff --git a/test/features/admin/site/home.test.ts b/test/features/admin/site/home.test.ts index d0e2f1d7f5..7b1bc7bf40 100644 --- a/test/features/admin/site/home.test.ts +++ b/test/features/admin/site/home.test.ts @@ -29,6 +29,8 @@ describeWithEnv("server (admin site home)", { db: true }, () => { "Displayed as the main heading on all public pages (max 128 characters).", ); expect(inputNamed(html, "website_title")).toContain('autocomplete="off"'); + // The id ties the label to the field; an emptied id breaks that link. + expect(inputNamed(html, "website_title")).toContain('id="website_title"'); expect(html).toContain("Homepage text"); const homepage = inputNamed(html, "homepage_text"); expect(homepage).toContain('placeholder="Welcome to our site..."'); diff --git a/test/ui/templates/admin/guide/payments.test.tsx b/test/ui/templates/admin/guide/payments.test.tsx index 99493040d3..302356ceab 100644 --- a/test/ui/templates/admin/guide/payments.test.tsx +++ b/test/ui/templates/admin/guide/payments.test.tsx @@ -29,9 +29,13 @@ describe("payments guide schema", () => { test("explains the booking fee with worked example prices", () => { const html = String(renderGuideSections([sectionByTitle("payments")])); expect(html).toContain(t("guide.q.what_is_booking_fee")); - expect(html).toContain(formatCurrency(1000)); - expect(html).toContain(formatCurrency(1020)); - expect(html).toContain("Booking Fee"); + // The full phrases, not bare amounts: "£10" alone also matches inside + // "£10.20", so a mutated example price could hide behind the other one. + expect(html).toContain(`on a ${formatCurrency(1000)} ticket`); + expect(html).toContain(`pays ${formatCurrency(1020)} in total`); + expect(html).toContain( + 'Settings under Booking Fee', + ); }); test("names the booking duration field the way the listing form does", () => { @@ -39,7 +43,9 @@ describe("payments guide schema", () => { renderGuideSections([sectionByTitle("daily_listings_and_holidays")]), ); expect(html).toContain(t("guide.q.booking_duration_field")); - expect(html).toContain("Booking duration (days)"); + expect(html).toContain( + "For daily listings, Booking duration (days)", + ); expect(html).toContain(`up to ${MAX_DURATION_DAYS} days`); }); }); From 1748578952b7727ee3c86326b8523de4705c284f Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 21:43:21 +0000 Subject: [PATCH 41/47] Catalog the provider labels, bound the field ranking, drop optional chains The builder and built-site provider labels now come from the message catalog, with the shared Deno Deploy option built on demand so the words resolve after the page's message group loads. Form ranking in the test browser now matches real name attributes only, so a data-name cannot claim a field, with a regression test. The attribute options tests throw when a fixture attribute is missing instead of optional chaining, and the regex escaping used in three places is one shared helper. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7 --- src/features/admin/builder.ts | 15 +++++++----- src/locales/en/built-sites.json | 8 +++++++ src/ui/templates/fields/admin.ts | 21 ++++++++++------ .../features/admin/attributes/options.test.ts | 24 ++++++++++--------- test/scripts/i18n-coverage.test.ts | 1 - test/test-utils/assertions.ts | 7 +++--- test/test-utils/regex.ts | 3 +++ test/test-utils/test-browser.test.ts | 20 ++++++++++++++++ test/test-utils/test-browser.ts | 13 +++++++--- 9 files changed, 80 insertions(+), 32 deletions(-) create mode 100644 test/test-utils/regex.ts diff --git a/src/features/admin/builder.ts b/src/features/admin/builder.ts index e9e80db52d..6082081207 100644 --- a/src/features/admin/builder.ts +++ b/src/features/admin/builder.ts @@ -39,7 +39,7 @@ import { } from "#templates/admin/builder.tsx"; import { builtSiteBox, - DENO_DEPLOY_OPTION, + denoDeployOption, providerChoices, } from "#templates/fields/admin.ts"; @@ -80,13 +80,16 @@ export const builderForm = defineForm({ }, ...providerChoices({ db: [ - { label: "Bunny DB (auto-provision)", value: "bunny" }, - { label: "Turso (auto-provision)", value: "turso" }, - { label: "Manual (enter URL below)", value: "manual" }, + { + label: t("fields.built_site.provider.bunny_db_auto"), + value: "bunny", + }, + { label: t("fields.built_site.provider.turso_auto"), value: "turso" }, + { label: t("fields.built_site.provider.manual_db"), value: "manual" }, ], hosting: [ - { label: "Bunny Edge Scripting", value: "bunny" }, - DENO_DEPLOY_OPTION, + { label: t("fields.built_site.provider.bunny_edge"), value: "bunny" }, + denoDeployOption(), ], }), { diff --git a/src/locales/en/built-sites.json b/src/locales/en/built-sites.json index 7b8df32755..84b1001104 100644 --- a/src/locales/en/built-sites.json +++ b/src/locales/en/built-sites.json @@ -112,6 +112,14 @@ "fields.built_site.db_token": "Database token", "fields.built_site.db_token_placeholder": "Database auth token", "fields.built_site.auto_provision_hint": "Leave blank to auto-provision a database.", + "fields.built_site.provider.bunny_db": "Bunny DB", + "fields.built_site.provider.bunny_db_auto": "Bunny DB (auto-provision)", + "fields.built_site.provider.bunny_edge": "Bunny Edge Scripting", + "fields.built_site.provider.bunny_hosting": "Bunny", + "fields.built_site.provider.deno_deploy": "Deno Deploy", + "fields.built_site.provider.manual_db": "Manual (enter URL below)", + "fields.built_site.provider.turso": "Turso", + "fields.built_site.provider.turso_auto": "Turso (auto-provision)", "fields.built_site.hosting_id": "Hosting ID", "fields.built_site.hosting_id_placeholder": "Script ID or app ID", "fields.built_site.hosting_provider": "Hosting provider", diff --git a/src/ui/templates/fields/admin.ts b/src/ui/templates/fields/admin.ts index db49d58e2e..2e7d7a674e 100644 --- a/src/ui/templates/fields/admin.ts +++ b/src/ui/templates/fields/admin.ts @@ -109,11 +109,12 @@ const builtSiteChoice = < type: "select", }); -/** The one hosting option both provider lists share, word for word. */ -export const DENO_DEPLOY_OPTION = { - label: "Deno Deploy", +/** The one hosting option both provider lists share, word for word. Built on + * demand, after the page's message group is loaded. */ +export const denoDeployOption = (): { label: string; value: "deno" } => ({ + label: t("fields.built_site.provider.deno_deploy"), value: "deno", -} as const; +}); /** The hosting and database provider choices, worded per form. */ export const providerChoices = < @@ -151,10 +152,16 @@ const getBuiltSiteFields = () => builtSiteBox("hosting_id", "hosting_id", "text" as const), ...providerChoices({ db: [ - { label: "Bunny DB", value: "bunny" }, - { label: "Turso", value: "turso" }, + { label: t("fields.built_site.provider.bunny_db"), value: "bunny" }, + { label: t("fields.built_site.provider.turso"), value: "turso" }, + ], + hosting: [ + { + label: t("fields.built_site.provider.bunny_hosting"), + value: "bunny", + }, + denoDeployOption(), ], - hosting: [{ label: "Bunny", value: "bunny" }, DENO_DEPLOY_OPTION], }), checkboxField("assignable", { hint: t("fields.built_site.assignable_hint"), diff --git a/test/features/admin/attributes/options.test.ts b/test/features/admin/attributes/options.test.ts index d9b8b17d20..e145b0ff8a 100644 --- a/test/features/admin/attributes/options.test.ts +++ b/test/features/admin/attributes/options.test.ts @@ -15,6 +15,14 @@ import { import { withFailingOrderTrigger } from "#test-utils/db-helpers/failing-order.ts"; import { adminFormPost, adminGet } from "#test-utils/session.ts"; +/** The stored options of an attribute the test knows exists — a missing + * attribute is a broken fixture, so it fails the test by name. */ +const attributeOptions = async (id: number) => { + const attribute = await getAttributeWithOptions(id); + if (!attribute) throw new Error(`No attribute ${id} in the database`); + return attribute.options; +}; + /** Ask the delete route to remove Difficulty's only option, "Easy". */ const postEasyOptionDelete = async ( confirmIdentifier: string, @@ -67,9 +75,7 @@ describeWithEnv("server (admin attribute options)", { db: true }, () => { }), ).rejects.toThrow("order write failed"); }); - expect((await getAttributeWithOptions(attribute.id))?.options).toEqual( - [], - ); + expect(await attributeOptions(attribute.id)).toEqual([]); }); test("an empty option is refused back to the attribute page", async () => { @@ -80,7 +86,7 @@ describeWithEnv("server (admin attribute options)", { db: true }, () => { "Option text is required", false, )((await adminFormPost(`/admin/attributes/${id}/options`)).response); - expect((await getAttributeWithOptions(id))?.options).toEqual([]); + expect(await attributeOptions(id)).toEqual([]); }); }); @@ -164,9 +170,7 @@ describeWithEnv("server (admin attribute options)", { db: true }, () => { ).response, ); expect( - (await getAttributeWithOptions(attribute.id))?.options.map( - (row) => row.text, - ), + (await attributeOptions(attribute.id)).map((row) => row.text), ).toEqual(["Only"]); }); }); @@ -181,9 +185,7 @@ describeWithEnv("server (admin attribute options)", { db: true }, () => { "Option text does not match. Please type the exact option text to confirm deletion.", false, )(response); - expect( - (await getAttributeWithOptions(attributeId))?.options, - ).toHaveLength(1); + expect(await attributeOptions(attributeId)).toHaveLength(1); }); test("deleting an option removes it, logs it, and says so", async () => { @@ -194,7 +196,7 @@ describeWithEnv("server (admin attribute options)", { db: true }, () => { "Option deleted", true, )(response); - expect((await getAttributeWithOptions(attributeId))?.options).toEqual([]); + expect(await attributeOptions(attributeId)).toEqual([]); expect(await activityMessages()).toContain( "Attribute option 'Easy' deleted from Difficulty", ); diff --git a/test/scripts/i18n-coverage.test.ts b/test/scripts/i18n-coverage.test.ts index 274304af1d..517166502d 100644 --- a/test/scripts/i18n-coverage.test.ts +++ b/test/scripts/i18n-coverage.test.ts @@ -85,7 +85,6 @@ const LEFTOVER_ALLOWLIST = new Map([ ["ui/templates/admin/site.tsx", 2], ["ui/templates/email/defaults.ts", 12], ["ui/templates/fields/add-attendee.ts", 3], - ["ui/templates/fields/admin.ts", 4], ["ui/templates/fields/listing.ts", 4], ["ui/templates/fields/modifier.ts", 20], ["ui/templates/fields/ticket.ts", 5], diff --git a/test/test-utils/assertions.ts b/test/test-utils/assertions.ts index 0e79a835fa..28175a1188 100644 --- a/test/test-utils/assertions.ts +++ b/test/test-utils/assertions.ts @@ -3,6 +3,7 @@ import { it } from "@std/testing/bdd"; import { once } from "#fp"; import { getSessionCookieName, parseFlashValue } from "#shared/cookies.ts"; import { BROKEN_IMAGE_PNG } from "#shared/images/broken.ts"; +import { escapeForRegex } from "#test-utils/regex.ts"; export const FLASH_TEST_ID = "t001"; @@ -211,10 +212,9 @@ export const inputNamed = (html: string, name: string): string => { // The lookahead keeps a longer tag like input-widget out, the whitespace // before name= keeps a longer attribute like data-name out, and escaping // keeps regex characters in a name literal. - const literal = name.replace(/[\\^$.*+?()[\]{}|]/g, "\\$&"); const tag = html.match( new RegExp( - `<(?:input|textarea|select)(?=\\s)[^>]*\\sname="${literal}"[^>]*>`, + `<(?:input|textarea|select)(?=\\s)[^>]*\\sname="${escapeForRegex(name)}"[^>]*>`, ), )?.[0]; if (!tag) throw new Error(`No control named ${name} on the page`); @@ -611,10 +611,9 @@ export const selectOptionLabels = ( html: string, ariaLabel: string, ): (string | undefined)[] => { - const escaped = ariaLabel.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); const inner = html.match( new RegExp( - `]*aria-label="${escaped}"[^>]*>([\\s\\S]*?)<\\/select>`, + `]*aria-label="${escapeForRegex(ariaLabel)}"[^>]*>([\\s\\S]*?)<\\/select>`, ), )![1]!; return [...inner.matchAll(/]*>([^<]+) m[1]); diff --git a/test/test-utils/regex.ts b/test/test-utils/regex.ts new file mode 100644 index 0000000000..cbf2df2f0e --- /dev/null +++ b/test/test-utils/regex.ts @@ -0,0 +1,3 @@ +/** Escape text so a RegExp built from it matches the text literally. */ +export const escapeForRegex = (text: string): string => + text.replace(/[\\^$.*+?()[\]{}|]/g, "\\$&"); diff --git a/test/test-utils/test-browser.test.ts b/test/test-utils/test-browser.test.ts index 0733a6e4a3..4d7d7df970 100644 --- a/test/test-utils/test-browser.test.ts +++ b/test/test-utils/test-browser.test.ts @@ -608,6 +608,26 @@ describe("TestBrowser forms", () => { expect(postedPath()).toBe("/words"); }); + it("ignores a data-name attribute when ranking forms by field", async () => { + const { browser, postedPath } = postedPathBrowser(); + // Only a real name attribute counts as rendering the field — a longer + // attribute like data-name on another form must not win the ranking. + browser.currentHtml = ` +
+
+ +
+
+ + +
+ `; + + await browser.submitForm({ intro: "Welcome" }, "Save"); + + expect(postedPath()).toBe("/real"); + }); + it("keeps the first matching form when nothing renders a sent field", async () => { const { browser, postedPath } = postedPathBrowser(); // A field no form renders is a plain override, so form choice falls back diff --git a/test/test-utils/test-browser.ts b/test/test-utils/test-browser.ts index 8dea155258..632b6123de 100644 --- a/test/test-utils/test-browser.ts +++ b/test/test-utils/test-browser.ts @@ -6,6 +6,7 @@ */ import { map, pipe } from "#fp"; +import { escapeForRegex } from "#test-utils/regex.ts"; /** Extract all cookies from a Set-Cookie header and merge into a cookie jar */ const parseCookies = (response: Response, jar: Map): void => { @@ -190,7 +191,10 @@ const findForms = (html: string): FormInfo[] => /** Extract all checkbox values for a given field name from form HTML */ const extractCheckboxValues = (formHtml: string, fieldName: string): string[] => regexCollect( - new RegExp(`]*name="${fieldName}"[^>]*>`, "gi"), + new RegExp( + `]*\\sname="${escapeForRegex(fieldName)}"[^>]*>`, + "gi", + ), formHtml, (m) => m[0], ) @@ -269,9 +273,12 @@ const findFormByButton = ( const lower = buttonText.toLowerCase(); // A page can serve two forms behind one button wording; the one rendering // every field being sent is the one a person filling them in would submit, - // so it wins whenever any form renders them all. + // so it wins whenever any form renders them all. The whitespace before + // name= keeps a longer attribute like data-name from counting as a field. const rendersEveryField = (body: string): boolean => - fieldNames.every((field) => body.includes(`name="${field}"`)); + fieldNames.every((field) => + new RegExp(`\\sname="${escapeForRegex(field)}"`).test(body), + ); const preferred = forms.filter((f) => rendersEveryField(f.body)); const candidates = preferred.length > 0 ? preferred : forms; let switchedOff = false; From 79bbcfd9f765422c337b7735a6cd5fa79c2d32ee Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 21:58:12 +0000 Subject: [PATCH 42/47] Sentence-case the builder, logistics, questions, join, setup, and holiday copy The remaining Title Case actions, headings, and links on the surfaces this branch touched now follow the sentence-case rule, and the two payment credential errors name their fields the way the labels do and end with full stops. Test pins and the shared e2e flows follow suit. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7 --- e2e-payments/src/flow.ts | 2 +- src/locales/en/builder.json | 6 +++--- src/locales/en/built-sites.json | 2 +- src/locales/en/holidays.json | 18 +++++++++--------- src/locales/en/login.json | 14 +++++++------- src/locales/en/logistics.json | 6 +++--- src/locales/en/questions.json | 18 +++++++++--------- src/locales/en/settings.json | 4 ++-- src/locales/en/setup.json | 18 +++++++++--------- test/features/admin/builder/server.test.ts | 6 +++--- test/features/admin/built-sites/server.test.ts | 4 ++-- test/features/join.test.ts | 4 ++-- test/integration/server/holidays.test.ts | 2 +- .../server/holidays/entity-page.test.ts | 2 +- test/integration/server/logistics.test.ts | 2 +- test/integration/server/setup.test.ts | 18 +++++++++--------- test/test-utils/db-contracts.test.ts | 4 ++-- test/test-utils/e2e.ts | 4 ++-- test/ui/templates/admin/builder.test.ts | 2 +- test/ui/templates/admin/dashboard.test.ts | 2 +- test/ui/templates/admin/holidays.test.tsx | 6 +++--- test/ui/templates/admin/logistics.test.ts | 6 +++--- .../templates/admin/questions/delete.test.ts | 2 +- test/ui/templates/components/actions.test.ts | 4 ++-- test/ui/templates/join.test.ts | 14 +++++++------- 25 files changed, 85 insertions(+), 85 deletions(-) diff --git a/e2e-payments/src/flow.ts b/e2e-payments/src/flow.ts index f5996e9bc2..06c53eb6d7 100644 --- a/e2e-payments/src/flow.ts +++ b/e2e-payments/src/flow.ts @@ -32,7 +32,7 @@ export const runSetup = async ( await session.fill("admin_password_confirm", config.adminPassword); await session.select("country", country); await session.check("accept_agreement"); - await session.clickButton("Complete Setup"); + await session.clickButton("Complete setup"); log(` setup complete (admin=${config.adminUsername}, country=${country})`); }; diff --git a/src/locales/en/builder.json b/src/locales/en/builder.json index 226e1c7402..c4591bacff 100644 --- a/src/locales/en/builder.json +++ b/src/locales/en/builder.json @@ -1,5 +1,5 @@ { - "builder.create_new_site": "Create New Site", + "builder.create_new_site": "Create new site", "builder.create_description": "This will create a new Tickets instance on your chosen hosting provider, copy host configuration, and configure the database.", "builder.available_for_assignment": "Available for assignment", "builder.available_for_assignment_help": "Make this site available for automatic assignment when a ticket is purchased", @@ -7,6 +7,6 @@ "builder.no_sites_yet": "No sites have been built yet.", "builder.table_url": "URL", "builder.table_built": "Built", - "builder.site_builder_title": "Site Builder", - "builder.built_sites_title": "Built Sites" + "builder.site_builder_title": "Site builder", + "builder.built_sites_title": "Built sites" } diff --git a/src/locales/en/built-sites.json b/src/locales/en/built-sites.json index 84b1001104..925a2fb99a 100644 --- a/src/locales/en/built-sites.json +++ b/src/locales/en/built-sites.json @@ -1,7 +1,7 @@ { "built_sites.add_built_site": "Add Built Site", "built_sites.guide_link": "Built sites guide", - "built_sites.list_title": "Built Sites", + "built_sites.list_title": "Built sites", "built_sites.build_new_site": "Build New Site", "built_sites.no_built_sites": "No built sites recorded.", "built_sites.table_site_url": "Site URL", diff --git a/src/locales/en/holidays.json b/src/locales/en/holidays.json index 2aaa25fee8..4d57aa0cb5 100644 --- a/src/locales/en/holidays.json +++ b/src/locales/en/holidays.json @@ -1,20 +1,20 @@ { - "holidays.add_holiday": "Add Holiday", + "holidays.add_holiday": "Add holiday", "holidays.guide_link": "Holidays guide", "holidays.no_holidays": "No holidays configured.", - "holidays.upcoming_heading": "Upcoming Holidays", + "holidays.upcoming_heading": "Upcoming holidays", "holidays.col.start_date": "Start Date", "holidays.col.end_date": "End Date", - "holidays.add.title": "Add Holiday", - "holidays.add.heading": "Add Holiday", - "holidays.add.submit": "Create Holiday", - "holidays.edit.title": "Edit Holiday", - "holidays.edit.heading": "Edit Holiday", - "holidays.delete.heading": "Delete Holiday", + "holidays.add.title": "Add holiday", + "holidays.add.heading": "Add holiday", + "holidays.add.submit": "Create holiday", + "holidays.edit.title": "Edit holiday", + "holidays.edit.heading": "Edit holiday", + "holidays.delete.heading": "Delete holiday", "holidays.delete.confirm": "Are you sure you want to delete the holiday {name} ({start} to {end})?", "holidays.delete.confirm_prompt": "Type the holiday name \"{name}\" to confirm:", "holidays.delete.confirm_label": "Holiday name", - "holidays.delete.submit": "Delete Holiday", + "holidays.delete.submit": "Delete holiday", "holidays.fields.name": "Holiday Name", "holidays.fields.name_placeholder": "Bank Holiday", "holidays.fields.start_date": "Start Date", diff --git a/src/locales/en/login.json b/src/locales/en/login.json index 46ba433c49..b731f9c59a 100644 --- a/src/locales/en/login.json +++ b/src/locales/en/login.json @@ -3,20 +3,20 @@ "login.submit": "Login", "login.password": "Password", "login.reset_database": "Reset database", - "join.set_password.title": "Set Your Password", + "join.set_password.title": "Set your password", "join.set_password.welcome": "Welcome, {username}", "join.set_password.instructions": "Set your password to complete your account setup.", - "join.set_password.submit": "Set Password", + "join.set_password.submit": "Set password", "join.set_password.password": "Password", "join.set_password.confirm_password": "Confirm password", "join.set_password.min_chars": "Minimum 8 characters", - "join.success.title": "Account Created", - "join.success.heading": "Password Set", + "join.success.title": "Account created", + "join.success.heading": "Password set", "join.success.message": "Your password has been set successfully.", "join.success.ready": "Your account is ready — you can log in now.", - "join.success.login_link": "Log In", - "join.invalid.title": "Invalid Invite", - "join.invalid.heading": "Invalid Invite", + "join.success.login_link": "Log in", + "join.invalid.title": "Invalid invite", + "join.invalid.heading": "Invalid invite", "password.current": "Current Password", "password.new": "New Password", "password.new_hint": "Minimum 8 characters", diff --git a/src/locales/en/logistics.json b/src/locales/en/logistics.json index 9a3031b9f7..7a8dac2efc 100644 --- a/src/locales/en/logistics.json +++ b/src/locales/en/logistics.json @@ -4,8 +4,8 @@ "logistics.agents_heading": "Logistics Agents", "logistics.agents_hint": "Agents (e.g. vans, drivers, or crew) you can assign as the start and end agent on a logistics listing's attendees.", "logistics.no_agents_yet": "No logistics agents yet.", - "logistics.add_agent": "Add Agent", - "logistics.create_agent": "Create Agent", + "logistics.add_agent": "Add agent", + "logistics.create_agent": "Create agent", "logistics.guide_link": "Logistics guide", "logistics.assigned_users": "Assigned users", "logistics.assigned_users_hint": "These users see this agent's deliveries and collections on the deliveries page. Agent-class users can only see the deliveries page; owners and managers reach it from the Calendar menu.", @@ -16,7 +16,7 @@ "logistics.agent_name": "Agent name", "logistics.delete_confirm": "Are you sure you want to delete the logistics agent {name}? Any attendees assigned to this agent will have that assignment cleared.", "logistics.type_name_to_confirm": "Type the agent name \"{name}\" to confirm:", - "logistics.add_logistics_agent": "Add Logistics Agent", + "logistics.add_logistics_agent": "Add logistics agent", "logistics.delete_logistics_agent": "Delete Logistics Agent", "attendee_logistics.address_heading": "Address", "attendee_logistics.location_heading": "Pinned location", diff --git a/src/locales/en/questions.json b/src/locales/en/questions.json index d95bedb9ca..ae4c3b1b80 100644 --- a/src/locales/en/questions.json +++ b/src/locales/en/questions.json @@ -13,10 +13,10 @@ "questions.all_listings": "All", "questions.answer_column": "Answer", "questions.selected_column": "Times selected", - "questions.edit.text_label": "Question Text", + "questions.edit.text_label": "Question text", "questions.edit.update": "Update", - "questions.edit.answers_heading": "Answer Options", - "questions.edit.new_answer_label": "New Answer", + "questions.edit.answers_heading": "Answer options", + "questions.edit.new_answer_label": "New answer", "questions.edit.new_answer_placeholder": "e.g. Medium", "questions.edit.add_answer": "Add answer", "questions.edit.no_answers": "No answers yet. Add at least 2 answer options.", @@ -26,8 +26,8 @@ "questions.delete.confirm_prompt": "To delete this question, type its text \"{text}\" into the box below:", "questions.delete.confirm_label": "Question text", "questions.delete.submit": "Delete question", - "questions.edit_answer.title": "Edit Answer", - "questions.edit_answer.heading": "Edit Answer", + "questions.edit_answer.title": "Edit answer", + "questions.edit_answer.heading": "Edit answer", "questions.edit_answer.question_context": "Answer for: {text}", "questions.edit_answer.times_selected": "Selected {count} {count, plural, one {time} other {times}} across all bookings.", "questions.edit_answer.back_to_question": "Back to question", @@ -47,14 +47,14 @@ "questions.edit_answer.modifier_label": "Price modifier", "questions.edit_answer.modifier_none": "No price modifier", "questions.edit_answer.modifier_hint": "Applies an \"answer\"-triggered modifier whenever a buyer picks this answer. Create one on the Modifiers page first.", - "questions.edit_answer.save": "Save Answer", + "questions.edit_answer.save": "Save answer", "questions.edit_answer.edit_link": "Edit", - "questions.delete_answer.title": "Delete Answer", - "questions.delete_answer.heading": "Delete Answer", + "questions.delete_answer.title": "Delete answer", + "questions.delete_answer.heading": "Delete answer", "questions.delete_answer.warning": "This will permanently delete the answer \"{answerText}\" from the question \"{questionText}\" and remove all attendee responses for it.", "questions.delete_answer.confirm_prompt": "To delete this answer, type its text \"{text}\" into the box below:", "questions.delete_answer.confirm_label": "Answer text", - "questions.delete_answer.submit": "Delete Answer", + "questions.delete_answer.submit": "Delete answer", "questions.listing.heading": "Questions for {listing}", "questions.listing.no_questions": "No questions created yet. Create questions first.", "questions.listing.manage": "Manage Questions", diff --git a/src/locales/en/settings.json b/src/locales/en/settings.json index 0d9fe05b80..effe5db4b8 100644 --- a/src/locales/en/settings.json +++ b/src/locales/en/settings.json @@ -210,11 +210,11 @@ "error.invalid_payment_provider": "Invalid payment provider", "error.provider_currency_unsupported": "{provider} cannot take payments in {currency}. Choose a different payment provider.", "error.stripe_demo_mode": "Cannot configure Stripe in demo mode", - "error.stripe_key_required": "Stripe Secret Key is required", + "error.stripe_key_required": "Stripe secret key is required.", "error.stripe_key_format": "Invalid Stripe key format. Keys must start with sk_test_ (test mode) or sk_live_ (live mode).", "error.square_demo_mode": "Cannot configure Square in demo mode", "error.square_location_required": "Location ID is required", - "error.square_token_required": "Square Access Token is required", + "error.square_token_required": "Square access token is required.", "error.booking_fee_range": "Booking fee must be a number between 0 and 10", "error.email_format": "Invalid email format. Please use format: name@domain.com", "error.invalid_theme": "Invalid theme selection", diff --git a/src/locales/en/setup.json b/src/locales/en/setup.json index 88a497fd87..d68b92d29d 100644 --- a/src/locales/en/setup.json +++ b/src/locales/en/setup.json @@ -1,8 +1,8 @@ { "setup.title": "Setup", - "setup.heading": "Initial Setup", + "setup.heading": "Initial setup", "setup.welcome": "Welcome! Please configure your ticket reservation system.", - "setup.agreement.title": "Data Controller Agreement", + "setup.agreement.title": "Data controller agreement", "setup.agreement.intro": "By completing setup, you confirm:", "setup.agreement.controller_title": "You are the data controller", "setup.agreement.controller_text": "You decide what data to collect and are responsible for your own GDPR/data protection compliance", @@ -18,19 +18,19 @@ "setup.agreement.deletion_text": "Your data is deleted when you delete your listings or close your account", "setup.agreement.password_warning": "If you lose your password you will be permanently unable to view attendee lists. Do not lose your password.", "setup.agreement.accept": "I understand and accept these terms", - "setup.country_label": "Your Country", + "setup.country_label": "Your country", "setup.country_hint": "Sets your timezone, currency, and phone prefix.", - "setup.submit": "Complete Setup", - "setup.complete.title": "Setup Complete", - "setup.complete.heading": "Setup Complete!", + "setup.submit": "Complete setup", + "setup.complete.title": "Setup complete", + "setup.complete.heading": "Setup complete!", "setup.complete.message": "Your ticket reservation system has been configured successfully.", - "setup.complete.dashboard_link": "Go to Admin Dashboard", - "setup.complete.login_link": "Log In", + "setup.complete.dashboard_link": "Go to admin dashboard", + "setup.complete.login_link": "Log in", "fields.setup.username": "Admin username *", "fields.setup.username_hint": "Letters, numbers, hyphens, underscores (2-32 chars)", "fields.setup.password": "Admin password *", "fields.setup.password_hint": "Minimum 8 characters", "fields.setup.confirm_password": "Confirm admin password *", - "error.agreement_required": "You must accept the Data Controller Agreement to continue", + "error.agreement_required": "You must accept the Data controller agreement to continue", "error.country_invalid": "Please select a valid country" } diff --git a/test/features/admin/builder/server.test.ts b/test/features/admin/builder/server.test.ts index a753855dcb..c62a1e73ef 100644 --- a/test/features/admin/builder/server.test.ts +++ b/test/features/admin/builder/server.test.ts @@ -166,12 +166,12 @@ describeWithEnv( await expectHtmlResponse( response, 200, - "Site Builder", - "Create New Site", + "Site builder", + "Create new site", "Site name", "Database URL", "Database token", - "Built Sites", + "Built sites", ); }); diff --git a/test/features/admin/built-sites/server.test.ts b/test/features/admin/built-sites/server.test.ts index 304012fbc4..691aa95a82 100644 --- a/test/features/admin/built-sites/server.test.ts +++ b/test/features/admin/built-sites/server.test.ts @@ -40,7 +40,7 @@ describeWithEnv("server (admin built sites)", builtSitesTestEnv, () => { await expectHtmlResponse( response, 200, - "Built Sites", + "Built sites", "No built sites recorded", ); }); @@ -565,7 +565,7 @@ describeWithEnv("server (admin built sites)", builtSitesTestEnv, () => { const response = await adminGet("/admin/built-sites"); const body = await response.text(); expect(body).toContain("/admin/built-sites"); - // The nav link is labelled "Builds" (the page title stays "Built Sites"). + // The nav link is labelled "Builds" (the page title stays "Built sites"). expect(body).toContain(">Builds<"); } finally { Deno.env.delete("CAN_BUILD_SITES"); diff --git a/test/features/join.test.ts b/test/features/join.test.ts index a47b50366c..7530e05354 100644 --- a/test/features/join.test.ts +++ b/test/features/join.test.ts @@ -109,9 +109,9 @@ describeWithEnv("server (multi-user admin)", { db: true }, () => { test("GET /join/complete shows confirmation page", async () => { await assertPublicHtml( "/join/complete", - "Password Set", + "Password set", 'href="/admin/login"', - "Log In", + "Log in", ); }); diff --git a/test/integration/server/holidays.test.ts b/test/integration/server/holidays.test.ts index 36f184375d..88f9fb2bd1 100644 --- a/test/integration/server/holidays.test.ts +++ b/test/integration/server/holidays.test.ts @@ -131,7 +131,7 @@ describeWithEnv("server (admin holidays)", { db: true }, () => { await expectHtmlResponse( response, 200, - "Add Holiday", + "Add holiday", "Holiday name", "Start date", "End date", diff --git a/test/integration/server/holidays/entity-page.test.ts b/test/integration/server/holidays/entity-page.test.ts index 7464288f2f..2b0dbd2db1 100644 --- a/test/integration/server/holidays/entity-page.test.ts +++ b/test/integration/server/holidays/entity-page.test.ts @@ -189,7 +189,7 @@ describeWithEnv("holiday entity page", { db: true }, () => { await expectHtmlResponse( response, 200, - "Delete Holiday", + "Delete holiday", "Christmas", "confirm_identifier", ); diff --git a/test/integration/server/logistics.test.ts b/test/integration/server/logistics.test.ts index c96e14d674..fbc3fd2fd6 100644 --- a/test/integration/server/logistics.test.ts +++ b/test/integration/server/logistics.test.ts @@ -120,7 +120,7 @@ describeWithEnv("server (admin logistics)", { db: true }, () => { await expectHtmlResponse( response, 200, - "Add Logistics Agent", + "Add logistics agent", "Agent name", ); }); diff --git a/test/integration/server/setup.test.ts b/test/integration/server/setup.test.ts index 8c9472c33c..85688f88f2 100644 --- a/test/integration/server/setup.test.ts +++ b/test/integration/server/setup.test.ts @@ -313,7 +313,7 @@ describeWithEnv("server (setup)", { db: true }, () => { const response = await handleRequest(mockRequest("/setup/")); - await expectHtmlResponse(response, 200, "Initial Setup"); + await expectHtmlResponse(response, 200, "Initial setup"); expect(await settingsTableExists()).toBe(true); }); @@ -323,7 +323,7 @@ describeWithEnv("server (setup)", { db: true }, () => { const response = await handleRequest(mockRequest("/setup/")); - await expectHtmlResponse(response, 200, "Initial Setup"); + await expectHtmlResponse(response, 200, "Initial setup"); expect(await settingsTableExists()).toBe(true); expect(await tableExists("listings")).toBe(true); expect(await schemaMarkerKeys()).toEqual([ @@ -335,15 +335,15 @@ describeWithEnv("server (setup)", { db: true }, () => { test("GET /setup/ shows setup page", async () => { await assertPublicHtml( "/setup/", - "Initial Setup", + "Initial setup", "Admin password", - "Your Country", - "Data Controller Agreement", + "Your country", + "Data controller agreement", ); }); test("GET /setup (without trailing slash) shows setup page", async () => { - await assertPublicHtml("/setup", "Initial Setup"); + await assertPublicHtml("/setup", "Initial setup"); }); test("POST /setup/ with valid data completes setup", async () => { @@ -440,7 +440,7 @@ describeWithEnv("server (setup)", { db: true }, () => { }); await expectFlashRedirect( "/setup/", - expect.stringContaining("must accept the Data Controller Agreement"), + expect.stringContaining("must accept the Data controller agreement"), false, )(response); }); @@ -530,9 +530,9 @@ describeWithEnv("server (setup)", { db: true }, () => { test("GET /setup/complete shows success page when setup is done", async () => { await assertPublicHtml( "/setup/complete", - "Setup Complete", + "Setup complete", 'href="/admin/login"', - "Log In", + "Log in", ); }); }); diff --git a/test/test-utils/db-contracts.test.ts b/test/test-utils/db-contracts.test.ts index 5252de6b90..f5110d8b67 100644 --- a/test/test-utils/db-contracts.test.ts +++ b/test/test-utils/db-contracts.test.ts @@ -347,8 +347,8 @@ describe("test-utils — db-backed & settings contracts", () => { expect(actions).toEqual([ "visit:/setup/", - "submit:Complete Setup", - "click:Log In", + "submit:Complete setup", + "click:Log in", "submit:Login", "click:Back to dashboard", ]); diff --git a/test/test-utils/e2e.ts b/test/test-utils/e2e.ts index e87cf18ab2..d44d1ea0fb 100644 --- a/test/test-utils/e2e.ts +++ b/test/test-utils/e2e.ts @@ -39,10 +39,10 @@ export const setupAndLogin = async (browser: TestBrowser): Promise => { admin_username: "admin", country: "GB", }, - "Complete Setup", + "Complete setup", ); invalidateAllCaches(); - await browser.clickLink("Log In"); + await browser.clickLink("Log in"); await browser.submitForm( { password: "password", username: "admin" }, "Login", diff --git a/test/ui/templates/admin/builder.test.ts b/test/ui/templates/admin/builder.test.ts index 13a09e796d..e5b66275a6 100644 --- a/test/ui/templates/admin/builder.test.ts +++ b/test/ui/templates/admin/builder.test.ts @@ -69,6 +69,6 @@ describe("adminBuilderPage", () => { test("renders page title", () => { const html = adminBuilderPage(OWNER_SESSION, []); - expect(html).toContain("Site Builder"); + expect(html).toContain("Site builder"); }); }); diff --git a/test/ui/templates/admin/dashboard.test.ts b/test/ui/templates/admin/dashboard.test.ts index 69b1d16291..54d605ac10 100644 --- a/test/ui/templates/admin/dashboard.test.ts +++ b/test/ui/templates/admin/dashboard.test.ts @@ -107,7 +107,7 @@ describe("adminDashboardPage", () => { ], ); - expect(html).toContain("Upcoming Holidays"); + expect(html).toContain("Upcoming holidays"); expect(html).toContain('class="table-scroll dashboard-holidays-scroll"'); expect(html).toContain('href="/admin/holidays/1"'); expect(html).toContain("Winter Break"); diff --git a/test/ui/templates/admin/holidays.test.tsx b/test/ui/templates/admin/holidays.test.tsx index b823b477b6..1e5af2ed84 100644 --- a/test/ui/templates/admin/holidays.test.tsx +++ b/test/ui/templates/admin/holidays.test.tsx @@ -26,7 +26,7 @@ describe("holiday templates", () => { test("renders the add-holiday action and guide link in the action row", () => { const html = adminHolidaysPage([holiday], OWNER_SESSION); expect(html).toContain('href="/admin/holidays/new"'); - expect(html).toContain("Add Holiday"); + expect(html).toContain("Add holiday"); expect(html).toContain('href="/admin/guide#holidays"'); }); @@ -71,7 +71,7 @@ describe("holiday templates", () => { expect(html).toContain('name="start_date"'); expect(html).toContain('name="end_date"'); // Submit button uses the create label and plus icon. - expect(html).toContain("Create Holiday"); + expect(html).toContain("Create holiday"); }); test("renders the error flash when an error is passed", () => { @@ -118,7 +118,7 @@ describe("holiday templates", () => { expect(html).toContain('name="confirm_identifier"'); // The name to type is shown as the confirm target. expect(html).toContain("Christmas"); - expect(html).toContain("Delete Holiday"); + expect(html).toContain("Delete holiday"); expect(html).not.toContain('"); + expect(html).toContain(">Set password"); }); test("renders every account-created message", () => { const html = joinCompletePage(); - expect(html).toContain("Account Created"); - expect(html).toContain("

Password Set

"); + expect(html).toContain("Account created"); + expect(html).toContain("

Password set

"); expect(html).toContain("Your password has been set successfully."); expect(html).toContain("Your account is ready"); expect(html).toContain('href="/admin/login"'); - expect(html).toContain("Log In"); + expect(html).toContain("Log in"); }); test("renders an invalid invite without trusting the message", () => { const html = joinErrorPage("Expired "); - expect(html).toContain("Invalid Invite"); - expect(html).toContain("

Invalid Invite

"); + expect(html).toContain("Invalid invite"); + expect(html).toContain("

Invalid invite

"); expect(html).toContain("Expired <invite>"); }); }); From e7a69125c35d1b7d58cdcd254bb53b5272a9553f Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 22:01:37 +0000 Subject: [PATCH 43/47] Sentence-case the last Title Case labels on the touched pages The questions, holidays, builder, password, and logistics catalogs had a few more Title Case holdouts next to the labels the last commit fixed. They now follow the same rule, with test pins updated. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7 --- src/locales/en/builder.json | 2 +- src/locales/en/holidays.json | 14 +++++++------- src/locales/en/login.json | 6 +++--- src/locales/en/logistics.json | 12 ++++++------ src/locales/en/questions.json | 10 +++++----- test/integration/server/logistics.test.ts | 4 ++-- test/specs/steps/booking-journey.ts | 2 +- test/ui/templates/admin/builder.test.ts | 2 +- test/ui/templates/admin/logistics.test.ts | 6 +++--- test/ui/templates/admin/questions/list.test.ts | 2 +- .../admin/questions/listing-panel.test.ts | 4 ++-- test/ui/templates/fields/admin/schema.test.ts | 2 +- 12 files changed, 33 insertions(+), 33 deletions(-) diff --git a/src/locales/en/builder.json b/src/locales/en/builder.json index c4591bacff..5f980d59f8 100644 --- a/src/locales/en/builder.json +++ b/src/locales/en/builder.json @@ -3,7 +3,7 @@ "builder.create_description": "This will create a new Tickets instance on your chosen hosting provider, copy host configuration, and configure the database.", "builder.available_for_assignment": "Available for assignment", "builder.available_for_assignment_help": "Make this site available for automatic assignment when a ticket is purchased", - "builder.build_site_button": "Build Site", + "builder.build_site_button": "Build site", "builder.no_sites_yet": "No sites have been built yet.", "builder.table_url": "URL", "builder.table_built": "Built", diff --git a/src/locales/en/holidays.json b/src/locales/en/holidays.json index 4d57aa0cb5..8430b32637 100644 --- a/src/locales/en/holidays.json +++ b/src/locales/en/holidays.json @@ -3,8 +3,8 @@ "holidays.guide_link": "Holidays guide", "holidays.no_holidays": "No holidays configured.", "holidays.upcoming_heading": "Upcoming holidays", - "holidays.col.start_date": "Start Date", - "holidays.col.end_date": "End Date", + "holidays.col.start_date": "Start date", + "holidays.col.end_date": "End date", "holidays.add.title": "Add holiday", "holidays.add.heading": "Add holiday", "holidays.add.submit": "Create holiday", @@ -15,13 +15,13 @@ "holidays.delete.confirm_prompt": "Type the holiday name \"{name}\" to confirm:", "holidays.delete.confirm_label": "Holiday name", "holidays.delete.submit": "Delete holiday", - "holidays.fields.name": "Holiday Name", - "holidays.fields.name_placeholder": "Bank Holiday", - "holidays.fields.start_date": "Start Date", - "holidays.fields.end_date": "End Date", + "holidays.fields.name": "Holiday name", + "holidays.fields.name_placeholder": "Bank holiday", + "holidays.fields.start_date": "Start date", + "holidays.fields.end_date": "End date", "holidays.fields.end_date_hint": "Must be on or after the start date", "fields.holiday.name": "Holiday name", - "fields.holiday.name_placeholder": "Bank Holiday", + "fields.holiday.name_placeholder": "Bank holiday", "fields.holiday.start_date": "Start date", "fields.holiday.end_date": "End date", "fields.holiday.end_date_hint": "Must be on or after the start date", diff --git a/src/locales/en/login.json b/src/locales/en/login.json index b731f9c59a..a2241f9db3 100644 --- a/src/locales/en/login.json +++ b/src/locales/en/login.json @@ -17,10 +17,10 @@ "join.success.login_link": "Log in", "join.invalid.title": "Invalid invite", "join.invalid.heading": "Invalid invite", - "password.current": "Current Password", - "password.new": "New Password", + "password.current": "Current password", + "password.new": "New password", "password.new_hint": "Minimum 8 characters", - "password.confirm": "Confirm New Password", + "password.confirm": "Confirm New password", "logout.title": "Log out", "logout.confirm_heading": "Are you sure you want to log out?", "logout.confirm_body": "You will need to log in again to manage tickets.", diff --git a/src/locales/en/logistics.json b/src/locales/en/logistics.json index 7a8dac2efc..574c544562 100644 --- a/src/locales/en/logistics.json +++ b/src/locales/en/logistics.json @@ -1,7 +1,7 @@ { "logistics.title": "Logistics", "logistics.enable_hint": "Enable logistics for listings handled by an agent at the customer's location \u2014 deliveries, equipment hire, transport, set-up and teardown. When on, listings gain a \"Needs logistics\" option and logistics agents can be managed below.", - "logistics.agents_heading": "Logistics Agents", + "logistics.agents_heading": "Logistics agents", "logistics.agents_hint": "Agents (e.g. vans, drivers, or crew) you can assign as the start and end agent on a logistics listing's attendees.", "logistics.no_agents_yet": "No logistics agents yet.", "logistics.add_agent": "Add agent", @@ -10,14 +10,14 @@ "logistics.assigned_users": "Assigned users", "logistics.assigned_users_hint": "These users see this agent's deliveries and collections on the deliveries page. Agent-class users can only see the deliveries page; owners and managers reach it from the Calendar menu.", "logistics.no_users_to_assign": "No users to assign yet.", - "logistics.edit_agent": "Edit Logistics Agent", + "logistics.edit_agent": "Edit logistics agent", "logistics.agent_details": "Agent details", - "logistics.delete_agent": "Delete Agent", + "logistics.delete_agent": "Delete agent", "logistics.agent_name": "Agent name", "logistics.delete_confirm": "Are you sure you want to delete the logistics agent {name}? Any attendees assigned to this agent will have that assignment cleared.", "logistics.type_name_to_confirm": "Type the agent name \"{name}\" to confirm:", "logistics.add_logistics_agent": "Add logistics agent", - "logistics.delete_logistics_agent": "Delete Logistics Agent", + "logistics.delete_logistics_agent": "Delete logistics agent", "attendee_logistics.address_heading": "Address", "attendee_logistics.location_heading": "Pinned location", "attendee_logistics.location_hint": "The latitude and longitude pin the exact spot on the map. Search by postcode to fill them in, or type them yourself.", @@ -26,9 +26,9 @@ "attendee_logistics.location_error": "Enter a latitude between -90 and 90 and a longitude between -180 and 180, or leave both empty", "attendee_logistics.map_label": "Map of the pinned location", "attendee_logistics.diff_heading": "The chosen address differs from what's typed:", - "attendee_logistics.save": "Save Logistics", + "attendee_logistics.save": "Save logistics", "attendee_logistics.saved": "Logistics saved for ''{value}''", - "attendee_logistics.others_heading": "Other Attendees", + "attendee_logistics.others_heading": "Other attendees", "attendee_logistics.others_hint": "Everyone else with a booking on the same dates. Open one to plan its times alongside this job.", "attendee_logistics.col_attendee": "Attendee", "attendee_logistics.col_dates": "Dates", diff --git a/src/locales/en/questions.json b/src/locales/en/questions.json index ae4c3b1b80..42c8bfeb22 100644 --- a/src/locales/en/questions.json +++ b/src/locales/en/questions.json @@ -1,8 +1,8 @@ { - "questions.title": "Custom Questions", - "questions.heading": "Custom Questions", + "questions.title": "Custom questions", + "questions.heading": "Custom questions", "questions.breadcrumb": "Dashboard", - "questions.new_label": "New Question", + "questions.new_label": "New question", "questions.new_placeholder": "e.g. What is your T-shirt size?", "questions.add_submit": "Add question", "questions.no_questions": "No custom questions yet.", @@ -57,11 +57,11 @@ "questions.delete_answer.submit": "Delete answer", "questions.listing.heading": "Questions for {listing}", "questions.listing.no_questions": "No questions created yet. Create questions first.", - "questions.listing.manage": "Manage Questions", + "questions.listing.manage": "Manage questions", "questions.assign_to_listings": "Assign to Listings", "questions.assign_all_listings": "Assign to all listings", "questions.linked_all_listings": "Linked {type} (all):", - "questions.save_listings": "Save Listings", + "questions.save_listings": "Save listings", "fields.answer.times_selected": "Times selected", "questions.answers.free_text_no_options": "Free-text questions don't have answer options." } diff --git a/test/integration/server/logistics.test.ts b/test/integration/server/logistics.test.ts index fbc3fd2fd6..9e794d7ff8 100644 --- a/test/integration/server/logistics.test.ts +++ b/test/integration/server/logistics.test.ts @@ -59,7 +59,7 @@ describeWithEnv("server (admin logistics)", { db: true }, () => { test("shows logistics agent management", async () => { const response = await adminGet("/admin/logistics"); - await expectHtmlResponse(response, 200, "Logistics", "Logistics Agents"); + await expectHtmlResponse(response, 200, "Logistics", "Logistics agents"); }); test("nav hides Logistics until the feature is enabled", async () => { @@ -232,7 +232,7 @@ describeWithEnv("server (admin logistics)", { db: true }, () => { const confirm = await adminGet(`/admin/logistics/${id}/delete`); const confirmHtml = await confirm.text(); expect(confirmHtml).toContain(`action="/admin/logistics/${id}/delete"`); - expect(confirmHtml).toContain("Delete Logistics Agent"); + expect(confirmHtml).toContain("Delete logistics agent"); expect(confirmHtml).toContain("Doomed Van"); const { response } = await adminFormPost( `/admin/logistics/${id}/delete`, diff --git a/test/specs/steps/booking-journey.ts b/test/specs/steps/booking-journey.ts index b815f1396e..38171bfd16 100644 --- a/test/specs/steps/booking-journey.ts +++ b/test/specs/steps/booking-journey.ts @@ -67,7 +67,7 @@ const askSizeQuestion = async ( id: number, ): Promise => { await browser.visit("/admin/questions"); - expect(browser.containsText("Custom Questions")).toBe(true); + expect(browser.containsText("Custom questions")).toBe(true); // Adding a question opens its own page, where its answers are added. await browser.submitForm({ text: QUESTION }, "Add question"); expect(browser.containsText(QUESTION)).toBe(true); diff --git a/test/ui/templates/admin/builder.test.ts b/test/ui/templates/admin/builder.test.ts index e5b66275a6..021c14caa0 100644 --- a/test/ui/templates/admin/builder.test.ts +++ b/test/ui/templates/admin/builder.test.ts @@ -15,7 +15,7 @@ describe("adminBuilderPage", () => { expect(html).toContain('name="site_name"'); expect(html).toContain('name="db_url"'); expect(html).toContain('name="db_token"'); - expect(html).toContain("Build Site"); + expect(html).toContain("Build site"); }); test("renders empty state when no sites", () => { diff --git a/test/ui/templates/admin/logistics.test.ts b/test/ui/templates/admin/logistics.test.ts index 7bfb58fa1a..987a896637 100644 --- a/test/ui/templates/admin/logistics.test.ts +++ b/test/ui/templates/admin/logistics.test.ts @@ -135,7 +135,7 @@ describeWithEnv("adminLogisticsPage", { db: true, encryptionKey: true }, () => { expect(html).not.toContain('name="has_logistics"'); expect(html).toContain('href="/admin/guide#logistics"'); expect(html).toContain("Logistics guide"); - expect(html).toContain("Logistics Agents"); + expect(html).toContain("Logistics agents"); expect(html).toContain('href="/admin/logistics/7"'); }); @@ -143,7 +143,7 @@ describeWithEnv("adminLogisticsPage", { db: true, encryptionKey: true }, () => { await enableFeature("logistics"); const html = adminLogisticsPage([agent], OWNER_SESSION); - expect(html).toContain("Logistics Agents"); + expect(html).toContain("Logistics agents"); expect(html).toContain("Agents (e.g. vans, drivers, or crew)"); expect(html).toContain('href="/admin/logistics/7"'); expect(html).toContain("Van 1"); @@ -161,7 +161,7 @@ describeWithEnv("adminLogisticsPage", { db: true, encryptionKey: true }, () => { test("shows the empty-state placeholder with no agents", () => { const html = adminLogisticsPage([], OWNER_SESSION); - expect(html).toContain("Logistics Agents"); + expect(html).toContain("Logistics agents"); expect(html).toContain("No logistics agents yet."); // No agent rows are rendered. expect(html).not.toContain('href="/admin/logistics/7"'); diff --git a/test/ui/templates/admin/questions/list.test.ts b/test/ui/templates/admin/questions/list.test.ts index df9c3a9c75..45161d3737 100644 --- a/test/ui/templates/admin/questions/list.test.ts +++ b/test/ui/templates/admin/questions/list.test.ts @@ -25,7 +25,7 @@ describe("adminQuestionsPage", () => { expect(html).toContain("

No custom questions yet.

"); }); - test("removes the Custom Questions heading", () => { + test("removes the Custom questions heading", () => { expect(adminQuestionsPage([], OWNER_SESSION)).not.toContain(" { expect(html).not.toContain("1 options"); }); - test("shows Manage Questions link below form", () => { + test("shows Manage questions link below form", () => { const listing = testListingWithCount({ id: 1, name: "My Listing" }); const questions = [ testQuestion({ @@ -77,7 +77,7 @@ describe("adminListingQuestionsPage", () => { }), ); expect(html).toContain('href="/admin/questions"'); - expect(html).toContain("Manage Questions"); + expect(html).toContain("Manage questions"); }); test("lists option names in parentheses", () => { diff --git a/test/ui/templates/fields/admin/schema.test.ts b/test/ui/templates/fields/admin/schema.test.ts index cda2ff4894..95479f17b1 100644 --- a/test/ui/templates/fields/admin/schema.test.ts +++ b/test/ui/templates/fields/admin/schema.test.ts @@ -49,7 +49,7 @@ describe("admin field schemas", () => { { label: "Holiday name", name: "name", - placeholder: "Bank Holiday", + placeholder: "Bank holiday", required: true, type: "text", }, From e1fdb04103ead24cd71caeed8b97899c14c644d0 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 22:09:35 +0000 Subject: [PATCH 44/47] Match the Other attendees pins to the renamed heading MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The logistics catalog change renamed the heading, and the tab's own template tests still pinned the old capitals — the one place the earlier sweep missed. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7 --- test/shared/db/attendees/overlap.test.ts | 2 +- .../admin/attendee-logistics-tab/others.test.ts | 10 +++++----- .../admin/attendee-logistics-tab/page.test.ts | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/test/shared/db/attendees/overlap.test.ts b/test/shared/db/attendees/overlap.test.ts index 7990921ff8..0424149f67 100644 --- a/test/shared/db/attendees/overlap.test.ts +++ b/test/shared/db/attendees/overlap.test.ts @@ -1,5 +1,5 @@ /** - * `getOverlappingBookings` — the Logistics tab's "Other Attendees" query. + * `getOverlappingBookings` — the Logistics tab's "Other attendees" query. * * Locks the half-open overlap contract (`start_at < windowEnd AND end_at > * windowStart`), the exclusions (the queried attendee itself, servicing-kind diff --git a/test/ui/templates/admin/attendee-logistics-tab/others.test.ts b/test/ui/templates/admin/attendee-logistics-tab/others.test.ts index 98e1256f76..1fff4914b3 100644 --- a/test/ui/templates/admin/attendee-logistics-tab/others.test.ts +++ b/test/ui/templates/admin/attendee-logistics-tab/others.test.ts @@ -1,6 +1,6 @@ /** * The attendee Logistics tab, part two: demo-mode masking, the failed save - * keeping the submitted selectors, the Other Attendees list, and the booked + * keeping the submitted selectors, the Other attendees list, and the booked * window helpers. The tab's core GET/POST behaviour lives in `page.test.ts`. */ @@ -146,7 +146,7 @@ describeWithEnv( ); describeWithEnv( - "attendee Logistics tab — Other Attendees", + "attendee Logistics tab — Other attendees", { db: true }, () => { test("lists overlapping attendees with links to their Logistics tabs", async () => { @@ -162,7 +162,7 @@ describeWithEnv( ]); const html = await logisticsTabHtml(current); - expect(html).toContain("Other Attendees"); + expect(html).toContain("Other attendees"); expect(html).toContain("Overlap Person"); expect(html).toContain(`/admin/attendees/${overlapping}/logistics`); expect(html).not.toContain("Far Away Person"); @@ -229,7 +229,7 @@ describeWithEnv( { date: "2030-01-10", durationDays: 1, listingId: daily.id }, ]); const html = await logisticsTabHtml(alone); - expect(html).not.toContain("Other Attendees"); + expect(html).not.toContain("Other attendees"); }); test("the section is hidden for a date-less booking", async () => { @@ -242,7 +242,7 @@ describeWithEnv( { listingId: listing.id }, ]); const html = await logisticsTabHtml(dateless); - expect(html).not.toContain("Other Attendees"); + expect(html).not.toContain("Other attendees"); }); }, ); diff --git a/test/ui/templates/admin/attendee-logistics-tab/page.test.ts b/test/ui/templates/admin/attendee-logistics-tab/page.test.ts index 754cbb4a1e..df56186e46 100644 --- a/test/ui/templates/admin/attendee-logistics-tab/page.test.ts +++ b/test/ui/templates/admin/attendee-logistics-tab/page.test.ts @@ -1,7 +1,7 @@ /** * The attendee Logistics tab: rendering (address form, pin inputs, map) and * saving (address + pin into the PII blob, selectors onto booking rows), - * including validation failures re-rendering in place. The Other Attendees + * including validation failures re-rendering in place. The Other attendees * list, demo mode, and the failed-save selector behaviour live in * `others.test.ts`. */ From c63fa7dd789364b2f06ec852295460eee0bdc1fb Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 22:11:13 +0000 Subject: [PATCH 45/47] Finish the agreement error, write copy-backs whole or not at all MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The setup agreement error now ends as a full sentence. A copy-back write goes through a sibling temp file and a rename, so a write that dies part-way — a full disk, a killed process — leaves the live registry file holding either the old text or the new, never a truncated middle. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7 --- scripts/mutation/snapshot-copy-back.ts | 14 +++++++++++++- src/locales/en/setup.json | 2 +- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/scripts/mutation/snapshot-copy-back.ts b/scripts/mutation/snapshot-copy-back.ts index 5bb9154012..ac5907d093 100644 --- a/scripts/mutation/snapshot-copy-back.ts +++ b/scripts/mutation/snapshot-copy-back.ts @@ -40,6 +40,18 @@ const assertUnchanged = async ( } }; +/** Write through a sibling temp file and rename it into place, so a write + * that dies part-way (a full disk, a killed process) can never leave the + * live file truncated — it holds either the old text or the new. */ +const writeWholeOrNotAtAll = async ( + path: string, + text: string, +): Promise => { + const temp = `${path}.writing`; + await Deno.writeTextFile(temp, text); + await Deno.rename(temp, path); +}; + /** One file this run wrote back, with the text before and after the write. */ export interface WrittenFile { after: string; @@ -93,7 +105,7 @@ export const bringFilesBack = async ( await assertUnchanged(root, entry); const after = await Deno.readTextFile(join(workRoot, entry.file)); if (after === entry.before) continue; - await Deno.writeTextFile(join(root, entry.file), after); + await writeWholeOrNotAtAll(join(root, entry.file), after); written.push({ after, before: entry.before, file: entry.file }); console.log(`Updated ${entry.file}`); } diff --git a/src/locales/en/setup.json b/src/locales/en/setup.json index d68b92d29d..9c1f3bf451 100644 --- a/src/locales/en/setup.json +++ b/src/locales/en/setup.json @@ -31,6 +31,6 @@ "fields.setup.password": "Admin password *", "fields.setup.password_hint": "Minimum 8 characters", "fields.setup.confirm_password": "Confirm admin password *", - "error.agreement_required": "You must accept the Data controller agreement to continue", + "error.agreement_required": "You must accept the Data controller agreement to continue.", "error.country_invalid": "Please select a valid country" } From 4fdf5047b37917d40694b33b58ca11bdc595f077 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 22:23:41 +0000 Subject: [PATCH 46/47] Make every registry write whole-or-nothing, tick the real order box MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The atomic write helper moves to its own module and now covers all three registry writes: the audit's shard prune inside the snapshot, the copy-back into the live checkout, and the rollback restore. A crash at any of them leaves each shard holding the old text or the new — never a truncated middle — and since each registry record stands alone, a partly-pruned set of shards is still a valid registry. The order-page story now checks the rendered box really sends "true" before ticking it, the same way the listing tabs do. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7 --- scripts/mutation/equivalent-audit.ts | 3 ++- scripts/mutation/snapshot-copy-back.ts | 15 ++------------- scripts/mutation/write-whole.ts | 11 +++++++++++ test/specs/support/front-pages.ts | 8 +++++++- 4 files changed, 22 insertions(+), 15 deletions(-) create mode 100644 scripts/mutation/write-whole.ts diff --git a/scripts/mutation/equivalent-audit.ts b/scripts/mutation/equivalent-audit.ts index 69eed764bb..15997b6f6c 100644 --- a/scripts/mutation/equivalent-audit.ts +++ b/scripts/mutation/equivalent-audit.ts @@ -2,6 +2,7 @@ import { isAbsolute, relative, resolve, SEPARATOR } from "@std/path"; import { createStaticGates, type StaticGate } from "./execution.ts"; import { applyMutant, generateMutants, type Mutant } from "./generate.ts"; import { mutantKeyForPath, parseIgnoreLine } from "./ignore.ts"; +import { writeWholeOrNotAtAll } from "./write-whole.ts"; interface PhysicalEntry { chunk: string; @@ -208,7 +209,7 @@ const pruneKilledEntries = async ( if ((await Deno.readTextFile(file)) !== original) { throw new Error("Equivalent-mutant file changed during the audit."); } - await Deno.writeTextFile(file, kept); + await writeWholeOrNotAtAll(file, kept); } }; diff --git a/scripts/mutation/snapshot-copy-back.ts b/scripts/mutation/snapshot-copy-back.ts index ac5907d093..bb93647e57 100644 --- a/scripts/mutation/snapshot-copy-back.ts +++ b/scripts/mutation/snapshot-copy-back.ts @@ -7,6 +7,7 @@ */ import { join } from "@std/path"; +import { writeWholeOrNotAtAll } from "#scripts/mutation/write-whole.ts"; import { errorMessage } from "#shared/error-message.ts"; /** One file to bring back, and what it held when the run started. */ @@ -40,18 +41,6 @@ const assertUnchanged = async ( } }; -/** Write through a sibling temp file and rename it into place, so a write - * that dies part-way (a full disk, a killed process) can never leave the - * live file truncated — it holds either the old text or the new. */ -const writeWholeOrNotAtAll = async ( - path: string, - text: string, -): Promise => { - const temp = `${path}.writing`; - await Deno.writeTextFile(temp, text); - await Deno.rename(temp, path); -}; - /** One file this run wrote back, with the text before and after the write. */ export interface WrittenFile { after: string; @@ -73,7 +62,7 @@ export const putBackOwnWrites = async ( for (const { after, before, file } of written) { try { if ((await Deno.readTextFile(join(root, file))) === after) { - await Deno.writeTextFile(join(root, file), before); + await writeWholeOrNotAtAll(join(root, file), before); console.log(`Put back ${file}`); } } catch (error) { diff --git a/scripts/mutation/write-whole.ts b/scripts/mutation/write-whole.ts new file mode 100644 index 0000000000..b4c2003e89 --- /dev/null +++ b/scripts/mutation/write-whole.ts @@ -0,0 +1,11 @@ +/** Write through a sibling temp file and rename it into place, so a write + * that dies part-way (a full disk, a killed process) can never leave the + * live file truncated — it holds either the old text or the new. */ +export const writeWholeOrNotAtAll = async ( + path: string, + text: string, +): Promise => { + const temp = `${path}.writing`; + await Deno.writeTextFile(temp, text); + await Deno.rename(temp, path); +}; diff --git a/test/specs/support/front-pages.ts b/test/specs/support/front-pages.ts index f7bd523c96..53b3f7752c 100644 --- a/test/specs/support/front-pages.ts +++ b/test/specs/support/front-pages.ts @@ -12,7 +12,10 @@ import { type PageRead, submitRenderedAdminForm, } from "#test/specs/support/browser.ts"; -import { fillInAndSend } from "#test/specs/support/form-controls.ts"; +import { + fillInAndSend, + requireCheckboxOffered, +} from "#test/specs/support/form-controls.ts"; import type { TicketsWorld } from "#test/specs/support/world.ts"; import { enablePublicSite } from "#test-utils/settings.ts"; @@ -56,6 +59,9 @@ export const ownerTurnsOrderPageOn = async ( ): Promise => { await enablePublicSite(); const browser = await openAdminPage(world, "/admin/site/order"); + // The tick only counts if the page's own box really sends "true" — the + // value the route reads — not a value the story made up. + requireCheckboxOffered(browser.currentHtml, "order_enabled", "true"); await fillInAndSend(browser, { order_enabled: "true" }, "Save"); expect(browser.pageText).toContain("Order page enabled"); await fillInAndSend(browser, { order_intro_text: intro }, "Save"); From 28c2a219e41721acc84b622b3605c525dd8e3723 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 22:27:21 +0000 Subject: [PATCH 47/47] Name the registry file path once, string or URL alike CI's type check caught the prune write handing a possible URL to the string-only atomic writer. A shared registryFilePath helper now turns either form into a plain path, used by the prune write and the copy-back manifest, with both forms pinned by direct tests. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0199Psv4jd85ZgQst3nGYXA7 --- scripts/audit-equivalent-mutants.ts | 9 ++++++--- scripts/mutation/equivalent-audit.ts | 8 ++++++-- scripts/mutation/ignore.ts | 6 +++++- test/scripts/mutation/ignore.test.ts | 13 +++++++++++++ 4 files changed, 30 insertions(+), 6 deletions(-) diff --git a/scripts/audit-equivalent-mutants.ts b/scripts/audit-equivalent-mutants.ts index 4afb44d775..df55b1509c 100644 --- a/scripts/audit-equivalent-mutants.ts +++ b/scripts/audit-equivalent-mutants.ts @@ -1,9 +1,12 @@ #!/usr/bin/env -S deno run --allow-all -import { fromFileUrl, relative, resolve } from "@std/path"; +import { relative, resolve } from "@std/path"; import { splitFlagValues } from "#scripts/flag-values.ts"; import { auditEquivalentMutants } from "#scripts/mutation/equivalent-audit.ts"; -import { listRegistryFiles } from "#scripts/mutation/ignore.ts"; +import { + listRegistryFiles, + registryFilePath, +} from "#scripts/mutation/ignore.ts"; import { runInSnapshot } from "#scripts/mutation/isolation.ts"; import { isSnapshotChild, @@ -77,7 +80,7 @@ const runAudit = async (options: AuditOptions): Promise => { * audit can carry each pruned file back to the live checkout. */ const registryCopyBackPaths = async (): Promise => (await listRegistryFiles()).map((file) => - relative(projectRoot, typeof file === "string" ? file : fromFileUrl(file)), + relative(projectRoot, registryFilePath(file)), ); if (import.meta.main) { diff --git a/scripts/mutation/equivalent-audit.ts b/scripts/mutation/equivalent-audit.ts index 15997b6f6c..f15cd75412 100644 --- a/scripts/mutation/equivalent-audit.ts +++ b/scripts/mutation/equivalent-audit.ts @@ -1,7 +1,11 @@ import { isAbsolute, relative, resolve, SEPARATOR } from "@std/path"; import { createStaticGates, type StaticGate } from "./execution.ts"; import { applyMutant, generateMutants, type Mutant } from "./generate.ts"; -import { mutantKeyForPath, parseIgnoreLine } from "./ignore.ts"; +import { + mutantKeyForPath, + parseIgnoreLine, + registryFilePath, +} from "./ignore.ts"; import { writeWholeOrNotAtAll } from "./write-whole.ts"; interface PhysicalEntry { @@ -209,7 +213,7 @@ const pruneKilledEntries = async ( if ((await Deno.readTextFile(file)) !== original) { throw new Error("Equivalent-mutant file changed during the audit."); } - await writeWholeOrNotAtAll(file, kept); + await writeWholeOrNotAtAll(registryFilePath(file), kept); } }; diff --git a/scripts/mutation/ignore.ts b/scripts/mutation/ignore.ts index fc2a410b10..41e480c930 100644 --- a/scripts/mutation/ignore.ts +++ b/scripts/mutation/ignore.ts @@ -20,7 +20,7 @@ * real surviving mutant, so a stale/redundant/duplicate entry fails the run. */ -import { join } from "@std/path"; +import { fromFileUrl, join } from "@std/path"; import { namesInDirectory, rethrowUnlessNotFound } from "#scripts/not-found.ts"; import { seenBefore } from "#shared/seen-before.ts"; import type { Mutant } from "./generate.ts"; @@ -33,6 +33,10 @@ const EQUIVALENT_MUTANTS_DIR = new URL( /** Every registry file in the directory, in name order so loads are stable. * A checkout without the directory simply has no records, so it reads empty. */ +/** A registry file as a plain path, whichever form the listing produced. */ +export const registryFilePath = (file: string | URL): string => + typeof file === "string" ? file : fromFileUrl(file); + export const listRegistryFiles = async ( dir: string | URL = EQUIVALENT_MUTANTS_DIR, ): Promise<(string | URL)[]> => { diff --git a/test/scripts/mutation/ignore.test.ts b/test/scripts/mutation/ignore.test.ts index c296a5c57f..32675657bf 100644 --- a/test/scripts/mutation/ignore.test.ts +++ b/test/scripts/mutation/ignore.test.ts @@ -8,6 +8,7 @@ import { listRegistryFiles, loadIgnoreList, mutantKey, + registryFilePath, } from "#scripts/mutation/ignore.ts"; import type { MutantResult } from "#scripts/mutation/summary.ts"; import { projectRoot } from "#scripts/project-root.ts"; @@ -206,4 +207,16 @@ describe("mutation ignore list", () => { `redundant (a test kills this mutant, not a survivor): ${redundant}`, ]); }); + + test("registryFilePath keeps a plain path as it is", () => { + expect(registryFilePath("scripts/mutation/equivalent-mutants/a.txt")).toBe( + "scripts/mutation/equivalent-mutants/a.txt", + ); + }); + + test("registryFilePath turns a file URL into its path", () => { + expect(registryFilePath(new URL("file:///tmp/registry/a.txt"))).toBe( + "/tmp/registry/a.txt", + ); + }); });