From 529ae2252b9c48637671ae388fa334e6b2d2e2fe Mon Sep 17 00:00:00 2001 From: Juan Cobo Betancourt Date: Fri, 29 May 2026 16:03:32 -0700 Subject: [PATCH 1/4] Centralize controlled vocabularies and fix test_images entry saving Entries marked as the test_images type could not be saved: autosave stalled and the manual "Save now" button could not recover the entry, even though the type was selectable in the outline. The root cause was controlled vocabularies duplicated as hand-copied literal arrays across the database schema, validators, and type definitions, with the save validator's allowlist omitting test_images. Hoist the controlled vocabularies (entry types, resource types, project roles, descriptive standards, quality-control flag types and resolution actions, GeoNames feature classes, and volume statuses) to a single source of truth in validation/enums.ts. The schema, Zod validators, role allow-lists, and derived types now all reference these constants, so save validation derives from the canonical entry-type set. Add a drift guard (tests/db/enum-drift.test.ts) that pins each schema enum column to its canonical constant so a future divergence fails in CI rather than silently breaking a save, plus a round-trip regression test for entry-type save validation. --- .../qc-flags/qc-flag-card-expandable.tsx | 5 +- .../qc-flags/resolve-qc-flag-dialog.tsx | 3 +- app/db/schema.ts | 28 ++++--- app/lib/boundary-types.ts | 15 ++-- app/lib/description-types.ts | 5 +- app/lib/description.server.ts | 5 +- app/lib/entries.server.ts | 5 +- app/lib/invites.server.ts | 5 +- app/lib/operator-actions.server.ts | 3 +- app/lib/permissions.server.ts | 5 +- app/lib/validation/enums.ts | 70 +++++++++++++++- app/lib/validation/place.ts | 4 +- app/lib/workflow.ts | 20 +++-- app/routes/_auth.admin.cataloguing.team.tsx | 3 +- app/routes/_auth.admin.users.$id.tsx | 9 +- .../_auth.description.$projectId.$entryId.tsx | 3 +- app/routes/_auth.projects.$id.members.tsx | 9 +- app/routes/_auth.projects.$id.overview.tsx | 3 +- app/routes/_auth.projects.$id.tsx | 3 +- ....projects.$id.volumes.$volumeId.manage.tsx | 2 +- .../_auth.viewer.$projectId.$volumeId.tsx | 3 +- app/routes/api.comments.tsx | 5 +- app/routes/api.entries.save.tsx | 3 +- app/routes/api.qc-flags.tsx | 38 ++++----- app/routes/api.resegmentation.tsx | 3 +- app/routes/api.workflow.tsx | 3 +- tests/db/enum-drift.test.ts | 82 +++++++++++++++++++ tests/entries/save.test.ts | 30 ++++++- 28 files changed, 285 insertions(+), 87 deletions(-) create mode 100644 tests/db/enum-drift.test.ts diff --git a/app/components/qc-flags/qc-flag-card-expandable.tsx b/app/components/qc-flags/qc-flag-card-expandable.tsx index f0490c1..84ae729 100644 --- a/app/components/qc-flags/qc-flag-card-expandable.tsx +++ b/app/components/qc-flags/qc-flag-card-expandable.tsx @@ -14,12 +14,13 @@ import { ChevronDown } from "lucide-react"; import { QcFlagCard, type QcFlagCardData, type QcStatus } from "./qc-flag-card"; import { CommentThread } from "../comments/comment-thread"; import type { CommentWithAuthor } from "../../lib/description-types"; +import type { ProjectRole } from "../../lib/validation/enums"; export type QCFlagCardExpandableProps = { flag: QcFlagCardData; volumeId: string; comments: CommentWithAuthor[]; - userRole: "lead" | "cataloguer" | "reviewer"; + userRole: ProjectRole; onResolveClick?: (flagId: string) => void; onCommentAdded?: () => void; }; @@ -34,7 +35,7 @@ export type QCFlagCardExpandableProps = { * body calls this helper to decide whether to forward `onResolveClick`. */ export function shouldForwardResolve( - userRole: "lead" | "cataloguer" | "reviewer", + userRole: ProjectRole, flagStatus: QcStatus, ): boolean { return userRole === "lead" && flagStatus === "open"; diff --git a/app/components/qc-flags/resolve-qc-flag-dialog.tsx b/app/components/qc-flags/resolve-qc-flag-dialog.tsx index aa42ee6..895736e 100644 --- a/app/components/qc-flags/resolve-qc-flag-dialog.tsx +++ b/app/components/qc-flags/resolve-qc-flag-dialog.tsx @@ -13,6 +13,7 @@ import { useState, useEffect } from "react"; import { useFetcher } from "react-router"; import { useTranslation } from "react-i18next"; import { CheckCircle2, X } from "lucide-react"; +import type { ProjectRole } from "../../lib/validation/enums"; export type QcStatus = "resolved" | "wontfix"; @@ -55,7 +56,7 @@ export type ResolveQcFlagDialogProps = { open: boolean; onClose: () => void; flagId: string; - userRole: "lead" | "cataloguer" | "reviewer"; + userRole: ProjectRole; onResolved?: () => void; onError?: (message: string) => void; }; diff --git a/app/db/schema.ts b/app/db/schema.ts index ad8d2d8..7d55e0f 100644 --- a/app/db/schema.ts +++ b/app/db/schema.ts @@ -79,7 +79,7 @@ * left in the database for migration purity and should be treated as * legacy for any future schema work. * - * @version v0.4.0 + * @version v0.4.1 */ import { @@ -99,6 +99,14 @@ import { ENTITY_ROLES, PLACE_ROLES, VOCABULARY_STATUSES, + VOLUME_STATUSES, + ENTRY_TYPES, + RESOURCE_TYPES_ES, + PROJECT_ROLES, + DESCRIPTIVE_STANDARDS, + QC_PROBLEM_TYPES, + QC_RESOLUTION_ACTIONS, + GEONAMES_FCLASSES, } from "../lib/validation/enums"; // AUDIT_LOG_ACTIONS lives in a tiny constants module so // `app/lib/audit.server.ts` (which imports the auditLog table from @@ -115,7 +123,7 @@ export const tenants = sqliteTable( slug: text("slug").notNull().unique(), name: text("name").notNull(), kind: text("kind", { enum: ["tenant", "platform"] }).notNull().default("tenant"), - descriptiveStandard: text("descriptive_standard", { enum: ["isadg", "dacs", "rad"] }), + descriptiveStandard: text("descriptive_standard", { enum: [...DESCRIPTIVE_STANDARDS] }), status: text("status", { enum: ["active", "suspended"] }).notNull().default("active"), crowdsourcingEnabled: integer("crowdsourcing_enabled", { mode: "boolean" }).notNull().default(false), vocabularyHubEnabled: integer("vocabulary_hub_enabled", { mode: "boolean" }).notNull().default(true), @@ -315,7 +323,7 @@ export const projectMembers = sqliteTable( id: text("id").primaryKey(), projectId: text("project_id").notNull().references(() => projects.id), userId: text("user_id").notNull().references(() => users.id), - role: text("role", { enum: ["lead", "cataloguer", "reviewer"] }).notNull(), + role: text("role", { enum: [...PROJECT_ROLES] }).notNull(), createdAt: integer("created_at").notNull(), }, (table) => [ @@ -349,7 +357,7 @@ export const volumes = sqliteTable( manifestUrl: text("manifest_url").notNull(), pageCount: integer("page_count").notNull(), status: text("status", { - enum: ["unstarted", "in_progress", "segmented", "sent_back", "reviewed", "approved"], + enum: [...VOLUME_STATUSES], }) .notNull() .default("unstarted"), @@ -399,7 +407,7 @@ export const entries = sqliteTable( endPage: integer("end_page"), // explicit for children, null for top-level endY: real("end_y"), // fraction 0-1, null for top-level type: text("type", { - enum: ["item", "blank", "front_matter", "back_matter", "test_images"], + enum: [...ENTRY_TYPES], }), // nullable: unset by default // Per-project document subtype label (e.g. "Escritura", "Poder", or a // free-typed "OTRO" value). Only meaningful when `type = 'item'`; null @@ -418,7 +426,7 @@ export const entries = sqliteTable( // Description metadata fields translatedTitle: text("translated_title"), resourceType: text("resource_type", { - enum: ["texto", "imagen", "cartografico", "mixto"], + enum: [...RESOURCE_TYPES_ES], }), dateExpression: text("date_expression"), dateStart: text("date_start"), @@ -488,14 +496,14 @@ export const qcFlags = sqliteTable( .references(() => volumePages.id, { onDelete: "cascade" }), reportedBy: text("reported_by").notNull().references(() => users.id), problemType: text("problem_type", { - enum: ["damaged", "repeated", "out_of_order", "missing", "blank", "other"], + enum: [...QC_PROBLEM_TYPES], }).notNull(), description: text("description").notNull(), status: text("status", { enum: ["open", "resolved", "wontfix"] }) .notNull() .default("open"), resolutionAction: text("resolution_action", { - enum: ["retake_requested", "reordered", "marked_duplicate", "ignored", "other"], + enum: [...QC_RESOLUTION_ACTIONS], }), resolverNote: text("resolver_note"), resolvedBy: text("resolved_by").references(() => users.id), @@ -536,7 +544,7 @@ export const comments = sqliteTable( parentId: text("parent_id"), // null = top-level, references comments.id for nesting authorId: text("author_id").notNull().references(() => users.id), authorRole: text("author_role", { - enum: ["cataloguer", "reviewer", "lead"], + enum: [...PROJECT_ROLES], }).notNull(), text: text("text").notNull(), createdAt: integer("created_at").notNull(), @@ -812,7 +820,7 @@ export const places = sqliteTable( // column. The DB-level CHECK on fclass // (`IS NULL OR IN ('P','H','A','T','S')`) lives in the migration; // Drizzle's `enum:` hint here is the TypeScript-side mirror. - fclass: text("fclass", { enum: ["P", "H", "A", "T", "S"] }), + fclass: text("fclass", { enum: [...GEONAMES_FCLASSES] }), legacyIds: text("legacy_ids").notNull().default("[]"), createdAt: integer("created_at").notNull(), updatedAt: integer("updated_at").notNull(), diff --git a/app/lib/boundary-types.ts b/app/lib/boundary-types.ts index 3a9a6c9..5ff78a9 100644 --- a/app/lib/boundary-types.ts +++ b/app/lib/boundary-types.ts @@ -23,15 +23,16 @@ * the SaveStatus pill component, so `app/lib/` stays unit-testable * without dragging in the component layer. * - * @version v0.4.0 + * @version v0.4.1 */ -export type EntryType = - | "item" - | "blank" - | "front_matter" - | "back_matter" - | "test_images"; +// EntryType is derived from the canonical `ENTRY_TYPES` array +// (app/lib/validation/enums.ts) so the segmentation type vocabulary has +// a single source of truth shared with the Drizzle schema and the save +// validator. Imported locally for use below and re-exported so the many +// modules that import `EntryType` from here keep working. +import type { EntryType } from "./validation/enums"; +export type { EntryType }; export type Entry = { id: string; diff --git a/app/lib/description-types.ts b/app/lib/description-types.ts index eb0aaef..33a9eaf 100644 --- a/app/lib/description-types.ts +++ b/app/lib/description-types.ts @@ -10,6 +10,7 @@ */ import type { DescriptionStatus } from "./description-workflow"; +import type { ResourceTypeEs, ProjectRole } from "./validation/enums"; /** * Entry fields relevant to the description form context. @@ -22,7 +23,7 @@ export type DescriptionEntry = { endPage: number | null; title: string | null; translatedTitle: string | null; - resourceType: "texto" | "imagen" | "cartografico" | "mixto" | null; + resourceType: ResourceTypeEs | null; dateExpression: string | null; dateStart: string | null; dateEnd: string | null; @@ -60,7 +61,7 @@ export type Comment = { regionH: number | null; parentId: string | null; authorId: string; - authorRole: "cataloguer" | "reviewer" | "lead"; + authorRole: ProjectRole; text: string; createdAt: number; updatedAt: number; diff --git a/app/lib/description.server.ts b/app/lib/description.server.ts index f99d306..a201db2 100644 --- a/app/lib/description.server.ts +++ b/app/lib/description.server.ts @@ -47,12 +47,13 @@ import { import type { WorkflowRole } from "./workflow"; import { logActivity } from "./workflow.server"; import { createComment } from "./comments.server"; +import { RESOURCE_TYPES_ES, type ResourceTypeEs } from "./validation/enums"; // --- Validation schema for submit-for-review --- const submitSchema = z.object({ title: z.string().min(1, "Title is required"), - resourceType: z.enum(["texto", "imagen", "cartografico", "mixto"]), + resourceType: z.enum(RESOURCE_TYPES_ES), dateExpression: z.string().min(1, "Date expression is required"), scopeContent: z.string().min(1, "Scope and content is required"), language: z.string().min(1, "Language is required"), @@ -62,7 +63,7 @@ const submitSchema = z.object({ export type DescriptionFields = { title?: string | null; translatedTitle?: string | null; - resourceType?: "texto" | "imagen" | "cartografico" | "mixto" | null; + resourceType?: ResourceTypeEs | null; dateExpression?: string | null; dateStart?: string | null; dateEnd?: string | null; diff --git a/app/lib/entries.server.ts b/app/lib/entries.server.ts index 3f6263f..8154dc5 100644 --- a/app/lib/entries.server.ts +++ b/app/lib/entries.server.ts @@ -22,12 +22,13 @@ * so a malformed payload fails fast with a useful message rather than * surfacing as a CHECK violation deep in the batch. * - * @version v0.4.0 + * @version v0.4.1 */ import { eq } from "drizzle-orm"; import type { DrizzleD1Database } from "drizzle-orm/d1"; import { entries } from "../db/schema"; import type { Entry } from "./boundary-types"; +import { ENTRY_TYPES } from "./validation/enums"; /** * Load all entries for a volume, ordered by position. @@ -238,7 +239,7 @@ function validateEntries(entriesToSave: Entry[], volumeId: string): void { throw new Error("endY must be a number between 0 and 1, or null"); } } - if (entry.type !== null && !["item", "blank", "front_matter", "back_matter"].includes(entry.type)) { + if (entry.type !== null && !(ENTRY_TYPES as readonly string[]).includes(entry.type)) { throw new Error(`invalid entry type: ${entry.type}`); } } diff --git a/app/lib/invites.server.ts b/app/lib/invites.server.ts index 678e319..be6c70e 100644 --- a/app/lib/invites.server.ts +++ b/app/lib/invites.server.ts @@ -29,6 +29,7 @@ import { sendExistingUserInviteEmail, } from "./email.server"; import { getAppConfig } from "./config.server"; +import type { ProjectRole } from "./validation/enums"; type InviterUser = { id: string; @@ -108,7 +109,7 @@ export async function createInvite( id: crypto.randomUUID(), projectId, userId: existingUser.id, - role: role as "lead" | "cataloguer" | "reviewer", + role: role as ProjectRole, createdAt: now, }); } @@ -253,7 +254,7 @@ export async function acceptInvite( id: crypto.randomUUID(), projectId: invite.projectId, userId: user.id, - role: role as "lead" | "cataloguer" | "reviewer", + role: role as ProjectRole, createdAt: now, }); } diff --git a/app/lib/operator-actions.server.ts b/app/lib/operator-actions.server.ts index 1974d34..39f0fdd 100644 --- a/app/lib/operator-actions.server.ts +++ b/app/lib/operator-actions.server.ts @@ -60,6 +60,7 @@ import { z } from "zod"; import { SlugSchema } from "./tenant"; +import { DESCRIPTIVE_STANDARDS } from "./validation/enums"; /** * Boolean coercion for HTML form inputs. A checkbox sends its `value` @@ -106,7 +107,7 @@ const checkboxSchema = z export const CreateTenantSchema = z.object({ slug: SlugSchema, name: z.string().min(1, { message: "Name is required" }).max(120), - descriptiveStandard: z.enum(["isadg", "dacs", "rad"]), + descriptiveStandard: z.enum(DESCRIPTIVE_STANDARDS), crowdsourcingEnabled: checkboxSchema.default(false), vocabularyHubEnabled: checkboxSchema.default(true), publishPipelineEnabled: checkboxSchema.default(true), diff --git a/app/lib/permissions.server.ts b/app/lib/permissions.server.ts index 81f3031..e2b5993 100644 --- a/app/lib/permissions.server.ts +++ b/app/lib/permissions.server.ts @@ -38,6 +38,7 @@ // --- TEMPLATE INFRASTRUCTURE --- do not modify when extending import { eq, and } from "drizzle-orm"; +import { PROJECT_ROLES } from "./validation/enums"; import type { DrizzleD1Database } from "drizzle-orm/d1"; import { projectMembers, entries, volumes, volumePages } from "../db/schema"; import type { DescriptionStatus } from "./description-workflow"; @@ -202,7 +203,7 @@ export async function requireEntryAccess( db, userId, volume.projectId, - ["lead", "cataloguer", "reviewer"], + [...PROJECT_ROLES], isAdmin ); @@ -351,7 +352,7 @@ export async function requirePageAccess( db, userId, volume.projectId, - ["lead", "cataloguer", "reviewer"], + [...PROJECT_ROLES], isAdmin ); diff --git a/app/lib/validation/enums.ts b/app/lib/validation/enums.ts index 4f89a6c..91038c8 100644 --- a/app/lib/validation/enums.ts +++ b/app/lib/validation/enums.ts @@ -3,11 +3,23 @@ * * This module deals with the single source of truth for the * controlled-vocabulary arrays used by the Drizzle schema, the Zod - * validation schemas, and the vocabularies admin surfaces. Values - * mirror the Django backend (`catalog/models.py`) exactly so a - * payload that validates here validates there too. + * validation schemas, and the vocabularies admin surfaces. Several of + * the description-side vocabularies still mirror the legacy Django + * backend (`catalog/models.py`) exactly so a payload that validates + * here validates there too; the workflow-side enums (entry types, + * project roles, QC vocabularies) are Fisqua-native. * - * @version v0.4.0 + * The contract is simple: every enum here is consumed in three places + * — the Drizzle column hint (`enum: [...CONST]`), the Zod validator + * (`z.enum(CONST)` or `(CONST as readonly string[]).includes(x)`), and + * the derived TypeScript type (`(typeof CONST)[number]`). Adding a + * value here therefore propagates everywhere, which is the whole point: + * it removes the hand-copied literal lists that previously let a value + * be settable in the UI and valid in the DB yet rejected by a stale + * validator (the `test_images` autosave hang). `tests/db/enum-drift.test.ts` + * pins each schema column's enum against the constant it must equal. + * + * @version v0.4.1 */ // Description levels @@ -63,3 +75,53 @@ export const FUNCTION_CATEGORIES = [ "honorific", "occupation_trade", "documentary_role", "kinship", "status_condition", "institutional_ref", ] as const; + +// --- Workflow-side enums (Fisqua-native, not Django-mirrored) --- + +// Segmentation entry types. `null` (unset) is allowed at the column and +// validator level and is NOT a member here — it is the "no type chosen +// yet" state, distinct from the closed set of chosen types. +export const ENTRY_TYPES = [ + "item", "blank", "front_matter", "back_matter", "test_images", +] as const; +export type EntryType = (typeof ENTRY_TYPES)[number]; + +// Editor-side (Spanish) resource types used by the entries table and the +// description editor. Distinct from the English `RESOURCE_TYPES` above, +// which is the published/Dublin-Core vocabulary; `lib/promote/types.ts` +// bridges the two (e.g. `texto` -> `text`). Do not conflate them. +export const RESOURCE_TYPES_ES = [ + "texto", "imagen", "cartografico", "mixto", +] as const; +export type ResourceTypeEs = (typeof RESOURCE_TYPES_ES)[number]; + +// Project membership roles. Used both as a closed validation set ("is +// this a real role") and as the allow-list for member-only mutation +// endpoints. Adding a role here grants it access to every endpoint that +// guards on the full set — intended, but worth knowing. +export const PROJECT_ROLES = ["lead", "cataloguer", "reviewer"] as const; +export type ProjectRole = (typeof PROJECT_ROLES)[number]; + +// Volume segmentation-lifecycle statuses. The allowed transitions +// between them live in `app/lib/workflow.ts` (a state machine, not a +// flat set); this is just the closed set of valid status values shared +// by the schema column and the `VolumeStatus` type. +export const VOLUME_STATUSES = [ + "unstarted", "in_progress", "segmented", "sent_back", "reviewed", "approved", +] as const; +export type VolumeStatus = (typeof VOLUME_STATUSES)[number]; + +// Tenant descriptive standards. +export const DESCRIPTIVE_STANDARDS = ["isadg", "dacs", "rad"] as const; + +// QC flag problem types and resolution actions (page-level quality +// control). Mirrored by the `qcFlags` table columns. +export const QC_PROBLEM_TYPES = [ + "damaged", "repeated", "out_of_order", "missing", "blank", "other", +] as const; +export const QC_RESOLUTION_ACTIONS = [ + "retake_requested", "reordered", "marked_duplicate", "ignored", "other", +] as const; + +// GeoNames feature classes (fixed external vocabulary). +export const GEONAMES_FCLASSES = ["P", "H", "A", "T", "S"] as const; diff --git a/app/lib/validation/place.ts b/app/lib/validation/place.ts index b4a8b04..da3042c 100644 --- a/app/lib/validation/place.ts +++ b/app/lib/validation/place.ts @@ -21,7 +21,7 @@ */ import { z } from "zod/v4"; -import { PLACE_TYPES } from "./enums"; +import { PLACE_TYPES, GEONAMES_FCLASSES } from "./enums"; export const placeSchema = z.object({ id: z.string().uuid(), @@ -40,7 +40,7 @@ export const placeSchema = z.object({ hgisId: z.string().max(50).nullable().optional(), whgId: z.string().max(50).nullable().optional(), // 5-value GeoNames feature class enum (added in 0036). - fclass: z.enum(["P", "H", "A", "T", "S"]).nullable().optional(), + fclass: z.enum(GEONAMES_FCLASSES).nullable().optional(), // Generic legacy id JSON column (0036). Stored as a JSON string at // the DB layer; full Zod shape lives in app/lib/validation/legacy-ids.ts. legacyIds: z.string().default("[]"), diff --git a/app/lib/workflow.ts b/app/lib/workflow.ts index 835472e..0209cd2 100644 --- a/app/lib/workflow.ts +++ b/app/lib/workflow.ts @@ -20,18 +20,20 @@ * keeping the two state machines separate reflects the fact that * segmentation and description proceed at different cadences. * - * @version v0.3.0 + * @version v0.4.1 */ -export type VolumeStatus = - | "unstarted" - | "in_progress" - | "segmented" - | "sent_back" - | "reviewed" - | "approved"; +import type { ProjectRole, VolumeStatus } from "./validation/enums"; -export type WorkflowRole = "cataloguer" | "reviewer" | "lead"; +// VolumeStatus is derived from the canonical `VOLUME_STATUSES` array +// (app/lib/validation/enums.ts). Imported locally for the state machine +// below and re-exported so the many modules that import `VolumeStatus` +// from here keep working. +export type { VolumeStatus }; + +// Project roles, viewed through the workflow lens. Aliased to the +// canonical `ProjectRole` so the role vocabulary has one source. +export type WorkflowRole = ProjectRole; const TRANSITIONS: Record< WorkflowRole, diff --git a/app/routes/_auth.admin.cataloguing.team.tsx b/app/routes/_auth.admin.cataloguing.team.tsx index 758742f..204f52f 100644 --- a/app/routes/_auth.admin.cataloguing.team.tsx +++ b/app/routes/_auth.admin.cataloguing.team.tsx @@ -19,6 +19,7 @@ import { useState } from "react"; import { useFetcher } from "react-router"; import { useTranslation } from "react-i18next"; import { tenantContext, userContext } from "../context"; +import { PROJECT_ROLES } from "../lib/validation/enums"; import type { Route } from "./+types/_auth.admin.cataloguing.team"; interface TeamMember { @@ -196,7 +197,7 @@ export async function action({ request, context }: Route.ActionArgs) { const intent = formData.get("_action") as string; if (intent === "assignToProject") { - const roleSchema = z.enum(["lead", "cataloguer", "reviewer"]); + const roleSchema = z.enum(PROJECT_ROLES); const userId = formData.get("userId") as string; const projectId = formData.get("projectId") as string; const roleRaw = formData.get("role") as string; diff --git a/app/routes/_auth.admin.users.$id.tsx b/app/routes/_auth.admin.users.$id.tsx index 3ead3c1..ca00650 100644 --- a/app/routes/_auth.admin.users.$id.tsx +++ b/app/routes/_auth.admin.users.$id.tsx @@ -37,6 +37,7 @@ import { useTranslation } from "react-i18next"; import { tenantContext, userContext } from "../context"; import { formatDate } from "../lib/format"; import type { Route } from "./+types/_auth.admin.users.$id"; +import { PROJECT_ROLES, type ProjectRole } from "../lib/validation/enums"; // --------------------------------------------------------------------------- // Loader @@ -250,9 +251,9 @@ export async function action({ request, params, context }: Route.ActionArgs) { if (intent === "assignToProject") { const projectId = formData.get("projectId") as string; - const role = formData.get("role") as "lead" | "cataloguer" | "reviewer"; + const role = formData.get("role") as ProjectRole; - if (!projectId || !["lead", "cataloguer", "reviewer"].includes(role)) { + if (!projectId || !(PROJECT_ROLES as readonly string[]).includes(role)) { return { ok: false, error: i18n.t("user_admin:error_invalid_request") }; } @@ -284,9 +285,9 @@ export async function action({ request, params, context }: Route.ActionArgs) { if (intent === "changeRole") { const membershipId = formData.get("membershipId") as string; - const role = formData.get("role") as "lead" | "cataloguer" | "reviewer"; + const role = formData.get("role") as ProjectRole; - if (!membershipId || !["lead", "cataloguer", "reviewer"].includes(role)) { + if (!membershipId || !(PROJECT_ROLES as readonly string[]).includes(role)) { return { ok: false, error: i18n.t("user_admin:error_invalid_request") }; } diff --git a/app/routes/_auth.description.$projectId.$entryId.tsx b/app/routes/_auth.description.$projectId.$entryId.tsx index c8ce9e4..1559dee 100644 --- a/app/routes/_auth.description.$projectId.$entryId.tsx +++ b/app/routes/_auth.description.$projectId.$entryId.tsx @@ -153,6 +153,7 @@ import { buildDescriptionBeaconBody, } from "../lib/beacon-save"; import type { Route } from "./+types/_auth.description.$projectId.$entryId"; +import { PROJECT_ROLES } from "../lib/validation/enums"; export async function loader({ params, context }: Route.LoaderArgs) { const { drizzle } = await import("drizzle-orm/d1"); @@ -186,7 +187,7 @@ export async function loader({ params, context }: Route.LoaderArgs) { db, user.id, params.projectId, - ["lead", "cataloguer", "reviewer"], + [...PROJECT_ROLES], user.isAdmin ); diff --git a/app/routes/_auth.projects.$id.members.tsx b/app/routes/_auth.projects.$id.members.tsx index 07585f8..f45c5e4 100644 --- a/app/routes/_auth.projects.$id.members.tsx +++ b/app/routes/_auth.projects.$id.members.tsx @@ -14,6 +14,7 @@ import { Form, useActionData, useFetcher } from "react-router"; import { useTranslation } from "react-i18next"; import { userContext } from "../context"; +import { PROJECT_ROLES, type ProjectRole } from "../lib/validation/enums"; import type { Route } from "./+types/_auth.projects.$id.members"; type MemberRow = { @@ -96,7 +97,7 @@ export async function action({ request, params, context }: Route.ActionArgs) { return { ok: false, error: i18n.t("admin:error.user_not_found") }; } - const validRoles = ["lead", "cataloguer", "reviewer"]; + const validRoles: readonly string[] = PROJECT_ROLES; if (!validRoles.includes(role)) { return { ok: false, error: i18n.t("project:error.role_required") }; } @@ -132,7 +133,7 @@ export async function action({ request, params, context }: Route.ActionArgs) { id: crypto.randomUUID(), projectId: params.id, userId: targetUser.id, - role: role as "lead" | "cataloguer" | "reviewer", + role: role as ProjectRole, createdAt: (Date.now() / 1000) | 0, }); @@ -147,14 +148,14 @@ export async function action({ request, params, context }: Route.ActionArgs) { return { ok: false, error: "Missing membership ID" }; } - const validRoles = ["lead", "cataloguer", "reviewer"]; + const validRoles: readonly string[] = PROJECT_ROLES; if (!validRoles.includes(role)) { return { ok: false, error: i18n.t("project:error.role_required") }; } await db .update(projectMembers) - .set({ role: role as "lead" | "cataloguer" | "reviewer" }) + .set({ role: role as ProjectRole }) .where(eq(projectMembers.id, membershipId)); return { ok: true }; diff --git a/app/routes/_auth.projects.$id.overview.tsx b/app/routes/_auth.projects.$id.overview.tsx index 487d176..b546489 100644 --- a/app/routes/_auth.projects.$id.overview.tsx +++ b/app/routes/_auth.projects.$id.overview.tsx @@ -18,6 +18,7 @@ import { PipelineColumn } from "../components/pipeline/pipeline-column"; import { AssignDescriberPopover } from "../components/pipeline/assign-describer-popover"; import type { Route } from "./+types/_auth.projects.$id.overview"; import type { PipelineColumn as PipelineColumnType } from "../lib/pipeline/pipeline.server"; +import { PROJECT_ROLES } from "../lib/validation/enums"; export async function loader({ params, context }: Route.LoaderArgs) { const { drizzle } = await import("drizzle-orm/d1"); @@ -34,7 +35,7 @@ export async function loader({ params, context }: Route.LoaderArgs) { db, user.id, params.id, - ["lead", "cataloguer", "reviewer"], + [...PROJECT_ROLES], user.isAdmin ); diff --git a/app/routes/_auth.projects.$id.tsx b/app/routes/_auth.projects.$id.tsx index 9b8c847..49251a1 100644 --- a/app/routes/_auth.projects.$id.tsx +++ b/app/routes/_auth.projects.$id.tsx @@ -13,6 +13,7 @@ import { Outlet, NavLink, Link } from "react-router"; import { useTranslation } from "react-i18next"; import { userContext } from "../context"; +import { PROJECT_ROLES } from "../lib/validation/enums"; import type { Route } from "./+types/_auth.projects.$id"; export async function loader({ params, context }: Route.LoaderArgs) { @@ -32,7 +33,7 @@ export async function loader({ params, context }: Route.LoaderArgs) { } // Check membership (admin bypasses) - await requireProjectRole(db, user.id, params.id, ["lead", "cataloguer", "reviewer"], user.isAdmin); + await requireProjectRole(db, user.id, params.id, [...PROJECT_ROLES], user.isAdmin); // Get user's specific role for conditional UI const membership = await db diff --git a/app/routes/_auth.projects.$id.volumes.$volumeId.manage.tsx b/app/routes/_auth.projects.$id.volumes.$volumeId.manage.tsx index 18e5f80..0c3a892 100644 --- a/app/routes/_auth.projects.$id.volumes.$volumeId.manage.tsx +++ b/app/routes/_auth.projects.$id.volumes.$volumeId.manage.tsx @@ -166,7 +166,7 @@ export async function loader({ params, context }: Route.LoaderArgs) { // The loader's top-line requireProjectRole already restricted access to // leads; surface the role explicitly so the client dialog's role-gate // has a single source of truth. - const userRole: "lead" | "cataloguer" | "reviewer" = "lead"; + const userRole: WorkflowRole = "lead"; return { volume, diff --git a/app/routes/_auth.viewer.$projectId.$volumeId.tsx b/app/routes/_auth.viewer.$projectId.$volumeId.tsx index 8ace2d4..2d14b03 100644 --- a/app/routes/_auth.viewer.$projectId.$volumeId.tsx +++ b/app/routes/_auth.viewer.$projectId.$volumeId.tsx @@ -75,6 +75,7 @@ import { findCurrentEntry } from "../lib/entry-ownership"; import { partitionComments } from "../lib/comment-partition"; import type { BoundaryAction } from "../lib/boundary-types"; import type { Route } from "./+types/_auth.viewer.$projectId.$volumeId"; +import { PROJECT_ROLES } from "../lib/validation/enums"; export async function loader({ params, context }: Route.LoaderArgs) { const { drizzle } = await import("drizzle-orm/d1"); @@ -95,7 +96,7 @@ export async function loader({ params, context }: Route.LoaderArgs) { // Any project member can access the viewer (access level determined by role + assignment) const memberships = await requireProjectRole( db, user.id, params.projectId, - ["lead", "cataloguer", "reviewer"], + [...PROJECT_ROLES], user.isAdmin ); diff --git a/app/routes/api.comments.tsx b/app/routes/api.comments.tsx index 0e6213a..1952d1e 100644 --- a/app/routes/api.comments.tsx +++ b/app/routes/api.comments.tsx @@ -11,6 +11,7 @@ */ import { userContext } from "../context"; import type { WorkflowRole } from "../lib/workflow"; +import { PROJECT_ROLES } from "../lib/validation/enums"; import type { Route } from "./+types/api.comments"; export async function action({ request, context }: Route.ActionArgs) { @@ -121,7 +122,7 @@ export async function action({ request, context }: Route.ActionArgs) { db, user.id, volume.projectId, - ["lead", "cataloguer", "reviewer"], + [...PROJECT_ROLES], user.isAdmin ); const roleOrder: WorkflowRole[] = ["lead", "reviewer", "cataloguer"]; @@ -168,7 +169,7 @@ export async function action({ request, context }: Route.ActionArgs) { db, user.id, volume.projectId, - ["lead", "cataloguer", "reviewer"], + [...PROJECT_ROLES], user.isAdmin ); const roleOrder: WorkflowRole[] = ["lead", "reviewer", "cataloguer"]; diff --git a/app/routes/api.entries.save.tsx b/app/routes/api.entries.save.tsx index c3c2189..932b133 100644 --- a/app/routes/api.entries.save.tsx +++ b/app/routes/api.entries.save.tsx @@ -25,6 +25,7 @@ */ import { userContext } from "../context"; +import { PROJECT_ROLES } from "../lib/validation/enums"; import type { Route } from "./+types/api.entries.save"; export async function action({ request, context }: Route.ActionArgs) { @@ -90,7 +91,7 @@ export async function action({ request, context }: Route.ActionArgs) { db, user.id, volume.projectId, - ["lead", "cataloguer", "reviewer"], + [...PROJECT_ROLES], user.isAdmin ); diff --git a/app/routes/api.qc-flags.tsx b/app/routes/api.qc-flags.tsx index b0111f0..120764f 100644 --- a/app/routes/api.qc-flags.tsx +++ b/app/routes/api.qc-flags.tsx @@ -7,28 +7,24 @@ * additionally enforces the lead-only resolver rule. GET lists open * flags for the requested volume or entry. * - * @version v0.3.0 + * @version v0.4.1 */ import { userContext } from "../context"; import type { Route } from "./+types/api.qc-flags"; - -const ALLOWED_PROBLEM_TYPES = [ - "damaged", - "repeated", - "out_of_order", - "missing", - "blank", - "other", -] as const; - -const ALLOWED_RESOLUTION_ACTIONS = [ - "retake_requested", - "reordered", - "marked_duplicate", - "ignored", - "other", -] as const; - +import { + QC_PROBLEM_TYPES, + QC_RESOLUTION_ACTIONS, + PROJECT_ROLES, +} from "../lib/validation/enums"; + +// Canonical QC vocabularies live in validation/enums.ts so the schema +// column, this validator, and the UI cannot drift. Local aliases keep +// the call sites below readable. +const ALLOWED_PROBLEM_TYPES = QC_PROBLEM_TYPES; +const ALLOWED_RESOLUTION_ACTIONS = QC_RESOLUTION_ACTIONS; + +// Subset of the qc-flag status enum: the statuses a flag may be resolved +// *to* (excludes "open"). Intentionally not the full enum. const ALLOWED_RESOLVE_STATUSES = ["resolved", "wontfix"] as const; export async function action({ request, context }: Route.ActionArgs) { @@ -104,7 +100,7 @@ export async function action({ request, context }: Route.ActionArgs) { db, user.id, volume.projectId, - ["lead", "cataloguer", "reviewer"], + [...PROJECT_ROLES], user.isAdmin ); @@ -316,7 +312,7 @@ export async function loader({ request, context }: Route.LoaderArgs) { db, user.id, volume.projectId, - ["lead", "cataloguer", "reviewer"], + [...PROJECT_ROLES], user.isAdmin ); diff --git a/app/routes/api.resegmentation.tsx b/app/routes/api.resegmentation.tsx index 458dae2..4fe2181 100644 --- a/app/routes/api.resegmentation.tsx +++ b/app/routes/api.resegmentation.tsx @@ -19,6 +19,7 @@ */ import { userContext } from "../context"; +import { PROJECT_ROLES } from "../lib/validation/enums"; import type { Route } from "./+types/api.resegmentation"; export async function action({ request, context }: Route.ActionArgs) { @@ -171,7 +172,7 @@ export async function loader({ request, context }: Route.LoaderArgs) { db, user.id, volume.projectId, - ["lead", "cataloguer", "reviewer"], + [...PROJECT_ROLES], user.isAdmin ); diff --git a/app/routes/api.workflow.tsx b/app/routes/api.workflow.tsx index d6015e3..d9383a8 100644 --- a/app/routes/api.workflow.tsx +++ b/app/routes/api.workflow.tsx @@ -20,6 +20,7 @@ import { userContext } from "../context"; import type { VolumeStatus, WorkflowRole } from "../lib/workflow"; +import { PROJECT_ROLES } from "../lib/validation/enums"; import type { Route } from "./+types/api.workflow"; export async function action({ request, context }: Route.ActionArgs) { @@ -52,7 +53,7 @@ export async function action({ request, context }: Route.ActionArgs) { db, user.id, projectId, - ["lead", "cataloguer", "reviewer"], + [...PROJECT_ROLES], user.isAdmin ); diff --git a/tests/db/enum-drift.test.ts b/tests/db/enum-drift.test.ts new file mode 100644 index 0000000..2b93a54 --- /dev/null +++ b/tests/db/enum-drift.test.ts @@ -0,0 +1,82 @@ +/** + * Tests — enum drift guard + * + * This suite is the structural backstop for the bug class behind the + * `test_images` autosave hang: an enum value that is settable in the UI + * and valid in the DB schema, yet rejected by a hand-copied validator + * literal that drifted from the schema. The fix hoisted every such + * vocabulary into `app/lib/validation/enums.ts` as the single source of + * truth, consumed by the Drizzle column hint, the Zod validators, and + * the derived TypeScript types. + * + * These assertions pin each Drizzle enum column's `enumValues` to (a) the + * canonical constant it must equal and (b) an explicit expected set. (a) + * fails if anyone reintroduces a divergent literal in the schema; (b) + * fails if anyone edits the canonical constant without updating this + * checkpoint — forcing a deliberate review rather than a silent change. + * Because the validators now reference the same constants directly, a + * passing schema↔constant assertion means the validator cannot reject a + * value the schema accepts. + * + * Scope note: `resegmentationFlags.problemType` is intentionally NOT + * pinned here — it has no second app-level allow-list to drift against + * (the create route trusts the value and the DB enum is the sole gate), + * so it was deliberately left out of the hoist. + * + * @version v0.4.1 + */ +import { describe, it, expect } from "vitest"; +import { + tenants, + projectMembers, + entries, + qcFlags, + comments, + places, + volumes, +} from "../../app/db/schema"; +import { + ENTRY_TYPES, + RESOURCE_TYPES_ES, + PROJECT_ROLES, + DESCRIPTIVE_STANDARDS, + QC_PROBLEM_TYPES, + QC_RESOLUTION_ACTIONS, + GEONAMES_FCLASSES, + VOLUME_STATUSES, +} from "../../app/lib/validation/enums"; + +const sorted = (xs: readonly string[]) => [...xs].sort(); + +// [schema column .enumValues, canonical constant, explicit expected set] +const CASES: [string, readonly string[], readonly string[], readonly string[]][] = [ + ["entries.type", entries.type.enumValues, ENTRY_TYPES, + ["item", "blank", "front_matter", "back_matter", "test_images"]], + ["entries.resourceType", entries.resourceType.enumValues, RESOURCE_TYPES_ES, + ["texto", "imagen", "cartografico", "mixto"]], + ["projectMembers.role", projectMembers.role.enumValues, PROJECT_ROLES, + ["lead", "cataloguer", "reviewer"]], + ["comments.authorRole", comments.authorRole.enumValues, PROJECT_ROLES, + ["lead", "cataloguer", "reviewer"]], + ["tenants.descriptiveStandard", tenants.descriptiveStandard.enumValues, DESCRIPTIVE_STANDARDS, + ["isadg", "dacs", "rad"]], + ["qcFlags.problemType", qcFlags.problemType.enumValues, QC_PROBLEM_TYPES, + ["damaged", "repeated", "out_of_order", "missing", "blank", "other"]], + ["qcFlags.resolutionAction", qcFlags.resolutionAction.enumValues, QC_RESOLUTION_ACTIONS, + ["retake_requested", "reordered", "marked_duplicate", "ignored", "other"]], + ["places.fclass", places.fclass.enumValues, GEONAMES_FCLASSES, + ["P", "H", "A", "T", "S"]], + ["volumes.status", volumes.status.enumValues, VOLUME_STATUSES, + ["unstarted", "in_progress", "segmented", "sent_back", "reviewed", "approved"]], +]; + +describe("enum drift guard (schema column === canonical constant)", () => { + for (const [label, columnValues, constant, expected] of CASES) { + it(`${label} matches its canonical constant and expected set`, () => { + // Schema column and the validation constant agree. + expect(sorted(columnValues)).toEqual(sorted(constant)); + // Both agree with the explicit checkpoint set. + expect(sorted(constant)).toEqual(sorted(expected)); + }); + } +}); diff --git a/tests/entries/save.test.ts b/tests/entries/save.test.ts index 8b5a602..c5a6155 100644 --- a/tests/entries/save.test.ts +++ b/tests/entries/save.test.ts @@ -17,7 +17,7 @@ * to another tenant rather than partially writing across the * boundary. * - * @version v0.3.0 + * @version v0.4.1 */ import { describe, @@ -223,6 +223,34 @@ describe("entry persistence (loadEntries / saveEntries)", () => { ).rejects.toThrow("invalid entry type"); }); + it("accepts every EntryType the schema enum allows (incl. test_images)", async () => { + // Regression: `validateEntries` once allowlisted only item / blank / + // front_matter / back_matter, so a `test_images` entry — settable in + // the outline UI and valid in the schema enum — failed validation and + // its autosave hung on "saving…" forever. Every member of the closed + // enum must round-trip through save → load. + const db = drizzle(env.DB, { schema }); + + const entriesToSave: Entry[] = [ + makeEntry({ id: "e1", volumeId, position: 0, startPage: 1, type: "test_images" }), + makeEntry({ id: "e2", volumeId, position: 1, startPage: 2, type: "front_matter" }), + makeEntry({ id: "e3", volumeId, position: 2, startPage: 3, type: "item" }), + makeEntry({ id: "e4", volumeId, position: 3, startPage: 4, type: "blank" }), + makeEntry({ id: "e5", volumeId, position: 4, startPage: 5, type: "back_matter" }), + ]; + + await saveEntries(db, volumeId, entriesToSave); + const loaded = await loadEntries(db, volumeId); + + expect(loaded.map((e) => e.type)).toEqual([ + "test_images", + "front_matter", + "item", + "blank", + "back_matter", + ]); + }); + it("saves and loads entries with startY and endY (y-position roundtrip)", async () => { const db = drizzle(env.DB, { schema }); From 1dbf68b1dc81df6a2dfb0e5f61cc9532389ff87f Mon Sep 17 00:00:00 2001 From: Juan Cobo Betancourt Date: Fri, 29 May 2026 16:03:44 -0700 Subject: [PATCH 2/4] Add manual-dispatch test workflow A workflow_dispatch CI job that runs the Vitest suite and the TypeScript type-check on demand, on an Ubicloud runner sized for the Cloudflare Workers vitest pool. Manual trigger is deliberate: the suite is too heavy to run on every push, and concurrency is keyed by branch ref so a second dispatch cancels the in-flight run. --- .github/workflows/tests.yml | 46 +++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 .github/workflows/tests.yml diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml new file mode 100644 index 0000000..642b2b7 --- /dev/null +++ b/.github/workflows/tests.yml @@ -0,0 +1,46 @@ +# Tests +# +# Manually-dispatched CI workflow that runs Vitest and the TypeScript +# type-check on demand. Trigger from the GitHub Actions UI +# (Actions → Tests → Run workflow) or `gh workflow run tests.yml` after +# pushing changes you want to verify against the full v0.4 regression +# surface (~2520 tests across the Cloudflare Workers vitest pool). +# +# Manual trigger is deliberate: the test suite is too heavy to run on +# every push during active development, and the local laptop can't +# host it. Run this when a milestone or risky phase wants cross-cutting +# verification, not on every commit. +# +# Concurrency is keyed by branch ref so a second manual dispatch on the +# same branch cancels the in-flight run rather than queueing. +# +# Runner: Ubicloud (`ubicloud-standard-4`, 4 vCPU / 16 GB) — comfortable +# headroom for the Workers vitest pool. Requires the Ubicloud GitHub +# App installed on the account; without it, runs queue indefinitely. +# Bump to `ubicloud-standard-8` if pool cold-starts begin pushing past +# the timeout. +# +# @version v0.4.1 + +name: Tests + +on: + workflow_dispatch: + +concurrency: + group: tests-${{ github.ref }} + cancel-in-progress: true + +jobs: + test: + runs-on: ubicloud-standard-4 + timeout-minutes: 15 + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version: 22 + cache: npm + - run: npm ci + - run: npx vitest --run + - run: npm run typecheck From 4f3f7916706f8965ae9de53469a06f15b1c9cb30 Mon Sep 17 00:00:00 2001 From: Juan Cobo Betancourt Date: Fri, 29 May 2026 16:23:10 -0700 Subject: [PATCH 3/4] Bump version to v0.4.1 The site footer reads "Fisqua v0.4.1" and links to the v0.4.1 release tag, and package.json metadata matches the release. --- app/components/layout/footer.tsx | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/components/layout/footer.tsx b/app/components/layout/footer.tsx index 87f832e..2f31b76 100644 --- a/app/components/layout/footer.tsx +++ b/app/components/layout/footer.tsx @@ -9,10 +9,10 @@ * keeps the gap between the two clusters click-through so users can still * interact with anything below. * - * @version v0.4.0 + * @version v0.4.1 */ -const VERSION = "0.4.0"; +const VERSION = "0.4.1"; export function Footer() { return ( diff --git a/package.json b/package.json index a30803e..729d35c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "fisqua", - "version": "0.4.0", + "version": "0.4.1", "private": true, "type": "module", "description": "Collaborative cataloguing and data-management platform for archival materials, built on Cloudflare Workers.", From f8abd11a31d6b0ddcb512d85690d192e10bbb3ce Mon Sep 17 00:00:00 2001 From: Juan Cobo Betancourt Date: Fri, 29 May 2026 16:23:10 -0700 Subject: [PATCH 4/4] Document v0.4.1 release in changelog --- CHANGELOG.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b2f2495..ea2c112 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,24 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased] +## [0.4.1] - 2026-05-29 + +### Fixed + +- **Saving test and calibration entries.** Entries marked as the `test_images` + type could not be saved: autosave stalled and the manual "Save now" button + could not recover the entry, even though the type was selectable in the + outline. Save validation now accepts every valid entry type. + +### Changed + +- **Controlled vocabularies consolidated.** The catalogue's controlled + vocabularies — entry types, resource types, project roles, descriptive + standards, quality-control flag types, and volume statuses — are now defined + in a single place and shared across the database schema, validators, and type + definitions, removing the hand-copied duplicates that caused the save fault + above. An automated test now fails the build if these definitions drift apart. + ## [0.4.0] - 2026-05-18 ### Added