Skip to content

Commit d95dc2e

Browse files
tofikwestclaude
andauthored
feat(isms): document versioning and history (CS-701) (#3361)
* feat(isms): document versioning and history (CS-701) Bring ISMS documents onto the Policies versioning model. Approving a document freezes an immutable published version (v1). Editing afterwards creates a draft without touching the published version, which stays live and exportable. Re-approving publishes the next version and moves the old one into history. Rendered DOCX/PDF are retained per published version so an auditor sampling v1 later downloads exactly what was approved. - schema: IsmsDocument gains currentVersion/draftNarrative/draftSnapshot; IsmsDocumentVersion gains contentSnapshot/changelog/publishedBy. Backfill migration (pre-GA, flag-gated) moves the old single-version model over. - api: new IsmsVersionService freezes a version at approval and renders + uploads PDF/DOCX to S3 after the txn; lists history; serves a version's export (stored file -> snapshot re-render -> live fallback). Editing an approved document reverts status to draft but preserves the published version. Drift baseline + draft narrative moved onto the document. New GET /v1/isms/documents/:id/versions; optional versionId on export. - app: version-history section on every document detail page with per-version PDF/DOCX download, and a "Published vN / editing creates a new draft" signal. - tests: 312 api + 119 app tests passing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012CweXoSSEP89mX93u3Bdcp * fix(isms): address CS-701 PR review — export integrity, download UX, org scoping Fixes the real issues from the automated PR review: - Versioned export never serves live/draft data (audit integrity): getVersionExport now falls back to the immutable snapshot if a stored S3 object is missing, and throws for a version with neither a file nor a snapshot instead of rendering current live data. - Default export of a clean approved document serves its retained published artifact (byte-identical to approval); only in-progress drafts export the draft. - Regenerating ("Generate from platform data") now invalidates approval, matching every other edit path, so an approved doc can't silently diverge from its published version. - Version-history download buttons are gated on real availability (file or snapshot); unavailable formats are hidden with a clear note. - Version-history fetch passes the route org (X-Organization-Id), matching the per-version download path. - Approval success is separated from the best-effort post-approval refresh so a revalidation hiccup no longer reports "Failed to approve". - Version history retains the approver via the frozen snapshot metadata if the approving member is later deleted. Not changed (deliberate): approval stays committed if export rendering fails — the immutable contentSnapshot makes every version reproducible and the AWS SDK retries transient errors; throwing would strand a doc "approved but unretryable". Tests: 316 api + 122 app ISMS tests passing; typecheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012CweXoSSEP89mX93u3Bdcp * fix(isms): second review pass — default export, download errors, tx isolation, a11y Addresses the follow-up automated review on the fix commit: - Default export (no versionId) now serves the current PUBLISHED version whenever one exists — so v1 stays the live/exportable default while v2 is being drafted (CS-701). Only a never-published document renders the live draft. (P1) - Per-version downloads now catch export failures and surface a toast instead of failing silently / as an unhandled rejection. (P2) - The approve snapshot reads the org profile through the same transaction client (threaded through resolveOrgProfile/loadOrgProfile) so profile + register rows share one point-in-time. (P2) - Version-history download buttons get version-specific accessible labels ("Download v2 as PDF") for screen-reader / voice-control users. (P3) Tests: 318 api + 123 app ISMS tests passing; typecheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012CweXoSSEP89mX93u3Bdcp * fix(isms): third review pass — no snapshot-less published versions, download races - Migration no longer promotes legacy approved docs to a published version. The pre-CS-701 version row had no contentSnapshot, so serving it as currentVersion would 404 on export (regression from the default-export + no-live-fallback fixes). Legacy approved docs revert to draft and re-approve once to establish a real v1; all legacy version rows are dropped. currentVersionId is now only ever set by the approve flow, which always writes a contentSnapshot — so it can never point at a snapshot-less version. (P1) - Per-version download indicator clears only if the finishing request still owns it, so starting a second download no longer clears the first's spinner early. (P3) Not changed (deliberate, re-confirmed): approval stays committed if publishRenders fails — createPublishedVersion always persists an immutable contentSnapshot, so the version re-renders on demand (no lost export); forcing a throw would strand a doc "approved but unretryable". Tests: 318 api + 123 app ISMS tests passing; typecheck clean; migration SQL validated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012CweXoSSEP89mX93u3Bdcp * fix(isms): migration — preserve approvals, add FKs after legacy cleanup Addresses the latest review on the CS-701 migration: - Non-destructive: existing approved documents keep their approved status. The migration no longer resets them to draft — it only drops the legacy (pre-CS-701, snapshot-less) version rows and leaves currentVersionId null, so an approved doc exports its current content on demand and captures a real versioned artifact on its next approval. currentVersionId is only ever set by the approve flow, which always writes a contentSnapshot. (P2) - Defensive ordering: the legacy-row DELETE now runs BEFORE the foreign keys are added, so FK validation can never trip on a stale publishedById. (In practice publishedById was a dead/always-null column pre-migration, so it could not have failed — but the ordering makes it robust regardless.) (P2) Verified: migration applies cleanly from scratch (fresh DB, all migrations deploy); 318 api + 123 app ISMS tests passing; typecheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012CweXoSSEP89mX93u3Bdcp --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent fe4394b commit d95dc2e

49 files changed

Lines changed: 2293 additions & 615 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎apps/api/src/isms/documents/generate.spec.ts‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,10 @@ function buildTx() {
4040
ismsInterestedParty: registerTable(),
4141
ismsInterestedPartyRequirement: registerTable(),
4242
ismsObjective: registerTable(),
43-
ismsDocument: { findFirst: jest.fn().mockResolvedValue(null) },
44-
ismsDocumentVersion: {
43+
ismsDocument: {
4544
findFirst: jest.fn().mockResolvedValue(null),
46-
create: jest.fn().mockResolvedValue({}),
45+
// CS-701: the draft narrative lives on IsmsDocument, not a version row.
46+
findUnique: jest.fn().mockResolvedValue(null),
4747
update: jest.fn().mockResolvedValue({}),
4848
},
4949
};
@@ -132,24 +132,27 @@ describe('runDerivation', () => {
132132
expect(tx.ismsObjective.createMany).toHaveBeenCalled();
133133
});
134134

135-
it('creates a version with the derived narrative for isms_scope', async () => {
135+
it('writes the derived narrative onto an empty document draft (isms_scope)', async () => {
136136
const tx = buildTx();
137137
await runDerivation({ tx: asTx(tx), type: 'isms_scope', ...baseArgs });
138-
expect(tx.ismsDocumentVersion.create).toHaveBeenCalled();
139-
const created = tx.ismsDocumentVersion.create.mock.calls[0][0].data;
140-
expect(created.narrative.certificateScopeSentence).toBeDefined();
138+
expect(tx.ismsDocument.update).toHaveBeenCalledWith(
139+
expect.objectContaining({ where: { id: 'doc_1' } }),
140+
);
141+
const updated = tx.ismsDocument.update.mock.calls[0][0].data;
142+
expect(updated.draftNarrative.certificateScopeSentence).toBeDefined();
141143
});
142144

143-
it('updates the existing version narrative for leadership', async () => {
145+
it('preserves a non-empty existing draft narrative (CS-437 override)', async () => {
144146
const tx = buildTx();
145-
tx.ismsDocumentVersion.findFirst.mockResolvedValue({ id: 'ver_1' });
147+
tx.ismsDocument.findUnique.mockResolvedValue({
148+
draftNarrative: { certificateScopeSentence: 'Manual edit' },
149+
});
146150
await runDerivation({
147151
tx: asTx(tx),
148152
type: 'leadership_commitment',
149153
...baseArgs,
150154
});
151-
expect(tx.ismsDocumentVersion.update).toHaveBeenCalledWith(
152-
expect.objectContaining({ where: { id: 'ver_1' } }),
153-
);
155+
// A regenerate must never clobber the customer's manual narrative.
156+
expect(tx.ismsDocument.update).not.toHaveBeenCalled();
154157
});
155158
});

‎apps/api/src/isms/documents/generate.ts‎

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -174,22 +174,17 @@ async function generateNarrative({
174174
if (!derived) return;
175175
const narrative: Prisma.InputJsonValue = JSON.parse(JSON.stringify(derived));
176176

177-
const latest = await tx.ismsDocumentVersion.findFirst({
178-
where: { documentId, isLatest: true },
177+
// The draft narrative lives on IsmsDocument (CS-701). Preserve a non-empty
178+
// existing draft so a regenerate never clobbers the customer's manual edits
179+
// (CS-437 override); seed an absent/empty draft with the derived narrative.
180+
const document = await tx.ismsDocument.findUnique({
181+
where: { id: documentId },
182+
select: { draftNarrative: true },
179183
});
180-
if (latest) {
181-
// Preserve a non-empty narrative so a regenerate never clobbers the
182-
// customer's manual edits (CS-437 override). An absent or empty ({}) value —
183-
// e.g. a snapshot-only version — is still seeded with the derived narrative.
184-
if (hasNarrativeContent(latest.narrative)) return;
185-
await tx.ismsDocumentVersion.update({
186-
where: { id: latest.id },
187-
data: { narrative },
188-
});
189-
return;
190-
}
191-
await tx.ismsDocumentVersion.create({
192-
data: { documentId, version: 1, isLatest: true, narrative },
184+
if (document && hasNarrativeContent(document.draftNarrative)) return;
185+
await tx.ismsDocument.update({
186+
where: { id: documentId },
187+
data: { draftNarrative: narrative },
193188
});
194189
}
195190

‎apps/api/src/isms/documents/org-profile.ts‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { db } from '@db';
2+
import type { Prisma } from '@db';
23
import { DEFAULT_INTENDED_OUTCOMES } from '../wizard/wizard-defaults';
34
import { parseStoredAnswers } from '../wizard/wizard-schema';
45
import type { IsmsKeyValue } from '../utils/export-shared';
@@ -29,23 +30,30 @@ const QUESTIONS = {
2930
export async function loadOrgProfile({
3031
organizationId,
3132
frameworkId,
33+
client = db,
3234
}: {
3335
organizationId: string;
3436
frameworkId: string;
37+
/**
38+
* DB client to read through. Defaults to the global `db`; the approve flow
39+
* passes its transaction client so the snapshot's org profile reads share the
40+
* same isolation context as the register rows written in the same transaction.
41+
*/
42+
client?: Prisma.TransactionClient;
3543
}): Promise<IsmsOrgProfile> {
3644
const [organization, contextEntries, profile] = await Promise.all([
37-
db.organization.findUnique({
45+
client.organization.findUnique({
3846
where: { id: organizationId },
3947
select: { name: true, website: true },
4048
}),
41-
db.context.findMany({
49+
client.context.findMany({
4250
where: { organizationId },
4351
// Deterministic ordering so duplicate questions resolve to the same answer
4452
// every export; the earliest-created entry wins (id breaks createdAt ties).
4553
orderBy: [{ createdAt: 'asc' }, { id: 'asc' }],
4654
select: { question: true, answer: true },
4755
}),
48-
db.ismsProfile.findUnique({
56+
client.ismsProfile.findUnique({
4957
where: { organizationId_frameworkId: { organizationId, frameworkId } },
5058
select: { answers: true },
5159
}),
Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import { ApiProperty } from '@nestjs/swagger';
2-
import { IsIn } from 'class-validator';
1+
import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger';
2+
import { IsIn, IsOptional, IsString } from 'class-validator';
33

44
export class ExportIsmsDocumentDto {
55
@ApiProperty({
@@ -9,4 +9,13 @@ export class ExportIsmsDocumentDto {
99
})
1010
@IsIn(['pdf', 'docx'])
1111
format!: 'pdf' | 'docx';
12+
13+
@ApiPropertyOptional({
14+
description:
15+
'Published version to export. Omit to export the current working draft; ' +
16+
'provide a version id to download exactly what was approved at that version.',
17+
})
18+
@IsOptional()
19+
@IsString()
20+
versionId?: string;
1221
}

0 commit comments

Comments
 (0)