From 7584b372aec938e681ebca0a3a2fb8c3ca21440c Mon Sep 17 00:00:00 2001 From: Stefan Date: Sun, 19 Jul 2026 11:53:24 +0100 Subject: [PATCH 1/6] Normalize declarative form inputs --- scripts/mutation/equivalent-mutants.txt | 7 + src/features/admin/aggregate-recalculation.ts | 20 +- src/features/admin/dashboard.ts | 4 +- src/features/admin/settings-general.ts | 39 ++-- .../admin/settings-listing-defaults.ts | 25 ++- src/shared/column-order.ts | 202 ++++++++++++++---- src/shared/columns/attendee-columns.ts | 25 +-- src/shared/columns/listing-columns.ts | 58 ++--- src/shared/day-names.ts | 10 +- src/shared/db/settings.ts | 11 + src/shared/forms/definition.ts | 35 +-- src/shared/forms/repeated-picklist.ts | 31 +++ src/shared/forms/saved-data.ts | 1 + src/shared/forms/submitted-value.ts | 10 - src/shared/forms/validation.ts | 59 ++--- src/ui/templates/admin/dashboard.tsx | 26 +-- src/ui/templates/admin/groups/overview.tsx | 11 +- src/ui/templates/admin/guide/operations.tsx | 16 +- src/ui/templates/admin/listing-table.tsx | 12 +- .../templates/admin/settings/column-order.tsx | 20 +- src/ui/templates/attendee-table.tsx | 38 ++-- .../admin/aggregate-recalculation.test.ts | 153 +++++++++++++ .../admin/settings-listing-defaults.test.ts | 29 ++- .../server/settings/column-order.test.ts | 29 ++- .../server/settings}/listing-defaults.test.ts | 3 + test/lib/server-editor.test.ts | 3 +- .../lib/server-images/attachment-edit.test.ts | 7 +- test/lib/server-images/create.test.ts | 7 +- test/shared/column-order.test.ts | 133 +++++------- test/shared/columns/attendee-columns.test.ts | 11 +- test/shared/db/attendees/api/errors.test.ts | 2 +- .../forms/definition/field-schema.test.ts | 48 +++-- test/shared/forms/repeated-picklist.test.ts | 44 ++++ test/shared/forms/validation.test.ts | 160 ++++++++++++-- test/test-utils/db-helpers/listing-forms.ts | 81 ++++--- test/test-utils/db-helpers/request.ts | 14 +- test/test-utils/form-values.test.ts | 18 ++ test/test-utils/form-values.ts | 24 +++ test/test-utils/mocks.ts | 16 +- test/test-utils/validation.ts | 17 +- test/ui/templates/admin/dashboard.test.ts | 5 +- .../templates/attendee-table/template.test.ts | 31 +-- test/ui/templates/fields/listing.test.ts | 33 ++- 43 files changed, 1069 insertions(+), 459 deletions(-) create mode 100644 src/shared/forms/repeated-picklist.ts create mode 100644 test/features/admin/aggregate-recalculation.test.ts rename test/{lib/server-settings => integration/server/settings}/listing-defaults.test.ts (99%) create mode 100644 test/shared/forms/repeated-picklist.test.ts create mode 100644 test/test-utils/form-values.test.ts create mode 100644 test/test-utils/form-values.ts diff --git a/scripts/mutation/equivalent-mutants.txt b/scripts/mutation/equivalent-mutants.txt index b3e84213d..e0b494d16 100644 --- a/scripts/mutation/equivalent-mutants.txt +++ b/scripts/mutation/equivalent-mutants.txt @@ -28,6 +28,13 @@ # ` — exit 0 means lint did not kill it. src/ui/templates/components/data-table.tsx:85:43 0 → -1 # For the only changed case, an empty array, both branches render the same empty tbody; non-empty arrays make both comparisons false +src/shared/forms/validation.ts:63:8 === → == # value is string|number|null and cannot be undefined, so strict and loose equality agree when comparing it with null +src/shared/forms/validation.ts:70:10 === → == # readSubmittedFieldValue returns string|null and cannot return undefined, so strict and loose equality agree when comparing it with null +src/shared/forms/validation.ts:122:46 = → += # this assignment runs only when trimmed is the empty string, so replacing it and appending to it produce the same defaultValue +src/shared/forms/validation.ts:126:16 !== → != # validateFieldText returns string|null and cannot return undefined, so strict and loose inequality agree when comparing it with null +src/features/admin/settings-listing-defaults.ts:55:15 === → == # parseNonNegativeInt returns number|null and cannot return undefined, so strict and loose equality agree when comparing it with null +src/shared/column-order.ts:107:38 validatedColumnLayout → "" # Symbol descriptions are diagnostic text only and do not affect symbol identity or any observable layout behavior +src/shared/column-order.ts:107:38 validatedColumnLayout → "validatedColumnLayout mutated" # Symbol descriptions are diagnostic text only and do not affect symbol identity or any observable layout behavior src/features/admin/entity-pages.ts:275:24 ?? → || # opts.query is a URLSearchParams object when present, so it is always truthy src/features/admin/site-content-page.ts:87:38 ?? → || # extraTabs is an array when present, and arrays are always truthy diff --git a/src/features/admin/aggregate-recalculation.ts b/src/features/admin/aggregate-recalculation.ts index dbce71171..3e15032e9 100644 --- a/src/features/admin/aggregate-recalculation.ts +++ b/src/features/admin/aggregate-recalculation.ts @@ -1,9 +1,12 @@ /* jscpd:ignore-start */ + +import * as v from "valibot"; import { AUTH_FORM, requireSessionOr, withAuth } from "#routes/auth.ts"; import { htmlResponse, redirect } from "#routes/response.ts"; import { getFlash } from "#shared/flash-context.ts"; import type { FormParams } from "#shared/form-data.ts"; import type { Field } from "#shared/forms/field.ts"; +import { readRepeatedPicklist } from "#shared/forms/repeated-picklist.ts"; import { type ValidationResult, validateForm, @@ -35,10 +38,17 @@ export const parseEditableAggregateForm = ( export const selectedRecalculationFields = ( form: FormParams, - allowed: readonly T[], + allowed: readonly [T, ...T[]], ): T[] => { - const selected = new Set(form.getAll(RECALCULATE_FIELD_NAME)); - return allowed.filter((field) => selected.has(field)); + const selection = readRepeatedPicklist( + v.picklist(allowed), + form, + RECALCULATE_FIELD_NAME, + ); + if (selection.state === "invalid") { + throw new Error(`Invalid recalculation field: ${selection.value}`); + } + return selection.state === "selected" ? selection.values : []; }; /** @@ -51,7 +61,7 @@ export const selectedRecalculationFields = ( */ export const runRecalculatePost = async (config: { form: FormParams; - fields: readonly T[]; + fields: readonly [T, ...T[]]; /** Re-render the recalculate page with the "choose a field" error. */ renderChoose: ResponseHandler; reset: (selected: T[]) => Promise; @@ -108,7 +118,7 @@ export const createRecalculateHandlers = (config: { error?: string, success?: string, ) => Promise; - fields: readonly F[]; + fields: readonly [F, ...F[]]; entityId: (entity: T) => number; reset: (entityId: number, selected: F[]) => Promise; log: (entity: T) => Promise; diff --git a/src/features/admin/dashboard.ts b/src/features/admin/dashboard.ts index 7dc7f5f93..b97f498ad 100644 --- a/src/features/admin/dashboard.ts +++ b/src/features/admin/dashboard.ts @@ -149,7 +149,7 @@ const handleAdminGet = (request: Request): Promise => newestAttendees, successMessage, stats, - settings.listingColumnOrder, + settings.listingColumnLayout, activeType, holidays, unbookableIds, @@ -170,7 +170,7 @@ const handleAdminListingsGet: TypedRouteHandler<"GET /admin/listings"> = return adminListingsPage( listings, session, - settings.listingColumnOrder, + settings.listingColumnLayout, await loadListingAttributeFilterContext(request, listings), ); }); diff --git a/src/features/admin/settings-general.ts b/src/features/admin/settings-general.ts index f0aae4f2a..fdc9ed974 100644 --- a/src/features/admin/settings-general.ts +++ b/src/features/admin/settings-general.ts @@ -15,9 +15,7 @@ import { settingsHandler, settingsToggle, } from "#routes/admin/settings-helpers.ts"; -import { validateColumnTemplate } from "#shared/column-order.ts"; -import { ATTENDEE_TABLE_COLUMNS } from "#shared/columns/attendee-columns.ts"; -import { LISTING_TABLE_COLUMNS } from "#shared/columns/listing-columns.ts"; +import { COLUMN_LAYOUTS, type ColumnLayoutKind } from "#shared/column-order.ts"; import { clearSessionCookie } from "#shared/cookies.ts"; import { logActivity } from "#shared/db/activityLog.ts"; import { settings } from "#shared/db/settings.ts"; @@ -194,24 +192,31 @@ export const handleBookingFeePost = settingsHandler({ * Build a column-order settings handler for the listing or attendee table. * Handles POST /admin/settings/{listing,attendee}-column-order - owner only */ -const columnOrderHandler = (kind: "listing" | "attendee") => { - const columns = - kind === "listing" ? LISTING_TABLE_COLUMNS : ATTENDEE_TABLE_COLUMNS; - const update = - kind === "listing" - ? settings.update.listingColumnOrder - : settings.update.attendeeColumnOrder; - const label = - kind === "listing" ? "Listing column order" : "Attendee column order"; +type ConfigurableColumnLayoutKind = Exclude; + +const COLUMN_ORDER_SETTINGS = { + attendee: { + label: "Attendee column order", + update: settings.update.attendeeColumnOrder, + }, + listing: { + label: "Listing column order", + update: settings.update.listingColumnOrder, + }, +} satisfies Record< + ConfigurableColumnLayoutKind, + { label: string; update: (value: string) => Promise } +>; + +const columnOrderHandler = (kind: ConfigurableColumnLayoutKind) => { + const config = COLUMN_ORDER_SETTINGS[kind]; return settingsHandler({ advanced: true, extract: (form) => form.getString("column_order").trim(), formId: `settings-${kind}-column-order`, - label, - save: (value) => update(value), - // Empty value clears to the default column order - validate: (value) => - value ? validateColumnTemplate(value, Object.keys(columns)) : null, + label: config.label, + save: config.update, + validate: COLUMN_LAYOUTS[kind].validate, }); }; diff --git a/src/features/admin/settings-listing-defaults.ts b/src/features/admin/settings-listing-defaults.ts index 307098dbc..37b46d965 100644 --- a/src/features/admin/settings-listing-defaults.ts +++ b/src/features/admin/settings-listing-defaults.ts @@ -7,6 +7,7 @@ * their next read (defaults resolve live — see `resolveListingDefaults`). */ +import * as v from "valibot"; import { t } from "#i18n"; import { settingsHandler } from "#routes/admin/settings-helpers.ts"; import { ownerPage } from "#routes/auth.ts"; @@ -16,6 +17,7 @@ import { invalidateListingsCache } from "#shared/db/listings/records.ts"; import { settings } from "#shared/db/settings.ts"; import { isDemoMode } from "#shared/demo/mode.ts"; import type { FormParams } from "#shared/form-data.ts"; +import { readRepeatedPicklist } from "#shared/forms/repeated-picklist.ts"; import { LISTING_DEFAULT_FIELDS, type ListingDefaultField, @@ -80,22 +82,25 @@ const parseUrlField = ( /** Parse the bookable-days default: only set when its enable box is ticked, with * at least one valid day (in canonical order). */ const parseDaysField = (form: FormParams): FieldParse => { - if (form.getString("default_bookable_days_enabled") !== "1") return {}; - const submittedDays = form.getAll("default_bookable_days"); - const invalidDay = submittedDays.find( - (submittedDay) => - !VALID_DAY_NAMES.some((validDay) => validDay === submittedDay), + const selection = readRepeatedPicklist( + v.picklist(VALID_DAY_NAMES), + form, + "default_bookable_days", + form.getFlag("default_bookable_days_enabled"), ); - if (invalidDay !== undefined) + if (selection.state === "disabled") return {}; + if (selection.state === "invalid") { return { error: t("fields.validation.invalid_day", { - day: invalidDay, + day: selection.value, valid: VALID_DAY_NAMES.join(", "), }), }; - const days = VALID_DAY_NAMES.filter((day) => submittedDays.includes(day)); - if (days.length === 0) return { error: t("listing_defaults.days_required") }; - return { value: days }; + } + if (selection.state === "absent") { + return { error: t("listing_defaults.days_required") }; + } + return { value: selection.values }; }; /** Per-kind parser. The `Record` is keyed by {@link ListingDefaultKind}, so a diff --git a/src/shared/column-order.ts b/src/shared/column-order.ts index 125e26f4e..0bfbb8902 100644 --- a/src/shared/column-order.ts +++ b/src/shared/column-order.ts @@ -10,6 +10,7 @@ * {{price | currency}} → "£25.00" */ +import * as v from "valibot"; import { createBaseLiquidEngine } from "#shared/liquid-engine.ts"; import { requireSuccess } from "#shared/result.ts"; @@ -47,6 +48,71 @@ export type ColumnGenerators = Record< ColumnDef >; +export const ListingColumnSchema = v.picklist([ + "attendees", + "cost", + "created", + "date", + "description", + "location", + "name", + "price", + "profit", + "renewal", + "revenue", + "status", + "tickets", +]); +export type ListingColumn = v.InferOutput; + +export const EditorListingColumnSchema = v.picklist([ + "attendees", + "created", + "date", + "description", + "location", + "name", + "price", + "renewal", + "status", + "tickets", +]); +export type EditorListingColumn = v.InferOutput< + typeof EditorListingColumnSchema +>; + +export const AttendeeColumnSchema = v.picklist([ + "address", + "answers", + "date", + "email", + "listings", + "name", + "phone", + "qty", + "registered", + "special_instructions", + "status", + "ticket", +]); +export type AttendeeColumn = v.InferOutput; + +export const ColumnLayoutKindSchema = v.picklist([ + "listing", + "editor-listing", + "attendee", +]); +export type ColumnLayoutKind = v.InferOutput; + +const validatedColumnLayout = Symbol("validatedColumnLayout"); + +export type ColumnLayout = { + readonly [validatedColumnLayout]: true; + readonly columnKeys: readonly TColumn[]; + readonly filters: ReadonlyMap; + readonly template: string; +}; + // --------------------------------------------------------------------------- // Liquid engine — single instance for rendering filtered values // --------------------------------------------------------------------------- @@ -79,37 +145,43 @@ const parseTagBody = ( * Parse a column-order template and extract the ordered list of column keys * plus any per-column Liquid filter expressions. * - * Validation is done by checking extracted keys against the valid set. - * No Liquid engine is needed for validation — just regex + Set.has(). + * Validation is done by parsing extracted keys through the column schema. + * No Liquid engine is needed for validation. */ -const parseColumnTemplate = ( +const parseColumnTemplate = < + const TOptions extends readonly [string, ...string[]], +>( template: string, - validKeys: readonly string[], + schema: v.PicklistSchema, ): - | { ok: true; columns: string[]; filters: Map } + | { + ok: true; + columns: TOptions[number][]; + filters: Map; + } | { ok: false; error: string; } => { - const validKeySet = new Set(validKeys); - const columns: string[] = []; - const filters = new Map(); + const columns: TOptions[number][] = []; + const filters = new Map(); const seen = new Set(); for (const match of template.matchAll(LIQUID_TAG_RE)) { const { key, filter } = parseTagBody(match[1]!); - if (!validKeySet.has(key)) { + const parsed = v.safeParse(schema, key); + if (!parsed.success) { return { - error: `Unknown column "${key}". Available columns: ${validKeys.join( + error: `Unknown column "${key}". Available columns: ${schema.options.join( ", ", )}`, ok: false, }; } - if (!seen.has(key)) { - seen.add(key); - columns.push(key); - if (filter) filters.set(key, filter); + if (!seen.has(parsed.output)) { + seen.add(parsed.output); + columns.push(parsed.output); + if (filter) filters.set(parsed.output, filter); } } @@ -124,14 +196,6 @@ const parseColumnTemplate = ( * Validate a column-order template without extracting columns. * Returns null if valid, or an error message string if invalid. */ -export const validateColumnTemplate = ( - template: string, - validKeys: readonly string[], -): string | null => { - const result = parseColumnTemplate(template, validKeys); - return result.ok ? null : result.error; -}; - /** * Build a default template from an ordered list of column keys. * Produces e.g. "{{name}}, {{description}}, {{actions}}" @@ -143,18 +207,81 @@ export const buildDefaultTemplate = (keys: readonly string[]): string => * Parse a template and return column keys and filters. * Shared by all listing/attendee table renderers. */ -export const resolveColumnLayout = ( - template: string, - validKeys: readonly string[], - defaultOrder: readonly string[], -): { columnKeys: string[]; filters: Map } => { - if (!template) { - return { columnKeys: [...defaultOrder], filters: new Map() }; - } - const result = parseColumnTemplate(template, validKeys); - requireSuccess(result); - return { columnKeys: result.columns, filters: result.filters }; -}; +const defineColumnLayout = < + const TOptions extends readonly [string, ...string[]], +>( + schema: v.PicklistSchema, + defaultOrder: readonly TOptions[number][], +) => ({ + defaultOrder, + parse(template: string): ColumnLayout { + if (!template) { + return { + [validatedColumnLayout]: true, + columnKeys: defaultOrder, + filters: new Map(), + template, + }; + } + const result = parseColumnTemplate(template, schema); + requireSuccess(result); + return { + [validatedColumnLayout]: true, + columnKeys: result.columns, + filters: result.filters, + template, + }; + }, + schema, + validate(template: string): string | null { + if (!template) return null; + const result = parseColumnTemplate(template, schema); + return result.ok ? null : result.error; + }, +}); + +export const COLUMN_LAYOUTS = { + attendee: defineColumnLayout(AttendeeColumnSchema, [ + "status", + "date", + "name", + "listings", + "email", + "phone", + "address", + "special_instructions", + "answers", + "qty", + "ticket", + "registered", + ]), + "editor-listing": defineColumnLayout(EditorListingColumnSchema, [ + "name", + "description", + "status", + "attendees", + "tickets", + "created", + ]), + listing: defineColumnLayout(ListingColumnSchema, [ + "name", + "description", + "status", + "attendees", + "tickets", + "revenue", + "cost", + "profit", + "created", + ]), +} satisfies Record>; + +export type ListingColumnLayout = ReturnType< + (typeof COLUMN_LAYOUTS)["listing"]["parse"] +>; +export type AttendeeColumnLayout = ReturnType< + (typeof COLUMN_LAYOUTS)["attendee"]["parse"] +>; /** Matches only when `date` is the first filter applied to the raw value */ const FIRST_FILTER_IS_DATE_RE = /^[^|]*\|\s*date\b/; @@ -196,15 +323,16 @@ export const renderFilteredValue = ( */ export const renderCells = ( row: TRow, - columnKeys: string[], + columnKeys: readonly string[], generators: ColumnGenerators, opts: TOpts, - filters: Map, + filters: ReadonlyMap, escapeHtml: (s: string) => string, ): string => { const cells: string[] = []; for (const key of columnKeys) { - const col = generators[key]!; + const col = generators[key]; + if (col === undefined) throw new Error(`Unknown column: ${key}`); const filterExpr = filters.get(key); const useFilter = filterExpr && col.rawValue; const content = useFilter diff --git a/src/shared/columns/attendee-columns.ts b/src/shared/columns/attendee-columns.ts index 470a67432..846a1a147 100644 --- a/src/shared/columns/attendee-columns.ts +++ b/src/shared/columns/attendee-columns.ts @@ -7,7 +7,7 @@ import { t } from "#i18n"; import { attendeeAdminPath } from "#shared/attendee-links.ts"; -import type { ColumnDef, ColumnGenerators } from "#shared/column-order.ts"; +import type { AttendeeColumn, ColumnDef } from "#shared/column-order.ts"; import { formatDateLabel, formatDatetimeShort } from "#shared/dates.ts"; import { isServicing } from "#shared/db/attendees/kind.ts"; import { nonBlankLines } from "#shared/lines.ts"; @@ -218,10 +218,7 @@ const formatInstructionsInline = (instructions: string): string => { // --------------------------------------------------------------------------- /** All available attendee table columns */ -export const ATTENDEE_TABLE_COLUMNS: ColumnGenerators< - AttendeeTableRow, - AttendeeColumnOpts -> = { +export const ATTENDEE_TABLE_COLUMNS = { address, answers, date, @@ -234,20 +231,4 @@ export const ATTENDEE_TABLE_COLUMNS: ColumnGenerators< special_instructions, status, ticket, -}; - -/** Default column order for the attendee table */ -export const ATTENDEE_DEFAULT_ORDER = [ - "status", - "date", - "name", - "listings", - "email", - "phone", - "address", - "special_instructions", - "answers", - "qty", - "ticket", - "registered", -] as const; +} satisfies Record; diff --git a/src/shared/columns/listing-columns.ts b/src/shared/columns/listing-columns.ts index d0b09261d..e9ab0b2eb 100644 --- a/src/shared/columns/listing-columns.ts +++ b/src/shared/columns/listing-columns.ts @@ -6,7 +6,11 @@ * cell rendering, and guide documentation. */ -import type { ColumnDef, ColumnGenerators } from "#shared/column-order.ts"; +import type { + ColumnDef, + EditorListingColumn, + ListingColumn, +} from "#shared/column-order.ts"; import { formatCurrency } from "#shared/currency.ts"; import type { ListingWithCount } from "#shared/types.ts"; import { escapeHtml } from "#templates/layout.tsx"; @@ -125,7 +129,7 @@ const renewal: ListingCol = { // --------------------------------------------------------------------------- /** All available listing table columns */ -export const LISTING_TABLE_COLUMNS: ColumnGenerators = { +export const LISTING_TABLE_COLUMNS = { attendees, cost, created, @@ -139,45 +143,21 @@ export const LISTING_TABLE_COLUMNS: ColumnGenerators = { revenue, status, tickets, -}; - -/** Default column order for the listing table */ -export const LISTING_DEFAULT_ORDER = [ - "name", - "description", - "status", - "attendees", - "tickets", - "revenue", - "cost", - "profit", - "created", -] as const; +} satisfies Record; /** Listing columns shown to editors: the ledger-derived money columns * (revenue/cost/profit) are omitted entirely — not just unordered — so a saved * column template can never surface them, and the name links to the edit form * rather than the forbidden detail page. */ -export const EDITOR_LISTING_TABLE_COLUMNS: ColumnGenerators = - { - attendees, - created, - date, - description, - location, - name: editorName, - price, - renewal, - status, - tickets, - }; - -/** Default column order for the editor listing table (no money columns). */ -export const EDITOR_LISTING_DEFAULT_ORDER = [ - "name", - "description", - "status", - "attendees", - "tickets", - "created", -] as const; +export const EDITOR_LISTING_TABLE_COLUMNS = { + attendees, + created, + date, + description, + location, + name: editorName, + price, + renewal, + status, + tickets, +} satisfies Record; diff --git a/src/shared/day-names.ts b/src/shared/day-names.ts index ca522ae7a..ff5351af2 100644 --- a/src/shared/day-names.ts +++ b/src/shared/day-names.ts @@ -16,4 +16,12 @@ export const DAY_NAMES = [ ] as const; /** Valid day names for bookable_days (Monday-first for display) */ -export const VALID_DAY_NAMES = [...DAY_NAMES.slice(1), DAY_NAMES[0]!]; +export const VALID_DAY_NAMES = [ + "Monday", + "Tuesday", + "Wednesday", + "Thursday", + "Friday", + "Saturday", + "Sunday", +] as const; diff --git a/src/shared/db/settings.ts b/src/shared/db/settings.ts index 3bce3a659..29a4d323d 100644 --- a/src/shared/db/settings.ts +++ b/src/shared/db/settings.ts @@ -34,6 +34,11 @@ import { type EnabledFeatures, parseEnabledFeatures, } from "#shared/admin-features.ts"; +import { + type AttendeeColumnLayout, + COLUMN_LAYOUTS, + type ListingColumnLayout, +} from "#shared/column-order.ts"; import { encrypt } from "#shared/crypto/encryption.ts"; import { boolUpdate, @@ -150,6 +155,9 @@ const settingsBase = { }, // --- Apple Wallet --- appleWallet: appleWallet.createReadSettings(snap as (k: string) => string), + get attendeeColumnLayout(): AttendeeColumnLayout { + return COLUMN_LAYOUTS.attendee.parse(snap("attendee_column_order")); + }, get autoPurgeOrphans(): boolean { return snap("auto_purge_orphans"); }, @@ -231,6 +239,9 @@ const settingsBase = { // --- Google Wallet --- googleWallet: googleWallet.createReadSettings(snap as (k: string) => string), invalidateCache, + get listingColumnLayout(): ListingColumnLayout { + return COLUMN_LAYOUTS.listing.parse(snap("listing_column_order")); + }, get listingDefaults(): ListingDefaults { return parseListingDefaults(snap(CONFIG_KEYS.LISTING_DEFAULTS)); }, diff --git a/src/shared/forms/definition.ts b/src/shared/forms/definition.ts index 369e2dab8..f4fb0753c 100644 --- a/src/shared/forms/definition.ts +++ b/src/shared/forms/definition.ts @@ -66,6 +66,18 @@ type FormSectionId = : never : never; +const requiredDefinition = ( + definitions: ReadonlyMap, + kind: "field" | "section", + id: string, +): T => { + const definition = definitions.get(id); + if (definition === undefined) { + throw new Error(`Unknown ${kind}: ${id}`); + } + return definition; +}; + export interface FormSchema { fields: readonly Field[]; validate: (form: FormParams) => ValidationResult; @@ -124,12 +136,12 @@ export const defineForm = < ), ), ] as FormSectionId[]; - - const fieldByName = (name: TFields[number]["name"]): Field => { - const field = fieldMap.get(name); - if (!field) throw new Error(`Unknown field: ${name}`); - return field; - }; + const sectionMap = new Map( + sectionIds.map((id) => [ + id, + fields.filter((field) => field.section === id), + ]), + ); const validate = ( form: FormParams, @@ -156,17 +168,16 @@ export const defineForm = < const section = ( id: FormSectionId, values: FormRenderValuesFor = {}, - ): string => { - if (!sectionIds.includes(id)) throw new Error(`Unknown section: ${id}`); - return renderFields( - fields.filter((field) => field.section === id), + ): string => + renderFields( + requiredDefinition(sectionMap, "section", id), values as FieldValues, ); - }; return { field: (name) => ({ - render: (value = "") => renderField(fieldByName(name), value), + render: (value = "") => + renderField(requiredDefinition(fieldMap, "field", name), value), }), fields: config.fields, id: config.id, diff --git a/src/shared/forms/repeated-picklist.ts b/src/shared/forms/repeated-picklist.ts new file mode 100644 index 000000000..f29a1fc54 --- /dev/null +++ b/src/shared/forms/repeated-picklist.ts @@ -0,0 +1,31 @@ +import * as v from "valibot"; +import type { FormParams } from "#shared/form-data.ts"; + +type PicklistOptions = readonly [string, ...string[]]; + +export type RepeatedPicklistValue = + | { state: "disabled" } + | { state: "absent" } + | { state: "invalid"; value: string } + | { state: "selected"; values: TValue[] }; + +/** Read a repeated form field against one ordered picklist schema. */ +export const readRepeatedPicklist = ( + schema: v.PicklistSchema, + form: FormParams, + name: string, + enabled = true, +): RepeatedPicklistValue => { + if (!enabled) return { state: "disabled" }; + if (!form.has(name)) return { state: "absent" }; + + const supplied = form.getAll(name); + const invalid = supplied.find((value) => !v.safeParse(schema, value).success); + if (invalid !== undefined) return { state: "invalid", value: invalid }; + + const selected = new Set(supplied); + return { + state: "selected", + values: schema.options.filter((value) => selected.has(value)), + }; +}; diff --git a/src/shared/forms/saved-data.ts b/src/shared/forms/saved-data.ts index ae34e0881..96cb01686 100644 --- a/src/shared/forms/saved-data.ts +++ b/src/shared/forms/saved-data.ts @@ -33,5 +33,6 @@ export const savedFormValue = (name: string): string => export const getSavedFieldValue = (field: Field): string => { const form = savedFormScope.current().form; if (!form || SENSITIVE_FIELD_TYPES.has(field.type)) return ""; + if (field.type === "checkbox-group") return form.getAll(field.name).join(","); return readSubmittedFieldValue(form, field) ?? ""; }; diff --git a/src/shared/forms/submitted-value.ts b/src/shared/forms/submitted-value.ts index 2ecba8255..d84c2e4dc 100644 --- a/src/shared/forms/submitted-value.ts +++ b/src/shared/forms/submitted-value.ts @@ -13,21 +13,11 @@ const getDatetimeValue = (form: FormParams, name: string): string | null => { return null; }; -const parseCheckboxGroup = (form: FormParams, name: string): string => - form - .getAll(name) - .map((value) => value.trim()) - .filter((value) => value) - .join(","); - /** Read one field from submitted form data using the field's input shape. */ export const readSubmittedFieldValue = ( form: FormParams, field: Field, ): string | null => { - if (field.type === "checkbox-group") { - return parseCheckboxGroup(form, field.name); - } if (field.type === "datetime") { return getDatetimeValue(form, field.name); } diff --git a/src/shared/forms/validation.ts b/src/shared/forms/validation.ts index 196a70fd6..ad38001d0 100644 --- a/src/shared/forms/validation.ts +++ b/src/shared/forms/validation.ts @@ -2,6 +2,7 @@ import * as v from "valibot"; import { t } from "#i18n"; import type { FormParams } from "#shared/form-data.ts"; import type { ChoiceField, Field } from "#shared/forms/field.ts"; +import { readRepeatedPicklist } from "#shared/forms/repeated-picklist.ts"; import { DATETIME_PARTIAL_ERROR, readSubmittedFieldValue, @@ -41,22 +42,23 @@ const choiceSchema = ( }; const hasInvalidChoice = ( - field: ChoiceField<"select" | "checkbox-group">, + field: ChoiceField<"select">, value: string, -): boolean => { - const values = - field.type === "checkbox-group" - ? value.split(",").map((choice) => choice.trim()) - : [value]; - const schema = choiceSchema(field); - return values.some((choice) => !v.safeParse(schema, choice).success); -}; +): boolean => !v.safeParse(choiceSchema(field), value).success; const requiredFieldError = (field: Field): FieldValidationResult => ({ error: field.requiredMessage ?? `${field.label} is required`, valid: false, }); +const invalidFieldMessage = (field: Field): string => + field.invalidMessage ?? t("error.field_invalid", { label: field.label }); + +const invalidFieldError = (field: Field): FieldValidationResult => ({ + error: invalidFieldMessage(field), + valid: false, +}); + const isUnusableParsedValue = (value: string | number | null): boolean => value === null || (typeof value === "number" && !Number.isFinite(value)); @@ -69,19 +71,29 @@ const collectFieldValue = ( return raw; }; +const validateCheckboxField = ( + form: FormParams, + field: ChoiceField<"checkbox-group">, +): FieldValidationResult => { + const selection = readRepeatedPicklist(choiceSchema(field), form, field.name); + if (selection.state === "invalid") { + const error = field.validate?.(selection.value); + return error ? { error, valid: false } : invalidFieldError(field); + } + const value = + selection.state === "selected" ? selection.values.join(",") : ""; + if (field.required && !value) return requiredFieldError(field); + const error = value ? field.validate?.(value) : undefined; + return error ? { error, valid: false } : { valid: true, value }; +}; + const validateFieldText = (field: Field, value: string): string | null => { if (field.validate && value) { const error = field.validate(value); if (error) return error; } - if ( - value && - (field.type === "select" || field.type === "checkbox-group") && - hasInvalidChoice(field, value) - ) { - return ( - field.invalidMessage ?? t("error.field_invalid", { label: field.label }) - ); + if (value && field.type === "select" && hasInvalidChoice(field, value)) { + return invalidFieldMessage(field); } if ( "maxlength" in field && @@ -99,6 +111,10 @@ const validateSingleField = ( ): FieldValidationResult => { if (field.type === "file") return { valid: true, value: null }; + if (field.type === "checkbox-group") { + return validateCheckboxField(form, field); + } + const collected = collectFieldValue(form, field); if (typeof collected !== "string") return collected; @@ -107,16 +123,11 @@ const validateSingleField = ( if (field.required && !trimmed) return requiredFieldError(field); const textError = validateFieldText(field, trimmed); - if (textError) return { error: textError, valid: false }; + if (textError !== null) return { error: textError, valid: false }; const value = parseFieldValue(field, trimmed); if (trimmed && isUnusableParsedValue(value)) { - return { - error: - field.invalidMessage ?? - t("error.field_invalid", { label: field.label }), - valid: false, - }; + return invalidFieldError(field); } return { valid: true, value }; }; diff --git a/src/ui/templates/admin/dashboard.tsx b/src/ui/templates/admin/dashboard.tsx index 25540521b..a7d540808 100644 --- a/src/ui/templates/admin/dashboard.tsx +++ b/src/ui/templates/admin/dashboard.tsx @@ -5,11 +5,12 @@ import { filter, joinStrings, map, pipe, unique } from "#fp"; import { t } from "#i18n"; import { groupAttendeeRows } from "#shared/attendee-table-rows.ts"; -import { resolveColumnLayout } from "#shared/column-order.ts"; import { - EDITOR_LISTING_DEFAULT_ORDER, + COLUMN_LAYOUTS, + type ListingColumnLayout, +} from "#shared/column-order.ts"; +import { EDITOR_LISTING_TABLE_COLUMNS, - LISTING_DEFAULT_ORDER, LISTING_TABLE_COLUMNS, } from "#shared/columns/listing-columns.ts"; import { getEffectiveDomain } from "#shared/config.ts"; @@ -223,18 +224,15 @@ export const adminDashboardPage = ( newestAttendees: DisplayAttendee[] = [], successMessage?: string, stats?: ActiveListingStats | null, - listingColumnTemplate?: string, + listingColumnLayout?: ListingColumnLayout, activeType: ListingFilter = "all", upcomingHolidays: Holiday[] = [], unbookableIds: ReadonlySet = new Set(), upcomingServicingEvents: ServicingEventSummary[] = [], attributeFilterView: ListingAttributeFilterView = emptyAttributeFilterView(), ): string => { - const { columnKeys, filters } = resolveColumnLayout( - listingColumnTemplate ?? "", - Object.keys(LISTING_TABLE_COLUMNS), - LISTING_DEFAULT_ORDER, - ); + const { columnKeys, filters } = + listingColumnLayout ?? COLUMN_LAYOUTS.listing.parse(""); // Type filter narrows the listing table only; the stats, multi-booking, and // newest-attendee sections below stay based on the full set. Offer the bar @@ -312,7 +310,7 @@ export const adminDashboardPage = ( export const adminListingsPage = ( listings: ListingWithCount[], session: AdminSession, - listingColumnTemplate?: string, + listingColumnLayout?: ListingColumnLayout, attributeFilterView: ListingAttributeFilterView = emptyAttributeFilterView(), ): string => { // Editors see a money-free, edit-linked table on a fixed order (their saved @@ -322,11 +320,9 @@ export const adminListingsPage = ( const columns = isEditor ? EDITOR_LISTING_TABLE_COLUMNS : LISTING_TABLE_COLUMNS; - const { columnKeys, filters } = resolveColumnLayout( - isEditor ? "" : (listingColumnTemplate ?? ""), - Object.keys(columns), - isEditor ? EDITOR_LISTING_DEFAULT_ORDER : LISTING_DEFAULT_ORDER, - ); + const { columnKeys, filters } = isEditor + ? COLUMN_LAYOUTS["editor-listing"].parse("") + : (listingColumnLayout ?? COLUMN_LAYOUTS.listing.parse("")); const activeListings = activeOnly(listings); const deactivatedListings = filter((e: ListingWithCount) => !e.active)( listings, diff --git a/src/ui/templates/admin/groups/overview.tsx b/src/ui/templates/admin/groups/overview.tsx index 3e62fdc5e..32b170598 100644 --- a/src/ui/templates/admin/groups/overview.tsx +++ b/src/ui/templates/admin/groups/overview.tsx @@ -1,11 +1,6 @@ import { sumOf } from "#fp"; import { t } from "#i18n"; import type { ListingMoneyTotals } from "#shared/accounting/listing-money-totals.ts"; -import { resolveColumnLayout } from "#shared/column-order.ts"; -import { - LISTING_DEFAULT_ORDER, - LISTING_TABLE_COLUMNS, -} from "#shared/columns/listing-columns.ts"; import { settings } from "#shared/db/settings.ts"; import { buildEmbedSnippets } from "#shared/embed.ts"; import { isReadOnly } from "#shared/env.ts"; @@ -183,11 +178,7 @@ export const GroupOverviewPanel = ({ shareable: boolean; questionData?: TableQuestionData; }): JSX.Element => { - const { columnKeys, filters } = resolveColumnLayout( - settings.listingColumnOrder, - Object.keys(LISTING_TABLE_COLUMNS), - LISTING_DEFAULT_ORDER, - ); + const { columnKeys, filters } = settings.listingColumnLayout; const listingRows = renderListingRows({ columnKeys, emptyText: t("groups.detail.no_listings"), diff --git a/src/ui/templates/admin/guide/operations.tsx b/src/ui/templates/admin/guide/operations.tsx index 32c1dbe0c..38d99af43 100644 --- a/src/ui/templates/admin/guide/operations.tsx +++ b/src/ui/templates/admin/guide/operations.tsx @@ -2,15 +2,9 @@ * Admin guide — Operations sections. */ -import { buildDefaultTemplate } from "#shared/column-order.ts"; -import { - ATTENDEE_DEFAULT_ORDER, - ATTENDEE_TABLE_COLUMNS, -} from "#shared/columns/attendee-columns.ts"; -import { - LISTING_DEFAULT_ORDER, - LISTING_TABLE_COLUMNS, -} from "#shared/columns/listing-columns.ts"; +import { buildDefaultTemplate, COLUMN_LAYOUTS } from "#shared/column-order.ts"; +import { ATTENDEE_TABLE_COLUMNS } from "#shared/columns/attendee-columns.ts"; +import { LISTING_TABLE_COLUMNS } from "#shared/columns/listing-columns.ts"; import { Raw } from "#shared/jsx/jsx-runtime.ts"; import { columnReferenceTable, @@ -139,14 +133,14 @@ export const operationsSections = (): GuideSection[] => [ custom( "listing_table_columns", <> - {defaultOrderParagraph(LISTING_DEFAULT_ORDER)} + {defaultOrderParagraph(COLUMN_LAYOUTS.listing.defaultOrder)} , ), custom( "attendee_table_columns", <> - {defaultOrderParagraph(ATTENDEE_DEFAULT_ORDER)} + {defaultOrderParagraph(COLUMN_LAYOUTS.attendee.defaultOrder)}

