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 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 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/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/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.", 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 });