From 2cff076171de2bf4037a0599446fe3ad9bd069fe Mon Sep 17 00:00:00 2001 From: Harry Randall Date: Tue, 22 Sep 2026 13:02:45 +1000 Subject: [PATCH 1/3] feat: give a record one history instead of six audit tables The Changelog tab listed raw rows: "Version 87 created", one line per autosave, no actor, no values, and a database id where a version number belongs. It is now one timeline grouped by day, in the vocabulary an administrator works in. Autosaves group by editing session, so a sitting reads as one entry from the value it started on to the value it ended on, with a field typed and taken back counting as nothing. Decisions taken in one sitting read as one review, and repeated quiet ANU checks collapse rather than filling the page. The grouping is presentation only: every raw event keeps its row, and an entry names the events it covers. Versions are numbered within their record and open as a page that renders the student view of that content, compares it against the draft, the publication or the preceding version through the same diff the Changes tab uses, and restores it as a draft. Restoring no longer destroys work. The draft it replaces becomes a version of its own and is offered back from the changelog, which is what makes the replacement warning true. A source decision event now names the review row it answered, because keeping a value changes no content and so leaves no field change row to say which field was kept. --- .../[year]/[code]/[[...section]]/page.tsx | 11 +- .../[year]/[code]/[[...section]]/page.tsx | 11 +- .../[year]/[code]/[[...section]]/page.tsx | 11 +- .../[year]/[code]/[[...section]]/page.tsx | 11 +- .../[year]/[code]/[[...section]]/page.tsx | 11 +- apps/web/lib/catalogue/changelog.ts | 183 +++++++++++++++ apps/web/lib/catalogue/drafts.ts | 21 +- .../lib/catalogue/source-review-decisions.ts | 10 +- .../lib/coursemap/admin-catalogue-actions.ts | 4 +- .../coursemap/admin-catalogue-changelog.ts | 169 ++++++++++++++ .../lib/coursemap/admin-catalogue-record.ts | 35 --- apps/web/lib/coursemap/catalogue-kinds.ts | 10 + .../catalogue-changelog-database.test.mjs | 205 ++++++++++++++++ .../catalogue-changelog-timeline.test.tsx | 162 +++++++++++++ apps/web/tests/catalogue-changelog.test.ts | 201 ++++++++++++++++ apps/web/tests/catalogue-routes.test.ts | 10 + .../catalogue-source-review-database.test.mjs | 11 +- apps/web/types/database.ts | 10 + .../admin/catalogue/catalogue-route-pages.tsx | 24 +- .../catalogue/changelog/changelog-entry.tsx | 165 +++++++++++++ .../changelog/changelog-timeline.tsx | 94 ++++++++ .../changelog/restore-version-button.tsx | 82 +++++++ .../catalogue/changelog/version-page.tsx | 220 ++++++++++++++++++ .../catalogue/changes/unpublished-changes.tsx | 38 +-- .../ui/admin/catalogue/field-change-list.tsx | 54 +++++ .../web/ui/admin/catalogue/record-history.tsx | 136 ----------- apps/web/ui/admin/catalogue/record-page.tsx | 22 +- docs/architecture.md | 5 +- docs/catalogue-completion-plan.md | 6 + docs/catalogue-operations.md | 26 ++- .../20260924100000_catalogue_changelog.sql | 22 ++ .../tests/database/catalogue_sync_changes.sql | 36 ++- 32 files changed, 1790 insertions(+), 226 deletions(-) create mode 100644 apps/web/lib/catalogue/changelog.ts create mode 100644 apps/web/lib/coursemap/admin-catalogue-changelog.ts create mode 100644 apps/web/tests/catalogue-changelog-database.test.mjs create mode 100644 apps/web/tests/catalogue-changelog-timeline.test.tsx create mode 100644 apps/web/tests/catalogue-changelog.test.ts create mode 100644 apps/web/ui/admin/catalogue/changelog/changelog-entry.tsx create mode 100644 apps/web/ui/admin/catalogue/changelog/changelog-timeline.tsx create mode 100644 apps/web/ui/admin/catalogue/changelog/restore-version-button.tsx create mode 100644 apps/web/ui/admin/catalogue/changelog/version-page.tsx create mode 100644 apps/web/ui/admin/catalogue/field-change-list.tsx delete mode 100644 apps/web/ui/admin/catalogue/record-history.tsx create mode 100644 supabase/migrations/20260924100000_catalogue_changelog.sql diff --git a/apps/web/app/admin/courses/[year]/[code]/[[...section]]/page.tsx b/apps/web/app/admin/courses/[year]/[code]/[[...section]]/page.tsx index f3e42ec9..816ee09f 100644 --- a/apps/web/app/admin/courses/[year]/[code]/[[...section]]/page.tsx +++ b/apps/web/app/admin/courses/[year]/[code]/[[...section]]/page.tsx @@ -1,9 +1,18 @@ +import type { SearchParams } from "@/ui/admin/catalogue/catalogue-pages"; import { CatalogueRecordRoute } from "@/ui/admin/catalogue/catalogue-route-pages"; export const dynamic = "force-dynamic"; export default async function Page({ params, + searchParams, }: { params: Promise<{ year: string; code: string; section?: string[] }>; + searchParams: SearchParams; }) { - return ; + return ( + + ); } diff --git a/apps/web/app/admin/majors/[year]/[code]/[[...section]]/page.tsx b/apps/web/app/admin/majors/[year]/[code]/[[...section]]/page.tsx index fea80fdf..6fe101d9 100644 --- a/apps/web/app/admin/majors/[year]/[code]/[[...section]]/page.tsx +++ b/apps/web/app/admin/majors/[year]/[code]/[[...section]]/page.tsx @@ -1,9 +1,18 @@ +import type { SearchParams } from "@/ui/admin/catalogue/catalogue-pages"; import { CatalogueRecordRoute } from "@/ui/admin/catalogue/catalogue-route-pages"; export const dynamic = "force-dynamic"; export default async function Page({ params, + searchParams, }: { params: Promise<{ year: string; code: string; section?: string[] }>; + searchParams: SearchParams; }) { - return ; + return ( + + ); } diff --git a/apps/web/app/admin/minors/[year]/[code]/[[...section]]/page.tsx b/apps/web/app/admin/minors/[year]/[code]/[[...section]]/page.tsx index e3e6f16f..1bb50101 100644 --- a/apps/web/app/admin/minors/[year]/[code]/[[...section]]/page.tsx +++ b/apps/web/app/admin/minors/[year]/[code]/[[...section]]/page.tsx @@ -1,9 +1,18 @@ +import type { SearchParams } from "@/ui/admin/catalogue/catalogue-pages"; import { CatalogueRecordRoute } from "@/ui/admin/catalogue/catalogue-route-pages"; export const dynamic = "force-dynamic"; export default async function Page({ params, + searchParams, }: { params: Promise<{ year: string; code: string; section?: string[] }>; + searchParams: SearchParams; }) { - return ; + return ( + + ); } diff --git a/apps/web/app/admin/programmes/[year]/[code]/[[...section]]/page.tsx b/apps/web/app/admin/programmes/[year]/[code]/[[...section]]/page.tsx index bc34635f..457bfa91 100644 --- a/apps/web/app/admin/programmes/[year]/[code]/[[...section]]/page.tsx +++ b/apps/web/app/admin/programmes/[year]/[code]/[[...section]]/page.tsx @@ -1,9 +1,18 @@ +import type { SearchParams } from "@/ui/admin/catalogue/catalogue-pages"; import { CatalogueRecordRoute } from "@/ui/admin/catalogue/catalogue-route-pages"; export const dynamic = "force-dynamic"; export default async function Page({ params, + searchParams, }: { params: Promise<{ year: string; code: string; section?: string[] }>; + searchParams: SearchParams; }) { - return ; + return ( + + ); } diff --git a/apps/web/app/admin/specialisations/[year]/[code]/[[...section]]/page.tsx b/apps/web/app/admin/specialisations/[year]/[code]/[[...section]]/page.tsx index e1adf57d..ad02a740 100644 --- a/apps/web/app/admin/specialisations/[year]/[code]/[[...section]]/page.tsx +++ b/apps/web/app/admin/specialisations/[year]/[code]/[[...section]]/page.tsx @@ -1,9 +1,18 @@ +import type { SearchParams } from "@/ui/admin/catalogue/catalogue-pages"; import { CatalogueRecordRoute } from "@/ui/admin/catalogue/catalogue-route-pages"; export const dynamic = "force-dynamic"; export default async function Page({ params, + searchParams, }: { params: Promise<{ year: string; code: string; section?: string[] }>; + searchParams: SearchParams; }) { - return ; + return ( + + ); } diff --git a/apps/web/lib/catalogue/changelog.ts b/apps/web/lib/catalogue/changelog.ts new file mode 100644 index 00000000..0469c725 --- /dev/null +++ b/apps/web/lib/catalogue/changelog.ts @@ -0,0 +1,183 @@ +import { fieldLabel } from "../coursemap/catalogue-kinds.ts"; + +export type ChangelogEventKind = + | "edit" + | "publish" + | "unpublish" + | "discard" + | "restore" + | "source_draft_created" + | "source_checked" + | "source_changed" + | "sync_failed" + | "source_accepted" + | "source_kept"; + +export type ChangelogEvent = { + id: number; + eventKind: ChangelogEventKind; + origin: "manual" | "source"; + actorId: string | null; + editingSessionId: string | null; + versionId: number | null; + syncChangeId: number | null; + createdAt: string; + fields: Array<{ fieldPath: string; oldValue: unknown; newValue: unknown }>; + /** The review row a source decision answered, when the event has one. */ + decision: { fieldPath: string; decision: "use_source" | "keep_local" } | null; +}; + +export type ChangelogFieldChange = { + fieldPath: string; + label: string; + oldValue: unknown; + newValue: unknown; +}; + +export type ChangelogEntry = { + id: string; + kind: ChangelogEventKind; + at: string; + startedAt: string; + actorId: string | null; + origin: "manual" | "source"; + eventIds: number[]; + versionId: number | null; + fields: ChangelogFieldChange[]; + usedFromSource: string[]; + keptLocal: string[]; +}; + +/** Decisions taken in one sitting read as one review, not as five rows. */ +const DECISION_WINDOW_MS = 30 * 60 * 1000; + +function sameEditingSession(left: ChangelogEvent, right: ChangelogEvent) { + return ( + left.eventKind === "edit" && + right.eventKind === "edit" && + left.editingSessionId !== null && + left.editingSessionId === right.editingSessionId && + left.actorId === right.actorId + ); +} + +function sameDecisionSitting(left: ChangelogEvent, right: ChangelogEvent) { + const decisions = new Set(["source_accepted", "source_kept"]); + return ( + decisions.has(left.eventKind) && + decisions.has(right.eventKind) && + left.actorId === right.actorId && + Math.abs(Date.parse(left.createdAt) - Date.parse(right.createdAt)) <= + DECISION_WINDOW_MS + ); +} + +function sameQuietCheck(left: ChangelogEvent, right: ChangelogEvent) { + return ( + left.eventKind === "source_checked" && right.eventKind === "source_checked" + ); +} + +function belongsToGroup(group: ChangelogEvent[], event: ChangelogEvent) { + const last = group[group.length - 1]!; + return ( + sameEditingSession(last, event) || + sameDecisionSitting(last, event) || + sameQuietCheck(last, event) + ); +} + +/** + * Collapses one group of raw events into the field changes a reader cares + * about: the value before the session started against the value it ended on, + * with paths that came back to where they started dropped entirely. + */ +function mergedFields(events: ChangelogEvent[]): ChangelogFieldChange[] { + const oldest = new Map(); + const newest = new Map(); + const order: string[] = []; + // Events arrive newest first, so the last one seen for a path is the oldest. + for (const event of events) { + for (const change of event.fields) { + if (!newest.has(change.fieldPath)) { + newest.set(change.fieldPath, change.newValue); + order.push(change.fieldPath); + } + oldest.set(change.fieldPath, change.oldValue); + } + } + return order.flatMap((fieldPath) => { + const oldValue = oldest.get(fieldPath) ?? null; + const newValue = newest.get(fieldPath) ?? null; + if (JSON.stringify(oldValue ?? null) === JSON.stringify(newValue ?? null)) + return []; + return [{ fieldPath, label: fieldLabel(fieldPath), oldValue, newValue }]; + }); +} + +/** + * One human timeline from raw audit events, newest first. Grouping is + * presentation only: every event keeps its own row in the database, and an + * entry names the events it covers so the raw history stays reachable. + */ +export function groupChangelogEvents( + events: readonly ChangelogEvent[], +): ChangelogEntry[] { + const groups: ChangelogEvent[][] = []; + for (const event of events) { + const current = groups[groups.length - 1]; + if (current && belongsToGroup(current, event)) current.push(event); + else groups.push([event]); + } + + return groups.map((group) => { + const newest = group[0]!; + const oldest = group[group.length - 1]!; + const decisions = group.flatMap((event) => + event.decision ? [event.decision] : [], + ); + return { + id: `event-${newest.id}`, + kind: newest.eventKind, + at: newest.createdAt, + startedAt: oldest.createdAt, + actorId: newest.actorId, + origin: newest.origin, + eventIds: group.map((event) => event.id), + versionId: newest.versionId, + fields: mergedFields(group), + usedFromSource: decisions + .filter((entry) => entry.decision === "use_source") + .map((entry) => fieldLabel(entry.fieldPath)), + keptLocal: decisions + .filter((entry) => entry.decision === "keep_local") + .map((entry) => fieldLabel(entry.fieldPath)), + } satisfies ChangelogEntry; + }); +} + +export const CHANGELOG_PAGE_SIZE = 40; + +export type ChangelogEntryView = ChangelogEntry & { + actorName: string | null; + versionOrdinal: number | null; + /** Review totals for the sync that produced a source change entry. */ + sourceChanges: { total: number; conflicts: number } | null; +}; + +export type CatalogueChangelog = { + entries: ChangelogEntryView[]; + shown: number; + hasMore: boolean; +}; + +/** "5 autosaves over 4 minutes", or null when there is nothing to summarise. */ +export function editingSessionSummary(entry: ChangelogEntry) { + if (entry.kind !== "edit" || entry.eventIds.length < 2) return null; + const minutes = Math.round( + (Date.parse(entry.at) - Date.parse(entry.startedAt)) / 60_000, + ); + const saves = `${entry.eventIds.length} autosaves`; + if (minutes < 1) return `${saves} in under a minute`; + return `${saves} over ${minutes} minute${minutes === 1 ? "" : "s"}`; +} diff --git a/apps/web/lib/catalogue/drafts.ts b/apps/web/lib/catalogue/drafts.ts index 75c5e176..3f8f963c 100644 --- a/apps/web/lib/catalogue/drafts.ts +++ b/apps/web/lib/catalogue/drafts.ts @@ -605,6 +605,7 @@ export async function restoreCatalogueVersion({ select * from public.catalogue_drafts where record_id = ${recordId} for update `; const existing = existingRow ? draftFromRow(existingRow) : null; + let replacedVersionId: number | null = null; if (existing) { if (!replaceExistingDraft) throw new CatalogueDraftError( @@ -613,6 +614,24 @@ export async function restoreCatalogueVersion({ ); if (expectedRevision === null || existing.revision !== expectedRevision) throw new CatalogueDraftConflictError(existing.revision); + // Restoring must not destroy work. The draft it replaces becomes a + // version of its own, so the changelog can offer it back. + if (await draftIsMeaningful(tx, record, existing)) { + replacedVersionId = await materialiseDraftVersion(tx, { + record, + draft: existing, + userId, + }); + await tx` + insert into public.catalogue_change_events ( + record_id, draft_revision, event_kind, origin, actor_id, + editing_session_id, version_id + ) values ( + ${recordId}, ${existing.revision}, 'discard', 'manual', + ${userId}::uuid, ${editingSessionId}::uuid, ${replacedVersionId} + ) + `; + } } const revision = existing ? existing.revision + 1 : 0; const contentHash = contentHashForCatalogueContent(content); @@ -648,7 +667,7 @@ export async function restoreCatalogueVersion({ ${editingSessionId}::uuid, ${versionId} ) `; - return { revision, content: restored }; + return { revision, content: restored, replacedVersionId }; }); return sql ? work(sql) : withSyncDatabaseClient(work); } diff --git a/apps/web/lib/catalogue/source-review-decisions.ts b/apps/web/lib/catalogue/source-review-decisions.ts index d3c5e94b..af39465e 100644 --- a/apps/web/lib/catalogue/source-review-decisions.ts +++ b/apps/web/lib/catalogue/source-review-decisions.ts @@ -160,10 +160,11 @@ export async function resolveSourceChange({ `; const [event] = await tx` insert into public.catalogue_change_events ( - record_id, draft_revision, event_kind, origin, actor_id, version_id + record_id, draft_revision, event_kind, origin, actor_id, version_id, + sync_change_id ) values ( ${recordId}, ${revision}, 'source_accepted', 'source', - ${userId}::uuid, ${sourceVersionId} + ${userId}::uuid, ${sourceVersionId}, ${changeId} ) returning id `; for (const [position, change] of fieldChanges.entries()) { @@ -186,10 +187,11 @@ export async function resolveSourceChange({ } else { await tx` insert into public.catalogue_change_events ( - record_id, draft_revision, event_kind, origin, actor_id, version_id + record_id, draft_revision, event_kind, origin, actor_id, version_id, + sync_change_id ) values ( ${recordId}, ${revision}, 'source_kept', 'source', ${userId}::uuid, - ${sourceVersionId} + ${sourceVersionId}, ${changeId} ) `; } diff --git a/apps/web/lib/coursemap/admin-catalogue-actions.ts b/apps/web/lib/coursemap/admin-catalogue-actions.ts index 7e5f9d59..20795bd5 100644 --- a/apps/web/lib/coursemap/admin-catalogue-actions.ts +++ b/apps/web/lib/coursemap/admin-catalogue-actions.ts @@ -215,7 +215,9 @@ export async function restoreCatalogueVersionAction({ return { ok: true, revision: result.revision, - message: "Version restored as a draft.", + message: result.replacedVersionId + ? "Version restored as a draft. Your previous draft is in the Changelog." + : "Version restored as a draft.", }; } catch (error) { return draftFailure(error, "The version could not be restored."); diff --git a/apps/web/lib/coursemap/admin-catalogue-changelog.ts b/apps/web/lib/coursemap/admin-catalogue-changelog.ts new file mode 100644 index 00000000..220ef036 --- /dev/null +++ b/apps/web/lib/coursemap/admin-catalogue-changelog.ts @@ -0,0 +1,169 @@ +import "server-only"; +import { + CHANGELOG_PAGE_SIZE, + type CatalogueChangelog, + type ChangelogEntryView, + type ChangelogEvent, + groupChangelogEvents, +} from "@/lib/catalogue/changelog"; +import { createClient } from "@/lib/supabase/server"; + +/** + * The record's timeline, newest first. Raw events are never rewritten; the + * grouping here is presentation, and an entry carries the event ids it covers. + * A group is not stitched across a page boundary, so the oldest entry on a + * page can be part of a session that continues on the next one. + */ +export async function loadCatalogueChangelog({ + recordId, + limit = CHANGELOG_PAGE_SIZE, +}: { + recordId: number; + limit?: number; +}): Promise { + const supabase = await createClient(); + const { data: eventRows, error } = await supabase + .from("catalogue_change_events") + .select( + "id,event_kind,origin,actor_id,editing_session_id,version_id,sync_change_id,created_at", + ) + .eq("record_id", recordId) + .order("created_at", { ascending: false }) + .order("id", { ascending: false }) + .limit(limit + 1); + if (error) throw error; + const hasMore = (eventRows?.length ?? 0) > limit; + const rows = (eventRows ?? []).slice(0, limit); + if (rows.length === 0) return { entries: [], shown: 0, hasMore: false }; + + const eventIds = rows.map((row) => row.id); + const syncChangeIds = rows.flatMap((row) => + row.sync_change_id === null ? [] : [row.sync_change_id], + ); + const actorIds = [ + ...new Set(rows.flatMap((row) => (row.actor_id ? [row.actor_id] : []))), + ]; + const sourceVersionIds = rows.flatMap((row) => + row.event_kind === "source_changed" && row.version_id !== null + ? [row.version_id] + : [], + ); + + const [fieldChanges, decisions, actors, versions, sourceVersions] = + await Promise.all([ + supabase + .from("catalogue_field_changes") + .select("event_id,position,field_path,old_value,new_value") + .in("event_id", eventIds) + .order("position"), + syncChangeIds.length + ? supabase + .from("catalogue_sync_changes") + .select("id,field_path,decision") + .in("id", syncChangeIds) + : { data: [], error: null }, + actorIds.length + ? supabase + .from("admin_users") + .select("user_id,display_name") + .in("user_id", actorIds) + : { data: [], error: null }, + supabase + .from("catalogue_versions") + .select("id") + .eq("record_id", recordId) + .order("id"), + sourceVersionIds.length + ? supabase + .from("catalogue_versions") + .select("id,sync_id") + .in("id", sourceVersionIds) + : { data: [], error: null }, + ]); + if (fieldChanges.error) throw fieldChanges.error; + if (decisions.error) throw decisions.error; + if (actors.error) throw actors.error; + if (versions.error) throw versions.error; + if (sourceVersions.error) throw sourceVersions.error; + + const syncIds = (sourceVersions.data ?? []).flatMap((version) => + version.sync_id ? [version.sync_id] : [], + ); + const reviewRows = syncIds.length + ? await supabase + .from("catalogue_sync_changes") + .select("sync_id,classification") + .in("sync_id", syncIds) + .in("classification", ["source_change", "conflict"]) + : { data: [], error: null }; + if (reviewRows.error) throw reviewRows.error; + + const fieldsByEvent = new Map(); + for (const change of fieldChanges.data ?? []) { + const list = fieldsByEvent.get(change.event_id) ?? []; + list.push({ + fieldPath: change.field_path, + oldValue: change.old_value, + newValue: change.new_value, + }); + fieldsByEvent.set(change.event_id, list); + } + const decisionById = new Map( + (decisions.data ?? []).map((row) => [ + row.id, + { + fieldPath: row.field_path, + decision: row.decision as "use_source" | "keep_local", + }, + ]), + ); + const actorNames = new Map( + (actors.data ?? []).flatMap((actor) => + actor.user_id && actor.display_name + ? [[actor.user_id, actor.display_name] as const] + : [], + ), + ); + const ordinalByVersion = new Map( + (versions.data ?? []).map((version, index) => [version.id, index + 1]), + ); + const syncByVersion = new Map( + (sourceVersions.data ?? []).map((version) => [version.id, version.sync_id]), + ); + const reviewCounts = new Map(); + for (const row of reviewRows.data ?? []) { + const counts = reviewCounts.get(row.sync_id) ?? { total: 0, conflicts: 0 }; + counts.total += 1; + if (row.classification === "conflict") counts.conflicts += 1; + reviewCounts.set(row.sync_id, counts); + } + + const events: ChangelogEvent[] = rows.map((row) => ({ + id: row.id, + eventKind: row.event_kind as ChangelogEvent["eventKind"], + origin: row.origin as ChangelogEvent["origin"], + actorId: row.actor_id, + editingSessionId: row.editing_session_id, + versionId: row.version_id, + syncChangeId: row.sync_change_id, + createdAt: row.created_at, + fields: fieldsByEvent.get(row.id) ?? [], + decision: + row.sync_change_id === null + ? null + : (decisionById.get(row.sync_change_id) ?? null), + })); + + const entries = groupChangelogEvents(events).map((entry) => { + const syncId = entry.versionId ? syncByVersion.get(entry.versionId) : null; + return { + ...entry, + actorName: entry.actorId ? (actorNames.get(entry.actorId) ?? null) : null, + versionOrdinal: entry.versionId + ? (ordinalByVersion.get(entry.versionId) ?? null) + : null, + sourceChanges: syncId ? (reviewCounts.get(syncId) ?? null) : null, + } satisfies ChangelogEntryView; + }); + return { entries, shown: rows.length, hasMore }; +} diff --git a/apps/web/lib/coursemap/admin-catalogue-record.ts b/apps/web/lib/coursemap/admin-catalogue-record.ts index ee7adf68..1c9fa026 100644 --- a/apps/web/lib/coursemap/admin-catalogue-record.ts +++ b/apps/web/lib/coursemap/admin-catalogue-record.ts @@ -63,23 +63,6 @@ export type CatalogueRecord = { unpublishedAt: string | null; unpublishedBy: string | null; }>; - changeEvents: Array<{ - id: number; - eventKind: - | "edit" - | "publish" - | "unpublish" - | "discard" - | "restore" - | "source_draft_created" - | "source_checked" - | "source_changed" - | "sync_failed"; - draftRevision: number | null; - editingSessionId: string | null; - versionId: number | null; - createdAt: string; - }>; syncs: CatalogueSync[]; }; @@ -134,7 +117,6 @@ export async function loadCatalogueRecord({ syncsResult, blockersResult, listingResult, - changeEventsResult, ] = await Promise.all([ supabase .from("catalogue_versions") @@ -163,20 +145,12 @@ export async function loadCatalogueRecord({ .select("title,is_current,last_seen_at") .eq("record_id", itemYear.id) .maybeSingle(), - supabase - .from("catalogue_change_events") - .select( - "id,event_kind,draft_revision,editing_session_id,version_id,created_at", - ) - .eq("record_id", itemYear.id) - .order("created_at", { ascending: false }), ]); if (versionsResult.error) throw versionsResult.error; if (publicationsResult.error) throw publicationsResult.error; if (syncsResult.error) throw syncsResult.error; if (blockersResult.error) throw blockersResult.error; if (listingResult.error) throw listingResult.error; - if (changeEventsResult.error) throw changeEventsResult.error; const currentVersionId = itemYear.published_version_id; const title = await versionTitle(supabase, kind, currentVersionId); @@ -215,15 +189,6 @@ export async function loadCatalogueRecord({ unpublishedAt: publication.unpublished_at, unpublishedBy: publication.unpublished_by, })), - changeEvents: (changeEventsResult.data ?? []).map((event) => ({ - id: event.id, - eventKind: - event.event_kind as CatalogueRecord["changeEvents"][number]["eventKind"], - draftRevision: event.draft_revision, - editingSessionId: event.editing_session_id, - versionId: event.version_id, - createdAt: event.created_at, - })), syncs: (syncsResult.data ?? []).map((sync) => ({ id: sync.id, status: sync.status as CatalogueSync["status"], diff --git a/apps/web/lib/coursemap/catalogue-kinds.ts b/apps/web/lib/coursemap/catalogue-kinds.ts index 11a4a53d..2695215a 100644 --- a/apps/web/lib/coursemap/catalogue-kinds.ts +++ b/apps/web/lib/coursemap/catalogue-kinds.ts @@ -37,6 +37,16 @@ export function adminCatalogueRecordPath( return `${adminCatalogueYearPath(kind, year)}/${encodeURIComponent(code.toLowerCase())}`; } +/** One immutable version, numbered within its record rather than by row id. */ +export function adminCatalogueVersionPath( + kind: CatalogueKind, + year: number, + code: string, + versionOrdinal: number, +) { + return `${adminCatalogueRecordPath(kind, year, code)}/changelog/${versionOrdinal}`; +} + export function publicCatalogueRecordPath( kind: CatalogueKind, year: number, diff --git a/apps/web/tests/catalogue-changelog-database.test.mjs b/apps/web/tests/catalogue-changelog-database.test.mjs new file mode 100644 index 00000000..1ffaf5da --- /dev/null +++ b/apps/web/tests/catalogue-changelog-database.test.mjs @@ -0,0 +1,205 @@ +import assert from "node:assert/strict"; +import { afterAll, beforeAll, test } from "vitest"; + +import { + createCatalogueDraft, + discardCatalogueDraft, + publishCatalogueDraft, + restoreCatalogueVersion, + saveCatalogueDraft, + unpublishCatalogueRecord, +} from "../lib/catalogue/drafts.ts"; +import { readVersionContent } from "../lib/catalogue-import/version-content.ts"; +import { createLocalDatabaseClient } from "../scripts/catalogue/lib/local-database.mjs"; +import { localTestEnvironment } from "../scripts/local/test-environment.mjs"; + +const ADMIN_ID = "99000000-0000-4000-8000-000000000051"; +const CODE = "TSTC9301"; +const YEAR = 2026; +const SESSION_ID = "99000000-0000-4000-8000-000000000052"; +const SECOND_SESSION_ID = "99000000-0000-4000-8000-000000000053"; + +let sql; +let recordId; + +async function removeFixture() { + await sql`delete from public.catalogue_listings where kind = 'course' and code = ${CODE}`; + await sql`alter table public.catalogue_versions disable trigger catalogue_versions_enforce_immutability`; + try { + await sql`delete from public.catalogue_codes where kind = 'course' and code = ${CODE}`; + } finally { + await sql`alter table public.catalogue_versions enable trigger catalogue_versions_enforce_immutability`; + } +} + +async function events() { + return sql` + select id, event_kind, origin, actor_id, editing_session_id, version_id, + draft_revision + from public.catalogue_change_events + where record_id = ${recordId} + order by id + `; +} + +beforeAll(async () => { + Object.assign(process.env, localTestEnvironment(), { + NODE_ENV: "development", + }); + sql = await createLocalDatabaseClient(); + await sql` + insert into auth.users ( + instance_id, id, aud, role, email, raw_app_meta_data, + raw_user_meta_data, created_at, updated_at + ) values ( + '00000000-0000-0000-0000-000000000000', ${ADMIN_ID}, + 'authenticated', 'authenticated', 'changelog-admin@example.test', + '{"provider":"email","providers":["email"]}'::jsonb, '{}'::jsonb, + now(), now() + ) on conflict (id) do nothing + `; + await removeFixture(); + const [year] = + await sql`select id from public.academic_years where year = ${YEAR}`; + const [code] = await sql` + insert into public.catalogue_codes (kind, code) + values ('course', ${CODE}) returning id + `; + const [record] = await sql` + insert into public.catalogue_records (code_id, kind, academic_year_id) + values (${code.id}, 'course', ${year.id}) returning id + `; + recordId = Number(record.id); + await sql` + insert into public.catalogue_listings ( + academic_year_id, kind, code, title, code_id, record_id, is_current, + first_seen_at, last_seen_at + ) values ( + ${year.id}, 'course', ${CODE}, 'Changelog Systems', ${code.id}, + ${record.id}, true, now(), now() + ) + `; +}); + +afterAll(async () => { + if (!sql) return; + await removeFixture(); + await sql`delete from auth.users where id = ${ADMIN_ID}`; + await sql.end({ timeout: 5 }); +}); + +test("every operation leaves one attributable audit event behind", async () => { + const draft = await createCatalogueDraft({ recordId, userId: ADMIN_ID, sql }); + let revision = draft.revision; + let content = draft.content; + for (const description of ["First pass.", "Second pass.", "Third pass."]) { + const next = structuredClone(content); + next.course.details.description = description; + const saved = await saveCatalogueDraft({ + recordId, + expectedRevision: revision, + content: next, + userId: ADMIN_ID, + editingSessionId: SESSION_ID, + sql, + }); + revision = saved.draft.revision; + content = saved.draft.content; + } + + const autosaves = (await events()).filter( + (event) => event.event_kind === "edit", + ); + assert.equal(autosaves.length, 3); + assert.equal( + new Set(autosaves.map((event) => event.editing_session_id)).size, + 1, + ); + assert.deepEqual( + autosaves.map((event) => Number(event.draft_revision)), + [1, 2, 3], + ); + assert.equal(autosaves[0].actor_id, ADMIN_ID); + + const published = await publishCatalogueDraft({ + recordId, + expectedRevision: revision, + userId: ADMIN_ID, + editingSessionId: SESSION_ID, + sql, + }); + await unpublishCatalogueRecord({ + recordId, + userId: ADMIN_ID, + editingSessionId: SESSION_ID, + sql, + }); + const lifecycle = (await events()).filter((event) => + ["publish", "unpublish"].includes(event.event_kind), + ); + assert.deepEqual( + lifecycle.map((event) => [event.event_kind, Number(event.version_id)]), + [ + ["publish", published.versionId], + ["unpublish", published.versionId], + ], + ); +}); + +test("restoring a version keeps the draft it replaces", async () => { + const draft = await createCatalogueDraft({ recordId, userId: ADMIN_ID, sql }); + const inProgress = structuredClone(draft.content); + inProgress.course.details.description = "Work in progress worth keeping."; + const saved = await saveCatalogueDraft({ + recordId, + expectedRevision: draft.revision, + content: inProgress, + userId: ADMIN_ID, + editingSessionId: SECOND_SESSION_ID, + sql, + }); + + const [oldest] = await sql` + select id from public.catalogue_versions where record_id = ${recordId} + order by id limit 1 + `; + const restored = await restoreCatalogueVersion({ + recordId, + versionId: Number(oldest.id), + expectedRevision: saved.draft.revision, + replaceExistingDraft: true, + userId: ADMIN_ID, + editingSessionId: SECOND_SESSION_ID, + sql, + }); + assert.ok(restored.replacedVersionId); + + const replaced = await readVersionContent(sql, restored.replacedVersionId); + assert.equal( + replaced.course.details.description, + "Work in progress worth keeping.", + ); + const [checkpoint] = (await events()).filter( + (event) => + event.event_kind === "discard" && + Number(event.version_id) === restored.replacedVersionId, + ); + assert.ok(checkpoint, "the replaced draft is offered back in the changelog"); + + const [restoreEvent] = (await events()).filter( + (event) => event.event_kind === "restore", + ); + assert.equal(Number(restoreEvent.version_id), Number(oldest.id)); + const [oldestAfter] = await sql` + select content_hash, sealed_at from public.catalogue_versions where id = ${oldest.id} + `; + assert.ok(oldestAfter.sealed_at, "the restored version is untouched history"); + + await discardCatalogueDraft({ + recordId, + expectedRevision: restored.revision, + userId: ADMIN_ID, + editingSessionId: SECOND_SESSION_ID, + sql, + }); +}); diff --git a/apps/web/tests/catalogue-changelog-timeline.test.tsx b/apps/web/tests/catalogue-changelog-timeline.test.tsx new file mode 100644 index 00000000..42ca1b9f --- /dev/null +++ b/apps/web/tests/catalogue-changelog-timeline.test.tsx @@ -0,0 +1,162 @@ +import { render, screen, within } from "@testing-library/react"; +import { expect, test } from "vitest"; + +import type { + CatalogueChangelog, + ChangelogEntryView, +} from "@/lib/catalogue/changelog"; +import { ChangelogTimeline } from "@/ui/admin/catalogue/changelog/changelog-timeline"; + +// Local dates, so the day grouping under test does not depend on the runner's +// time zone. +const TODAY = new Date(2026, 8, 22, 12, 0); +const TODAY_AT = new Date(2026, 8, 22, 10, 42).toISOString(); +const TODAY_STARTED_AT = new Date(2026, 8, 22, 10, 38).toISOString(); +const YESTERDAY_AT = new Date(2026, 8, 21, 15, 10).toISOString(); +const PATH = "/admin/courses/2027/comp2700"; + +function entry( + overrides: Partial = {}, +): ChangelogEntryView { + return { + id: "event-1", + kind: "edit", + at: TODAY_AT, + startedAt: TODAY_STARTED_AT, + actorId: "11111111-1111-4111-8111-111111111111", + origin: "manual", + eventIds: [5, 4, 3, 2, 1], + versionId: null, + fields: [ + { + fieldPath: "course.details.description", + label: "Description", + oldValue: "Original", + newValue: "Final", + }, + ], + usedFromSource: [], + keptLocal: [], + actorName: "Harry", + versionOrdinal: null, + sourceChanges: null, + ...overrides, + }; +} + +function renderTimeline( + changelog: Partial, + ordinals = new Map(), +) { + return render( + , + ); +} + +test("an empty changelog says what will fill it", () => { + renderTimeline({}); + expect(screen.getByText("Nothing has happened yet")).toBeTruthy(); +}); + +test("an editing session reads as one entry with its autosave count", () => { + renderTimeline({ entries: [entry()], shown: 1 }); + expect(screen.getByText("Harry edited Description")).toBeTruthy(); + expect(screen.getByText("5 autosaves over 4 minutes")).toBeTruthy(); + const details = screen.getByText("View change").closest("details"); + expect(details?.open).toBe(false); + expect(within(details!).getByText("Original")).toBeTruthy(); + expect(within(details!).getByText("Final")).toBeTruthy(); +}); + +test("entries are grouped under the day they happened", () => { + renderTimeline({ + entries: [ + entry(), + entry({ + id: "event-2", + at: YESTERDAY_AT, + startedAt: YESTERDAY_AT, + kind: "publish", + eventIds: [2], + versionId: 14, + versionOrdinal: 14, + fields: [], + }), + ], + shown: 2, + }); + expect(screen.getByText("Today")).toBeTruthy(); + expect(screen.getByText("Yesterday")).toBeTruthy(); + expect(screen.getByText("Published version 14")).toBeTruthy(); +}); + +test("a version entry links to the version rather than printing a row id", () => { + renderTimeline( + { + entries: [ + entry({ + kind: "publish", + versionId: 87, + versionOrdinal: 3, + fields: [], + eventIds: [9], + }), + ], + shown: 1, + }, + new Map([[87, 3]]), + ); + const link = screen.getByRole("link", { name: "View version 3" }); + expect(link.getAttribute("href")).toBe(`${PATH}/changelog/3`); + expect(screen.queryByText(/87/)).toBeNull(); +}); + +test("a review entry names what was used and what was kept", () => { + renderTimeline({ + entries: [ + entry({ + kind: "source_accepted", + origin: "source", + actorName: "Harry", + eventIds: [7, 6], + fields: [], + usedFromSource: ["Offerings", "Learning outcomes"], + keptLocal: ["Description"], + }), + ], + shown: 1, + }); + expect(screen.getByText("ANU changes reviewed")).toBeTruthy(); + expect(screen.getByText("Offerings, Learning outcomes")).toBeTruthy(); + expect(screen.getByText("Description")).toBeTruthy(); +}); + +test("a sync entry counts the changes it found", () => { + renderTimeline({ + entries: [ + entry({ + kind: "source_changed", + origin: "source", + actorName: null, + eventIds: [3], + fields: [], + sourceChanges: { total: 3, conflicts: 1 }, + }), + ], + shown: 1, + }); + expect(screen.getByText("ANU changes found")).toBeTruthy(); + expect(screen.getByText("3 changes to review, 1 conflict")).toBeTruthy(); + expect(screen.getByText("ANU sync")).toBeTruthy(); +}); + +test("a long history offers the rest of itself", () => { + renderTimeline({ entries: [entry()], shown: 40, hasMore: true }); + const link = screen.getByRole("link", { name: "Show earlier history" }); + expect(link.getAttribute("href")).toBe(`${PATH}/changelog?events=80`); +}); diff --git a/apps/web/tests/catalogue-changelog.test.ts b/apps/web/tests/catalogue-changelog.test.ts new file mode 100644 index 00000000..2c7e34e7 --- /dev/null +++ b/apps/web/tests/catalogue-changelog.test.ts @@ -0,0 +1,201 @@ +import assert from "node:assert/strict"; +import { test } from "vitest"; + +import { + type ChangelogEvent, + editingSessionSummary, + groupChangelogEvents, +} from "../lib/catalogue/changelog.ts"; + +const SESSION = "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa"; +const OTHER_SESSION = "bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb"; +const HARRY = "11111111-1111-4111-8111-111111111111"; +const SAM = "22222222-2222-4222-8222-222222222222"; + +function event( + overrides: Partial & { id: number }, +): ChangelogEvent { + return { + eventKind: "edit", + origin: "manual", + actorId: HARRY, + editingSessionId: SESSION, + versionId: null, + syncChangeId: null, + createdAt: "2026-09-21T10:00:00.000Z", + fields: [], + decision: null, + ...overrides, + }; +} + +function edit( + id: number, + minute: number, + from: string, + to: string, + extra?: Partial, +) { + return event({ + id, + createdAt: `2026-09-21T10:${String(minute).padStart(2, "0")}:00.000Z`, + fields: [ + { fieldPath: "course.details.description", oldValue: from, newValue: to }, + ], + ...extra, + }); +} + +test("one editing session reads as one entry from its first to its last value", () => { + // Newest first, as the loader supplies them. + const entries = groupChangelogEvents([ + edit(5, 4, "Fourth", "Final"), + edit(4, 3, "Third", "Fourth"), + edit(3, 2, "Second", "Third"), + edit(2, 1, "First", "Second"), + edit(1, 0, "Original", "First"), + ]); + assert.equal(entries.length, 1); + assert.deepEqual(entries[0]!.eventIds, [5, 4, 3, 2, 1]); + assert.deepEqual(entries[0]!.fields, [ + { + fieldPath: "course.details.description", + label: "Description", + oldValue: "Original", + newValue: "Final", + }, + ]); + assert.equal( + editingSessionSummary(entries[0]!), + "5 autosaves over 4 minutes", + ); +}); + +test("a value typed and taken back again is not a change", () => { + const entries = groupChangelogEvents([ + edit(2, 1, "Tried something", "Original"), + edit(1, 0, "Original", "Tried something"), + ]); + assert.deepEqual(entries[0]!.fields, []); +}); + +test("separate sessions and separate people stay separate", () => { + const entries = groupChangelogEvents([ + edit(3, 9, "B", "C", { editingSessionId: OTHER_SESSION }), + edit(2, 5, "A", "B", { actorId: SAM }), + edit(1, 0, "Start", "A"), + ]); + assert.equal(entries.length, 3); + assert.equal(editingSessionSummary(entries[0]!), null); +}); + +test("decisions taken in one sitting read as one review", () => { + const entries = groupChangelogEvents([ + event({ + id: 3, + eventKind: "source_kept", + origin: "source", + editingSessionId: null, + createdAt: "2026-09-21T10:12:00.000Z", + syncChangeId: 12, + decision: { + fieldPath: "course.details.description", + decision: "keep_local", + }, + }), + event({ + id: 2, + eventKind: "source_accepted", + origin: "source", + editingSessionId: null, + createdAt: "2026-09-21T10:10:00.000Z", + syncChangeId: 11, + decision: { + fieldPath: "course.learningOutcomes", + decision: "use_source", + }, + fields: [ + { fieldPath: "course.learningOutcomes", oldValue: [], newValue: [{}] }, + ], + }), + event({ + id: 1, + eventKind: "source_accepted", + origin: "source", + editingSessionId: null, + createdAt: "2026-09-21T09:00:00.000Z", + syncChangeId: 10, + decision: { fieldPath: "course.fees", decision: "use_source" }, + }), + ]); + assert.equal(entries.length, 2); + assert.deepEqual(entries[0]!.usedFromSource, ["Learning outcomes"]); + assert.deepEqual(entries[0]!.keptLocal, ["Description"]); + assert.deepEqual(entries[1]!.usedFromSource, ["Fees"]); +}); + +test("quiet ANU checks collapse instead of filling the timeline", () => { + const entries = groupChangelogEvents([ + event({ + id: 3, + eventKind: "source_checked", + origin: "source", + actorId: null, + editingSessionId: null, + }), + event({ + id: 2, + eventKind: "source_checked", + origin: "source", + actorId: null, + editingSessionId: null, + }), + event({ + id: 1, + eventKind: "source_checked", + origin: "source", + actorId: null, + editingSessionId: null, + }), + ]); + assert.equal(entries.length, 1); + assert.equal(entries[0]!.eventIds.length, 3); +}); + +test("publication and lifecycle events always stand alone", () => { + const entries = groupChangelogEvents([ + event({ + id: 4, + eventKind: "restore", + versionId: 40, + editingSessionId: null, + }), + event({ + id: 3, + eventKind: "discard", + versionId: 30, + editingSessionId: null, + }), + event({ + id: 2, + eventKind: "unpublish", + versionId: 20, + editingSessionId: null, + }), + event({ + id: 1, + eventKind: "publish", + versionId: 10, + editingSessionId: null, + }), + ]); + assert.deepEqual( + entries.map((entry) => [entry.kind, entry.versionId]), + [ + ["restore", 40], + ["discard", 30], + ["unpublish", 20], + ["publish", 10], + ], + ); +}); diff --git a/apps/web/tests/catalogue-routes.test.ts b/apps/web/tests/catalogue-routes.test.ts index ca95ec46..5605be37 100644 --- a/apps/web/tests/catalogue-routes.test.ts +++ b/apps/web/tests/catalogue-routes.test.ts @@ -1,6 +1,7 @@ import { expect, test } from "vitest"; import { adminCatalogueRecordPath, + adminCatalogueVersionPath, adminCatalogueYearPath, publicCatalogueRecordPath, } from "@/lib/coursemap/catalogue-kinds"; @@ -20,3 +21,12 @@ test("catalogue paths put the year before a lowercase human-readable code", () = "/specialisations/2027/data-sci", ); }); + +test("a version is addressed by its place in the record, not by a row id", () => { + expect(adminCatalogueVersionPath("course", 2027, "COMP2700", 3)).toBe( + "/admin/courses/2027/comp2700/changelog/3", + ); + expect(adminCatalogueVersionPath("programme", 2027, "BCOMP", 12)).toBe( + "/admin/programmes/2027/bcomp/changelog/12", + ); +}); diff --git a/apps/web/tests/catalogue-source-review-database.test.mjs b/apps/web/tests/catalogue-source-review-database.test.mjs index 74676e35..38f3d9c8 100644 --- a/apps/web/tests/catalogue-source-review-database.test.mjs +++ b/apps/web/tests/catalogue-source-review-database.test.mjs @@ -225,11 +225,17 @@ test("keeping a local value survives an unchanged source and reopens when ANU mo ); assert.equal(Number(afterKeep.revision), 1); const [keptEvent] = await sql` - select event_kind, origin, actor_id from public.catalogue_change_events + select event_kind, origin, actor_id, sync_change_id + from public.catalogue_change_events where record_id = ${recordId} and event_kind = 'source_kept' `; assert.equal(keptEvent.origin, "source"); assert.equal(keptEvent.actor_id, ADMIN_ID); + assert.equal( + Number(keptEvent.sync_change_id), + opened.conflicts[0].id, + "the changelog can name the field that was kept", + ); // ANU repeats the same wording. The baseline advanced, so this is an // override rather than a question the administrator already answered. @@ -355,11 +361,12 @@ test("using ANU writes one path, keeps unrelated edits and moves only its proven ); const [acceptedEvent] = await sql` - select events.id, events.event_kind, events.draft_revision + select events.id, events.event_kind, events.draft_revision, events.sync_change_id from public.catalogue_change_events as events where events.record_id = ${recordId} and events.event_kind = 'source_accepted' `; assert.equal(Number(acceptedEvent.draft_revision), 3); + assert.equal(Number(acceptedEvent.sync_change_id), unaffected.incoming[0].id); const fieldChanges = await sql` select field_path, new_value from public.catalogue_field_changes where event_id = ${acceptedEvent.id} diff --git a/apps/web/types/database.ts b/apps/web/types/database.ts index fa928359..e3909b8e 100644 --- a/apps/web/types/database.ts +++ b/apps/web/types/database.ts @@ -810,6 +810,7 @@ export type Database = { id: number origin: string record_id: number + sync_change_id: number | null version_id: number | null } Insert: { @@ -821,6 +822,7 @@ export type Database = { id?: never origin: string record_id: number + sync_change_id?: number | null version_id?: number | null } Update: { @@ -832,6 +834,7 @@ export type Database = { id?: never origin?: string record_id?: number + sync_change_id?: number | null version_id?: number | null } Relationships: [ @@ -849,6 +852,13 @@ export type Database = { referencedRelation: "published_course_summaries" referencedColumns: ["record_id"] }, + { + foreignKeyName: "catalogue_change_events_sync_change_id_fkey" + columns: ["sync_change_id"] + isOneToOne: false + referencedRelation: "catalogue_sync_changes" + referencedColumns: ["id"] + }, { foreignKeyName: "catalogue_change_events_version_fkey" columns: ["version_id", "record_id"] diff --git a/apps/web/ui/admin/catalogue/catalogue-route-pages.tsx b/apps/web/ui/admin/catalogue/catalogue-route-pages.tsx index 5e3cbe23..58e9b520 100644 --- a/apps/web/ui/admin/catalogue/catalogue-route-pages.tsx +++ b/apps/web/ui/admin/catalogue/catalogue-route-pages.tsx @@ -2,9 +2,14 @@ import { notFound, redirect } from "next/navigation"; import type { CatalogueKind } from "@/lib/coursemap/catalogue-kinds"; import { adminCatalogueRecordPath } from "@/lib/coursemap/catalogue-kinds"; import { CatalogueDirectoryPage, type SearchParams } from "./catalogue-pages"; +import { CatalogueVersionPage } from "./changelog/version-page"; import { CatalogueRecordPage } from "./record-page"; import type { RecordSection } from "./record-tabs"; +function first(value: string | string[] | undefined) { + return Array.isArray(value) ? value[0] : value; +} + const RECORD_SECTIONS = new Set([ "content", "student-view", @@ -42,11 +47,13 @@ export async function CatalogueRecordRoute({ year, code, section, + searchParams, }: { kind: CatalogueKind; year: string; code: string; section?: string[]; + searchParams: SearchParams; }) { const academicYear = Number(year); if ( @@ -60,15 +67,30 @@ export async function CatalogueRecordRoute({ redirect( section?.length ? `${canonicalPath}/${section.join("/")}` : canonicalPath, ); - if ((section?.length ?? 0) > 1) notFound(); const requested = section?.[0] ?? "content"; if (!RECORD_SECTIONS.has(requested as RecordSection)) notFound(); + const params = await searchParams; + if (requested === "changelog" && section?.length === 2) { + const ordinal = Number(section[1]); + if (!Number.isInteger(ordinal) || ordinal < 1) notFound(); + return ( + + ); + } + if ((section?.length ?? 0) > 1) notFound(); return ( ); } diff --git a/apps/web/ui/admin/catalogue/changelog/changelog-entry.tsx b/apps/web/ui/admin/catalogue/changelog/changelog-entry.tsx new file mode 100644 index 00000000..191b8e62 --- /dev/null +++ b/apps/web/ui/admin/catalogue/changelog/changelog-entry.tsx @@ -0,0 +1,165 @@ +import { + FileClock, + Pencil, + RefreshCw, + RotateCcw, + Send, + TriangleAlert, + Trash2, + Undo2, +} from "lucide-react"; +import Link from "next/link"; +import { + type ChangelogEntryView, + editingSessionSummary, +} from "@/lib/catalogue/changelog"; +import { FieldChangeList } from "../field-change-list"; + +function formatTime(value: string) { + return new Intl.DateTimeFormat("en-AU", { timeStyle: "short" }).format( + new Date(value), + ); +} + +function EntryIcon({ kind }: { kind: ChangelogEntryView["kind"] }) { + const shared = { + className: "mt-0.5 size-4 shrink-0 text-muted-foreground", + "aria-hidden": true, + } as const; + if (kind === "publish") return ; + if (kind === "unpublish") return ; + if (kind === "edit") return ; + if (kind === "discard") return ; + if (kind === "restore") return ; + if (kind === "sync_failed") return ; + if (kind === "source_checked" || kind === "source_changed") + return ; + return ; +} + +function editTitle(entry: ChangelogEntryView, actor: string) { + if (entry.fields.length === 1) + return `${actor} edited ${entry.fields[0]!.label}`; + if (entry.fields.length === 0) return `${actor} edited the draft`; + return `${actor} edited ${entry.fields.length} fields`; +} + +function entryTitle(entry: ChangelogEntryView) { + const actor = entry.actorName ?? "Someone"; + const version = entry.versionOrdinal + ? `version ${entry.versionOrdinal}` + : "a version"; + switch (entry.kind) { + case "edit": + return editTitle(entry, actor); + case "publish": + return `Published ${version}`; + case "unpublish": + return "Unpublished"; + case "discard": + return "Draft discarded"; + case "restore": + return `Restored ${version} as a draft`; + case "source_draft_created": + return "ANU content started the draft"; + case "source_checked": + return "Checked ANU"; + case "source_changed": + return "ANU changes found"; + case "sync_failed": + return "ANU sync failed"; + default: + return "ANU changes reviewed"; + } +} + +function entryDetail(entry: ChangelogEntryView) { + if (entry.kind === "edit") return editingSessionSummary(entry); + if (entry.kind === "source_checked") { + return entry.eventIds.length > 1 + ? `Checked ${entry.eventIds.length} times. No changes found.` + : "No changes found."; + } + if (entry.kind === "source_changed" && entry.sourceChanges) { + const { total, conflicts } = entry.sourceChanges; + const changes = `${total} change${total === 1 ? "" : "s"} to review`; + return conflicts > 0 + ? `${changes}, ${conflicts} conflict${conflicts === 1 ? "" : "s"}` + : changes; + } + if (entry.kind === "discard") + return entry.versionOrdinal + ? `Kept as version ${entry.versionOrdinal}, which can be restored.` + : null; + return null; +} + +/** + * One thing that happened, in the vocabulary an administrator works in. The + * raw events behind an entry stay in the database; this names what they add up + * to. + */ +export function ChangelogEntry({ + entry, + versionHref, +}: { + entry: ChangelogEntryView; + versionHref: string | null; +}) { + const detail = entryDetail(entry); + const actor = + entry.kind === "edit" + ? null + : (entry.actorName ?? (entry.origin === "source" ? "ANU sync" : null)); + return ( +
  • + +
    +
    +

    {entryTitle(entry)}

    + +
    + {actor ? ( +

    {actor}

    + ) : null} + {detail ? ( +

    {detail}

    + ) : null} + {entry.usedFromSource.length > 0 ? ( +

    + Used ANU: + {entry.usedFromSource.join(", ")} +

    + ) : null} + {entry.keptLocal.length > 0 ? ( +

    + Kept current: + {entry.keptLocal.join(", ")} +

    + ) : null} + {entry.fields.length > 0 ? ( +
    + + {entry.fields.length === 1 + ? "View change" + : `View ${entry.fields.length} changes`} + +
    + +
    +
    + ) : null} + {versionHref ? ( + + View version {entry.versionOrdinal} + + ) : null} +
    +
    • + ); +} diff --git a/apps/web/ui/admin/catalogue/changelog/changelog-timeline.tsx b/apps/web/ui/admin/catalogue/changelog/changelog-timeline.tsx new file mode 100644 index 00000000..d0d1bcab --- /dev/null +++ b/apps/web/ui/admin/catalogue/changelog/changelog-timeline.tsx @@ -0,0 +1,94 @@ +import Link from "next/link"; +import { + CHANGELOG_PAGE_SIZE, + type CatalogueChangelog, +} from "@/lib/catalogue/changelog"; +import { CatalogueEmpty } from "@/ui/admin/catalogue-table/catalogue-empty"; +import { ChangelogEntry } from "./changelog-entry"; + +function dayKey(value: string) { + return new Date(value).toDateString(); +} + +function dayLabel(value: string, today: Date) { + const day = new Date(value); + const yesterday = new Date(today); + yesterday.setDate(today.getDate() - 1); + if (day.toDateString() === today.toDateString()) return "Today"; + if (day.toDateString() === yesterday.toDateString()) return "Yesterday"; + return new Intl.DateTimeFormat("en-AU", { dateStyle: "long" }).format(day); +} + +/** + * The record's history as one story, newest first, grouped by day. Versions + * that can be opened link out to their own page rather than being rendered as + * a database row here. + */ +export function ChangelogTimeline({ + changelog, + path, + versionOrdinals, + today = new Date(), +}: { + changelog: CatalogueChangelog; + path: string; + versionOrdinals: ReadonlyMap; + today?: Date; +}) { + if (changelog.entries.length === 0) { + return ( + + ); + } + + const days: Array<{ + key: string; + label: string; + entries: typeof changelog.entries; + }> = []; + for (const entry of changelog.entries) { + const key = dayKey(entry.at); + const current = days[days.length - 1]; + if (current?.key === key) current.entries.push(entry); + else days.push({ key, label: dayLabel(entry.at, today), entries: [entry] }); + } + + return ( +
    + {days.map((day) => ( +
    +

    + {day.label} +

    +
      + {day.entries.map((entry) => ( + + ))} +
    +
    + ))} + {changelog.hasMore ? ( + + Show earlier history + + ) : null} +
    + ); +} diff --git a/apps/web/ui/admin/catalogue/changelog/restore-version-button.tsx b/apps/web/ui/admin/catalogue/changelog/restore-version-button.tsx new file mode 100644 index 00000000..1a0fb1e5 --- /dev/null +++ b/apps/web/ui/admin/catalogue/changelog/restore-version-button.tsx @@ -0,0 +1,82 @@ +"use client"; + +import { Button } from "@coursemap/ui/primitives/button"; +import { useRouter } from "next/navigation"; +import { useState, useTransition } from "react"; +import { toast } from "sonner"; +import { restoreCatalogueVersionAction } from "@/lib/coursemap/admin-catalogue-actions"; +import { ConfirmDialog } from "@/ui/common/confirm-dialog"; + +/** + * Restores historical content as the working draft. The version itself never + * changes, which is why this is not called a revert, and a draft it would + * replace becomes a version of its own first. + */ +export function RestoreVersionButton({ + recordId, + versionId, + draftRevision, + path, + label = "Restore as draft", +}: { + recordId: number; + versionId: number; + /** The current draft's revision, or null when the record has no draft. */ + draftRevision: number | null; + path: string; + label?: string; +}) { + const router = useRouter(); + const [isPending, startTransition] = useTransition(); + const [open, setOpen] = useState(false); + + function restore() { + startTransition(async () => { + const result = await restoreCatalogueVersionAction({ + recordId, + versionId, + expectedRevision: draftRevision, + replaceExistingDraft: draftRevision !== null, + editingSessionId: crypto.randomUUID(), + path, + }); + if (!result.ok) { + toast.error(result.error); + return; + } + toast.success(result.message ?? "Version restored as a draft."); + setOpen(false); + router.push(path); + router.refresh(); + }); + } + + if (draftRevision === null) { + return ( + + ); + } + + return ( + + {label} + + } + /> + ); +} diff --git a/apps/web/ui/admin/catalogue/changelog/version-page.tsx b/apps/web/ui/admin/catalogue/changelog/version-page.tsx new file mode 100644 index 00000000..b4ce3db0 --- /dev/null +++ b/apps/web/ui/admin/catalogue/changelog/version-page.tsx @@ -0,0 +1,220 @@ +import { ArrowLeft } from "lucide-react"; +import Link from "next/link"; +import { notFound } from "next/navigation"; +import { Badge } from "@coursemap/ui/components/badge"; +import { Button } from "@coursemap/ui/primitives/button"; +import { + canManageCatalogueSources, + canWriteCatalogue, +} from "@/lib/auth/viewer"; +import { diffSnapshotWrites } from "@/lib/catalogue-import/changes"; +import { loadCatalogueDraft } from "@/lib/catalogue/drafts"; +import { + loadCatalogueRecord, + loadVersionCoursePreview, + loadVersionWrite, +} from "@/lib/coursemap/admin-catalogue-record"; +import { + CATALOGUE_KIND_LABELS, + type CatalogueKind, + adminCatalogueRecordPath, + fieldLabel, +} from "@/lib/coursemap/catalogue-kinds"; +import { AccessDeniedError } from "@/ui/errors/access-denied-error"; +import { AppShell } from "@/ui/shell"; +import { FieldChangeList } from "../field-change-list"; +import { CoursePreview, StructurePreview } from "../version-preview"; +import { RestoreVersionButton } from "./restore-version-button"; + +function formatDate(value: string) { + return new Intl.DateTimeFormat("en-AU", { dateStyle: "long" }).format( + new Date(value), + ); +} + +/** + * One immutable version, as content rather than as a database row. Comparison + * targets are addressed in the URL so a specific comparison can be linked to. + */ +export async function CatalogueVersionPage({ + kind, + code, + academicYear, + versionOrdinal, + compare, +}: { + kind: CatalogueKind; + code: string; + academicYear: number; + versionOrdinal: number; + compare: string | null; +}) { + const [canManageImports, canWrite] = await Promise.all([ + canManageCatalogueSources(), + canWriteCatalogue(), + ]); + if (!canManageImports && !canWrite) return ; + const record = await loadCatalogueRecord({ kind, code, academicYear }); + if (!record) notFound(); + + const ordered = [...record.versions].sort( + (left, right) => left.id - right.id, + ); + const version = ordered[versionOrdinal - 1]; + if (!version) notFound(); + const labels = CATALOGUE_KIND_LABELS[kind]; + const path = adminCatalogueRecordPath(kind, academicYear, record.code); + const changelogPath = `${path}/changelog`; + const versionPath = `${changelogPath}/${versionOrdinal}`; + + const publication = record.publications.find( + (entry) => entry.versionId === version.id, + ); + const [content, coursePreview, draft] = await Promise.all([ + loadVersionWrite(version.id), + kind === "course" ? loadVersionCoursePreview(version.id) : null, + loadCatalogueDraft(record.recordId), + ]); + if (!content) notFound(); + + const comparisons = [ + ...(draft ? [{ key: "draft", label: "Current draft" }] : []), + ...(record.publishedVersionId + ? [{ key: "published", label: "Published" }] + : []), + ...(versionOrdinal > 1 + ? [ + { + key: String(versionOrdinal - 1), + label: `Version ${versionOrdinal - 1}`, + }, + ] + : []), + ].filter((option) => option.key !== String(versionOrdinal)); + + const comparedTo = + compare && comparisons.some((option) => option.key === compare) + ? compare + : null; + const comparedVersionId = + comparedTo === null || comparedTo === "draft" + ? null + : comparedTo === "published" + ? record.publishedVersionId + : (ordered[Number(comparedTo) - 1]?.id ?? null); + const comparedContent = + comparedTo === "draft" + ? (draft?.content ?? null) + : comparedVersionId + ? await loadVersionWrite(comparedVersionId) + : null; + const differences = comparedContent + ? diffSnapshotWrites(content, comparedContent) + : []; + + return ( + +
    + +
    +     + ); +} diff --git a/apps/web/ui/admin/catalogue/changes/unpublished-changes.tsx b/apps/web/ui/admin/catalogue/changes/unpublished-changes.tsx index 6f18a8d6..cb220d0f 100644 --- a/apps/web/ui/admin/catalogue/changes/unpublished-changes.tsx +++ b/apps/web/ui/admin/catalogue/changes/unpublished-changes.tsx @@ -1,16 +1,6 @@ import type { SnapshotChange } from "@/lib/catalogue-import/changes"; import { fieldLabel } from "@/lib/coursemap/catalogue-kinds"; - -function describe(value: unknown) { - if (value === null || value === undefined) return "Not set"; - if (Array.isArray(value)) - return `${value.length} item${value.length === 1 ? "" : "s"}`; - if (typeof value === "object") return "A value"; - if (typeof value === "boolean") return value ? "Yes" : "No"; - const text = String(value).trim(); - if (text === "") return "Not set"; - return text.length > 80 ? `${text.slice(0, 77)}…` : text; -} +import { FieldChangeList } from "../field-change-list"; /** Saved draft work that students will not see until the record is published. */ export function UnpublishedChanges({ @@ -29,23 +19,13 @@ export function UnpublishedChanges({ ); } return ( -
      - {changes.map((change) => ( -
    • -

      {fieldLabel(change.fieldPath)}

      -

      - {describe(change.oldValue)} - - becomes - - {describe(change.newValue)} - -

      -
      • - ))} -
    + ({ + fieldPath: change.fieldPath, + label: fieldLabel(change.fieldPath), + oldValue: change.oldValue, + newValue: change.newValue, + }))} + /> ); } diff --git a/apps/web/ui/admin/catalogue/field-change-list.tsx b/apps/web/ui/admin/catalogue/field-change-list.tsx new file mode 100644 index 00000000..16ae82af --- /dev/null +++ b/apps/web/ui/admin/catalogue/field-change-list.tsx @@ -0,0 +1,54 @@ +export type FieldChangeItem = { + fieldPath: string; + label: string; + oldValue: unknown; + newValue: unknown; +}; + +/** A short, readable stand-in for a stored value of any shape. */ +export function describeFieldValue(value: unknown) { + if (value === null || value === undefined) return "Not set"; + if (Array.isArray(value)) + return `${value.length} item${value.length === 1 ? "" : "s"}`; + if (typeof value === "object") return "A value"; + if (typeof value === "boolean") return value ? "Yes" : "No"; + const text = String(value).trim(); + if (text === "") return "Not set"; + return text.length > 120 ? `${text.slice(0, 117)}…` : text; +} + +/** What changed, one field to a line, oldest value first. */ +export function FieldChangeList({ + changes, + bordered = true, +}: { + changes: readonly FieldChangeItem[]; + bordered?: boolean; +}) { + return ( +
      + {changes.map((change) => ( +
    • +

      {change.label}

      +

      + + {describeFieldValue(change.oldValue)} + + + becomes + + {describeFieldValue(change.newValue)} + +

      +
      • + ))} +
    + ); +} diff --git a/apps/web/ui/admin/catalogue/record-history.tsx b/apps/web/ui/admin/catalogue/record-history.tsx deleted file mode 100644 index 16a94cbe..00000000 --- a/apps/web/ui/admin/catalogue/record-history.tsx +++ /dev/null @@ -1,136 +0,0 @@ -import { Badge } from "@coursemap/ui/components/badge"; -import { - FileClock, - Pencil, - RotateCcw, - Send, - Trash2, - Undo2, -} from "lucide-react"; -import type { CatalogueRecord } from "@/lib/coursemap/admin-catalogue-record"; - -function formatDateTime(value: string) { - return new Intl.DateTimeFormat("en-AU", { - dateStyle: "medium", - timeStyle: "short", - }).format(new Date(value)); -} - -export function RecordHistory({ record }: { record: CatalogueRecord }) { - const events = [ - ...record.versions.map((version) => ({ - id: `version-${version.id}`, - at: version.createdAt, - kind: "version" as const, - title: `Version ${version.id} created`, - detail: - version.origin === "source" ? "Synced from ANU" : "Created manually", - })), - ...record.publications.flatMap((publication, index) => [ - { - id: `published-${index}`, - at: publication.publishedAt, - kind: "published" as const, - title: `Version ${publication.versionId} published`, - detail: null, - }, - ...(publication.unpublishedAt - ? [ - { - id: `unpublished-${index}`, - at: publication.unpublishedAt, - kind: "unpublished" as const, - title: `Version ${publication.versionId} unpublished`, - detail: null, - }, - ] - : []), - ]), - ...record.changeEvents - .filter( - (event) => - event.eventKind !== "publish" && event.eventKind !== "unpublish", - ) - .map((event) => ({ - id: `change-${event.id}`, - at: event.createdAt, - kind: event.eventKind, - title: - event.eventKind === "edit" - ? "Draft updated" - : event.eventKind === "discard" - ? "Draft discarded" - : event.eventKind === "restore" - ? "Version restored as draft" - : event.eventKind === "source_draft_created" - ? "ANU source populated the draft" - : event.eventKind === "source_checked" - ? "ANU source checked" - : event.eventKind === "source_changed" - ? "ANU source changes detected" - : "ANU sync failed", - detail: - event.eventKind === "discard" && event.versionId - ? `Restorable checkpoint version ${event.versionId}` - : event.eventKind === "restore" && event.versionId - ? `Restored from version ${event.versionId}` - : null, - })), - ].sort((left, right) => Date.parse(right.at) - Date.parse(left.at)); - - if (events.length === 0) - return ( -
    -

    No changelog entries yet

    -

    - Versions and publication activity will appear here. -

    -
    - ); - - return ( -
      - {events.map((event) => { - const Icon = - event.kind === "published" - ? Send - : event.kind === "unpublished" - ? Undo2 - : event.kind === "edit" - ? Pencil - : event.kind === "discard" - ? Trash2 - : event.kind === "restore" - ? RotateCcw - : FileClock; - return ( -
    1. -
      2. - ); - })} -
    - ); -} diff --git a/apps/web/ui/admin/catalogue/record-page.tsx b/apps/web/ui/admin/catalogue/record-page.tsx index d77d01ed..149e2fa4 100644 --- a/apps/web/ui/admin/catalogue/record-page.tsx +++ b/apps/web/ui/admin/catalogue/record-page.tsx @@ -12,6 +12,8 @@ import { import { diffSnapshotWrites } from "@/lib/catalogue-import/changes"; import { contentHashForCatalogueContent } from "@/lib/catalogue-import/version-content"; import { loadSourceReview } from "@/lib/catalogue/source-review-store"; +import { CHANGELOG_PAGE_SIZE } from "@/lib/catalogue/changelog"; +import { loadCatalogueChangelog } from "@/lib/coursemap/admin-catalogue-changelog"; import { loadCatalogueRecord, loadVersionCoursePreview, @@ -25,8 +27,8 @@ import { import { AccessDeniedError } from "@/ui/errors/access-denied-error"; import { AppShell } from "@/ui/shell"; import { CatalogueChangesPanel } from "./changes/changes-panel"; +import { ChangelogTimeline } from "./changelog/changelog-timeline"; import { RecordHeader } from "./record-header"; -import { RecordHistory } from "./record-history"; import { RecordTabList, RecordTabs, type RecordSection } from "./record-tabs"; import { CatalogueContentEditor } from "./content-editor"; import { CoursePreview, StructurePreview } from "./version-preview"; @@ -53,11 +55,14 @@ export async function CatalogueRecordPage({ code, academicYear, section = "content", + changelogEvents = CHANGELOG_PAGE_SIZE, }: { kind: CatalogueKind; code: string; academicYear: number; section?: RecordSection; + /** How many raw audit events the changelog reads before paging. */ + changelogEvents?: number; }) { const [canManageImports, canWrite] = await Promise.all([ canManageCatalogueSources(), @@ -103,6 +108,15 @@ export async function CatalogueRecordPage({ const unpublished = draft ? diffSnapshotWrites(studentContent, draft.content) : []; + const changelog = await loadCatalogueChangelog({ + recordId: record.recordId, + limit: Math.min(Math.max(changelogEvents, CHANGELOG_PAGE_SIZE), 400), + }); + const versionOrdinals = new Map( + [...record.versions] + .sort((left, right) => left.id - right.id) + .map((version, index) => [version.id, index + 1]), + ); const openChanges = (review?.conflicts.length ?? 0) + (review?.incoming.length ?? 0); @@ -171,7 +185,11 @@ export async function CatalogueRecordPage({ /> - + diff --git a/docs/architecture.md b/docs/architecture.md index d6410d04..9e3fac17 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -46,8 +46,9 @@ and version model. These concepts are deliberately separate: A manual edit replaces provenance only for its semantic changed paths. - `catalogue_change_events` and `catalogue_field_changes`: append-only accepted editing, publication, discard and restore operations with exact old and new - values. `editing_session_id` lets the changelog group autosaves later without - rewriting raw history. + values. `editing_session_id` groups autosaves in the changelog without + rewriting raw history, and `sync_change_id` names the ANU review row a source + decision answered - `catalogue_publications`: historical visibility intervals recording the version, publisher, publication time, unpublisher and unpublication time. - `course_version_details` and `structure_version_details` hold the scalar diff --git a/docs/catalogue-completion-plan.md b/docs/catalogue-completion-plan.md index 1f93305c..3d5f4ae2 100644 --- a/docs/catalogue-completion-plan.md +++ b/docs/catalogue-completion-plan.md @@ -290,6 +290,12 @@ Turn `catalogue_change_events`, `catalogue_field_changes`, `catalogue_versions`, `catalogue_publications`, `catalogue_syncs` and `catalogue_sync_changes` into one human timeline. An administrator never learns which table produced a row. +Two schema notes from what landed: `catalogue_change_events.sync_change_id` +names the review row a source decision answered, because keeping a value +changes no content and so leaves no field change row to carry that meaning; and +`restoreCatalogueVersion` now materialises the draft it replaces as a version +first, which is what makes the replacement warning honest. + ## Grouping Branch 03 records `editing_session_id`. Use it. Five autosaves become one diff --git a/docs/catalogue-operations.md b/docs/catalogue-operations.md index 200f1b82..9a3063a5 100644 --- a/docs/catalogue-operations.md +++ b/docs/catalogue-operations.md @@ -81,9 +81,29 @@ using the student-facing view. **Publish draft** materialises and seals a new manual version, advances the publication pointer and clears the draft. **Unpublish** closes the visibility interval without deleting history. -The **Changelog** tab lists immutable versions and publication events. -**Restore as draft** copies historical content into a new draft. **Discard** -clears the draft without deleting versions. +## Read the changelog + +The **Changelog** tab is one timeline of everything that happened to the +record, newest first and grouped by day. It speaks in editing, review and +publication terms; technical execution detail belongs to operations, not here. + +Autosaves are grouped by editing session, so one sitting reads as "Harry edited +Description, 5 autosaves over 4 minutes" with the value it started from and the +value it ended on. A field typed and taken back within a session is not a +change. Repeated quiet ANU checks collapse into one line. The grouping is +presentation only: every raw event keeps its own row, and expanding an entry +shows the fields it covers. + +Versions are numbered within their record and open as a page, not a database +row. A version page renders the student view of that content and offers +**Compare** against the current draft, the published version or the preceding +version, and **Restore as draft**. + +**Restore as draft** copies historical content into the working draft. It is +not called a revert because the version itself never changes. If a draft would +be replaced, the draft is kept as a version of its own first and offered back +from the changelog, so nothing is lost. **Discard** works the same way: it +clears the draft and keeps a restorable checkpoint. ## Reliability and evidence diff --git a/supabase/migrations/20260924100000_catalogue_changelog.sql b/supabase/migrations/20260924100000_catalogue_changelog.sql new file mode 100644 index 00000000..c1ded440 --- /dev/null +++ b/supabase/migrations/20260924100000_catalogue_changelog.sql @@ -0,0 +1,22 @@ +begin; + +-- A source decision event names the review row it answered, so the changelog +-- can say which field was used or kept without matching events to review rows +-- by timestamp. Keeping current changes no content, so there is no field +-- change row to carry that meaning. +alter table public.catalogue_change_events + add column sync_change_id bigint + references public.catalogue_sync_changes (id) on delete set null, + add constraint catalogue_change_events_sync_change_check check ( + sync_change_id is null + or event_kind in ('source_accepted', 'source_kept') + ); + +create index catalogue_change_events_sync_change_idx + on public.catalogue_change_events (sync_change_id) + where sync_change_id is not null; + +comment on column public.catalogue_change_events.sync_change_id is + 'The ANU review row this event decided, for source_accepted and source_kept events.'; + +commit; diff --git a/supabase/tests/database/catalogue_sync_changes.sql b/supabase/tests/database/catalogue_sync_changes.sql index cbdde7ae..e6266d9a 100644 --- a/supabase/tests/database/catalogue_sync_changes.sql +++ b/supabase/tests/database/catalogue_sync_changes.sql @@ -6,7 +6,7 @@ begin; create extension if not exists pgtap with schema extensions; -select extensions.plan(9); +select extensions.plan(11); select extensions.has_table('public', 'catalogue_sync_changes', 'ANU review rows are stored'); @@ -99,11 +99,37 @@ select extensions.throws_ok( select extensions.lives_ok( $$ - insert into public.catalogue_change_events (record_id, event_kind, origin) - select record_id, 'source_accepted', 'source' from public.catalogue_syncs - where id = '33000000-0000-4000-8000-000000000001' + insert into public.catalogue_change_events ( + record_id, event_kind, origin, sync_change_id + ) + select changes.record_id, 'source_accepted', 'source', changes.id + from public.catalogue_sync_changes as changes + where changes.sync_id = '33000000-0000-4000-8000-000000000001' + $$, + 'accepting an ANU value is an auditable event that names the row it answered' +); + +select extensions.throws_ok( + $$ + insert into public.catalogue_change_events ( + record_id, event_kind, origin, sync_change_id + ) + select changes.record_id, 'edit', 'manual', changes.id + from public.catalogue_sync_changes as changes + where changes.sync_id = '33000000-0000-4000-8000-000000000001' $$, - 'accepting an ANU value is an auditable event kind' + '23514', + null, + 'only a source decision may name a review row' +); + +select extensions.is( + ( + select count(*)::int from public.catalogue_change_events + where event_kind = 'source_accepted' and sync_change_id is not null + ), + 1, + 'the changelog can join a decision to the field it decided' ); select * from extensions.finish(); From 10be8113e65c64a5fe07b900ca9e7f02b1e98225 Mon Sep 17 00:00:00 2001 From: Harry Randall Date: Tue, 22 Sep 2026 13:07:22 +1000 Subject: [PATCH 2/3] docs: stack the remaining catalogue branches in one line Each branch sits on the one before it and its pull request targets that branch, rather than 07 and 08 running in parallel from the sync branch. --- docs/catalogue-completion-plan.md | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/docs/catalogue-completion-plan.md b/docs/catalogue-completion-plan.md index 3d5f4ae2..6da3bd37 100644 --- a/docs/catalogue-completion-plan.md +++ b/docs/catalogue-completion-plan.md @@ -35,23 +35,24 @@ Neither needs a second implementation. ## Sequence and ownership -| Order | Branch | Base | Owner | Depends on | -| ----- | ---------------------------------------- | ---- | ------ | ----------------------- | -| 05 | `feat/catalogue-source-review` | 04 | Claude | 04 | -| 06 | `feat/catalogue-changelog` | 05 | Claude | 05 decision events | -| 07 | `feat/catalogue-student-view` | 04 | agent | none beyond 04 | -| 08 | `feat/catalogue-sync-operations` | 04 | agent | none beyond 04 | -| 09 | `feat/catalogue-automation-and-baseline` | 08 | agent | all of the above merged | +| Order | Branch | Base | Owner | Depends on | +| ----- | ---------------------------------------- | ---- | ------ | ---------- | +| 05 | `feat/catalogue-source-review` | 04 | Claude | landed | +| 06 | `feat/catalogue-changelog` | 05 | Claude | landed | +| 07 | `feat/catalogue-student-view` | 06 | Claude | 06 | +| 08 | `feat/catalogue-sync-operations` | 07 | Claude | 07 | +| 09 | `feat/catalogue-automation-and-baseline` | 08 | agent | 08 | -Branches 07 and 08 touch almost nothing that 05 and 06 touch, so they run in -parallel from 04. Branch 09 is the finalisation phase and starts only when -05 to 08 are merged into the stack. Split it into `09A feat/catalogue-automation` -and `09B refactor/catalogue-schema-baseline` if it grows past a reviewable size. +The stack is strictly sequential: every branch sits on the one before it and +its pull request targets that branch until it merges. Branch 09 is the +finalisation phase. Split it into `09A feat/catalogue-automation` and +`09B refactor/catalogue-schema-baseline` if it grows past a reviewable size. -### Working in parallel +### Working the stack - Branch from the base named above, never from `main`. Rebase forward when a - base branch changes. Use a separate worktree per concurrent branch. + base branch changes. Work in a separate worktree when another branch in the + stack is still open. - Reserved migration timestamp prefixes, so two branches never collide on file order: 05 uses `20260923*`, 06 uses `20260924*`, 07 uses `20260925*`, 08 uses `20260926*`, 09 uses `20260927*` and the baseline rewrite. From 8bb46a80da1fecb04265f227655e20e793e52934 Mon Sep 17 00:00:00 2001 From: Harry Randall Date: Tue, 22 Sep 2026 13:49:56 +1000 Subject: [PATCH 3/3] test: walk the changelog into the version it kept The workspace journey asserted a "Version N created" line, which the timeline no longer writes: a discarded draft appears as the discard it was, with a link to the version that holds it. Follow that link and check the restore the changelog now offers. --- apps/web/playwright/catalogue-workspace.spec.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/apps/web/playwright/catalogue-workspace.spec.ts b/apps/web/playwright/catalogue-workspace.spec.ts index 01792c27..d29be415 100644 --- a/apps/web/playwright/catalogue-workspace.spec.ts +++ b/apps/web/playwright/catalogue-workspace.spec.ts @@ -55,9 +55,19 @@ test("catalogue content autosaves and remains separate from student view", async await page.getByRole("tab", { name: "Changelog" }).click(); await expect(page).toHaveURL(/\/admin\/courses\/2026\/comp1100\/changelog$/); await expect(page.getByRole("list", { name: "Changelog" })).toBeVisible(); - await expect(page.getByText(/Version \d+ created/).first()).toBeVisible(); await expect( page.getByRole("heading", { name: "Draft discarded" }).first(), ).toBeVisible(); - await expect(page.getByRole("button", { name: /restore/i })).toHaveCount(0); + // The discarded draft was kept as a version, which the entry offers to open. + const checkpoint = page + .getByRole("link", { name: /View version \d+/ }) + .first(); + await expect(checkpoint).toBeVisible(); + await checkpoint.click(); + await expect(page).toHaveURL( + /\/admin\/courses\/2026\/comp1100\/changelog\/\d+$/, + ); + await expect( + page.getByRole("button", { name: "Restore as draft" }), + ).toBeVisible(); });