Columns referencing absent data (e.g. {"{{email}}"}{" "} when no attendees have an email) are hidden automatically even when diff --git a/src/ui/templates/admin/listing-table.tsx b/src/ui/templates/admin/listing-table.tsx index 7cf6b3952..b52f0bd8d 100644 --- a/src/ui/templates/admin/listing-table.tsx +++ b/src/ui/templates/admin/listing-table.tsx @@ -23,8 +23,8 @@ import { escapeHtml } from "#templates/layout.tsx"; /** The ordered column layout a listings table (or single row) renders from. */ type ListingColumnArgs = { - columnKeys: string[]; - filters: Map; + columnKeys: readonly string[]; + filters: ReadonlyMap; columns?: ColumnGenerators; }; @@ -51,7 +51,7 @@ export const ListingRow = ({ /** The subset of `columnKeys` that the given column set actually defines. */ export const validListingColumnKeys = ( - columnKeys: string[], + columnKeys: readonly string[], columns: ColumnGenerators, ): string[] => columnKeys.filter((key) => columns[key]); @@ -82,7 +82,7 @@ export const renderListingRows = ({ /** Render the listing table with dynamic column keys. `columns` defaults to the * staff column set; the editor variant passes its money-free set. */ export const renderListingTable = ( - columnKeys: string[], + columnKeys: readonly string[], rows: string, columns: ColumnGenerators = LISTING_TABLE_COLUMNS, ): string => { @@ -95,8 +95,8 @@ export const renderListingTable = ( export const renderListingsTableSection = ( listings: ListingWithCount[], - columnKeys: string[], - filters: Map, + columnKeys: readonly string[], + filters: ReadonlyMap, columns: ColumnGenerators = LISTING_TABLE_COLUMNS, ): string => { const listingRows = renderListingRows({ diff --git a/src/ui/templates/admin/settings/column-order.tsx b/src/ui/templates/admin/settings/column-order.tsx index 4427e4f92..259147dc5 100644 --- a/src/ui/templates/admin/settings/column-order.tsx +++ b/src/ui/templates/admin/settings/column-order.tsx @@ -8,23 +8,21 @@ /* jscpd:ignore-start */ import { t } from "#i18n"; -import { buildDefaultTemplate } from "#shared/column-order.ts"; -import { - ATTENDEE_DEFAULT_ORDER, - ATTENDEE_TABLE_COLUMNS, -} from "#shared/columns/attendee-columns.ts"; -import { - LISTING_DEFAULT_ORDER, - LISTING_TABLE_COLUMNS, -} from "#shared/columns/listing-columns.ts"; +import { buildDefaultTemplate, COLUMN_LAYOUTS } from "#shared/column-order.ts"; +import { ATTENDEE_TABLE_COLUMNS } from "#shared/columns/attendee-columns.ts"; +import { LISTING_TABLE_COLUMNS } from "#shared/columns/listing-columns.ts"; import { Raw } from "#shared/jsx/jsx-runtime.ts"; import type { AdvancedSettingsPageState } from "#templates/admin/settings-advanced.tsx"; import { textSettingsSection } from "#templates/components/settings-field-section.tsx"; /* jscpd:ignore-end */ -const listingDefault = buildDefaultTemplate(LISTING_DEFAULT_ORDER); -const attendeeDefault = buildDefaultTemplate(ATTENDEE_DEFAULT_ORDER); +const listingDefault = buildDefaultTemplate( + COLUMN_LAYOUTS.listing.defaultOrder, +); +const attendeeDefault = buildDefaultTemplate( + COLUMN_LAYOUTS.attendee.defaultOrder, +); /** Render available column tags as helper text */ const AvailableTags = ({ diff --git a/src/ui/templates/attendee-table.tsx b/src/ui/templates/attendee-table.tsx index bb8050ecf..5d5af2083 100644 --- a/src/ui/templates/attendee-table.tsx +++ b/src/ui/templates/attendee-table.tsx @@ -15,14 +15,12 @@ import { joinStrings, map, pipe, sort } from "#fp"; import { t } from "#i18n"; import { + type AttendeeColumn, + type AttendeeColumnLayout, getHeaderText, renderCells, - resolveColumnLayout, } from "#shared/column-order.ts"; -import { - ATTENDEE_DEFAULT_ORDER, - ATTENDEE_TABLE_COLUMNS, -} from "#shared/columns/attendee-columns.ts"; +import { ATTENDEE_TABLE_COLUMNS } from "#shared/columns/attendee-columns.ts"; import { isServicing } from "#shared/db/attendees/kind.ts"; import type { QuestionWithAnswers } from "#shared/db/question-types.ts"; import type { AttendeeQuestionData } from "#shared/db/questions/attendee-answers/reads.ts"; @@ -76,7 +74,7 @@ export type AttendeeTableOptions = { /** Question data for the Answers column */ questionData?: TableQuestionData | undefined; /** Liquid template controlling column order (e.g. "{{name}}, {{email}}, {{qty}}") */ - columnTemplate?: string; + columnLayout?: AttendeeColumnLayout; }; // --------------------------------------------------------------------------- @@ -87,7 +85,7 @@ export type AttendeeTableOptions = { const computeVisibilityMap = ( rows: AttendeeTableRow[], opts: AttendeeTableOptions, -): Record => { +): Record => { const showCheckin = opts.showCheckin !== false; return { address: rows.some((r) => !!r.attendee.address), @@ -111,15 +109,13 @@ const computeVisibilityMap = ( /** Get the ordered list of visible column keys and their filter expressions */ const getColumnLayout = ( - visMap: Record, - columnTemplate?: string, -): { visibleColumns: string[]; filters: Map } => { - const template = columnTemplate || settings.attendeeColumnOrder; - const { columnKeys, filters } = resolveColumnLayout( - template, - Object.keys(ATTENDEE_TABLE_COLUMNS), - ATTENDEE_DEFAULT_ORDER, - ); + visMap: Record, + columnLayout?: AttendeeColumnLayout, +): { + visibleColumns: AttendeeColumn[]; + filters: ReadonlyMap; +} => { + const { columnKeys, filters } = columnLayout ?? settings.attendeeColumnLayout; return { filters, visibleColumns: columnKeys.filter((k) => visMap[k]), @@ -259,9 +255,9 @@ const createStatusRenderer = /** Render a single attendee row using ordered column defs */ const AttendeeRow = ( row: AttendeeTableRow, - visibleColumns: string[], + visibleColumns: AttendeeColumn[], colOpts: AttendeeColumnOpts, - filters: Map, + filters: ReadonlyMap, ): string => ` { const visMap = computeVisibilityMap(orderedRows, opts); const { visibleColumns, filters } = getColumnLayout( visMap, - opts.columnTemplate, + opts.columnLayout, ); const colCount = visibleColumns.length; @@ -319,8 +315,8 @@ export const AttendeeTable = (opts: AttendeeTableOptions): string => { }`; const headers = pipe( - map((key: string) => { - const col = ATTENDEE_TABLE_COLUMNS[key]!; + map((key: AttendeeColumn) => { + const col = ATTENDEE_TABLE_COLUMNS[key]; const cls = col.headerClassName; return `${getHeaderText(col)}`; }), diff --git a/test/features/admin/aggregate-recalculation.test.ts b/test/features/admin/aggregate-recalculation.test.ts new file mode 100644 index 000000000..156ea090e --- /dev/null +++ b/test/features/admin/aggregate-recalculation.test.ts @@ -0,0 +1,153 @@ +import { expect } from "@std/expect"; +import { describe, it as test } from "@std/testing/bdd"; +import { + createRecalculatePageRenderer, + parseEditableAggregateForm, + runRecalculatePost, + selectedRecalculationFields, +} from "#routes/admin/aggregate-recalculation.ts"; +import { parseFlashValue } from "#shared/cookies.ts"; +import { FormParams } from "#shared/form-data.ts"; +import type { Field } from "#shared/forms/field.ts"; +import { runWithRequestId } from "#shared/logger.ts"; + +const FIELDS = ["income", "tickets"] as const; + +describe("selectedRecalculationFields", () => { + test("returns selected fields in their declared order", () => { + expect( + selectedRecalculationFields( + new FormParams("recalculate_fields=tickets&recalculate_fields=income"), + FIELDS, + ), + ).toEqual(["income", "tickets"]); + }); + + test("returns no fields when none were submitted", () => { + expect(selectedRecalculationFields(new FormParams(), FIELDS)).toEqual([]); + }); + + test("rejects the first unknown field", () => { + expect(() => + selectedRecalculationFields( + new FormParams("recalculate_fields=income&recalculate_fields=bogus"), + FIELDS, + ), + ).toThrow("Invalid recalculation field: bogus"); + }); +}); + +const editableFields: Field[] = [{ label: "Name", name: "name", type: "text" }]; + +describe("parseEditableAggregateForm", () => { + test("skips an aggregate whose fields were not submitted", () => { + expect( + parseEditableAggregateForm( + new FormParams({ other: "value" }), + editableFields, + (values) => values, + ), + ).toEqual({ input: null, ok: true }); + }); + + test("parses an aggregate when one of its fields was submitted", () => { + expect( + parseEditableAggregateForm<{ name: string }, string>( + new FormParams({ name: "New name" }), + editableFields, + (values) => values.name, + ), + ).toEqual({ input: "New name", ok: true }); + }); +}); + +describe("runRecalculatePost", () => { + test("renders the choice error without changing aggregates", async () => { + let resetCount = 0; + let logCount = 0; + const response = await runRecalculatePost({ + fields: FIELDS, + form: new FormParams(), + log: () => { + logCount += 1; + return Promise.resolve(); + }, + renderChoose: () => new Response("Choose a field", { status: 400 }), + reset: () => { + resetCount += 1; + return Promise.resolve(); + }, + successMessage: "Recalculated.", + successPath: "/admin/listings/1/edit", + }); + + expect(response.status).toBe(400); + expect(await response.text()).toBe("Choose a field"); + expect(resetCount).toBe(0); + expect(logCount).toBe(0); + }); + + test("resets selected aggregates, logs, and redirects", async () => { + const calls: string[] = []; + const response = await runWithRequestId(() => + runRecalculatePost({ + fields: FIELDS, + form: new FormParams("recalculate_fields=tickets"), + log: () => { + calls.push("log"); + return Promise.resolve(); + }, + renderChoose: () => new Response("wrong branch", { status: 400 }), + reset: (selected) => { + calls.push(`reset:${selected.join(",")}`); + return Promise.resolve(); + }, + successMessage: "Recalculated.", + successPath: "/admin/listings/1/edit", + }), + ); + + expect(calls).toEqual(["reset:tickets", "log"]); + expect(response.status).toBe(302); + expect(response.headers.get("location")).toMatch( + /^\/admin\/listings\/1\/edit\?flash=[a-f0-9]+$/, + ); + const cookieValue = response.headers + .get("set-cookie") + ?.split("=")[1] + ?.split(";")[0]; + if (cookieValue === undefined) throw new Error("Missing flash cookie"); + expect(parseFlashValue(cookieValue)).toEqual({ + error: undefined, + formToken: undefined, + info: undefined, + result: undefined, + success: "Recalculated.", + }); + }); +}); + +describe("createRecalculatePageRenderer", () => { + test("renders the snapshot and success response", async () => { + const render = createRecalculatePageRenderer( + (entity: { id: number }) => Promise.resolve(`snapshot-${entity.id}`), + (entity, snapshot, session: string, error, success) => + [entity.id, snapshot, session, error, success].join("|"), + ); + const response = await render({ id: 3 }, "owner", undefined, "Saved."); + + expect(response.status).toBe(200); + expect(await response.text()).toBe("3|snapshot-3|owner||Saved."); + }); + + test("uses status 400 when rendering an error", async () => { + const render = createRecalculatePageRenderer( + () => Promise.resolve("snapshot"), + (_entity: string, _snapshot, _session: string, error) => error ?? "", + ); + const response = await render("entity", "owner", "Choose a field"); + + expect(response.status).toBe(400); + expect(await response.text()).toBe("Choose a field"); + }); +}); diff --git a/test/features/admin/settings-listing-defaults.test.ts b/test/features/admin/settings-listing-defaults.test.ts index 7cce6d34a..b26c2bb60 100644 --- a/test/features/admin/settings-listing-defaults.test.ts +++ b/test/features/admin/settings-listing-defaults.test.ts @@ -1,6 +1,7 @@ import { expect } from "@std/expect"; import { describe, it as test } from "@std/testing/bdd"; import { parseListingDefaultsForm } from "#routes/admin/settings-listing-defaults.ts"; +import { setDemoModeForTest } from "#shared/demo/mode.ts"; import { FormParams } from "#shared/form-data.ts"; const parse = (query: string, hasLogistics = true) => @@ -58,6 +59,22 @@ describe("admin > listing defaults form parse > urls", () => { expect(parse("default_webhook_url=ftp://example.com").error).not.toBeNull(); expect(parse("default_thank_you_url=not-a-url").error).not.toBeNull(); }); + + test("ignores the webhook default in demo mode", () => { + setDemoModeForTest(true); + try { + expect( + "webhookUrl" in + ok(parse("default_webhook_url=https://example.com/hook")), + ).toBe(false); + expect( + ok(parse("default_thank_you_url=https://example.com/thanks")) + .thankYouUrl, + ).toBe("https://example.com/thanks"); + } finally { + setDemoModeForTest(false); + } + }); }); describe("admin > listing defaults form parse > bookable days", () => { @@ -81,7 +98,17 @@ describe("admin > listing defaults form parse > bookable days", () => { "default_bookable_days_enabled=1&default_bookable_days=Funday&default_bookable_days=Monday", ).error; expect(error).toContain("Invalid day: Funday"); - expect(error).toContain("Monday"); + expect(error).toContain( + "Use: Monday, Tuesday, Wednesday, Thursday, Friday, Saturday, Sunday", + ); + }); + + test("reports the first malformed day supplied", () => { + const error = parse( + "default_bookable_days_enabled=1&default_bookable_days=Monday&default_bookable_days=Funday&default_bookable_days=Someday", + ).error; + expect(error).toContain("Invalid day: Funday"); + expect(error).not.toContain("Invalid day: Someday"); }); test("rejects enabled with no valid day chosen", () => { diff --git a/test/integration/server/settings/column-order.test.ts b/test/integration/server/settings/column-order.test.ts index 621a74f7c..b04ea0a98 100644 --- a/test/integration/server/settings/column-order.test.ts +++ b/test/integration/server/settings/column-order.test.ts @@ -1,6 +1,7 @@ import { expect } from "@std/expect"; import { describe, it as test } from "@std/testing/bdd"; -import { settings } from "#shared/db/settings.ts"; +import { execute } from "#shared/db/client.ts"; +import { CONFIG_KEYS, settings } from "#shared/db/settings.ts"; import { expectFlashRedirect } from "#test-utils/assertions.ts"; import { describeWithEnv } from "#test-utils/db.ts"; import { adminFormPost } from "#test-utils/session.ts"; @@ -20,6 +21,13 @@ describeWithEnv("server (admin settings: column order)", { db: true }, () => { "Listing column order updated", )(response); expect(settings.listingColumnOrder).toBe("{{name}}, {{status}}"); + expect(settings.listingColumnLayout.columnKeys).toEqual([ + "name", + "status", + ]); + expect(settings.listingColumnLayout.template).toBe( + "{{name}}, {{status}}", + ); }); test("rejects invalid column name", async () => { @@ -44,6 +52,20 @@ describeWithEnv("server (admin settings: column order)", { db: true }, () => { "Listing column order updated", )(response); expect(settings.listingColumnOrder).toBe(""); + expect(settings.listingColumnLayout.columnKeys[0]).toBe("name"); + }); + + test("rejects a malformed template loaded from settings", async () => { + await execute( + "INSERT OR REPLACE INTO settings (key, value) VALUES (?, ?)", + [CONFIG_KEYS.LISTING_COLUMN_ORDER, "{{not_a_column}}"], + ); + settings.invalidateCache(); + await settings.loadKeys([CONFIG_KEYS.LISTING_COLUMN_ORDER]); + + expect(() => settings.listingColumnLayout).toThrow( + 'Unknown column "not_a_column"', + ); }); }); @@ -63,6 +85,11 @@ describeWithEnv("server (admin settings: column order)", { db: true }, () => { expect(settings.attendeeColumnOrder).toBe( "{{name}}, {{qty}}, {{ticket}}", ); + expect(settings.attendeeColumnLayout.columnKeys).toEqual([ + "name", + "qty", + "ticket", + ]); }); test("rejects invalid column name", async () => { diff --git a/test/lib/server-settings/listing-defaults.test.ts b/test/integration/server/settings/listing-defaults.test.ts similarity index 99% rename from test/lib/server-settings/listing-defaults.test.ts rename to test/integration/server/settings/listing-defaults.test.ts index dbdd9ec1f..0d4be262b 100644 --- a/test/lib/server-settings/listing-defaults.test.ts +++ b/test/integration/server/settings/listing-defaults.test.ts @@ -116,6 +116,9 @@ describeWithEnv("server (admin listing defaults)", { db: true }, () => { default_webhook_url: "https://example.com/hook", }); expect(response.status).toBe(302); + expect(response.headers.get("location")).toMatch( + /^\/admin\/listing-defaults\?flash=/, + ); expectFlash(response, expect.stringContaining("Listing defaults saved")); expect(settings.listingDefaults).toEqual({ hidden: true, diff --git a/test/lib/server-editor.test.ts b/test/lib/server-editor.test.ts index cc13004ea..00a0a5865 100644 --- a/test/lib/server-editor.test.ts +++ b/test/lib/server-editor.test.ts @@ -21,6 +21,7 @@ import { createTestSitePage, } from "#test-utils/db-helpers/misc.ts"; import { testListingInput } from "#test-utils/factories.ts"; +import type { TestFormValues } from "#test-utils/form-values.ts"; import { awaitTestRequest, mockFormRequest, @@ -51,7 +52,7 @@ const postFormAs = async ( const postMultipartAs = async ( path: string, cookie: string, - data: Record, + data: TestFormValues, ): Promise => { const csrf_token = await signCsrfToken(); return handleRequest( diff --git a/test/lib/server-images/attachment-edit.test.ts b/test/lib/server-images/attachment-edit.test.ts index e58c91a15..34ac64e90 100644 --- a/test/lib/server-images/attachment-edit.test.ts +++ b/test/lib/server-images/attachment-edit.test.ts @@ -10,6 +10,7 @@ import { runWithStorageConfig } from "#shared/storage.ts"; import { expectFlashRedirect } from "#test-utils/assertions.ts"; import { describeWithEnv } from "#test-utils/db.ts"; import { PDF_BYTES } from "#test-utils/factories.ts"; +import type { TestFormValues } from "#test-utils/form-values.ts"; import { TEST_STORAGE_ZONE } from "#test-utils/internal.ts"; import { installUrlHandler, @@ -25,18 +26,18 @@ import { expectImageErrorRedirect } from "./helpers.ts"; const editFormData = async ( listingId: number, csrfToken: string, -): Promise> => { +): Promise => { const listing = await getListingWithCount(listingId); if (!listing) throw new Error(`Listing not found: ${listingId}`); return { - bookable_days: listing.bookable_days.join(","), + bookable_days: listing.bookable_days, closes_at_date: "", closes_at_time: "", csrf_token: csrfToken, date_date: "", date_time: "", description: listing.description, - fields: listing.fields || "email", + fields: (listing.fields || "email").split(","), listing_type: listing.listing_type, location: listing.location, max_attendees: String(listing.max_attendees), diff --git a/test/lib/server-images/create.test.ts b/test/lib/server-images/create.test.ts index dd4c5a38c..a0ee772d5 100644 --- a/test/lib/server-images/create.test.ts +++ b/test/lib/server-images/create.test.ts @@ -6,6 +6,7 @@ import type { ListingWithCount } from "#shared/types.ts"; import { expectFlashRedirect } from "#test-utils/assertions.ts"; import { describeWithEnv } from "#test-utils/db.ts"; import { PDF_BYTES } from "#test-utils/factories.ts"; +import type { TestFormValues } from "#test-utils/form-values.ts"; import { mockMultipartRequest, withStorageMock } from "#test-utils/mocks.ts"; import { testCookie, testCsrfToken } from "#test-utils/session.ts"; import { makeTestPng } from "#test-utils/test-image.ts"; @@ -14,15 +15,15 @@ import { makeTestPng } from "#test-utils/test-image.ts"; const newListingFormFields = ( csrfToken: string, name: string, -): Record => ({ - bookable_days: "Monday,Tuesday,Wednesday,Thursday,Friday", +): TestFormValues => ({ + bookable_days: ["Monday", "Tuesday", "Wednesday", "Thursday", "Friday"], closes_at_date: "", closes_at_time: "", csrf_token: csrfToken, date_date: "", date_time: "", description: "", - fields: "email", + fields: ["email"], listing_type: "standard", location: "", max_attendees: "50", diff --git a/test/shared/column-order.test.ts b/test/shared/column-order.test.ts index 300a54a3f..e8cdd7e45 100644 --- a/test/shared/column-order.test.ts +++ b/test/shared/column-order.test.ts @@ -3,18 +3,13 @@ import { describe, it as test } from "@std/testing/bdd"; import type { ColumnDef, ColumnGenerators } from "#shared/column-order.ts"; import { buildDefaultTemplate, + COLUMN_LAYOUTS, getHeaderText, renderCells, renderFilteredValue, - resolveColumnLayout, - validateColumnTemplate, } from "#shared/column-order.ts"; import { - ATTENDEE_DEFAULT_ORDER, - ATTENDEE_TABLE_COLUMNS, -} from "#shared/columns/attendee-columns.ts"; -import { - LISTING_DEFAULT_ORDER, + EDITOR_LISTING_TABLE_COLUMNS, LISTING_TABLE_COLUMNS, } from "#shared/columns/listing-columns.ts"; import { escapeHtml } from "#templates/layout.tsx"; @@ -23,134 +18,107 @@ import { testListingWithCount } from "#test-utils/factories.ts"; setupTestEncryptionKey(); -const VALID_LISTING_KEYS = Object.keys(LISTING_TABLE_COLUMNS); -const VALID_ATTENDEE_KEYS = Object.keys(ATTENDEE_TABLE_COLUMNS); +describe("column layout validation", () => { + test("derives every listing schema from the matching column definitions", () => { + expect(COLUMN_LAYOUTS.listing.schema.options).toEqual( + Object.keys(LISTING_TABLE_COLUMNS), + ); + expect(COLUMN_LAYOUTS["editor-listing"].schema.options).toEqual( + Object.keys(EDITOR_LISTING_TABLE_COLUMNS), + ); + }); -describe("validateColumnTemplate", () => { test("returns null for valid template", () => { - expect( - validateColumnTemplate("{{name}}, {{status}}", VALID_LISTING_KEYS), - ).toBeNull(); + expect(COLUMN_LAYOUTS.listing.validate("{{name}}, {{status}}")).toBeNull(); }); test("handles wonky spacing", () => { expect( - validateColumnTemplate( + COLUMN_LAYOUTS.listing.validate( "{{ name }},{{description}}, {{ status }}", - VALID_LISTING_KEYS, ), ).toBeNull(); }); test("rejects unknown column (typo)", () => { - const error = validateColumnTemplate( - "{{name}}, {{descritpion}}", - VALID_LISTING_KEYS, - ); + const error = COLUMN_LAYOUTS.listing.validate("{{name}}, {{descritpion}}"); expect(error).toContain("descritpion"); expect(error).toContain("Available columns"); }); - test("lists available columns comma-separated in the error message", () => { - const error = validateColumnTemplate("{{bogus}}", [ - "alpha", - "beta", - "gamma", - ]); + test("lists the schema columns in the error message", () => { + const error = COLUMN_LAYOUTS.attendee.validate("{{bogus}}"); expect(error).toBe( - 'Unknown column "bogus". Available columns: alpha, beta, gamma', + `Unknown column "bogus". Available columns: ${COLUMN_LAYOUTS.attendee.schema.options.join(", ")}`, ); }); - test("rejects empty template", () => { - const error = validateColumnTemplate("", VALID_LISTING_KEYS); - expect(error).toBe("Template must include at least one column"); + test("accepts an empty template as the default layout", () => { + expect(COLUMN_LAYOUTS.listing.validate("")).toBeNull(); + }); + + test("rejects a non-empty template with no column tags", () => { + expect(COLUMN_LAYOUTS.listing.validate("name, status")).toBe( + "Template must include at least one column", + ); }); test("accepts templates with date filter", () => { expect( - validateColumnTemplate( - '{{name}}, {{created | date: "%B"}}', - VALID_LISTING_KEYS, - ), + COLUMN_LAYOUTS.listing.validate('{{name}}, {{created | date: "%B"}}'), ).toBeNull(); }); test("accepts templates with currency filter", () => { expect( - validateColumnTemplate( - "{{name}}, {{price | currency}}", - VALID_LISTING_KEYS, - ), + COLUMN_LAYOUTS.listing.validate("{{name}}, {{price | currency}}"), ).toBeNull(); }); }); -describe("resolveColumnLayout", () => { +describe("column layout parsing", () => { test("returns default order when template is empty", () => { - const { columnKeys, filters } = resolveColumnLayout( - "", - VALID_LISTING_KEYS, - LISTING_DEFAULT_ORDER, - ); - expect(columnKeys).toEqual([...LISTING_DEFAULT_ORDER]); + const { columnKeys, filters } = COLUMN_LAYOUTS.listing.parse(""); + expect(columnKeys).toEqual(COLUMN_LAYOUTS.listing.defaultOrder); expect(filters.size).toBe(0); }); test("returns columns in template order", () => { - const { columnKeys } = resolveColumnLayout( - "{{status}}, {{name}}", - VALID_LISTING_KEYS, - LISTING_DEFAULT_ORDER, - ); + const { columnKeys } = COLUMN_LAYOUTS.listing.parse("{{status}}, {{name}}"); expect(columnKeys).toEqual(["status", "name"]); }); test("deduplicates repeated columns", () => { - const { columnKeys } = resolveColumnLayout( + const { columnKeys } = COLUMN_LAYOUTS.listing.parse( "{{name}}, {{name}}, {{status}}", - VALID_LISTING_KEYS, - LISTING_DEFAULT_ORDER, ); expect(columnKeys).toEqual(["name", "status"]); }); test("throws for an invalid template", () => { - expect(() => - resolveColumnLayout( - "{{bogus}}", - VALID_LISTING_KEYS, - LISTING_DEFAULT_ORDER, - ), - ).toThrow('Unknown column "bogus"'); + expect(() => COLUMN_LAYOUTS.listing.parse("{{bogus}}")).toThrow( + 'Unknown column "bogus"', + ); }); test("extracts filter expression for filtered column", () => { - const { filters } = resolveColumnLayout( + const { filters } = COLUMN_LAYOUTS.listing.parse( '{{name}}, {{created | date: "%B %d"}}', - VALID_LISTING_KEYS, - LISTING_DEFAULT_ORDER, ); expect(filters.get("created")).toBe('created | date: "%B %d"'); }); test("does not create filter entry for unfiltered column", () => { - const { filters } = resolveColumnLayout( + const { filters } = COLUMN_LAYOUTS.listing.parse( '{{name}}, {{created | date: "%B %d"}}', - VALID_LISTING_KEYS, - LISTING_DEFAULT_ORDER, ); expect(filters.has("name")).toBe(false); }); test("resolves all attendee columns from default template", () => { - const template = buildDefaultTemplate(ATTENDEE_DEFAULT_ORDER); - const { columnKeys } = resolveColumnLayout( - template, - VALID_ATTENDEE_KEYS, - ATTENDEE_DEFAULT_ORDER, - ); - expect(columnKeys).toEqual([...ATTENDEE_DEFAULT_ORDER]); + const template = buildDefaultTemplate(COLUMN_LAYOUTS.attendee.defaultOrder); + const { columnKeys } = COLUMN_LAYOUTS.attendee.parse(template); + expect(columnKeys).toEqual(COLUMN_LAYOUTS.attendee.defaultOrder); }); }); @@ -230,16 +198,20 @@ describe("renderFilteredValue", () => { }); describe("renderCells", () => { + test("rejects a column missing from the supplied definitions", () => { + expect(() => + renderCells({}, ["missing"], {}, undefined, new Map(), escapeHtml), + ).toThrow("Unknown column: missing"); + }); + test("renders listing columns end-to-end through the full pipeline", () => { const listing = testListingWithCount({ date: "2026-06-15", name: "Jazz Night", unit_price: 0, }); - const { columnKeys, filters } = resolveColumnLayout( + const { columnKeys, filters } = COLUMN_LAYOUTS.listing.parse( "{{name}}, {{price}}, {{date}}", - VALID_LISTING_KEYS, - LISTING_DEFAULT_ORDER, ); const html = renderCells( listing, @@ -260,10 +232,8 @@ describe("renderCells", () => { date: "2026-03-15", unit_price: 2500, }); - const { columnKeys, filters } = resolveColumnLayout( + const { columnKeys, filters } = COLUMN_LAYOUTS.listing.parse( '{{date | date: "%d/%m/%Y"}}, {{created | date: "%B %Y"}}, {{price | currency}}', - VALID_LISTING_KEYS, - LISTING_DEFAULT_ORDER, ); const html = renderCells( listing, @@ -282,11 +252,8 @@ describe("renderCells", () => { const listing = testListingWithCount({ location: '', }); - const { columnKeys, filters } = resolveColumnLayout( - "{{location}}", - VALID_LISTING_KEYS, - LISTING_DEFAULT_ORDER, - ); + const { columnKeys, filters } = + COLUMN_LAYOUTS.listing.parse("{{location}}"); const html = renderCells( listing, columnKeys, diff --git a/test/shared/columns/attendee-columns.test.ts b/test/shared/columns/attendee-columns.test.ts index 43d8e2a70..e01fb8e66 100644 --- a/test/shared/columns/attendee-columns.test.ts +++ b/test/shared/columns/attendee-columns.test.ts @@ -1,7 +1,8 @@ import { expect } from "@std/expect"; import { describe, it as test } from "@std/testing/bdd"; +import type { AttendeeColumn } from "#shared/column-order.ts"; +import { COLUMN_LAYOUTS } from "#shared/column-order.ts"; import { - ATTENDEE_DEFAULT_ORDER, ATTENDEE_TABLE_COLUMNS, formatAddressInline, } from "#shared/columns/attendee-columns.ts"; @@ -85,7 +86,7 @@ const METADATA: Record = { describe("ATTENDEE_TABLE_COLUMNS metadata", () => { test("every column exposes its expected label/description/className/isHtml", () => { for (const [key, expected] of Object.entries(METADATA)) { - const col = ATTENDEE_TABLE_COLUMNS[key]!; + const col = ATTENDEE_TABLE_COLUMNS[key as AttendeeColumn]; expect(col.label).toBe(expected.label); expect(col.description).toBe(expected.description); expect(col.isHtml).toBe(expected.isHtml); @@ -102,9 +103,9 @@ describe("ATTENDEE_TABLE_COLUMNS metadata", () => { }); }); -describe("ATTENDEE_DEFAULT_ORDER", () => { +describe("the attendee default column order", () => { test("matches the expected default order exactly", () => { - expect(ATTENDEE_DEFAULT_ORDER).toEqual([ + expect(COLUMN_LAYOUTS.attendee.defaultOrder).toEqual([ "status", "date", "name", @@ -121,7 +122,7 @@ describe("ATTENDEE_DEFAULT_ORDER", () => { }); test("is a permutation of ATTENDEE_TABLE_COLUMNS' keys", () => { - expect([...ATTENDEE_DEFAULT_ORDER].sort()).toEqual( + expect([...COLUMN_LAYOUTS.attendee.defaultOrder].sort()).toEqual( Object.keys(ATTENDEE_TABLE_COLUMNS).sort(), ); }); diff --git a/test/shared/db/attendees/api/errors.test.ts b/test/shared/db/attendees/api/errors.test.ts index 20f7f7732..c730f6192 100644 --- a/test/shared/db/attendees/api/errors.test.ts +++ b/test/shared/db/attendees/api/errors.test.ts @@ -148,7 +148,7 @@ describeWithEnv( const { response } = await adminFormPost( `/admin/attendees/${attendee.id}`, { - name: attendee.name, + name: "Blank", ...attendeeLineFields([ { eventId: event.id, quantity: Number("0") }, ]), diff --git a/test/shared/forms/definition/field-schema.test.ts b/test/shared/forms/definition/field-schema.test.ts index c5353c8b9..fbe6e33a0 100644 --- a/test/shared/forms/definition/field-schema.test.ts +++ b/test/shared/forms/definition/field-schema.test.ts @@ -1,11 +1,28 @@ import { expect } from "@std/expect"; -import { describe, it as test } from "@std/testing/bdd"; +import { beforeAll, describe, it as test } from "@std/testing/bdd"; import { FormParams } from "#shared/form-data.ts"; import { defineForm } from "#shared/forms/definition.ts"; import type { Field } from "#shared/forms/field.ts"; import { renderField } from "#shared/forms/rendering.tsx"; +import { ensureMessageGroups } from "#shared/i18n.ts"; + +const singleActionSelect = (id: string, invalidMessage?: string) => + defineForm({ + fields: [ + { + ...(invalidMessage === undefined ? {} : { invalidMessage }), + label: "Action", + name: "action", + options: [{ label: "Pay", value: "pay" }], + type: "select", + }, + ] as const, + id, + }); describe("form field schema", () => { + beforeAll(() => ensureMessageGroups(["validation"])); + test("renders select option hints from the field schema", () => { const form = defineForm({ fields: [ @@ -50,17 +67,7 @@ describe("form field schema", () => { }); test("uses the shared invalid message when a choice has no custom message", () => { - const form = defineForm({ - fields: [ - { - label: "Action", - name: "action", - options: [{ label: "Pay", value: "pay" }], - type: "select", - }, - ] as const, - id: "default-choice-message", - }); + const form = singleActionSelect("default-choice-message"); expect(form.validate(new FormParams({ action: "refund" }))).toEqual({ error: "Action is invalid.", @@ -92,6 +99,15 @@ describe("form field schema", () => { } }); + test("normalizes an empty select value to null", () => { + const form = singleActionSelect("empty-select-value"); + + expect(form.validate(new FormParams())).toEqual({ + valid: true, + values: { action: null }, + }); + }); + test("requires options when declaring a select field", () => { const acceptField = (_field: Field): void => {}; // @ts-expect-error A select without options is not a valid Field. @@ -129,7 +145,7 @@ describe("form field schema", () => { expect(defineEmptySelect).toThrow("Action must define at least one option"); }); - test("accepts spaces in comma-separated checkbox choices", () => { + test("rejects checkbox choices submitted as one comma-separated token", () => { const form = defineForm({ fields: [ { @@ -148,8 +164,8 @@ describe("form field schema", () => { expect( form.validate(new FormParams({ days: "Monday, Wednesday" })), ).toEqual({ - valid: true, - values: { days: "Monday, Wednesday" }, + error: "Days is invalid.", + valid: false, }); }); @@ -199,6 +215,7 @@ describe("form field schema", () => { const form = defineForm({ fields: [ { label: "Name", name: "name", section: "main", type: "text" }, + { label: "Summary", name: "summary", type: "text" }, { label: "Private", name: "private", @@ -216,6 +233,7 @@ describe("form field schema", () => { expect(form.sections).toEqual(["main", "advanced"]); expect(form.section("main")).toContain('name="name"'); + expect(form.section("main")).not.toContain('name="summary"'); expect(form.section("advanced")).not.toContain('name="private"'); expect(rejectUnknownSection).toThrow("Unknown section: missing"); }); diff --git a/test/shared/forms/repeated-picklist.test.ts b/test/shared/forms/repeated-picklist.test.ts new file mode 100644 index 000000000..2d140cd4e --- /dev/null +++ b/test/shared/forms/repeated-picklist.test.ts @@ -0,0 +1,44 @@ +import { expect } from "@std/expect"; +import { describe, it as test } from "@std/testing/bdd"; +import * as v from "valibot"; +import { FormParams } from "#shared/form-data.ts"; +import { readRepeatedPicklist } from "#shared/forms/repeated-picklist.ts"; + +const DaySchema = v.picklist(["Monday", "Tuesday", "Wednesday"]); + +const read = (query: string, enabled = true) => + readRepeatedPicklist(DaySchema, new FormParams(query), "days", enabled); + +describe("readRepeatedPicklist", () => { + test("is enabled when the flag is omitted", () => { + expect( + readRepeatedPicklist(DaySchema, new FormParams("days=Monday"), "days"), + ).toEqual({ state: "selected", values: ["Monday"] }); + }); + + test("returns disabled without accepting supplied values", () => { + expect(read("days=Monday", false)).toEqual({ state: "disabled" }); + }); + + test("distinguishes an absent enabled selection", () => { + expect(read("")).toEqual({ state: "absent" }); + }); + + test("returns the first invalid supplied token", () => { + expect(read("days=Monday&days=Funday&days=Someday")).toEqual({ + state: "invalid", + value: "Funday", + }); + }); + + test("returns unique values in schema order", () => { + expect(read("days=Wednesday&days=Monday&days=Wednesday")).toEqual({ + state: "selected", + values: ["Monday", "Wednesday"], + }); + }); + + test("treats a supplied empty token as invalid", () => { + expect(read("days=")).toEqual({ state: "invalid", value: "" }); + }); +}); diff --git a/test/shared/forms/validation.test.ts b/test/shared/forms/validation.test.ts index c59db8a5a..bc566eb16 100644 --- a/test/shared/forms/validation.test.ts +++ b/test/shared/forms/validation.test.ts @@ -1,8 +1,9 @@ import { expect } from "@std/expect"; -import { describe, it as test } from "@std/testing/bdd"; +import { beforeAll, describe, it as test } from "@std/testing/bdd"; import { FormParams } from "#shared/form-data.ts"; import type { Field } from "#shared/forms/field.ts"; import { validateForm } from "#shared/forms/validation.ts"; +import { ensureMessageGroups } from "#shared/i18n.ts"; const field = ( overrides: Partial & { name: string; label: string }, @@ -12,7 +13,32 @@ const requiredName: Field[] = [ field({ label: "Name", name: "name", required: true }), ]; +const dayCheckboxFields = (invalidMessage?: string): Field[] => [ + field({ + ...(invalidMessage ? { invalidMessage } : {}), + label: "Days", + name: "days", + options: [ + { label: "Monday", value: "Monday" }, + { label: "Wednesday", value: "Wednesday" }, + ], + type: "checkbox-group", + }), +]; + +const colorSelectFields = (invalidMessage?: string): Field[] => [ + field({ + ...(invalidMessage === undefined ? {} : { invalidMessage }), + label: "Color", + name: "color", + options: [{ label: "Red", value: "red" }], + type: "select", + }), +]; + describe("validateForm", () => { + beforeAll(() => ensureMessageGroups(["validation"])); + test("rejects empty required field", () => { const result = validateForm(new FormParams({ name: "" }), requiredName); expect(result.valid).toBe(false); @@ -114,26 +140,110 @@ describe("validateForm", () => { expect(validateForm(new FormParams({ code: "" }), fields).valid).toBe(true); }); - test("collects checkbox-group values from multiple form entries", () => { + test("uses a field parser instead of the built-in value", () => { const fields: Field[] = [ - field({ - label: "Days", - name: "days", - options: [ - { label: "Monday", value: "Monday" }, - { label: "Wednesday", value: "Wednesday" }, - ], - type: "checkbox-group", - }), + field({ label: "Code", name: "code", parse: () => "parsed" }), ]; + expect(validateForm(new FormParams({ code: "raw" }), fields)).toEqual({ + valid: true, + values: { code: "parsed" }, + }); + }); + + test("keeps an explicitly empty required message", () => { + expect( + validateForm(new FormParams(), [ + field({ + label: "Name", + name: "name", + required: true, + requiredMessage: "", + }), + ]), + ).toEqual({ error: "", valid: false }); + }); + + test("keeps an explicitly empty invalid message", () => { + expect( + validateForm(new FormParams({ color: "blue" }), colorSelectFields("")), + ).toEqual({ error: "", valid: false }); + }); + + test("accepts a declared select value", () => { + expect( + validateForm(new FormParams({ color: "red" }), colorSelectFields()), + ).toEqual({ + valid: true, + values: { color: "red" }, + }); + }); + + test("rejects an undeclared select value with the default message", () => { + expect( + validateForm(new FormParams({ color: "blue" }), colorSelectFields()), + ).toEqual({ error: "Color is invalid.", valid: false }); + }); + + test("rejects parsers that return unusable values", () => { + for (const [name, parse] of [ + ["missing", () => null], + ["infinite", () => Number.POSITIVE_INFINITY], + ] as const) { + expect( + validateForm(new FormParams({ [name]: "value" }), [ + field({ label: name, name, parse }), + ]), + ).toEqual({ error: `${name} is invalid.`, valid: false }); + } + }); + + test("uses a default when the submitted value is empty", () => { + const fields: Field[] = [ + field({ defaultValue: "fallback", label: "Code", name: "code" }), + ]; + expect(validateForm(new FormParams(), fields)).toEqual({ + valid: true, + values: { code: "fallback" }, + }); + }); + + test("keeps a submitted value instead of its default", () => { + const fields: Field[] = [ + field({ defaultValue: "fallback", label: "Code", name: "code" }), + ]; + expect(validateForm(new FormParams({ code: "given" }), fields)).toEqual({ + valid: true, + values: { code: "given" }, + }); + }); + + test("collects checkbox-group values from multiple form entries", () => { const form = new FormParams(); form.append("days", "Monday"); form.append("days", "Wednesday"); - const result = validateForm(form, fields); + const result = validateForm(form, dayCheckboxFields()); expect(result.valid).toBe(true); if (result.valid) expect(result.values.days).toBe("Monday,Wednesday"); }); + test("normalizes checkbox-group values to option order", () => { + const form = new FormParams("days=Wednesday&days=Monday&days=Wednesday"); + + expect(validateForm(form, dayCheckboxFields())).toEqual({ + valid: true, + values: { days: "Monday,Wednesday" }, + }); + }); + + test("rejects a bad checkbox value mixed into valid selections", () => { + expect( + validateForm( + new FormParams("days=Monday&days=Funday&days=Wednesday"), + dayCheckboxFields("Choose listed days only."), + ), + ).toEqual({ error: "Choose listed days only.", valid: false }); + }); + test("returns empty string for empty checkbox-group", () => { const fields: Field[] = [ field({ @@ -148,6 +258,32 @@ describe("validateForm", () => { if (result.valid) expect(result.values.days).toBe(""); }); + test("rejects an empty required checkbox group", () => { + const fields = dayCheckboxFields(); + fields[0]!.required = true; + expect(validateForm(new FormParams(), fields)).toEqual({ + error: "Days is required", + valid: false, + }); + }); + + test("runs checkbox-group validation on selected values", () => { + const fields = dayCheckboxFields(); + fields[0]!.validate = () => "Days cannot be combined."; + expect( + validateForm(new FormParams("days=Monday&days=Wednesday"), fields), + ).toEqual({ error: "Days cannot be combined.", valid: false }); + }); + + test("does not run checkbox-group validation without a selection", () => { + const fields = dayCheckboxFields(); + fields[0]!.validate = () => "Unexpected validation."; + expect(validateForm(new FormParams(), fields)).toEqual({ + valid: true, + values: { days: "" }, + }); + }); + test("skips file fields and returns null", () => { const fields: Field[] = [ field({ label: "Image", name: "image", type: "file" }), diff --git a/test/test-utils/db-helpers/listing-forms.ts b/test/test-utils/db-helpers/listing-forms.ts index 4023a6439..33c7d15e1 100644 --- a/test/test-utils/db-helpers/listing-forms.ts +++ b/test/test-utils/db-helpers/listing-forms.ts @@ -1,18 +1,22 @@ import { toMajorUnits } from "#shared/currency.ts"; import type { ListingInput } from "#shared/db/listings/table.ts"; import type { DayPrices, ListingWithCount } from "#shared/types.ts"; +import type { TestFormValues } from "#test-utils/form-values.ts"; -const bool = (v: unknown): string => (v ? "1" : ""); +const checked = (name: string, value: unknown): TestFormValues => + value ? { [name]: "1" } : {}; +const flagChoice = (value: unknown): string => (value ? "1" : ""); const optionalNumber = (v: number | null | undefined): string => v != null ? String(v) : ""; const optionalPrice = (v: number | null | undefined): string => v != null ? toMajorUnits(v) : ""; -const formatBookableDaysForForm = (days: string[]): string => days.join(","); +const repeated = (value: string): string[] => + value ? value.split(",").map((part) => part.trim()) : []; /** Serialize a DayPrices map into the form's `day_price_` fields. */ const dayPriceFormFields = ( dayPrices: DayPrices | undefined, -): Record => { +): TestFormValues => { const result: Record = {}; for (const [days, price] of Object.entries(dayPrices ?? {})) { result[`day_price_${days}`] = toMajorUnits(price); @@ -44,30 +48,28 @@ export const priceFormValue = (minorUnits: number): string => export const buildCreateListingForm = ( input: Omit, -): Record => { +): TestFormValues => { const closesAtParts = splitClosesAt(input.closesAt, null); const dateParts = splitClosesAt(input.date, null); const initialSiteMonths = input.assignBuiltSite ? (input.initialSiteMonths ?? 1) : (input.initialSiteMonths ?? 0); return { - assign_built_site: bool(input.assignBuiltSite), - bookable_alone: bool(input.bookableAlone), - bookable_days: input.bookableDays - ? formatBookableDaysForForm(input.bookableDays) - : "", - can_pay_more: bool(input.canPayMore), + ...checked("assign_built_site", input.assignBuiltSite), + ...checked("bookable_alone", input.bookableAlone), + bookable_days: input.bookableDays ?? [], + ...checked("can_pay_more", input.canPayMore), closes_at_date: closesAtParts.date, closes_at_time: closesAtParts.time, - customisable_days: bool(input.customisableDays), + ...checked("customisable_days", input.customisableDays), date_date: dateParts.date, date_time: dateParts.time, description: input.description ?? "", duration_days: optionalNumber(input.durationDays), - uses_logistics: bool(input.usesLogistics), + ...checked("uses_logistics", input.usesLogistics), ...dayPriceFormFields(input.dayPrices), - fields: input.fields ?? "email", - hidden: bool(input.hidden), + fields: repeated(input.fields ?? "email"), + ...checked("hidden", input.hidden), initial_site_months: String(initialSiteMonths), listing_type: input.listingType ?? "", location: input.location ?? "", @@ -78,11 +80,11 @@ export const buildCreateListingForm = ( minimum_days_before: optionalNumber(input.minimumDaysBefore), months_per_unit: String(input.monthsPerUnit ?? 0), name: input.name, - non_transferable: bool(input.nonTransferable), - purchase_only: bool(input.purchaseOnly), + non_transferable: flagChoice(input.nonTransferable), + ...checked("purchase_only", input.purchaseOnly), thank_you_url: input.thankYouUrl ?? "", unit_price: optionalPrice(input.unitPrice), - use_defaults: bool(input.useDefaults), + ...checked("use_defaults", input.useDefaults), webhook_url: input.webhookUrl ?? "", }; }; @@ -90,21 +92,33 @@ export const buildCreateListingForm = ( const buildUpdateBoolFields = ( updates: Partial, existing: ListingWithCount, -): Record => ({ - assign_built_site: bool( +): TestFormValues => ({ + ...checked( + "assign_built_site", pickField(updates.assignBuiltSite, existing.assign_built_site), ), - bookable_alone: bool( + ...checked( + "bookable_alone", pickField(updates.bookableAlone, existing.bookable_alone), ), - can_pay_more: bool(pickField(updates.canPayMore, existing.can_pay_more)), - hidden: bool(pickField(updates.hidden, existing.hidden)), - non_transferable: bool( + ...checked( + "can_pay_more", + pickField(updates.canPayMore, existing.can_pay_more), + ), + ...checked("hidden", pickField(updates.hidden, existing.hidden)), + non_transferable: flagChoice( pickField(updates.nonTransferable, existing.non_transferable), ), - purchase_only: bool(pickField(updates.purchaseOnly, existing.purchase_only)), - use_defaults: bool(pickField(updates.useDefaults, existing.use_defaults)), - uses_logistics: bool( + ...checked( + "purchase_only", + pickField(updates.purchaseOnly, existing.purchase_only), + ), + ...checked( + "use_defaults", + pickField(updates.useDefaults, existing.use_defaults), + ), + ...checked( + "uses_logistics", pickField(updates.usesLogistics, existing.uses_logistics), ), }); @@ -112,7 +126,7 @@ const buildUpdateBoolFields = ( const buildUpdateNumericFields = ( updates: Partial, existing: ListingWithCount, -): Record => { +): TestFormValues => { const assignsBuiltSite = pickField( updates.assignBuiltSite, existing.assign_built_site, @@ -146,12 +160,10 @@ const buildUpdateNumericFields = ( const buildUpdateStringFields = ( updates: Partial, existing: ListingWithCount, -): Record => ({ - bookable_days: formatBookableDaysForForm( - pickField(updates.bookableDays, existing.bookable_days), - ), +): TestFormValues => ({ + bookable_days: pickField(updates.bookableDays, existing.bookable_days), description: pickField(updates.description, existing.description), - fields: pickField(updates.fields, existing.fields), + fields: repeated(pickField(updates.fields, existing.fields)), listing_type: pickField(updates.listingType, existing.listing_type), location: pickField(updates.location, existing.location), name: pickField(updates.name, existing.name), @@ -163,7 +175,7 @@ const buildUpdateStringFields = ( export const buildUpdateListingForm = ( updates: Partial, existing: ListingWithCount, -): Record => { +): TestFormValues => { const closesAtParts = splitClosesAt(updates.closesAt, existing.closes_at); const dateParts = splitClosesAt(updates.date, existing.date); return { @@ -173,7 +185,8 @@ export const buildUpdateListingForm = ( ...dayPriceFormFields(updates.dayPrices ?? existing.day_prices), closes_at_date: closesAtParts.date, closes_at_time: closesAtParts.time, - customisable_days: bool( + ...checked( + "customisable_days", updates.customisableDays ?? existing.customisable_days, ), date_date: dateParts.date, diff --git a/test/test-utils/db-helpers/request.ts b/test/test-utils/db-helpers/request.ts index 8889b2dca..d486072b5 100644 --- a/test/test-utils/db-helpers/request.ts +++ b/test/test-utils/db-helpers/request.ts @@ -4,12 +4,8 @@ * admin create/update/delete test helper below. */ async function doAuthenticatedRequest( path: string, - formData: Record, - buildRequest: ( - path: string, - data: Record, - cookie: string, - ) => Request, + formData: TestFormValues, + buildRequest: (path: string, data: TestFormValues, cookie: string) => Request, onSuccess: () => Promise, errorContext: string, ): Promise { @@ -31,7 +27,7 @@ async function doAuthenticatedRequest( export const doAuthenticatedFormRequest = async ( path: string, - formData: Record, + formData: TestFormValues, onSuccess: () => Promise, errorContext: string, ): Promise => { @@ -47,7 +43,7 @@ export const doAuthenticatedFormRequest = async ( export const doAuthenticatedMultipartFormRequest = async ( path: string, - formData: Record, + formData: TestFormValues, onSuccess: () => Promise, errorContext: string, ): Promise => { @@ -60,3 +56,5 @@ export const doAuthenticatedMultipartFormRequest = async ( errorContext, ); }; + +import type { TestFormValues } from "#test-utils/form-values.ts"; diff --git a/test/test-utils/form-values.test.ts b/test/test-utils/form-values.test.ts new file mode 100644 index 000000000..d62401446 --- /dev/null +++ b/test/test-utils/form-values.test.ts @@ -0,0 +1,18 @@ +import { expect } from "@std/expect"; +import { it as test } from "@std/testing/bdd"; +import { testFormParams } from "#test-utils/form-values.ts"; + +test("testFormParams appends repeated values and skips missing values", () => { + const params = testFormParams({ + absent: undefined, + empty: null, + selected: ["first", "second"], + single: "value", + }); + + expect([...params.entries()]).toEqual([ + ["selected", "first"], + ["selected", "second"], + ["single", "value"], + ]); +}); diff --git a/test/test-utils/form-values.ts b/test/test-utils/form-values.ts new file mode 100644 index 000000000..0f7de8b43 --- /dev/null +++ b/test/test-utils/form-values.ts @@ -0,0 +1,24 @@ +import { FormParams } from "#shared/form-data.ts"; + +export type TestFormValues = Record< + string, + string | string[] | null | undefined +>; + +export const appendTestFormValues = ( + params: URLSearchParams | FormData, + data: TestFormValues = {}, +): void => { + for (const [key, values] of Object.entries(data)) { + if (values == null) continue; + for (const value of typeof values === "string" ? [values] : values) { + params.append(key, value); + } + } +}; + +export const testFormParams = (data: TestFormValues): FormParams => { + const params = new FormParams(); + appendTestFormValues(params, data); + return params; +}; diff --git a/test/test-utils/mocks.ts b/test/test-utils/mocks.ts index 8a33412a6..704aa1089 100644 --- a/test/test-utils/mocks.ts +++ b/test/test-utils/mocks.ts @@ -10,6 +10,10 @@ import { runWithStorageConfig } from "#shared/storage.ts"; import { type EnvScope, withEnv } from "#test-utils/env.ts"; import { stubFetch } from "#test-utils/fetch-stub.ts"; import { withTempDir } from "#test-utils/files.ts"; +import { + appendTestFormValues, + type TestFormValues, +} from "#test-utils/form-values.ts"; import { TEST_STORAGE_ZONE, type TestRequestOptions, @@ -30,10 +34,12 @@ export const mockRequest = (path: string, options: RequestInit = {}): Request => export const mockFormRequest = ( path: string, - data: Record, + data: TestFormValues = {}, cookie?: string, ): Request => { - const body = new URLSearchParams(data).toString(); + const params = new URLSearchParams(); + appendTestFormValues(params, data); + const body = params.toString(); const headers: HeadersInit = { "content-type": "application/x-www-form-urlencoded", host: "localhost", @@ -73,7 +79,7 @@ export const mockAdminLoginRequest = async ( export const mockMultipartRequest = ( path: string, - data: Record, + data: TestFormValues = {}, cookie?: string, file?: { name: string; @@ -83,9 +89,7 @@ export const mockMultipartRequest = ( }, ): Request => { const formData = new FormData(); - for (const [key, value] of Object.entries(data)) { - formData.append(key, value); - } + appendTestFormValues(formData, data); if (file) { // deno-lint-ignore no-explicit-any const blob = new Blob([file.data as any], { type: file.contentType }); diff --git a/test/test-utils/validation.ts b/test/test-utils/validation.ts index 3a6977047..b77a86307 100644 --- a/test/test-utils/validation.ts +++ b/test/test-utils/validation.ts @@ -1,16 +1,17 @@ import { expect } from "@std/expect"; -import { FormParams } from "#shared/form-data.ts"; import type { Field } from "#shared/forms/field.ts"; import { validateForm } from "#shared/forms/validation.ts"; +import { + type TestFormValues, + testFormParams, +} from "#test-utils/form-values.ts"; -const validateFormData = ( - fields: readonly Field[], - data: Record, -) => validateForm(new FormParams(data), fields); +const validateFormData = (fields: readonly Field[], data: TestFormValues) => + validateForm(testFormParams(data), fields); export const expectValid = ( fields: readonly Field[], - data: Record, + data: TestFormValues, ): Record => { const result = validateFormData(fields, data); expect(result.valid).toBe(true); @@ -19,7 +20,7 @@ export const expectValid = ( export const expectInvalid = (expectedError: string) => - (fields: readonly Field[], data: Record): void => { + (fields: readonly Field[], data: TestFormValues): void => { const result = validateFormData(fields, data); expect(result.valid).toBe(false); if (!result.valid) expect(result.error).toBe(expectedError); @@ -27,7 +28,7 @@ export const expectInvalid = export const expectInvalidForm = ( fields: readonly Field[], - data: Record, + data: TestFormValues, ): void => { expect(validateFormData(fields, data).valid).toBe(false); }; diff --git a/test/ui/templates/admin/dashboard.test.ts b/test/ui/templates/admin/dashboard.test.ts index 81a32ef76..5e0dd84d0 100644 --- a/test/ui/templates/admin/dashboard.test.ts +++ b/test/ui/templates/admin/dashboard.test.ts @@ -4,6 +4,7 @@ import { NO_QUANTITY_PREFIX, QTY_PREFIX, } from "#routes/admin/attendee-form-model.ts"; +import { COLUMN_LAYOUTS } from "#shared/column-order.ts"; import { signCsrfToken } from "#shared/csrf.ts"; import { getDb } from "#shared/db/client.ts"; import { @@ -265,7 +266,7 @@ describe("adminDashboardPage with column template filters", () => { [], undefined, null, - '{{name}}, {{created | date: "%B %Y"}}', + COLUMN_LAYOUTS.listing.parse('{{name}}, {{created | date: "%B %Y"}}'), ); expect(html).toContain("April 2026"); }); @@ -281,7 +282,7 @@ describe("adminDashboardPage with column template filters", () => { [], undefined, null, - "{{name}}, {{created}}", + COLUMN_LAYOUTS.listing.parse("{{name}}, {{created}}"), ); // Default uses toLocaleDateString — locale format, not Liquid strftime expect(html).toContain("2026"); diff --git a/test/ui/templates/attendee-table/template.test.ts b/test/ui/templates/attendee-table/template.test.ts index eb3a934a6..5465a7e50 100644 --- a/test/ui/templates/attendee-table/template.test.ts +++ b/test/ui/templates/attendee-table/template.test.ts @@ -1,5 +1,6 @@ import { expect } from "@std/expect"; import { it as test } from "@std/testing/bdd"; +import { COLUMN_LAYOUTS } from "#shared/column-order.ts"; import { AttendeeTable } from "#templates/attendee-table.tsx"; import { testAttendee } from "#test-utils/factories.ts"; import { attendeeTableSuite, makeOpts, makeRow } from "./shared.ts"; @@ -11,28 +12,28 @@ attendeeTableSuite(() => { test("renders only specified columns in template order", () => { const html = AttendeeTable( makeOpts({ - columnTemplate: "{{name}}, {{qty}}, {{registered}}", + columnLayout: COLUMN_LAYOUTS.attendee.parse( + "{{name}}, {{qty}}, {{registered}}", + ), showCheckin: false, }), ); expect(headersOf(html)).toEqual(["Name", "Qty", "Registered"]); }); - test("throws for an invalid template loaded outside the settings form", () => { - expect(() => - AttendeeTable( - makeOpts({ - columnTemplate: "{{invalid_column}}", - }), - ), - ).toThrow('Unknown column "invalid_column"'); + test("rejects an invalid template before rendering", () => { + expect(() => COLUMN_LAYOUTS.attendee.parse("{{invalid_column}}")).toThrow( + 'Unknown column "invalid_column"', + ); }); test("hides a data-dependent template column with no data", () => { const attendee = testAttendee({ email: "" }); const html = AttendeeTable( makeOpts({ - columnTemplate: "{{name}}, {{email}}, {{qty}}", + columnLayout: COLUMN_LAYOUTS.attendee.parse( + "{{name}}, {{email}}, {{qty}}", + ), rows: [makeRow({ attendee })], showCheckin: false, }), @@ -44,7 +45,9 @@ attendeeTableSuite(() => { const attendee = testAttendee({ email: "a@b.com" }); const html = AttendeeTable( makeOpts({ - columnTemplate: "{{qty}}, {{name}}, {{email}}", + columnLayout: COLUMN_LAYOUTS.attendee.parse( + "{{qty}}, {{name}}, {{email}}", + ), rows: [makeRow({ attendee })], showCheckin: false, }), @@ -56,7 +59,9 @@ attendeeTableSuite(() => { const attendee = testAttendee({ created: "2026-04-10T14:00:00Z" }); const html = AttendeeTable( makeOpts({ - columnTemplate: '{{name}}, {{registered | date: "%B %d, %Y"}}', + columnLayout: COLUMN_LAYOUTS.attendee.parse( + '{{name}}, {{registered | date: "%B %d, %Y"}}', + ), rows: [makeRow({ attendee })], showCheckin: false, }), @@ -68,7 +73,7 @@ attendeeTableSuite(() => { const attendee = testAttendee({ created: "2026-04-10T14:00:00Z" }); const html = AttendeeTable( makeOpts({ - columnTemplate: "{{name}}, {{registered}}", + columnLayout: COLUMN_LAYOUTS.attendee.parse("{{name}}, {{registered}}"), rows: [makeRow({ attendee })], showCheckin: false, }), diff --git a/test/ui/templates/fields/listing.test.ts b/test/ui/templates/fields/listing.test.ts index bd34f2db5..024147a50 100644 --- a/test/ui/templates/fields/listing.test.ts +++ b/test/ui/templates/fields/listing.test.ts @@ -14,11 +14,10 @@ import { validateDate, } from "#templates/fields/validators.ts"; import { baseListingForm } from "#test-utils/factories.ts"; +import type { TestFormValues } from "#test-utils/form-values.ts"; import { expectInvalid, expectValid } from "#test-utils/validation.ts"; -const listingForm = ( - overrides: Record = {}, -): Record => ({ +const listingForm = (overrides: TestFormValues = {}): TestFormValues => ({ ...baseListingForm, ...overrides, }); @@ -259,16 +258,20 @@ describe("listing form contact fields setting", () => { test("rejects unknown contact field name", () => { expectInvalid("Invalid contact field: invalid")( getListingForm().fields, - listingForm({ fields: "invalid" }), + listingForm({ fields: ["invalid"] }), ); }); test("accepts known contact field values", () => { // Derived from the schema, so a new ContactField member is covered here the // moment it is added — no hand-maintained list to forget to update. - for (const value of [...CONTACT_FIELDS, "email,phone"]) { - expectValid(getListingForm().fields, listingForm({ fields: value })); + for (const value of CONTACT_FIELDS) { + expectValid(getListingForm().fields, listingForm({ fields: [value] })); } + expectValid( + getListingForm().fields, + listingForm({ fields: ["email", "phone"] }), + ); }); test("warns that attendees won't be emailed their ticket without email collection", () => { @@ -332,13 +335,20 @@ describe("listing form bookable_days", () => { test("accepts valid day names", () => { expectValid( getListingForm().fields, - listingForm({ bookable_days: "Monday,Wednesday,Friday" }), + listingForm({ bookable_days: ["Monday", "Wednesday", "Friday"] }), ); expectValid( getListingForm().fields, listingForm({ - bookable_days: - "Monday,Tuesday,Wednesday,Thursday,Friday,Saturday,Sunday", + bookable_days: [ + "Monday", + "Tuesday", + "Wednesday", + "Thursday", + "Friday", + "Saturday", + "Sunday", + ], }), ); }); @@ -346,7 +356,10 @@ describe("listing form bookable_days", () => { test("rejects invalid day name", () => { expectInvalid( "Invalid day: Funday. Use: Monday, Tuesday, Wednesday, Thursday, Friday, Saturday, Sunday", - )(getListingForm().fields, listingForm({ bookable_days: "Monday,Funday" })); + )( + getListingForm().fields, + listingForm({ bookable_days: ["Monday", "Funday"] }), + ); }); test("rejects empty-after-trimming value", () => { From a942e547841ed158718b05e308c40b89426f736f Mon Sep 17 00:00:00 2001 From: Stefan Date: Sun, 19 Jul 2026 16:23:01 +0100 Subject: [PATCH 2/6] Consolidate declarative input handling --- scripts/mutation/equivalent-mutants.txt | 14 +- src/features/admin/aggregate-recalculation.ts | 14 +- src/features/admin/settings-general.ts | 11 +- .../admin/settings-listing-defaults.ts | 18 +- src/shared/column-order.ts | 241 +++++++----------- src/shared/columns/listing-columns.ts | 21 +- src/shared/form-data.ts | 18 +- src/shared/forms/definition.ts | 63 ++--- src/shared/forms/repeated-picklist.ts | 31 --- src/shared/forms/validation.ts | 62 ++--- src/ui/templates/admin/dashboard.tsx | 7 +- src/ui/templates/admin/guide/operations.tsx | 10 +- src/ui/templates/admin/ledger/entry-pages.tsx | 4 +- src/ui/templates/admin/news.tsx | 4 +- src/ui/templates/admin/questions.tsx | 2 +- .../templates/admin/settings/column-order.tsx | 10 +- src/ui/templates/admin/site-pages.tsx | 4 +- .../server/settings/column-order.test.ts | 3 - test/shared/column-order.test.ts | 26 +- test/shared/columns/attendee-columns.test.ts | 17 -- test/shared/form-data.test.ts | 32 +++ .../forms/definition/define-form.test.ts | 2 +- .../forms/definition/field-schema.test.ts | 4 +- test/shared/forms/repeated-picklist.test.ts | 44 ---- test/shared/forms/validation.test.ts | 7 + test/ui/templates/admin/ledger/forms.test.ts | 2 +- 26 files changed, 267 insertions(+), 404 deletions(-) delete mode 100644 src/shared/forms/repeated-picklist.ts delete mode 100644 test/shared/forms/repeated-picklist.test.ts diff --git a/scripts/mutation/equivalent-mutants.txt b/scripts/mutation/equivalent-mutants.txt index e0b494d16..e61bbf1f5 100644 --- a/scripts/mutation/equivalent-mutants.txt +++ b/scripts/mutation/equivalent-mutants.txt @@ -28,13 +28,13 @@ # ` — exit 0 means lint did not kill it. src/ui/templates/components/data-table.tsx:85:43 0 → -1 # For the only changed case, an empty array, both branches render the same empty tbody; non-empty arrays make both comparisons false -src/shared/forms/validation.ts:63:8 === → == # value is string|number|null and cannot be undefined, so strict and loose equality agree when comparing it with null -src/shared/forms/validation.ts:70:10 === → == # readSubmittedFieldValue returns string|null and cannot return undefined, so strict and loose equality agree when comparing it with null -src/shared/forms/validation.ts:122:46 = → += # this assignment runs only when trimmed is the empty string, so replacing it and appending to it produce the same defaultValue -src/shared/forms/validation.ts:126:16 !== → != # validateFieldText returns string|null and cannot return undefined, so strict and loose inequality agree when comparing it with null -src/features/admin/settings-listing-defaults.ts:55:15 === → == # parseNonNegativeInt returns number|null and cannot return undefined, so strict and loose equality agree when comparing it with null -src/shared/column-order.ts:107:38 validatedColumnLayout → "" # Symbol descriptions are diagnostic text only and do not affect symbol identity or any observable layout behavior -src/shared/column-order.ts:107:38 validatedColumnLayout → "validatedColumnLayout mutated" # Symbol descriptions are diagnostic text only and do not affect symbol identity or any observable layout behavior +src/shared/form-data.ts:16:33 ?? → || # trim returns a string, and its only falsy value is the empty string supplied by the fallback +src/shared/form-data.ts:35:23 !== → != # parsePositiveIntId returns number|null and cannot return undefined, so strict and loose inequality agree when comparing it with null +src/shared/forms/validation.ts:51:8 === → == # value is string|number|null and cannot be undefined, so strict and loose equality agree when comparing it with null +src/shared/forms/validation.ts:69:10 === → == # readSubmittedFieldValue returns string|null and cannot return undefined, so strict and loose equality agree when comparing it with null +src/shared/forms/validation.ts:101:46 = → += # this assignment runs only when trimmed is the empty string, so replacing it and appending to it produce the same defaultValue +src/shared/forms/validation.ts:105:16 !== → != # validateFieldText returns string|null and cannot return undefined, so strict and loose inequality agree when comparing it with null +src/features/admin/settings-listing-defaults.ts:53:15 === → == # parseNonNegativeInt returns number|null and cannot return undefined, so strict and loose equality agree when comparing it with null src/features/admin/entity-pages.ts:275:24 ?? → || # opts.query is a URLSearchParams object when present, so it is always truthy src/features/admin/site-content-page.ts:87:38 ?? → || # extraTabs is an array when present, and arrays are always truthy diff --git a/src/features/admin/aggregate-recalculation.ts b/src/features/admin/aggregate-recalculation.ts index 3e15032e9..0b0a62235 100644 --- a/src/features/admin/aggregate-recalculation.ts +++ b/src/features/admin/aggregate-recalculation.ts @@ -1,12 +1,10 @@ /* jscpd:ignore-start */ -import * as v from "valibot"; import { AUTH_FORM, requireSessionOr, withAuth } from "#routes/auth.ts"; import { htmlResponse, redirect } from "#routes/response.ts"; import { getFlash } from "#shared/flash-context.ts"; import type { FormParams } from "#shared/form-data.ts"; import type { Field } from "#shared/forms/field.ts"; -import { readRepeatedPicklist } from "#shared/forms/repeated-picklist.ts"; import { type ValidationResult, validateForm, @@ -40,15 +38,11 @@ export const selectedRecalculationFields = ( form: FormParams, allowed: readonly [T, ...T[]], ): T[] => { - const selection = readRepeatedPicklist( - v.picklist(allowed), - form, - RECALCULATE_FIELD_NAME, - ); - if (selection.state === "invalid") { - throw new Error(`Invalid recalculation field: ${selection.value}`); + const selection = form.getRepeatedPicklist(RECALCULATE_FIELD_NAME, allowed); + if (!selection.ok) { + throw new Error(`Invalid recalculation field: ${selection.error}`); } - return selection.state === "selected" ? selection.values : []; + return selection.value; }; /** diff --git a/src/features/admin/settings-general.ts b/src/features/admin/settings-general.ts index fdc9ed974..da80e1542 100644 --- a/src/features/admin/settings-general.ts +++ b/src/features/admin/settings-general.ts @@ -15,7 +15,7 @@ import { settingsHandler, settingsToggle, } from "#routes/admin/settings-helpers.ts"; -import { COLUMN_LAYOUTS, type ColumnLayoutKind } from "#shared/column-order.ts"; +import { COLUMN_LAYOUTS } from "#shared/column-order.ts"; import { clearSessionCookie } from "#shared/cookies.ts"; import { logActivity } from "#shared/db/activityLog.ts"; import { settings } from "#shared/db/settings.ts"; @@ -192,8 +192,6 @@ export const handleBookingFeePost = settingsHandler({ * Build a column-order settings handler for the listing or attendee table. * Handles POST /admin/settings/{listing,attendee}-column-order - owner only */ -type ConfigurableColumnLayoutKind = Exclude; - const COLUMN_ORDER_SETTINGS = { attendee: { label: "Attendee column order", @@ -203,10 +201,9 @@ const COLUMN_ORDER_SETTINGS = { label: "Listing column order", update: settings.update.listingColumnOrder, }, -} satisfies Record< - ConfigurableColumnLayoutKind, - { label: string; update: (value: string) => Promise } ->; +}; + +type ConfigurableColumnLayoutKind = keyof typeof COLUMN_ORDER_SETTINGS; const columnOrderHandler = (kind: ConfigurableColumnLayoutKind) => { const config = COLUMN_ORDER_SETTINGS[kind]; diff --git a/src/features/admin/settings-listing-defaults.ts b/src/features/admin/settings-listing-defaults.ts index 37b46d965..830f9ae88 100644 --- a/src/features/admin/settings-listing-defaults.ts +++ b/src/features/admin/settings-listing-defaults.ts @@ -7,7 +7,6 @@ * their next read (defaults resolve live — see `resolveListingDefaults`). */ -import * as v from "valibot"; import { t } from "#i18n"; import { settingsHandler } from "#routes/admin/settings-helpers.ts"; import { ownerPage } from "#routes/auth.ts"; @@ -17,7 +16,6 @@ import { invalidateListingsCache } from "#shared/db/listings/records.ts"; import { settings } from "#shared/db/settings.ts"; import { isDemoMode } from "#shared/demo/mode.ts"; import type { FormParams } from "#shared/form-data.ts"; -import { readRepeatedPicklist } from "#shared/forms/repeated-picklist.ts"; import { LISTING_DEFAULT_FIELDS, type ListingDefaultField, @@ -82,25 +80,23 @@ const parseUrlField = ( /** Parse the bookable-days default: only set when its enable box is ticked, with * at least one valid day (in canonical order). */ const parseDaysField = (form: FormParams): FieldParse => { - const selection = readRepeatedPicklist( - v.picklist(VALID_DAY_NAMES), - form, + if (!form.getFlag("default_bookable_days_enabled")) return {}; + const selection = form.getRepeatedPicklist( "default_bookable_days", - form.getFlag("default_bookable_days_enabled"), + VALID_DAY_NAMES, ); - if (selection.state === "disabled") return {}; - if (selection.state === "invalid") { + if (!selection.ok) { return { error: t("fields.validation.invalid_day", { - day: selection.value, + day: selection.error, valid: VALID_DAY_NAMES.join(", "), }), }; } - if (selection.state === "absent") { + if (selection.value.length === 0) { return { error: t("listing_defaults.days_required") }; } - return { value: selection.values }; + return { value: selection.value }; }; /** Per-kind parser. The `Record` is keyed by {@link ListingDefaultKind}, so a diff --git a/src/shared/column-order.ts b/src/shared/column-order.ts index 0bfbb8902..8e9ad5053 100644 --- a/src/shared/column-order.ts +++ b/src/shared/column-order.ts @@ -12,7 +12,7 @@ import * as v from "valibot"; import { createBaseLiquidEngine } from "#shared/liquid-engine.ts"; -import { requireSuccess } from "#shared/result.ts"; +import type { Result } from "#shared/result.ts"; // --------------------------------------------------------------------------- // Types @@ -48,69 +48,9 @@ export type ColumnGenerators = Record< ColumnDef >; -export const ListingColumnSchema = v.picklist([ - "attendees", - "cost", - "created", - "date", - "description", - "location", - "name", - "price", - "profit", - "renewal", - "revenue", - "status", - "tickets", -]); -export type ListingColumn = v.InferOutput; - -export const EditorListingColumnSchema = v.picklist([ - "attendees", - "created", - "date", - "description", - "location", - "name", - "price", - "renewal", - "status", - "tickets", -]); -export type EditorListingColumn = v.InferOutput< - typeof EditorListingColumnSchema ->; - -export const AttendeeColumnSchema = v.picklist([ - "address", - "answers", - "date", - "email", - "listings", - "name", - "phone", - "qty", - "registered", - "special_instructions", - "status", - "ticket", -]); -export type AttendeeColumn = v.InferOutput; - -export const ColumnLayoutKindSchema = v.picklist([ - "listing", - "editor-listing", - "attendee", -]); -export type ColumnLayoutKind = v.InferOutput; - -const validatedColumnLayout = Symbol("validatedColumnLayout"); - export type ColumnLayout = { - readonly [validatedColumnLayout]: true; readonly columnKeys: readonly TColumn[]; readonly filters: ReadonlyMap; - readonly template: string; }; // --------------------------------------------------------------------------- @@ -148,40 +88,32 @@ const parseTagBody = ( * Validation is done by parsing extracted keys through the column schema. * No Liquid engine is needed for validation. */ -const parseColumnTemplate = < - const TOptions extends readonly [string, ...string[]], ->( +type ParsedLayout = Result<{ + columns: T[]; + filters: Map; +}>; + +const parseColumnTemplate = ( template: string, - schema: v.PicklistSchema, -): - | { - ok: true; - columns: TOptions[number][]; - filters: Map; - } - | { - ok: false; - error: string; - } => { - const columns: TOptions[number][] = []; - const filters = new Map(); + options: readonly T[], +): ParsedLayout => { + const columns: T[] = []; + const filters = new Map(); const seen = new Set(); for (const match of template.matchAll(LIQUID_TAG_RE)) { const { key, filter } = parseTagBody(match[1]!); - const parsed = v.safeParse(schema, key); - if (!parsed.success) { + const option = options.find((value) => value === key); + if (option === undefined) { return { - error: `Unknown column "${key}". Available columns: ${schema.options.join( - ", ", - )}`, + error: `Unknown column "${key}". Available columns: ${options.join(", ")}`, ok: false, }; } - if (!seen.has(parsed.output)) { - seen.add(parsed.output); - columns.push(parsed.output); - if (filter) filters.set(parsed.output, filter); + if (!seen.has(option)) { + seen.add(option); + columns.push(option); + if (filter) filters.set(option, filter); } } @@ -189,7 +121,7 @@ const parseColumnTemplate = < return { error: "Template must include at least one column", ok: false }; } - return { columns, filters, ok: true }; + return { ok: true, value: { columns, filters } }; }; /** @@ -208,73 +140,84 @@ export const buildDefaultTemplate = (keys: readonly string[]): string => * Shared by all listing/attendee table renderers. */ const defineColumnLayout = < - const TOptions extends readonly [string, ...string[]], + const TDefault extends readonly [string, ...string[]], + const TExtra extends readonly string[], >( - schema: v.PicklistSchema, - defaultOrder: readonly TOptions[number][], -) => ({ - defaultOrder, - parse(template: string): ColumnLayout { - if (!template) { + defaultOrder: TDefault, + extra: TExtra, +) => { + const [first, ...rest] = defaultOrder; + const schema = v.picklist([first, ...rest, ...extra]); + const options: readonly (TDefault[number] | TExtra[number])[] = [ + ...defaultOrder, + ...extra, + ]; + const defaultLayout: ColumnLayout = { + columnKeys: defaultOrder, + filters: new Map(), + }; + return { + defaultLayout, + defaultOrder, + defaultTemplate: buildDefaultTemplate(defaultOrder), + options, + parse(template: string): ColumnLayout { + if (!template) return defaultLayout; + const result = parseColumnTemplate(template, options); + if (!result.ok) throw new Error(result.error); return { - [validatedColumnLayout]: true, - columnKeys: defaultOrder, - filters: new Map(), - template, + columnKeys: result.value.columns, + filters: result.value.filters, }; - } - const result = parseColumnTemplate(template, schema); - requireSuccess(result); - return { - [validatedColumnLayout]: true, - columnKeys: result.columns, - filters: result.filters, - template, - }; - }, - schema, - validate(template: string): string | null { - if (!template) return null; - const result = parseColumnTemplate(template, schema); - return result.ok ? null : result.error; - }, -}); + }, + schema, + validate(template: string): string | null { + if (!template) return null; + const result = parseColumnTemplate(template, options); + return result.ok ? null : result.error; + }, + }; +}; export const COLUMN_LAYOUTS = { - attendee: defineColumnLayout(AttendeeColumnSchema, [ - "status", - "date", - "name", - "listings", - "email", - "phone", - "address", - "special_instructions", - "answers", - "qty", - "ticket", - "registered", - ]), - "editor-listing": defineColumnLayout(EditorListingColumnSchema, [ - "name", - "description", - "status", - "attendees", - "tickets", - "created", - ]), - listing: defineColumnLayout(ListingColumnSchema, [ - "name", - "description", - "status", - "attendees", - "tickets", - "revenue", - "cost", - "profit", - "created", - ]), -} satisfies Record>; + attendee: defineColumnLayout( + [ + "status", + "date", + "name", + "listings", + "email", + "phone", + "address", + "special_instructions", + "answers", + "qty", + "ticket", + "registered", + ], + [], + ), + listing: defineColumnLayout( + [ + "name", + "description", + "status", + "attendees", + "tickets", + "revenue", + "cost", + "profit", + "created", + ], + ["date", "location", "price", "renewal"], + ), +}; + +export type ColumnLayoutKind = keyof typeof COLUMN_LAYOUTS; +export type ListingColumn = + (typeof COLUMN_LAYOUTS)["listing"]["options"][number]; +export type AttendeeColumn = + (typeof COLUMN_LAYOUTS)["attendee"]["options"][number]; export type ListingColumnLayout = ReturnType< (typeof COLUMN_LAYOUTS)["listing"]["parse"] diff --git a/src/shared/columns/listing-columns.ts b/src/shared/columns/listing-columns.ts index e9ab0b2eb..9ba59b720 100644 --- a/src/shared/columns/listing-columns.ts +++ b/src/shared/columns/listing-columns.ts @@ -8,7 +8,7 @@ import type { ColumnDef, - EditorListingColumn, + ColumnLayout, ListingColumn, } from "#shared/column-order.ts"; import { formatCurrency } from "#shared/currency.ts"; @@ -149,15 +149,26 @@ export const LISTING_TABLE_COLUMNS = { * (revenue/cost/profit) are omitted entirely — not just unordered — so a saved * column template can never surface them, and the name links to the edit form * rather than the forbidden detail page. */ +export const EDITOR_LISTING_COLUMN_KEYS = [ + "name", + "description", + "status", + "attendees", + "tickets", + "created", +] as const; +type EditorListingColumn = (typeof EDITOR_LISTING_COLUMN_KEYS)[number]; + export const EDITOR_LISTING_TABLE_COLUMNS = { attendees, created, - date, description, - location, name: editorName, - price, - renewal, status, tickets, } satisfies Record; + +export const EDITOR_LISTING_LAYOUT: ColumnLayout = { + columnKeys: EDITOR_LISTING_COLUMN_KEYS, + filters: new Map(), +}; diff --git a/src/shared/form-data.ts b/src/shared/form-data.ts index df77df726..8216948e4 100644 --- a/src/shared/form-data.ts +++ b/src/shared/form-data.ts @@ -2,6 +2,7 @@ * Utilities for reading values from form data (URLSearchParams). */ +import type { Result } from "#shared/result.ts"; import { parseNonNegativeInt, parsePositiveIntId, @@ -24,8 +25,7 @@ export class FormParams extends URLSearchParams { /** A single field parsed as a strict non-negative integer, or null when blank/invalid. */ getOptionalInt(key: string): number | null { - const raw = this.getString(key); - return raw === "" ? null : parseNonNegativeInt(raw); + return parseNonNegativeInt(this.getString(key)); } /** All repeated values parsed as strict positive decimal ids, dropping invalid values. */ @@ -34,4 +34,18 @@ export class FormParams extends URLSearchParams { .map(parsePositiveIntId) .filter((n) => n !== null); } + + /** Validate a repeated field and return selected values in declared order. */ + getRepeatedPicklist( + key: string, + allowed: readonly T[], + ): Result { + const supplied = this.getAll(key); + const invalid = supplied.find( + (value) => !allowed.some((option) => option === value), + ); + if (invalid !== undefined) return { error: invalid, ok: false }; + const selected = new Set(supplied); + return { ok: true, value: allowed.filter((value) => selected.has(value)) }; + } } diff --git a/src/shared/forms/definition.ts b/src/shared/forms/definition.ts index f4fb0753c..1a94d6e75 100644 --- a/src/shared/forms/definition.ts +++ b/src/shared/forms/definition.ts @@ -6,7 +6,6 @@ import { } from "#shared/forms/field.ts"; import { renderField, renderFields } from "#shared/forms/rendering.tsx"; import { - type FieldValueNormalizer, type ValidationResult, validateForm, } from "#shared/forms/validation.ts"; @@ -55,10 +54,6 @@ export type FormValues = ? FormValuesFor : never; -type FormFieldRenderHelper = { - render: (value?: string) => string; -}; - type FormSectionId = TFields[number] extends infer TField ? TField extends { section: infer TSection extends string } @@ -66,18 +61,6 @@ type FormSectionId = : never : never; -const requiredDefinition = ( - definitions: ReadonlyMap, - kind: "field" | "section", - id: string, -): T => { - const definition = definitions.get(id); - if (definition === undefined) { - throw new Error(`Unknown ${kind}: ${id}`); - } - return definition; -}; - export interface FormSchema { fields: readonly Field[]; validate: (form: FormParams) => ValidationResult; @@ -90,8 +73,7 @@ export type FormDefinition< id: string; fields: TFields; render: (values?: FormRenderValuesFor) => string; - renderFields: (values?: FormRenderValuesFor) => string; - field: (name: TFields[number]["name"]) => FormFieldRenderHelper; + renderField: (name: TFields[number]["name"], value?: string) => string; section: ( id: FormSectionId, values?: FormRenderValuesFor, @@ -103,11 +85,6 @@ export type FormDefinition< ) => ValidationResult>; }; -const normalizeSelectValue: FieldValueNormalizer = ( - field: Field, - value: string | number | null, -) => (field.type === "select" && value === "" ? null : value); - /** Define a typed form schema that can render and validate from one source. */ export const defineForm = < TFields extends FormFieldDefinitions, @@ -120,14 +97,14 @@ export const defineForm = < context: TContext, ) => string | null; }): FormDefinition => { - const fields = config.fields.map((field) => { + const fields = config.fields; + for (const field of fields) { if (field.type === "select") { requireChoiceOptions(field.label, field.options); } else if (field.type === "checkbox-group") { requireCheckboxOptions(field.label, field.options); } - return field; - }); + } const fieldMap = new Map(fields.map((field) => [field.name, field] as const)); const sectionIds = [ ...new Set( @@ -136,22 +113,18 @@ export const defineForm = < ), ), ] as FormSectionId[]; - const sectionMap = new Map( - sectionIds.map((id) => [ - id, - fields.filter((field) => field.section === id), - ]), - ); + + const fieldByName = (name: TFields[number]["name"]): Field => { + const field = fieldMap.get(name); + if (!field) throw new Error(`Unknown field: ${name}`); + return field; + }; const validate = ( form: FormParams, context?: TContext, ): ValidationResult> => { - const base = validateForm>( - form, - fields, - normalizeSelectValue, - ); + const base = validateForm>(form, fields); if (!base.valid) return base; const values = base.values; @@ -168,21 +141,19 @@ export const defineForm = < const section = ( id: FormSectionId, values: FormRenderValuesFor = {}, - ): string => - renderFields( - requiredDefinition(sectionMap, "section", id), + ): string => { + if (!sectionIds.includes(id)) throw new Error(`Unknown section: ${id}`); + return renderFields( + fields.filter((field) => field.section === id), values as FieldValues, ); + }; return { - field: (name) => ({ - render: (value = "") => - renderField(requiredDefinition(fieldMap, "field", name), value), - }), fields: config.fields, id: config.id, render, - renderFields: render, + renderField: (name, value = "") => renderField(fieldByName(name), value), section, sections: sectionIds, validate, diff --git a/src/shared/forms/repeated-picklist.ts b/src/shared/forms/repeated-picklist.ts deleted file mode 100644 index f29a1fc54..000000000 --- a/src/shared/forms/repeated-picklist.ts +++ /dev/null @@ -1,31 +0,0 @@ -import * as v from "valibot"; -import type { FormParams } from "#shared/form-data.ts"; - -type PicklistOptions = readonly [string, ...string[]]; - -export type RepeatedPicklistValue = - | { state: "disabled" } - | { state: "absent" } - | { state: "invalid"; value: string } - | { state: "selected"; values: TValue[] }; - -/** Read a repeated form field against one ordered picklist schema. */ -export const readRepeatedPicklist = ( - schema: v.PicklistSchema, - form: FormParams, - name: string, - enabled = true, -): RepeatedPicklistValue => { - if (!enabled) return { state: "disabled" }; - if (!form.has(name)) return { state: "absent" }; - - const supplied = form.getAll(name); - const invalid = supplied.find((value) => !v.safeParse(schema, value).success); - if (invalid !== undefined) return { state: "invalid", value: invalid }; - - const selected = new Set(supplied); - return { - state: "selected", - values: schema.options.filter((value) => selected.has(value)), - }; -}; diff --git a/src/shared/forms/validation.ts b/src/shared/forms/validation.ts index ad38001d0..8e3252b2c 100644 --- a/src/shared/forms/validation.ts +++ b/src/shared/forms/validation.ts @@ -1,19 +1,12 @@ -import * as v from "valibot"; import { t } from "#i18n"; import type { FormParams } from "#shared/form-data.ts"; import type { ChoiceField, Field } from "#shared/forms/field.ts"; -import { readRepeatedPicklist } from "#shared/forms/repeated-picklist.ts"; import { DATETIME_PARTIAL_ERROR, readSubmittedFieldValue, } from "#shared/forms/submitted-value.ts"; import type { FieldValues } from "#shared/forms/values.ts"; -export type FieldValueNormalizer = ( - field: Field, - value: string | number | null, -) => string | number | null; - export type ValidationResult = | { valid: true; values: T } | { valid: false; error: string }; @@ -28,23 +21,18 @@ const parseFieldValue = ( ): string | number | null => field.parse ? field.parse(trimmed) - : field.type === "number" - ? trimmed - ? Number(trimmed) - : null - : trimmed; - -const choiceSchema = ( - field: ChoiceField<"select" | "checkbox-group">, -): v.PicklistSchema => { - const [first, ...rest] = field.options; - return v.picklist([first.value, ...rest.map((option) => option.value)]); -}; + : field.type === "select" && !trimmed + ? null + : field.type === "number" + ? trimmed + ? Number(trimmed) + : null + : trimmed; const hasInvalidChoice = ( field: ChoiceField<"select">, value: string, -): boolean => !v.safeParse(choiceSchema(field), value).success; +): boolean => !field.options.some((option) => option.value === value); const requiredFieldError = (field: Field): FieldValidationResult => ({ error: field.requiredMessage ?? `${field.label} is required`, @@ -66,27 +54,22 @@ const collectFieldValue = ( form: FormParams, field: Field, ): string | FieldValidationResult => { + if (field.type === "checkbox-group") { + const selection = form.getRepeatedPicklist( + field.name, + field.options.map((option) => option.value), + ); + if (!selection.ok) { + const error = field.validate?.(selection.error); + return error ? { error, valid: false } : invalidFieldError(field); + } + return selection.value.join(","); + } const raw = readSubmittedFieldValue(form, field); if (raw === null) return { error: DATETIME_PARTIAL_ERROR, valid: false }; return raw; }; -const validateCheckboxField = ( - form: FormParams, - field: ChoiceField<"checkbox-group">, -): FieldValidationResult => { - const selection = readRepeatedPicklist(choiceSchema(field), form, field.name); - if (selection.state === "invalid") { - const error = field.validate?.(selection.value); - return error ? { error, valid: false } : invalidFieldError(field); - } - const value = - selection.state === "selected" ? selection.values.join(",") : ""; - if (field.required && !value) return requiredFieldError(field); - const error = value ? field.validate?.(value) : undefined; - return error ? { error, valid: false } : { valid: true, value }; -}; - const validateFieldText = (field: Field, value: string): string | null => { if (field.validate && value) { const error = field.validate(value); @@ -111,10 +94,6 @@ const validateSingleField = ( ): FieldValidationResult => { if (field.type === "file") return { valid: true, value: null }; - if (field.type === "checkbox-group") { - return validateCheckboxField(form, field); - } - const collected = collectFieldValue(form, field); if (typeof collected !== "string") return collected; @@ -135,13 +114,12 @@ const validateSingleField = ( export const validateForm = ( form: FormParams, fields: readonly Field[], - normalizeValue: FieldValueNormalizer = (_field, value) => value, ): ValidationResult => { const values: FieldValues = {}; for (const field of fields) { const result = validateSingleField(form, field); if (!result.valid) return result; - values[field.name] = normalizeValue(field, result.value); + values[field.name] = result.value; } return { valid: true, values: values as T }; }; diff --git a/src/ui/templates/admin/dashboard.tsx b/src/ui/templates/admin/dashboard.tsx index a7d540808..24418d936 100644 --- a/src/ui/templates/admin/dashboard.tsx +++ b/src/ui/templates/admin/dashboard.tsx @@ -10,6 +10,7 @@ import { type ListingColumnLayout, } from "#shared/column-order.ts"; import { + EDITOR_LISTING_LAYOUT, EDITOR_LISTING_TABLE_COLUMNS, LISTING_TABLE_COLUMNS, } from "#shared/columns/listing-columns.ts"; @@ -232,7 +233,7 @@ export const adminDashboardPage = ( attributeFilterView: ListingAttributeFilterView = emptyAttributeFilterView(), ): string => { const { columnKeys, filters } = - listingColumnLayout ?? COLUMN_LAYOUTS.listing.parse(""); + listingColumnLayout ?? COLUMN_LAYOUTS.listing.defaultLayout; // Type filter narrows the listing table only; the stats, multi-booking, and // newest-attendee sections below stay based on the full set. Offer the bar @@ -321,8 +322,8 @@ export const adminListingsPage = ( ? EDITOR_LISTING_TABLE_COLUMNS : LISTING_TABLE_COLUMNS; const { columnKeys, filters } = isEditor - ? COLUMN_LAYOUTS["editor-listing"].parse("") - : (listingColumnLayout ?? COLUMN_LAYOUTS.listing.parse("")); + ? EDITOR_LISTING_LAYOUT + : (listingColumnLayout ?? COLUMN_LAYOUTS.listing.defaultLayout); const activeListings = activeOnly(listings); const deactivatedListings = filter((e: ListingWithCount) => !e.active)( listings, diff --git a/src/ui/templates/admin/guide/operations.tsx b/src/ui/templates/admin/guide/operations.tsx index 38d99af43..422544e46 100644 --- a/src/ui/templates/admin/guide/operations.tsx +++ b/src/ui/templates/admin/guide/operations.tsx @@ -2,7 +2,7 @@ * Admin guide — Operations sections. */ -import { buildDefaultTemplate, COLUMN_LAYOUTS } from "#shared/column-order.ts"; +import { COLUMN_LAYOUTS } from "#shared/column-order.ts"; import { ATTENDEE_TABLE_COLUMNS } from "#shared/columns/attendee-columns.ts"; import { LISTING_TABLE_COLUMNS } from "#shared/columns/listing-columns.ts"; import { Raw } from "#shared/jsx/jsx-runtime.ts"; @@ -14,9 +14,9 @@ import { } from "#templates/admin/guide/components.tsx"; /** The "Default order: ``" line a table-columns FAQ answer opens with. */ -const defaultOrderParagraph = (order: readonly string[]): JSX.Element => ( +const defaultOrderParagraph = (template: string): JSX.Element => (

- Default order: {buildDefaultTemplate(order)} + Default order: {template}

); @@ -133,14 +133,14 @@ export const operationsSections = (): GuideSection[] => [ custom( "listing_table_columns", <> - {defaultOrderParagraph(COLUMN_LAYOUTS.listing.defaultOrder)} + {defaultOrderParagraph(COLUMN_LAYOUTS.listing.defaultTemplate)} , ), custom( "attendee_table_columns", <> - {defaultOrderParagraph(COLUMN_LAYOUTS.attendee.defaultOrder)} + {defaultOrderParagraph(COLUMN_LAYOUTS.attendee.defaultTemplate)}

Columns referencing absent data (e.g. {"{{email}}"}{" "} when no attendees have an email) are hidden automatically even when diff --git a/src/ui/templates/admin/ledger/entry-pages.tsx b/src/ui/templates/admin/ledger/entry-pages.tsx index 369b22182..0cffd6b44 100644 --- a/src/ui/templates/admin/ledger/entry-pages.tsx +++ b/src/ui/templates/admin/ledger/entry-pages.tsx @@ -103,7 +103,7 @@ export const adminLedgerEntryAddPage = ({ action={`/admin/ledger/${account.type}/${account.id}/add`} afterFields={} buttonText={t("admin.ledger.add.submit")} - fieldsHtml={defineLedgerEntryAddForm(options).renderFields(values)} + fieldsHtml={defineLedgerEntryAddForm(options).render(values)} icon="plus" returnUrl={returnUrl} > @@ -135,7 +135,7 @@ export const adminLedgerEntryEditPage = ({ diff --git a/src/ui/templates/admin/news.tsx b/src/ui/templates/admin/news.tsx index bfbe6bb7a..99c8af0ef 100644 --- a/src/ui/templates/admin/news.tsx +++ b/src/ui/templates/admin/news.tsx @@ -73,7 +73,7 @@ export const adminNewsNewPage = ( active: ACTIVE, children: ( <> - + {t("news.create_submit")} ), @@ -88,7 +88,7 @@ export const adminNewsNewPage = ( export const newsEditPanel = (post: NewsPost): JSX.Element => contentEditPanel( `${LIST}/${post.id}/edit`, - newsPostEditForm.renderFields(newsPostToValues(post)), + newsPostEditForm.render(newsPostToValues(post)), ); export const adminNewsDeletePage = ( diff --git a/src/ui/templates/admin/questions.tsx b/src/ui/templates/admin/questions.tsx index 044c78c3e..00cf7448d 100644 --- a/src/ui/templates/admin/questions.tsx +++ b/src/ui/templates/admin/questions.tsx @@ -222,7 +222,7 @@ export const adminQuestionPage = ( action={`/admin/questions/${question.id}/edit`} submitLabel={t("questions.edit.update")} > - + {question.display_type === "free_text" ? ( // Free-text questions can't become choice questions (it would orphan // any stored text answers), so lock the type rather than offering it. diff --git a/src/ui/templates/admin/settings/column-order.tsx b/src/ui/templates/admin/settings/column-order.tsx index 259147dc5..1e723ea53 100644 --- a/src/ui/templates/admin/settings/column-order.tsx +++ b/src/ui/templates/admin/settings/column-order.tsx @@ -8,7 +8,7 @@ /* jscpd:ignore-start */ import { t } from "#i18n"; -import { buildDefaultTemplate, COLUMN_LAYOUTS } from "#shared/column-order.ts"; +import { COLUMN_LAYOUTS } from "#shared/column-order.ts"; import { ATTENDEE_TABLE_COLUMNS } from "#shared/columns/attendee-columns.ts"; import { LISTING_TABLE_COLUMNS } from "#shared/columns/listing-columns.ts"; import { Raw } from "#shared/jsx/jsx-runtime.ts"; @@ -17,12 +17,8 @@ import { textSettingsSection } from "#templates/components/settings-field-sectio /* jscpd:ignore-end */ -const listingDefault = buildDefaultTemplate( - COLUMN_LAYOUTS.listing.defaultOrder, -); -const attendeeDefault = buildDefaultTemplate( - COLUMN_LAYOUTS.attendee.defaultOrder, -); +const listingDefault = COLUMN_LAYOUTS.listing.defaultTemplate; +const attendeeDefault = COLUMN_LAYOUTS.attendee.defaultTemplate; /** Render available column tags as helper text */ const AvailableTags = ({ diff --git a/src/ui/templates/admin/site-pages.tsx b/src/ui/templates/admin/site-pages.tsx index 239e35404..dc9de3b6c 100644 --- a/src/ui/templates/admin/site-pages.tsx +++ b/src/ui/templates/admin/site-pages.tsx @@ -177,7 +177,7 @@ export const adminSitePageNewPage = ( t("site.pages.new_title"), LIST, session, - sitePageForm.renderFields(), + sitePageForm.render(), error, {t("site.pages.create_submit")}, ); @@ -220,7 +220,7 @@ const ItemPicker = ({ export const sitePageEditPanel = (page: SitePage): JSX.Element => contentEditPanel( `${LIST}/${page.id}/edit`, - sitePageEditForm.renderFields(contentFieldValues(page)), + sitePageEditForm.render(contentFieldValues(page)), ); /** The Items tab's panel: the page's current contents (reorderable, each diff --git a/test/integration/server/settings/column-order.test.ts b/test/integration/server/settings/column-order.test.ts index b04ea0a98..c410e981a 100644 --- a/test/integration/server/settings/column-order.test.ts +++ b/test/integration/server/settings/column-order.test.ts @@ -25,9 +25,6 @@ describeWithEnv("server (admin settings: column order)", { db: true }, () => { "name", "status", ]); - expect(settings.listingColumnLayout.template).toBe( - "{{name}}, {{status}}", - ); }); test("rejects invalid column name", async () => { diff --git a/test/shared/column-order.test.ts b/test/shared/column-order.test.ts index e8cdd7e45..d54fd29c0 100644 --- a/test/shared/column-order.test.ts +++ b/test/shared/column-order.test.ts @@ -9,6 +9,7 @@ import { renderFilteredValue, } from "#shared/column-order.ts"; import { + EDITOR_LISTING_LAYOUT, EDITOR_LISTING_TABLE_COLUMNS, LISTING_TABLE_COLUMNS, } from "#shared/columns/listing-columns.ts"; @@ -20,11 +21,11 @@ setupTestEncryptionKey(); describe("column layout validation", () => { test("derives every listing schema from the matching column definitions", () => { - expect(COLUMN_LAYOUTS.listing.schema.options).toEqual( - Object.keys(LISTING_TABLE_COLUMNS), + expect([...COLUMN_LAYOUTS.listing.schema.options].sort()).toEqual( + Object.keys(LISTING_TABLE_COLUMNS).sort(), ); - expect(COLUMN_LAYOUTS["editor-listing"].schema.options).toEqual( - Object.keys(EDITOR_LISTING_TABLE_COLUMNS), + expect([...EDITOR_LISTING_LAYOUT.columnKeys].sort()).toEqual( + Object.keys(EDITOR_LISTING_TABLE_COLUMNS).sort(), ); }); @@ -76,6 +77,23 @@ describe("column layout validation", () => { }); }); +test("keeps the attendee default column order", () => { + expect(COLUMN_LAYOUTS.attendee.defaultOrder).toEqual([ + "status", + "date", + "name", + "listings", + "email", + "phone", + "address", + "special_instructions", + "answers", + "qty", + "ticket", + "registered", + ]); +}); + describe("column layout parsing", () => { test("returns default order when template is empty", () => { const { columnKeys, filters } = COLUMN_LAYOUTS.listing.parse(""); diff --git a/test/shared/columns/attendee-columns.test.ts b/test/shared/columns/attendee-columns.test.ts index e01fb8e66..efbed6e33 100644 --- a/test/shared/columns/attendee-columns.test.ts +++ b/test/shared/columns/attendee-columns.test.ts @@ -104,23 +104,6 @@ describe("ATTENDEE_TABLE_COLUMNS metadata", () => { }); describe("the attendee default column order", () => { - test("matches the expected default order exactly", () => { - expect(COLUMN_LAYOUTS.attendee.defaultOrder).toEqual([ - "status", - "date", - "name", - "listings", - "email", - "phone", - "address", - "special_instructions", - "answers", - "qty", - "ticket", - "registered", - ]); - }); - test("is a permutation of ATTENDEE_TABLE_COLUMNS' keys", () => { expect([...COLUMN_LAYOUTS.attendee.defaultOrder].sort()).toEqual( Object.keys(ATTENDEE_TABLE_COLUMNS).sort(), diff --git a/test/shared/form-data.test.ts b/test/shared/form-data.test.ts index 8ba435dbb..57c3e7695 100644 --- a/test/shared/form-data.test.ts +++ b/test/shared/form-data.test.ts @@ -81,3 +81,35 @@ describe("FormParams.getNumberArray", () => { expect(form.getNumberArray("ids")).toEqual([]); }); }); + +describe("FormParams.getRepeatedPicklist", () => { + const allowed = ["Monday", "Tuesday", "Wednesday"] as const; + const read = (query: string) => + new FormParams(query).getRepeatedPicklist("days", allowed); + + test("returns no selections when the field is absent", () => { + expect(read("")).toEqual({ ok: true, value: [] }); + }); + + test("returns the first invalid supplied token", () => { + expect(read("days=Monday&days=Funday&days=Someday")).toEqual({ + error: "Funday", + ok: false, + }); + }); + + test("returns unique values in declared order", () => { + expect(read("days=Wednesday&days=Monday&days=Wednesday")).toEqual({ + ok: true, + value: ["Monday", "Wednesday"], + }); + }); + + test("rejects empty and padded tokens", () => { + expect(read("days=")).toEqual({ error: "", ok: false }); + expect(read("days=%20Monday%20")).toEqual({ + error: " Monday ", + ok: false, + }); + }); +}); diff --git a/test/shared/forms/definition/define-form.test.ts b/test/shared/forms/definition/define-form.test.ts index 79b31d2a4..57ed553d8 100644 --- a/test/shared/forms/definition/define-form.test.ts +++ b/test/shared/forms/definition/define-form.test.ts @@ -175,7 +175,7 @@ describe("defineForm", () => { id: "test", }); - const html = form.field("color").render("blue"); + const html = form.renderField("color", "blue"); expect(html).toContain("blue"); }); diff --git a/test/shared/forms/definition/field-schema.test.ts b/test/shared/forms/definition/field-schema.test.ts index fbe6e33a0..d1958805e 100644 --- a/test/shared/forms/definition/field-schema.test.ts +++ b/test/shared/forms/definition/field-schema.test.ts @@ -245,7 +245,7 @@ describe("form field schema", () => { }); const renderUnknownField = () => { // @ts-expect-error Field names come from the field declarations. - form.field("missing").render(); + form.renderField("missing"); }; expect(renderUnknownField).toThrow("Unknown field: missing"); @@ -257,6 +257,6 @@ describe("form field schema", () => { id: "blank-field", }); - expect(form.field("name").render()).not.toContain("value="); + expect(form.renderField("name")).not.toContain("value="); }); }); diff --git a/test/shared/forms/repeated-picklist.test.ts b/test/shared/forms/repeated-picklist.test.ts deleted file mode 100644 index 2d140cd4e..000000000 --- a/test/shared/forms/repeated-picklist.test.ts +++ /dev/null @@ -1,44 +0,0 @@ -import { expect } from "@std/expect"; -import { describe, it as test } from "@std/testing/bdd"; -import * as v from "valibot"; -import { FormParams } from "#shared/form-data.ts"; -import { readRepeatedPicklist } from "#shared/forms/repeated-picklist.ts"; - -const DaySchema = v.picklist(["Monday", "Tuesday", "Wednesday"]); - -const read = (query: string, enabled = true) => - readRepeatedPicklist(DaySchema, new FormParams(query), "days", enabled); - -describe("readRepeatedPicklist", () => { - test("is enabled when the flag is omitted", () => { - expect( - readRepeatedPicklist(DaySchema, new FormParams("days=Monday"), "days"), - ).toEqual({ state: "selected", values: ["Monday"] }); - }); - - test("returns disabled without accepting supplied values", () => { - expect(read("days=Monday", false)).toEqual({ state: "disabled" }); - }); - - test("distinguishes an absent enabled selection", () => { - expect(read("")).toEqual({ state: "absent" }); - }); - - test("returns the first invalid supplied token", () => { - expect(read("days=Monday&days=Funday&days=Someday")).toEqual({ - state: "invalid", - value: "Funday", - }); - }); - - test("returns unique values in schema order", () => { - expect(read("days=Wednesday&days=Monday&days=Wednesday")).toEqual({ - state: "selected", - values: ["Monday", "Wednesday"], - }); - }); - - test("treats a supplied empty token as invalid", () => { - expect(read("days=")).toEqual({ state: "invalid", value: "" }); - }); -}); diff --git a/test/shared/forms/validation.test.ts b/test/shared/forms/validation.test.ts index bc566eb16..98db9d8ae 100644 --- a/test/shared/forms/validation.test.ts +++ b/test/shared/forms/validation.test.ts @@ -85,6 +85,13 @@ describe("validateForm", () => { if (result.valid) expect(result.values.note).toBe(""); }); + test("returns null for an empty optional select", () => { + expect(validateForm(new FormParams(), colorSelectFields())).toEqual({ + valid: true, + values: { color: null }, + }); + }); + test("runs custom validate function and surfaces its error", () => { const fields: Field[] = [ field({ diff --git a/test/ui/templates/admin/ledger/forms.test.ts b/test/ui/templates/admin/ledger/forms.test.ts index 49ad77518..ce54a77e7 100644 --- a/test/ui/templates/admin/ledger/forms.test.ts +++ b/test/ui/templates/admin/ledger/forms.test.ts @@ -38,7 +38,7 @@ describe("ledger entry forms", () => { type: MANUAL_ATTENDEE_WRITEOFF, }, ]; - const html = defineLedgerEntryAddForm(options).renderFields({ + const html = defineLedgerEntryAddForm(options).render({ amount: "5.00", entry_type: MANUAL_ATTENDEE_CHARGE, occurred_at: "2026-06-22T09:30", From 997e33a505bc7f5f1fc8ab6a6d13a7f4703621f4 Mon Sep 17 00:00:00 2001 From: Stefan Date: Sun, 19 Jul 2026 16:42:05 +0100 Subject: [PATCH 3/6] Address declarative input review feedback --- src/ui/templates/attendee-table.tsx | 2 +- test/shared/forms/validation.test.ts | 9 +++++++++ test/test-utils/validation.ts | 4 +++- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/ui/templates/attendee-table.tsx b/src/ui/templates/attendee-table.tsx index 5d5af2083..56f436825 100644 --- a/src/ui/templates/attendee-table.tsx +++ b/src/ui/templates/attendee-table.tsx @@ -73,7 +73,7 @@ export type AttendeeTableOptions = { presorted?: boolean; /** Question data for the Answers column */ questionData?: TableQuestionData | undefined; - /** Liquid template controlling column order (e.g. "{{name}}, {{email}}, {{qty}}") */ + /** Pre-parsed layout controlling column order and filters */ columnLayout?: AttendeeColumnLayout; }; diff --git a/test/shared/forms/validation.test.ts b/test/shared/forms/validation.test.ts index 98db9d8ae..11f7a6ad9 100644 --- a/test/shared/forms/validation.test.ts +++ b/test/shared/forms/validation.test.ts @@ -251,6 +251,15 @@ describe("validateForm", () => { ).toEqual({ error: "Choose listed days only.", valid: false }); }); + test("keeps a custom validation error for a bad checkbox value", () => { + const fields = dayCheckboxFields(); + fields[0]!.validate = () => "Choose a valid day."; + expect(validateForm(new FormParams("days=Funday"), fields)).toEqual({ + error: "Choose a valid day.", + valid: false, + }); + }); + test("returns empty string for empty checkbox-group", () => { const fields: Field[] = [ field({ diff --git a/test/test-utils/validation.ts b/test/test-utils/validation.ts index b77a86307..ca17ca41e 100644 --- a/test/test-utils/validation.ts +++ b/test/test-utils/validation.ts @@ -19,7 +19,9 @@ export const expectValid = ( }; export const expectInvalid = - (expectedError: string) => + ( + expectedError: string, + ): ((fields: readonly Field[], data: TestFormValues) => void) => (fields: readonly Field[], data: TestFormValues): void => { const result = validateFormData(fields, data); expect(result.valid).toBe(false); From 1384659ed448a008cf8a839b2c81c99dfdef8722 Mon Sep 17 00:00:00 2001 From: Stefan Date: Sun, 19 Jul 2026 17:09:23 +0100 Subject: [PATCH 4/6] Unify declarative input sources --- src/shared/column-order.ts | 35 +++++++++++-------------- src/shared/day-names.ts | 12 +++------ src/shared/forms/definition.ts | 8 +++--- test/shared/dates/pinned-values.test.ts | 19 +++----------- test/shared/day-names.test.ts | 23 ++++++++++++++++ test/shared/forms/validation.test.ts | 8 +++++- 6 files changed, 57 insertions(+), 48 deletions(-) create mode 100644 test/shared/day-names.test.ts diff --git a/src/shared/column-order.ts b/src/shared/column-order.ts index 8e9ad5053..8d113fb9c 100644 --- a/src/shared/column-order.ts +++ b/src/shared/column-order.ts @@ -93,23 +93,28 @@ type ParsedLayout = Result<{ filters: Map; }>; -const parseColumnTemplate = ( +type ColumnSchema = v.GenericSchema & { + readonly options: readonly string[]; +}; + +const parseColumnTemplate = ( template: string, - options: readonly T[], -): ParsedLayout => { - const columns: T[] = []; - const filters = new Map(); + schema: TSchema, +): ParsedLayout> => { + const columns: v.InferOutput[] = []; + const filters = new Map, string>(); const seen = new Set(); for (const match of template.matchAll(LIQUID_TAG_RE)) { const { key, filter } = parseTagBody(match[1]!); - const option = options.find((value) => value === key); - if (option === undefined) { + const parsed = v.safeParse(schema, key); + if (!parsed.success) { return { - error: `Unknown column "${key}". Available columns: ${options.join(", ")}`, + error: `Unknown column "${key}". Available columns: ${schema.options.join(", ")}`, ok: false, }; } + const option = parsed.output; if (!seen.has(option)) { seen.add(option); columns.push(option); @@ -124,10 +129,6 @@ const parseColumnTemplate = ( return { ok: true, value: { columns, filters } }; }; -/** - * Validate a column-order template without extracting columns. - * Returns null if valid, or an error message string if invalid. - */ /** * Build a default template from an ordered list of column keys. * Produces e.g. "{{name}}, {{description}}, {{actions}}" @@ -148,10 +149,6 @@ const defineColumnLayout = < ) => { const [first, ...rest] = defaultOrder; const schema = v.picklist([first, ...rest, ...extra]); - const options: readonly (TDefault[number] | TExtra[number])[] = [ - ...defaultOrder, - ...extra, - ]; const defaultLayout: ColumnLayout = { columnKeys: defaultOrder, filters: new Map(), @@ -160,10 +157,10 @@ const defineColumnLayout = < defaultLayout, defaultOrder, defaultTemplate: buildDefaultTemplate(defaultOrder), - options, + options: schema.options as readonly (TDefault[number] | TExtra[number])[], parse(template: string): ColumnLayout { if (!template) return defaultLayout; - const result = parseColumnTemplate(template, options); + const result = parseColumnTemplate(template, schema); if (!result.ok) throw new Error(result.error); return { columnKeys: result.value.columns, @@ -173,7 +170,7 @@ const defineColumnLayout = < schema, validate(template: string): string | null { if (!template) return null; - const result = parseColumnTemplate(template, options); + const result = parseColumnTemplate(template, schema); return result.ok ? null : result.error; }, }; diff --git a/src/shared/day-names.ts b/src/shared/day-names.ts index ff5351af2..549add991 100644 --- a/src/shared/day-names.ts +++ b/src/shared/day-names.ts @@ -15,13 +15,7 @@ export const DAY_NAMES = [ "Saturday", ] as const; +const [sunday, ...mondayFirst] = DAY_NAMES; + /** Valid day names for bookable_days (Monday-first for display) */ -export const VALID_DAY_NAMES = [ - "Monday", - "Tuesday", - "Wednesday", - "Thursday", - "Friday", - "Saturday", - "Sunday", -] as const; +export const VALID_DAY_NAMES = [...mondayFirst, sunday] as const; diff --git a/src/shared/forms/definition.ts b/src/shared/forms/definition.ts index 1a94d6e75..d8599e445 100644 --- a/src/shared/forms/definition.ts +++ b/src/shared/forms/definition.ts @@ -1,3 +1,4 @@ +import { reduce } from "#fp"; import type { FormParams } from "#shared/form-data.ts"; import { type Field, @@ -98,14 +99,15 @@ export const defineForm = < ) => string | null; }): FormDefinition => { const fields = config.fields; - for (const field of fields) { + const fieldMap = reduce((map: Map, field: Field) => { if (field.type === "select") { requireChoiceOptions(field.label, field.options); } else if (field.type === "checkbox-group") { requireCheckboxOptions(field.label, field.options); } - } - const fieldMap = new Map(fields.map((field) => [field.name, field] as const)); + map.set(field.name, field); + return map; + }, new Map())([...fields]); const sectionIds = [ ...new Set( fields.flatMap((field) => diff --git a/test/shared/dates/pinned-values.test.ts b/test/shared/dates/pinned-values.test.ts index 8d44d2650..c885d7f83 100644 --- a/test/shared/dates/pinned-values.test.ts +++ b/test/shared/dates/pinned-values.test.ts @@ -1,7 +1,7 @@ /** * Pins exact outputs of the date helpers so small slips can't hide: every - * day and month name in a rendered label, the Monday-first day list, hour - * rounding, calendar-grid boundaries, and booked-range arithmetic. Each + * day and month name in a rendered label, hour rounding, calendar-grid + * boundaries, and booked-range arithmetic. Each * assertion here exists to fail under a specific one-token change that the * broader behaviour tests in dates.test.ts did not distinguish. */ @@ -18,7 +18,7 @@ import { startOfHour, widestDatedEntry, } from "#shared/dates.ts"; -import { DAY_NAMES, VALID_DAY_NAMES } from "#shared/day-names.ts"; +import { VALID_DAY_NAMES } from "#shared/day-names.ts"; import { testListing } from "#test-utils/factories.ts"; import { useSetting } from "#test-utils/settings.ts"; @@ -62,19 +62,6 @@ describe("dates — pinned values", () => { } }); - test("VALID_DAY_NAMES is the Monday-first rotation of DAY_NAMES", () => { - expect(VALID_DAY_NAMES).toEqual([ - "Monday", - "Tuesday", - "Wednesday", - "Thursday", - "Friday", - "Saturday", - "Sunday", - ]); - expect(DAY_NAMES[0]).toBe("Sunday"); - }); - test("adding months clamps a month-end start to each target month's length", () => { // From 31 January, every non-leap target month clamps to its true length. const expectedDays = [28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31]; diff --git a/test/shared/day-names.test.ts b/test/shared/day-names.test.ts new file mode 100644 index 000000000..29261c6ce --- /dev/null +++ b/test/shared/day-names.test.ts @@ -0,0 +1,23 @@ +import { expect } from "@std/expect"; +import { DAY_NAMES, VALID_DAY_NAMES } from "#shared/day-names.ts"; + +Deno.test("bookable day names rotate the date order to Monday first", () => { + expect(DAY_NAMES).toEqual([ + "Sunday", + "Monday", + "Tuesday", + "Wednesday", + "Thursday", + "Friday", + "Saturday", + ]); + expect(VALID_DAY_NAMES).toEqual([ + "Monday", + "Tuesday", + "Wednesday", + "Thursday", + "Friday", + "Saturday", + "Sunday", + ]); +}); diff --git a/test/shared/forms/validation.test.ts b/test/shared/forms/validation.test.ts index 11f7a6ad9..07d6251ae 100644 --- a/test/shared/forms/validation.test.ts +++ b/test/shared/forms/validation.test.ts @@ -15,7 +15,7 @@ const requiredName: Field[] = [ const dayCheckboxFields = (invalidMessage?: string): Field[] => [ field({ - ...(invalidMessage ? { invalidMessage } : {}), + ...(invalidMessage === undefined ? {} : { invalidMessage }), label: "Days", name: "days", options: [ @@ -251,6 +251,12 @@ describe("validateForm", () => { ).toEqual({ error: "Choose listed days only.", valid: false }); }); + test("keeps an explicitly empty invalid checkbox message", () => { + expect( + validateForm(new FormParams("days=Funday"), dayCheckboxFields("")), + ).toEqual({ error: "", valid: false }); + }); + test("keeps a custom validation error for a bad checkbox value", () => { const fields = dayCheckboxFields(); fields[0]!.validate = () => "Choose a valid day."; From 01056a5568709cbadde19a2eef3f2eba518cd57d Mon Sep 17 00:00:00 2001 From: Stefan Date: Sun, 19 Jul 2026 21:05:12 +0100 Subject: [PATCH 5/6] Address final declarative input review --- scripts/mutation/equivalent-mutants.txt | 1 + src/features/admin/dashboard.ts | 8 +- src/features/admin/entity-write-tab.ts | 2 +- src/features/admin/listings-edit.ts | 2 +- src/features/admin/logistics-agent-page.tsx | 2 +- src/features/admin/modifiers.ts | 2 +- src/features/admin/settings-general.ts | 2 +- src/shared/column-layout.ts | 147 +++++++++++++++ src/shared/column-order.ts | 174 +----------------- src/shared/columns/attendee-columns.ts | 3 +- src/shared/columns/listing-columns.ts | 7 +- src/shared/db/settings.ts | 2 +- src/shared/form-data.ts | 8 + src/ui/templates/admin/dashboard.tsx | 2 +- src/ui/templates/admin/guide/operations.tsx | 2 +- .../templates/admin/settings/column-order.tsx | 2 +- src/ui/templates/attendee-table.tsx | 11 +- test/features/admin/dashboard.test.ts | 79 ++++++++ .../admin/dashboard/listings-csv.test.ts} | 5 + test/features/admin/dashboard/log.test.ts | 74 ++++++++ .../server/listings/create-validation.test.ts | 15 ++ .../server-listings/log-and-activity.test.ts | 70 +------ test/shared/column-layout.test.ts | 118 ++++++++++++ test/shared/column-order.test.ts | 136 +------------- test/shared/columns/attendee-columns.test.ts | 3 +- test/shared/form-data.test.ts | 10 + test/test-utils/session.ts | 3 +- test/ui/templates/admin/dashboard.test.ts | 2 +- .../templates/attendee-table/template.test.ts | 2 +- 29 files changed, 491 insertions(+), 403 deletions(-) create mode 100644 src/shared/column-layout.ts create mode 100644 test/features/admin/dashboard.test.ts rename test/{lib/server-listings-csv.test.ts => features/admin/dashboard/listings-csv.test.ts} (87%) create mode 100644 test/features/admin/dashboard/log.test.ts create mode 100644 test/shared/column-layout.test.ts diff --git a/scripts/mutation/equivalent-mutants.txt b/scripts/mutation/equivalent-mutants.txt index e61bbf1f5..c61ed71b6 100644 --- a/scripts/mutation/equivalent-mutants.txt +++ b/scripts/mutation/equivalent-mutants.txt @@ -35,6 +35,7 @@ src/shared/forms/validation.ts:69:10 === → == # readSubmittedFieldValue ret src/shared/forms/validation.ts:101:46 = → += # this assignment runs only when trimmed is the empty string, so replacing it and appending to it produce the same defaultValue src/shared/forms/validation.ts:105:16 !== → != # validateFieldText returns string|null and cannot return undefined, so strict and loose inequality agree when comparing it with null src/features/admin/settings-listing-defaults.ts:53:15 === → == # parseNonNegativeInt returns number|null and cannot return undefined, so strict and loose equality agree when comparing it with null +src/features/admin/dashboard.ts:235:65 1 → 2 # The query needs any number above the 200-row display limit only to detect overflow; both limits produce the same truncated flag and the same first 200 displayed rows src/features/admin/entity-pages.ts:275:24 ?? → || # opts.query is a URLSearchParams object when present, so it is always truthy src/features/admin/site-content-page.ts:87:38 ?? → || # extraTabs is an array when present, and arrays are always truthy diff --git a/src/features/admin/dashboard.ts b/src/features/admin/dashboard.ts index b97f498ad..fc59de32a 100644 --- a/src/features/admin/dashboard.ts +++ b/src/features/admin/dashboard.ts @@ -170,7 +170,9 @@ const handleAdminListingsGet: TypedRouteHandler<"GET /admin/listings"> = return adminListingsPage( listings, session, - settings.listingColumnLayout, + session.adminLevel === "editor" + ? undefined + : settings.listingColumnLayout, await loadListingAttributeFilterContext(request, listings), ); }); @@ -232,9 +234,7 @@ const handleAdminLog: TypedRouteHandler<"GET /admin/log"> = sessionPage( async (session) => { const entries = await getAllActivityLog(LOG_DISPLAY_LIMIT + 1); const truncated = entries.length > LOG_DISPLAY_LIMIT; - const displayEntries = truncated - ? entries.slice(0, LOG_DISPLAY_LIMIT) - : entries; + const displayEntries = entries.slice(0, LOG_DISPLAY_LIMIT); const refs = await loadActivityLogRefs(displayEntries); return adminGlobalActivityLogPage(displayEntries, truncated, session, refs); }, diff --git a/src/features/admin/entity-write-tab.ts b/src/features/admin/entity-write-tab.ts index 963ab5498..f14787d71 100644 --- a/src/features/admin/entity-write-tab.ts +++ b/src/features/admin/entity-write-tab.ts @@ -35,7 +35,7 @@ export const submittedValueProps = ( rejected ? { error: rejected.error, - values: Object.fromEntries(rejected.form.entries()), + values: rejected.form.toRenderValues(), } : {}; diff --git a/src/features/admin/listings-edit.ts b/src/features/admin/listings-edit.ts index 599aa2977..37c589121 100644 --- a/src/features/admin/listings-edit.ts +++ b/src/features/admin/listings-edit.ts @@ -222,7 +222,7 @@ const renderCreateListingError = async ( // re-renders their selection rather than dropping every group. selectedGroupIds: parseGroupIds(form), templateId, - values: Object.fromEntries(form.entries()), + values: form.toRenderValues(), }, 400, ); diff --git a/src/features/admin/logistics-agent-page.tsx b/src/features/admin/logistics-agent-page.tsx index 786cadf4b..fb461adf6 100644 --- a/src/features/admin/logistics-agent-page.tsx +++ b/src/features/admin/logistics-agent-page.tsx @@ -56,7 +56,7 @@ export const logisticsAgentPage: EditEntityPage = ...(rejected ? { error: rejected.error, - values: Object.fromEntries(rejected.form.entries()), + values: rejected.form.toRenderValues(), } : {}), }); diff --git a/src/features/admin/modifiers.ts b/src/features/admin/modifiers.ts index b9da2a35c..8d7410a83 100644 --- a/src/features/admin/modifiers.ts +++ b/src/features/admin/modifiers.ts @@ -316,7 +316,7 @@ const modifierPage: EditEntityPage = defineEditEntityPage({ modifier, ctx.session, rejected?.error, - rejected ? Object.fromEntries(rejected.form.entries()) : undefined, + rejected?.form.toRenderValues(), ), guard: requireSessionOr, guideFooter: () => Promise.resolve(ModifiersGuideFooter()), diff --git a/src/features/admin/settings-general.ts b/src/features/admin/settings-general.ts index da80e1542..1bd42dfbf 100644 --- a/src/features/admin/settings-general.ts +++ b/src/features/admin/settings-general.ts @@ -15,7 +15,7 @@ import { settingsHandler, settingsToggle, } from "#routes/admin/settings-helpers.ts"; -import { COLUMN_LAYOUTS } from "#shared/column-order.ts"; +import { COLUMN_LAYOUTS } from "#shared/column-layout.ts"; import { clearSessionCookie } from "#shared/cookies.ts"; import { logActivity } from "#shared/db/activityLog.ts"; import { settings } from "#shared/db/settings.ts"; diff --git a/src/shared/column-layout.ts b/src/shared/column-layout.ts new file mode 100644 index 000000000..77ee5be95 --- /dev/null +++ b/src/shared/column-layout.ts @@ -0,0 +1,147 @@ +/** Parse and validate configurable table column layouts without loading the + * Liquid rendering engine used later to format cell values. */ + +import * as v from "valibot"; +import type { Result } from "#shared/result.ts"; + +export type ColumnLayout = { + readonly columnKeys: readonly TColumn[]; + readonly filters: ReadonlyMap; +}; + +const LIQUID_TAG_RE = /\{\{\s*([^}]+?)\s*\}\}/g; + +const parseTagBody = ( + body: string, +): { key: string; filter: string | undefined } => { + const pipeIdx = body.indexOf("|"); + if (pipeIdx === -1) return { filter: undefined, key: body.trim() }; + return { filter: body.trim(), key: body.slice(0, pipeIdx).trim() }; +}; + +type ParsedLayout = Result<{ + columns: T[]; + filters: Map; +}>; + +type ColumnSchema = v.GenericSchema & { + readonly options: readonly string[]; +}; + +const parseColumnTemplate = ( + template: string, + schema: TSchema, +): ParsedLayout> => { + const columns: v.InferOutput[] = []; + const filters = new Map, string>(); + const seen = new Set(); + + for (const match of template.matchAll(LIQUID_TAG_RE)) { + const { key, filter } = parseTagBody(match[1]!); + const parsed = v.safeParse(schema, key); + if (!parsed.success) { + return { + error: `Unknown column "${key}". Available columns: ${schema.options.join(", ")}`, + ok: false, + }; + } + const option = parsed.output; + if (!seen.has(option)) { + seen.add(option); + columns.push(option); + if (filter) filters.set(option, filter); + } + } + + if (columns.length === 0) { + return { error: "Template must include at least one column", ok: false }; + } + + return { ok: true, value: { columns, filters } }; +}; + +export const buildDefaultTemplate = (keys: readonly string[]): string => + keys.map((key) => `{{${key}}}`).join(", "); + +const defineColumnLayout = < + const TDefault extends readonly [string, ...string[]], + const TExtra extends readonly string[], +>( + defaultOrder: TDefault, + extra: TExtra, +) => { + const [first, ...rest] = defaultOrder; + const schema = v.picklist([first, ...rest, ...extra]); + const defaultLayout: ColumnLayout = { + columnKeys: defaultOrder, + filters: new Map(), + }; + return { + defaultLayout, + defaultOrder, + defaultTemplate: buildDefaultTemplate(defaultOrder), + options: schema.options as readonly (TDefault[number] | TExtra[number])[], + parse(template: string): ColumnLayout { + if (!template) return defaultLayout; + const result = parseColumnTemplate(template, schema); + if (!result.ok) throw new Error(result.error); + return { + columnKeys: result.value.columns, + filters: result.value.filters, + }; + }, + schema, + validate(template: string): string | null { + if (!template) return null; + const result = parseColumnTemplate(template, schema); + return result.ok ? null : result.error; + }, + }; +}; + +export const COLUMN_LAYOUTS = { + attendee: defineColumnLayout( + [ + "status", + "date", + "name", + "listings", + "email", + "phone", + "address", + "special_instructions", + "answers", + "qty", + "ticket", + "registered", + ], + [], + ), + listing: defineColumnLayout( + [ + "name", + "description", + "status", + "attendees", + "tickets", + "revenue", + "cost", + "profit", + "created", + ], + ["date", "location", "price", "renewal"], + ), +}; + +export type ColumnLayoutKind = keyof typeof COLUMN_LAYOUTS; +export type ListingColumn = + (typeof COLUMN_LAYOUTS)["listing"]["options"][number]; +export type AttendeeColumn = + (typeof COLUMN_LAYOUTS)["attendee"]["options"][number]; + +export type ListingColumnLayout = ReturnType< + (typeof COLUMN_LAYOUTS)["listing"]["parse"] +>; +export type AttendeeColumnLayout = ReturnType< + (typeof COLUMN_LAYOUTS)["attendee"]["parse"] +>; diff --git a/src/shared/column-order.ts b/src/shared/column-order.ts index 8d113fb9c..8980f6b46 100644 --- a/src/shared/column-order.ts +++ b/src/shared/column-order.ts @@ -10,9 +10,8 @@ * {{price | currency}} → "£25.00" */ -import * as v from "valibot"; +import { once } from "#fp"; import { createBaseLiquidEngine } from "#shared/liquid-engine.ts"; -import type { Result } from "#shared/result.ts"; // --------------------------------------------------------------------------- // Types @@ -48,180 +47,13 @@ export type ColumnGenerators = Record< ColumnDef >; -export type ColumnLayout = { - readonly columnKeys: readonly TColumn[]; - readonly filters: ReadonlyMap; -}; - // --------------------------------------------------------------------------- // Liquid engine — single instance for rendering filtered values // --------------------------------------------------------------------------- // `currency` is custom; `date` is a LiquidJS built-in (strftime on Date objects). // ISO string → Date conversion happens in renderFilteredValue before calling Liquid. -const engine = createBaseLiquidEngine(); - -// --------------------------------------------------------------------------- -// Template parsing — regex-based extraction + validation -// --------------------------------------------------------------------------- - -/** Regex to extract Liquid output tags: {{ expression }} */ -const LIQUID_TAG_RE = /\{\{\s*([^}]+?)\s*\}\}/g; - -/** - * Extract the column key and optional filter expression from a Liquid tag body. - * "name" → { key: "name", filter: undefined } - * "date | date: \"%B\"" → { key: "date", filter: "date | date: \"%B\"" } - */ -const parseTagBody = ( - body: string, -): { key: string; filter: string | undefined } => { - const pipeIdx = body.indexOf("|"); - if (pipeIdx === -1) return { filter: undefined, key: body.trim() }; - return { filter: body.trim(), key: body.slice(0, pipeIdx).trim() }; -}; - -/** - * Parse a column-order template and extract the ordered list of column keys - * plus any per-column Liquid filter expressions. - * - * Validation is done by parsing extracted keys through the column schema. - * No Liquid engine is needed for validation. - */ -type ParsedLayout = Result<{ - columns: T[]; - filters: Map; -}>; - -type ColumnSchema = v.GenericSchema & { - readonly options: readonly string[]; -}; - -const parseColumnTemplate = ( - template: string, - schema: TSchema, -): ParsedLayout> => { - const columns: v.InferOutput[] = []; - const filters = new Map, string>(); - const seen = new Set(); - - for (const match of template.matchAll(LIQUID_TAG_RE)) { - const { key, filter } = parseTagBody(match[1]!); - const parsed = v.safeParse(schema, key); - if (!parsed.success) { - return { - error: `Unknown column "${key}". Available columns: ${schema.options.join(", ")}`, - ok: false, - }; - } - const option = parsed.output; - if (!seen.has(option)) { - seen.add(option); - columns.push(option); - if (filter) filters.set(option, filter); - } - } - - if (columns.length === 0) { - return { error: "Template must include at least one column", ok: false }; - } - - return { ok: true, value: { columns, filters } }; -}; - -/** - * Build a default template from an ordered list of column keys. - * Produces e.g. "{{name}}, {{description}}, {{actions}}" - */ -export const buildDefaultTemplate = (keys: readonly string[]): string => - keys.map((k) => `{{${k}}}`).join(", "); - -/** - * Parse a template and return column keys and filters. - * Shared by all listing/attendee table renderers. - */ -const defineColumnLayout = < - const TDefault extends readonly [string, ...string[]], - const TExtra extends readonly string[], ->( - defaultOrder: TDefault, - extra: TExtra, -) => { - const [first, ...rest] = defaultOrder; - const schema = v.picklist([first, ...rest, ...extra]); - const defaultLayout: ColumnLayout = { - columnKeys: defaultOrder, - filters: new Map(), - }; - return { - defaultLayout, - defaultOrder, - defaultTemplate: buildDefaultTemplate(defaultOrder), - options: schema.options as readonly (TDefault[number] | TExtra[number])[], - parse(template: string): ColumnLayout { - if (!template) return defaultLayout; - const result = parseColumnTemplate(template, schema); - if (!result.ok) throw new Error(result.error); - return { - columnKeys: result.value.columns, - filters: result.value.filters, - }; - }, - schema, - validate(template: string): string | null { - if (!template) return null; - const result = parseColumnTemplate(template, schema); - return result.ok ? null : result.error; - }, - }; -}; - -export const COLUMN_LAYOUTS = { - attendee: defineColumnLayout( - [ - "status", - "date", - "name", - "listings", - "email", - "phone", - "address", - "special_instructions", - "answers", - "qty", - "ticket", - "registered", - ], - [], - ), - listing: defineColumnLayout( - [ - "name", - "description", - "status", - "attendees", - "tickets", - "revenue", - "cost", - "profit", - "created", - ], - ["date", "location", "price", "renewal"], - ), -}; - -export type ColumnLayoutKind = keyof typeof COLUMN_LAYOUTS; -export type ListingColumn = - (typeof COLUMN_LAYOUTS)["listing"]["options"][number]; -export type AttendeeColumn = - (typeof COLUMN_LAYOUTS)["attendee"]["options"][number]; - -export type ListingColumnLayout = ReturnType< - (typeof COLUMN_LAYOUTS)["listing"]["parse"] ->; -export type AttendeeColumnLayout = ReturnType< - (typeof COLUMN_LAYOUTS)["attendee"]["parse"] ->; +const getEngine = once(createBaseLiquidEngine); /** Matches only when `date` is the first filter applied to the raw value */ const FIRST_FILTER_IS_DATE_RE = /^[^|]*\|\s*date\b/; @@ -251,7 +83,7 @@ export const renderFilteredValue = ( const d = new Date(rawValue); if (!Number.isNaN(d.getTime())) contextValue = d; } - const result = engine.parseAndRenderSync(`{{ ${expression} }}`, { + const result = getEngine().parseAndRenderSync(`{{ ${expression} }}`, { [key]: contextValue, }); return result.trim(); diff --git a/src/shared/columns/attendee-columns.ts b/src/shared/columns/attendee-columns.ts index 846a1a147..383f44bb6 100644 --- a/src/shared/columns/attendee-columns.ts +++ b/src/shared/columns/attendee-columns.ts @@ -7,7 +7,8 @@ import { t } from "#i18n"; import { attendeeAdminPath } from "#shared/attendee-links.ts"; -import type { AttendeeColumn, ColumnDef } from "#shared/column-order.ts"; +import type { AttendeeColumn } from "#shared/column-layout.ts"; +import type { ColumnDef } from "#shared/column-order.ts"; import { formatDateLabel, formatDatetimeShort } from "#shared/dates.ts"; import { isServicing } from "#shared/db/attendees/kind.ts"; import { nonBlankLines } from "#shared/lines.ts"; diff --git a/src/shared/columns/listing-columns.ts b/src/shared/columns/listing-columns.ts index 9ba59b720..05c8958ac 100644 --- a/src/shared/columns/listing-columns.ts +++ b/src/shared/columns/listing-columns.ts @@ -6,11 +6,8 @@ * cell rendering, and guide documentation. */ -import type { - ColumnDef, - ColumnLayout, - ListingColumn, -} from "#shared/column-order.ts"; +import type { ColumnLayout, ListingColumn } from "#shared/column-layout.ts"; +import type { ColumnDef } from "#shared/column-order.ts"; import { formatCurrency } from "#shared/currency.ts"; import type { ListingWithCount } from "#shared/types.ts"; import { escapeHtml } from "#templates/layout.tsx"; diff --git a/src/shared/db/settings.ts b/src/shared/db/settings.ts index 29a4d323d..685808bc0 100644 --- a/src/shared/db/settings.ts +++ b/src/shared/db/settings.ts @@ -38,7 +38,7 @@ import { type AttendeeColumnLayout, COLUMN_LAYOUTS, type ListingColumnLayout, -} from "#shared/column-order.ts"; +} from "#shared/column-layout.ts"; import { encrypt } from "#shared/crypto/encryption.ts"; import { boolUpdate, diff --git a/src/shared/form-data.ts b/src/shared/form-data.ts index 8216948e4..4d711ed1f 100644 --- a/src/shared/form-data.ts +++ b/src/shared/form-data.ts @@ -48,4 +48,12 @@ export class FormParams extends URLSearchParams { const selected = new Set(supplied); return { ok: true, value: allowed.filter((value) => selected.has(value)) }; } + + /** Values for re-rendering a rejected form. Repeated controls use the comma + * format consumed by checkbox-group fields instead of losing all but one. */ + toRenderValues(): Record { + return Object.fromEntries( + [...new Set(this.keys())].map((key) => [key, this.getAll(key).join(",")]), + ); + } } diff --git a/src/ui/templates/admin/dashboard.tsx b/src/ui/templates/admin/dashboard.tsx index 24418d936..f85cbe701 100644 --- a/src/ui/templates/admin/dashboard.tsx +++ b/src/ui/templates/admin/dashboard.tsx @@ -8,7 +8,7 @@ import { groupAttendeeRows } from "#shared/attendee-table-rows.ts"; import { COLUMN_LAYOUTS, type ListingColumnLayout, -} from "#shared/column-order.ts"; +} from "#shared/column-layout.ts"; import { EDITOR_LISTING_LAYOUT, EDITOR_LISTING_TABLE_COLUMNS, diff --git a/src/ui/templates/admin/guide/operations.tsx b/src/ui/templates/admin/guide/operations.tsx index 422544e46..19f3c0dfe 100644 --- a/src/ui/templates/admin/guide/operations.tsx +++ b/src/ui/templates/admin/guide/operations.tsx @@ -2,7 +2,7 @@ * Admin guide — Operations sections. */ -import { COLUMN_LAYOUTS } from "#shared/column-order.ts"; +import { COLUMN_LAYOUTS } from "#shared/column-layout.ts"; import { ATTENDEE_TABLE_COLUMNS } from "#shared/columns/attendee-columns.ts"; import { LISTING_TABLE_COLUMNS } from "#shared/columns/listing-columns.ts"; import { Raw } from "#shared/jsx/jsx-runtime.ts"; diff --git a/src/ui/templates/admin/settings/column-order.tsx b/src/ui/templates/admin/settings/column-order.tsx index 1e723ea53..eda8c9419 100644 --- a/src/ui/templates/admin/settings/column-order.tsx +++ b/src/ui/templates/admin/settings/column-order.tsx @@ -8,7 +8,7 @@ /* jscpd:ignore-start */ import { t } from "#i18n"; -import { COLUMN_LAYOUTS } from "#shared/column-order.ts"; +import { COLUMN_LAYOUTS } from "#shared/column-layout.ts"; import { ATTENDEE_TABLE_COLUMNS } from "#shared/columns/attendee-columns.ts"; import { LISTING_TABLE_COLUMNS } from "#shared/columns/listing-columns.ts"; import { Raw } from "#shared/jsx/jsx-runtime.ts"; diff --git a/src/ui/templates/attendee-table.tsx b/src/ui/templates/attendee-table.tsx index 56f436825..95727d696 100644 --- a/src/ui/templates/attendee-table.tsx +++ b/src/ui/templates/attendee-table.tsx @@ -14,12 +14,11 @@ import { joinStrings, map, pipe, sort } from "#fp"; import { t } from "#i18n"; -import { - type AttendeeColumn, - type AttendeeColumnLayout, - getHeaderText, - renderCells, -} from "#shared/column-order.ts"; +import type { + AttendeeColumn, + AttendeeColumnLayout, +} from "#shared/column-layout.ts"; +import { getHeaderText, renderCells } from "#shared/column-order.ts"; import { ATTENDEE_TABLE_COLUMNS } from "#shared/columns/attendee-columns.ts"; import { isServicing } from "#shared/db/attendees/kind.ts"; import type { QuestionWithAnswers } from "#shared/db/question-types.ts"; diff --git a/test/features/admin/dashboard.test.ts b/test/features/admin/dashboard.test.ts new file mode 100644 index 000000000..9a3f05623 --- /dev/null +++ b/test/features/admin/dashboard.test.ts @@ -0,0 +1,79 @@ +import { expect } from "@std/expect"; +import { describe, it as test } from "@std/testing/bdd"; +import { loginResponse } from "#routes/admin/dashboard.ts"; +import { describeWithEnv } from "#test-utils/db.ts"; +import { createTestAttendee } from "#test-utils/db-helpers/attendees.ts"; +import { awaitTestRequest } from "#test-utils/mocks.ts"; +import { + createTestAgentSession, + createTestEditorSession, + getTestSession, + setupListingAndLogin, +} from "#test-utils/session.ts"; +import { withSetting } from "#test-utils/settings.ts"; + +describeWithEnv("admin listings route", { db: true }, () => { + test("editor listings ignore an invalid owner column layout", async () => { + const { cookie } = await createTestEditorSession(); + const response = await withSetting( + { listing_column_order: "{{unknown}}" }, + () => awaitTestRequest("/admin/listings", { cookie }), + ); + expect(response.status).toBe(200); + expect(await response.text()).toContain("Listings"); + }); + + test("owner listings use the saved column layout", async () => { + const { cookie } = await getTestSession(); + const html = await withSetting( + { listing_column_order: "{{name}}" }, + async () => + (await awaitTestRequest("/admin/listings", { cookie })).text(), + ); + expect(html).toContain("Listing Name"); + expect(html).not.toContain("Status"); + }); + + test("dashboard redirects non-dashboard roles to their own landing pages", async () => { + const { cookie: agentCookie } = await createTestAgentSession(); + const { cookie: editorCookie } = await createTestEditorSession(); + expect( + (await awaitTestRequest("/admin/", { cookie: agentCookie })).headers.get( + "location", + ), + ).toBe("/admin/deliveries"); + expect( + (await awaitTestRequest("/admin/", { cookie: editorCookie })).headers.get( + "location", + ), + ).toBe("/admin/listings"); + }); + + test("dashboard shows only the ten newest attendees", async () => { + const { cookie, listing } = await setupListingAndLogin(); + for (let index = 0; index < 11; index++) { + await createTestAttendee( + listing.id, + listing.slug, + `Guest ${index}`, + `guest-${index}@example.com`, + ); + } + const html = await (await awaitTestRequest("/admin/", { cookie })).text(); + expect(html.match(/Guest \d+/g)).toHaveLength(10); + }); +}); + +describe("loginResponse", () => { + test("uses status 200 by default", async () => { + expect( + (await loginResponse(new Request("http://localhost/admin/"))).status, + ).toBe(200); + }); + + test("uses the supplied status", async () => { + expect( + (await loginResponse(new Request("http://localhost/admin/"), 418)).status, + ).toBe(418); + }); +}); diff --git a/test/lib/server-listings-csv.test.ts b/test/features/admin/dashboard/listings-csv.test.ts similarity index 87% rename from test/lib/server-listings-csv.test.ts rename to test/features/admin/dashboard/listings-csv.test.ts index aa1443299..f057c078b 100644 --- a/test/lib/server-listings-csv.test.ts +++ b/test/features/admin/dashboard/listings-csv.test.ts @@ -1,5 +1,6 @@ import { expect } from "@std/expect"; import { describe, it as test } from "@std/testing/bdd"; +import { wasActivityLogged } from "#test-utils/activity-log.ts"; import { testRequiresAuth } from "#test-utils/assertions.ts"; import { describeWithEnv } from "#test-utils/db.ts"; import { createTestListing } from "#test-utils/db-helpers/listings.ts"; @@ -26,6 +27,7 @@ describeWithEnv("server (admin listings CSV)", { db: true }, () => { const csv = await response.text(); expect(csv.split("\n")[0]).toContain("Name,Status,Type"); expect(csv).toContain("Gala Night"); + expect(await wasActivityLogged("Listings CSV exported")).toBe(true); }); test("filters the export to one type and names the file by type", async () => { @@ -38,6 +40,9 @@ describeWithEnv("server (admin listings CSV)", { db: true }, () => { const csv = await response.text(); expect(csv).toContain("Daily One"); expect(csv).not.toContain("Standard One"); + expect( + await wasActivityLogged("Listings CSV exported (type: daily)"), + ).toBe(true); }); }); }); diff --git a/test/features/admin/dashboard/log.test.ts b/test/features/admin/dashboard/log.test.ts new file mode 100644 index 000000000..c5439aec1 --- /dev/null +++ b/test/features/admin/dashboard/log.test.ts @@ -0,0 +1,74 @@ +import { expect } from "@std/expect"; +import { it as test } from "@std/testing/bdd"; +import { logActivity } from "#test-utils/activity-log.ts"; +import { + assertAdminHtmlWithCookie, + expectHtmlResponse, + testRequiresAuth, +} from "#test-utils/assertions.ts"; +import { describeWithEnv } from "#test-utils/db.ts"; +import { createTestAttendee } from "#test-utils/db-helpers/attendees.ts"; +import { createTestListing } from "#test-utils/db-helpers/listings.ts"; +import { awaitTestRequest } from "#test-utils/mocks.ts"; +import { + adminGet, + createTestManagerSession, + setupListingAndLogin, +} from "#test-utils/session.ts"; + +describeWithEnv("admin activity log", { db: true }, () => { + testRequiresAuth("/admin/log"); + + test("shows log page when authenticated", async () => { + await createTestListing({ maxAttendees: 50, name: "Log Test" }); + await expectHtmlResponse(await adminGet("/admin/log"), 200, "Log"); + }); + + test("shows log page for manager", async () => { + const managerCookie = await createTestManagerSession(); + await assertAdminHtmlWithCookie("/admin/log", managerCookie, "Log"); + }); + + test("shows only the most recent 200 entries", async () => { + for (let index = 0; index < 201; index++) { + await logActivity(`Action ${index}`); + } + const html = await (await adminGet("/admin/log")).text(); + expect(html).toContain("Showing the most recent 200 entries"); + expect(html).toContain("Action 200"); + expect(html).toContain("Action 1"); + expect(html).not.toContain("Action 0"); + }); + + test("does not mark exactly 200 entries as truncated", async () => { + for (let index = 0; index < 200; index++) { + await logActivity(`Exact action ${index}`); + } + const html = await (await adminGet("/admin/log")).text(); + expect(html).not.toContain("Showing the most recent 200 entries"); + }); + + test("links each entry to its attendee and listing by name", async () => { + const { listing, cookie } = await setupListingAndLogin({ + maxAttendees: 50, + name: "Gala Dinner", + }); + const attendee = await createTestAttendee( + listing.id, + listing.slug, + "Ada Lovelace", + "ada@example.com", + ); + await logActivity("Balance updated", listing.id, attendee.id); + + const html = await ( + await awaitTestRequest("/admin/log", { cookie }) + ).text(); + expect(html).toContain( + `Ada Lovelace`, + ); + expect(html).toContain( + `Gala Dinner`, + ); + }); +}); diff --git a/test/integration/server/listings/create-validation.test.ts b/test/integration/server/listings/create-validation.test.ts index 7c16092d9..6e3777fc7 100644 --- a/test/integration/server/listings/create-validation.test.ts +++ b/test/integration/server/listings/create-validation.test.ts @@ -56,6 +56,21 @@ describeWithEnv("server listings > create validation", { db: true }, () => { expect(await getListingWithCount(1)).toBeNull(); }); + test("keeps repeated bookable days after a rejected create", async () => { + const { response } = await adminMultipartPost("/admin/listing", { + bookable_days: ["Monday", "Wednesday"], + day_price_1: "10.005", + listing_type: "daily", + max_attendees: "50", + max_quantity: "1", + name: "Bad Day Price", + }); + expect(response.status).toBe(400); + const html = await response.text(); + expect(html).toContain('value="Monday" checked'); + expect(html).toContain('value="Wednesday" checked'); + }); + test("accepts a create with a valid day price and stores it", async () => { const { response } = await adminFormPost("/admin/listing", { day_price_1: "10.00", diff --git a/test/lib/server-listings/log-and-activity.test.ts b/test/lib/server-listings/log-and-activity.test.ts index 174c7c45e..f3c6e5394 100644 --- a/test/lib/server-listings/log-and-activity.test.ts +++ b/test/lib/server-listings/log-and-activity.test.ts @@ -1,79 +1,13 @@ // jscpd:ignore-start import { expect } from "@std/expect"; import { describe, it as test } from "@std/testing/bdd"; -import { logActivity } from "#test-utils/activity-log.ts"; -import { - assertAdminHtml, - assertAdminHtmlWithCookie, - expectHtmlResponse, - testRequiresAuth, -} from "#test-utils/assertions.ts"; +import { assertAdminHtml, testRequiresAuth } from "#test-utils/assertions.ts"; import { describeWithEnv } from "#test-utils/db.ts"; -import { createTestAttendee } from "#test-utils/db-helpers/attendees.ts"; -import { createTestListing } from "#test-utils/db-helpers/listings.ts"; -import { awaitTestRequest } from "#test-utils/mocks.ts"; -import { - adminGet, - createTestManagerSession, - setupListingAndLogin, -} from "#test-utils/session.ts"; +import { adminGet, setupListingAndLogin } from "#test-utils/session.ts"; // jscpd:ignore-end describeWithEnv("server listings > log and activity", { db: true }, () => { - describe("GET /admin/log", () => { - testRequiresAuth("/admin/log"); - - test("shows log page when authenticated", async () => { - // Create an listing to generate activity - await createTestListing({ - maxAttendees: 50, - name: "Log Test", - }); - - const response = await adminGet("/admin/log"); - await expectHtmlResponse(response, 200, "Log"); - }); - - test("shows log page for manager", async () => { - const managerCookie = await createTestManagerSession(); - await assertAdminHtmlWithCookie("/admin/log", managerCookie, "Log"); - }); - - test("shows truncation message when more than 200 entries", async () => { - // Create 201 log entries to trigger truncation - for (let i = 0; i < 201; i++) { - await logActivity(`Action ${i}`); - } - - const response = await adminGet("/admin/log"); - const html = await response.text(); - expect(html).toContain("Showing the most recent 200 entries"); - }); - - test("links each entry to its attendee and listing by name", async () => { - const { listing, cookie } = await setupListingAndLogin({ - maxAttendees: 50, - name: "Gala Dinner", - }); - const attendee = await createTestAttendee( - listing.id, - listing.slug, - "Ada Lovelace", - "ada@example.com", - ); - await logActivity("Balance updated", listing.id, attendee.id); - - const response = await awaitTestRequest("/admin/log", { cookie }); - const html = await response.text(); - expect(html).toContain( - `Ada Lovelace`, - ); - expect(html).toContain( - `Gala Dinner`, - ); - }); - }); describe("GET /admin/listing/:id/activity", () => { testRequiresAuth("/admin/listing/1/activity"); diff --git a/test/shared/column-layout.test.ts b/test/shared/column-layout.test.ts new file mode 100644 index 000000000..af2464502 --- /dev/null +++ b/test/shared/column-layout.test.ts @@ -0,0 +1,118 @@ +import { expect } from "@std/expect"; +import { describe, it as test } from "@std/testing/bdd"; +import { buildDefaultTemplate, COLUMN_LAYOUTS } from "#shared/column-layout.ts"; +import { + EDITOR_LISTING_LAYOUT, + EDITOR_LISTING_TABLE_COLUMNS, + LISTING_TABLE_COLUMNS, +} from "#shared/columns/listing-columns.ts"; + +describe("column layout validation", () => { + test("derives every listing schema from the matching column definitions", () => { + expect([...COLUMN_LAYOUTS.listing.schema.options].sort()).toEqual( + Object.keys(LISTING_TABLE_COLUMNS).sort(), + ); + expect([...EDITOR_LISTING_LAYOUT.columnKeys].sort()).toEqual( + Object.keys(EDITOR_LISTING_TABLE_COLUMNS).sort(), + ); + }); + + test("returns null for valid template", () => { + expect(COLUMN_LAYOUTS.listing.validate("{{name}}, {{status}}")).toBeNull(); + }); + + test("handles wonky spacing", () => { + expect( + COLUMN_LAYOUTS.listing.validate( + "{{ name }},{{description}}, {{ status }}", + ), + ).toBeNull(); + }); + + test("rejects unknown column", () => { + const error = COLUMN_LAYOUTS.listing.validate("{{name}}, {{descritpion}}"); + expect(error).toContain("descritpion"); + expect(error).toContain("Available columns"); + }); + + test("lists the schema columns in the error message", () => { + expect(COLUMN_LAYOUTS.attendee.validate("{{bogus}}")).toBe( + `Unknown column "bogus". Available columns: ${COLUMN_LAYOUTS.attendee.schema.options.join(", ")}`, + ); + }); + + test("accepts an empty template as the default layout", () => { + expect(COLUMN_LAYOUTS.listing.validate("")).toBeNull(); + }); + + test("rejects a non-empty template with no column tags", () => { + expect(COLUMN_LAYOUTS.listing.validate("name, status")).toBe( + "Template must include at least one column", + ); + }); + + test("accepts supported filters", () => { + expect( + COLUMN_LAYOUTS.listing.validate( + '{{name}}, {{created | date: "%B"}}, {{price | currency}}', + ), + ).toBeNull(); + }); +}); + +test("keeps the attendee default column order", () => { + expect(COLUMN_LAYOUTS.attendee.defaultOrder).toEqual([ + "status", + "date", + "name", + "listings", + "email", + "phone", + "address", + "special_instructions", + "answers", + "qty", + "ticket", + "registered", + ]); +}); + +describe("column layout parsing", () => { + test("returns default order when template is empty", () => { + const { columnKeys, filters } = COLUMN_LAYOUTS.listing.parse(""); + expect(columnKeys).toEqual(COLUMN_LAYOUTS.listing.defaultOrder); + expect(filters.size).toBe(0); + }); + + test("returns unique columns in template order", () => { + const { columnKeys } = COLUMN_LAYOUTS.listing.parse( + "{{status}}, {{name}}, {{status}}", + ); + expect(columnKeys).toEqual(["status", "name"]); + }); + + test("throws for an invalid template", () => { + expect(() => COLUMN_LAYOUTS.listing.parse("{{bogus}}")).toThrow( + 'Unknown column "bogus"', + ); + }); + + test("extracts only supplied filter expressions", () => { + const { filters } = COLUMN_LAYOUTS.listing.parse( + '{{name}}, {{created | date: "%B %d"}}', + ); + expect(filters.get("created")).toBe('created | date: "%B %d"'); + expect(filters.has("name")).toBe(false); + }); + + test("resolves all attendee columns from the default template", () => { + const template = buildDefaultTemplate(COLUMN_LAYOUTS.attendee.defaultOrder); + expect(COLUMN_LAYOUTS.attendee.parse(template).columnKeys).toEqual( + COLUMN_LAYOUTS.attendee.defaultOrder, + ); + }); +}); + +test("buildDefaultTemplate joins column tags", () => { + expect(buildDefaultTemplate(["a", "b", "c"])).toBe("{{a}}, {{b}}, {{c}}"); +}); diff --git a/test/shared/column-order.test.ts b/test/shared/column-order.test.ts index d54fd29c0..1af6f79ad 100644 --- a/test/shared/column-order.test.ts +++ b/test/shared/column-order.test.ts @@ -1,151 +1,19 @@ import { expect } from "@std/expect"; import { describe, it as test } from "@std/testing/bdd"; +import { COLUMN_LAYOUTS } from "#shared/column-layout.ts"; import type { ColumnDef, ColumnGenerators } from "#shared/column-order.ts"; import { - buildDefaultTemplate, - COLUMN_LAYOUTS, getHeaderText, renderCells, renderFilteredValue, } from "#shared/column-order.ts"; -import { - EDITOR_LISTING_LAYOUT, - EDITOR_LISTING_TABLE_COLUMNS, - LISTING_TABLE_COLUMNS, -} from "#shared/columns/listing-columns.ts"; +import { LISTING_TABLE_COLUMNS } from "#shared/columns/listing-columns.ts"; import { escapeHtml } from "#templates/layout.tsx"; import { setupTestEncryptionKey } from "#test-utils/env.ts"; import { testListingWithCount } from "#test-utils/factories.ts"; setupTestEncryptionKey(); -describe("column layout validation", () => { - test("derives every listing schema from the matching column definitions", () => { - expect([...COLUMN_LAYOUTS.listing.schema.options].sort()).toEqual( - Object.keys(LISTING_TABLE_COLUMNS).sort(), - ); - expect([...EDITOR_LISTING_LAYOUT.columnKeys].sort()).toEqual( - Object.keys(EDITOR_LISTING_TABLE_COLUMNS).sort(), - ); - }); - - test("returns null for valid template", () => { - expect(COLUMN_LAYOUTS.listing.validate("{{name}}, {{status}}")).toBeNull(); - }); - - test("handles wonky spacing", () => { - expect( - COLUMN_LAYOUTS.listing.validate( - "{{ name }},{{description}}, {{ status }}", - ), - ).toBeNull(); - }); - - test("rejects unknown column (typo)", () => { - const error = COLUMN_LAYOUTS.listing.validate("{{name}}, {{descritpion}}"); - expect(error).toContain("descritpion"); - expect(error).toContain("Available columns"); - }); - - test("lists the schema columns in the error message", () => { - const error = COLUMN_LAYOUTS.attendee.validate("{{bogus}}"); - expect(error).toBe( - `Unknown column "bogus". Available columns: ${COLUMN_LAYOUTS.attendee.schema.options.join(", ")}`, - ); - }); - - test("accepts an empty template as the default layout", () => { - expect(COLUMN_LAYOUTS.listing.validate("")).toBeNull(); - }); - - test("rejects a non-empty template with no column tags", () => { - expect(COLUMN_LAYOUTS.listing.validate("name, status")).toBe( - "Template must include at least one column", - ); - }); - - test("accepts templates with date filter", () => { - expect( - COLUMN_LAYOUTS.listing.validate('{{name}}, {{created | date: "%B"}}'), - ).toBeNull(); - }); - - test("accepts templates with currency filter", () => { - expect( - COLUMN_LAYOUTS.listing.validate("{{name}}, {{price | currency}}"), - ).toBeNull(); - }); -}); - -test("keeps the attendee default column order", () => { - expect(COLUMN_LAYOUTS.attendee.defaultOrder).toEqual([ - "status", - "date", - "name", - "listings", - "email", - "phone", - "address", - "special_instructions", - "answers", - "qty", - "ticket", - "registered", - ]); -}); - -describe("column layout parsing", () => { - test("returns default order when template is empty", () => { - const { columnKeys, filters } = COLUMN_LAYOUTS.listing.parse(""); - expect(columnKeys).toEqual(COLUMN_LAYOUTS.listing.defaultOrder); - expect(filters.size).toBe(0); - }); - - test("returns columns in template order", () => { - const { columnKeys } = COLUMN_LAYOUTS.listing.parse("{{status}}, {{name}}"); - expect(columnKeys).toEqual(["status", "name"]); - }); - - test("deduplicates repeated columns", () => { - const { columnKeys } = COLUMN_LAYOUTS.listing.parse( - "{{name}}, {{name}}, {{status}}", - ); - expect(columnKeys).toEqual(["name", "status"]); - }); - - test("throws for an invalid template", () => { - expect(() => COLUMN_LAYOUTS.listing.parse("{{bogus}}")).toThrow( - 'Unknown column "bogus"', - ); - }); - - test("extracts filter expression for filtered column", () => { - const { filters } = COLUMN_LAYOUTS.listing.parse( - '{{name}}, {{created | date: "%B %d"}}', - ); - expect(filters.get("created")).toBe('created | date: "%B %d"'); - }); - - test("does not create filter entry for unfiltered column", () => { - const { filters } = COLUMN_LAYOUTS.listing.parse( - '{{name}}, {{created | date: "%B %d"}}', - ); - expect(filters.has("name")).toBe(false); - }); - - test("resolves all attendee columns from default template", () => { - const template = buildDefaultTemplate(COLUMN_LAYOUTS.attendee.defaultOrder); - const { columnKeys } = COLUMN_LAYOUTS.attendee.parse(template); - expect(columnKeys).toEqual(COLUMN_LAYOUTS.attendee.defaultOrder); - }); -}); - -describe("buildDefaultTemplate", () => { - test("joins column tags with a comma and space", () => { - expect(buildDefaultTemplate(["a", "b", "c"])).toBe("{{a}}, {{b}}, {{c}}"); - }); -}); - describe("renderFilteredValue", () => { test("applies date filter with strftime format", () => { const result = renderFilteredValue( diff --git a/test/shared/columns/attendee-columns.test.ts b/test/shared/columns/attendee-columns.test.ts index efbed6e33..b21cc1146 100644 --- a/test/shared/columns/attendee-columns.test.ts +++ b/test/shared/columns/attendee-columns.test.ts @@ -1,7 +1,6 @@ import { expect } from "@std/expect"; import { describe, it as test } from "@std/testing/bdd"; -import type { AttendeeColumn } from "#shared/column-order.ts"; -import { COLUMN_LAYOUTS } from "#shared/column-order.ts"; +import { type AttendeeColumn, COLUMN_LAYOUTS } from "#shared/column-layout.ts"; import { ATTENDEE_TABLE_COLUMNS, formatAddressInline, diff --git a/test/shared/form-data.test.ts b/test/shared/form-data.test.ts index 57c3e7695..7eccdc2b7 100644 --- a/test/shared/form-data.test.ts +++ b/test/shared/form-data.test.ts @@ -113,3 +113,13 @@ describe("FormParams.getRepeatedPicklist", () => { }); }); }); + +describe("FormParams.toRenderValues", () => { + test("joins repeated controls without changing single values", () => { + const form = new FormParams("days=Monday&name=Alice&days=Wednesday"); + expect(form.toRenderValues()).toEqual({ + days: "Monday,Wednesday", + name: "Alice", + }); + }); +}); diff --git a/test/test-utils/session.ts b/test/test-utils/session.ts index 374e1c86e..783c8815c 100644 --- a/test/test-utils/session.ts +++ b/test/test-utils/session.ts @@ -11,6 +11,7 @@ import { } from "#shared/session-context.ts"; import type { Listing } from "#shared/types.ts"; import type { TestListingOverrides } from "#test-utils/factories.ts"; +import type { TestFormValues } from "#test-utils/form-values.ts"; import type { AdminTestContext } from "#test-utils/internal.ts"; import { getInternalTestSession, @@ -413,7 +414,7 @@ export const adminFormPost = async ( export const adminMultipartPost = async ( path: string, - data: Record = {}, + data: TestFormValues = {}, file?: { name: string; fieldName: string; diff --git a/test/ui/templates/admin/dashboard.test.ts b/test/ui/templates/admin/dashboard.test.ts index 5e0dd84d0..d5fd6a546 100644 --- a/test/ui/templates/admin/dashboard.test.ts +++ b/test/ui/templates/admin/dashboard.test.ts @@ -4,7 +4,7 @@ import { NO_QUANTITY_PREFIX, QTY_PREFIX, } from "#routes/admin/attendee-form-model.ts"; -import { COLUMN_LAYOUTS } from "#shared/column-order.ts"; +import { COLUMN_LAYOUTS } from "#shared/column-layout.ts"; import { signCsrfToken } from "#shared/csrf.ts"; import { getDb } from "#shared/db/client.ts"; import { diff --git a/test/ui/templates/attendee-table/template.test.ts b/test/ui/templates/attendee-table/template.test.ts index 5465a7e50..a7e395613 100644 --- a/test/ui/templates/attendee-table/template.test.ts +++ b/test/ui/templates/attendee-table/template.test.ts @@ -1,6 +1,6 @@ import { expect } from "@std/expect"; import { it as test } from "@std/testing/bdd"; -import { COLUMN_LAYOUTS } from "#shared/column-order.ts"; +import { COLUMN_LAYOUTS } from "#shared/column-layout.ts"; import { AttendeeTable } from "#templates/attendee-table.tsx"; import { testAttendee } from "#test-utils/factories.ts"; import { attendeeTableSuite, makeOpts, makeRow } from "./shared.ts"; From 6c5e34572fbf094b534b6173b7ba9f50ea81c81f Mon Sep 17 00:00:00 2001 From: Stefan Date: Sun, 19 Jul 2026 21:57:35 +0100 Subject: [PATCH 6/6] Use functional column layout parsing --- src/shared/column-layout.ts | 49 +++++++++++++++++++++++-------- test/shared/column-layout.test.ts | 2 +- 2 files changed, 38 insertions(+), 13 deletions(-) diff --git a/src/shared/column-layout.ts b/src/shared/column-layout.ts index 77ee5be95..29f083116 100644 --- a/src/shared/column-layout.ts +++ b/src/shared/column-layout.ts @@ -2,6 +2,7 @@ * Liquid rendering engine used later to format cell values. */ import * as v from "valibot"; +import { reduce } from "#fp"; import type { Result } from "#shared/result.ts"; export type ColumnLayout = { @@ -19,24 +20,29 @@ const parseTagBody = ( return { filter: body.trim(), key: body.slice(0, pipeIdx).trim() }; }; -type ParsedLayout = Result<{ +interface LayoutParts { columns: T[]; filters: Map; -}>; + seen: Set; +} + +type CollectedLayout = Result>; + +type ParsedLayout = Result< + Pick, "columns" | "filters"> +>; type ColumnSchema = v.GenericSchema & { readonly options: readonly string[]; }; -const parseColumnTemplate = ( - template: string, - schema: TSchema, -): ParsedLayout> => { - const columns: v.InferOutput[] = []; - const filters = new Map, string>(); - const seen = new Set(); - - for (const match of template.matchAll(LIQUID_TAG_RE)) { +const collectColumnTag = + (schema: TSchema) => + ( + result: CollectedLayout>, + match: RegExpMatchArray, + ): CollectedLayout> => { + if (!result.ok) return result; const { key, filter } = parseTagBody(match[1]!); const parsed = v.safeParse(schema, key); if (!parsed.success) { @@ -46,12 +52,31 @@ const parseColumnTemplate = ( }; } const option = parsed.output; + const { columns, filters, seen } = result.value; if (!seen.has(option)) { seen.add(option); columns.push(option); if (filter) filters.set(option, filter); } - } + return result; + }; + +const parseColumnTemplate = ( + template: string, + schema: TSchema, +): ParsedLayout> => { + const collected = reduce(collectColumnTag(schema), { + ok: true, + value: { + columns: [], + filters: new Map(), + seen: new Set(), + }, + } satisfies CollectedLayout>)([ + ...template.matchAll(LIQUID_TAG_RE), + ]); + if (!collected.ok) return collected; + const { columns, filters } = collected.value; if (columns.length === 0) { return { error: "Template must include at least one column", ok: false }; diff --git a/test/shared/column-layout.test.ts b/test/shared/column-layout.test.ts index af2464502..7e7e57211 100644 --- a/test/shared/column-layout.test.ts +++ b/test/shared/column-layout.test.ts @@ -92,7 +92,7 @@ describe("column layout parsing", () => { }); test("throws for an invalid template", () => { - expect(() => COLUMN_LAYOUTS.listing.parse("{{bogus}}")).toThrow( + expect(() => COLUMN_LAYOUTS.listing.parse("{{bogus}}, {{name}}")).toThrow( 'Unknown column "bogus"', ); });