From d9850a33fa4dd40064382b0f90bd9b8bf8effb7b Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Tue, 7 Jul 2026 10:41:56 -0400 Subject: [PATCH 1/5] 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) Claude-Session: https://claude.ai/code/session_012CweXoSSEP89mX93u3Bdcp --- apps/api/src/isms/documents/generate.spec.ts | 27 +- apps/api/src/isms/documents/generate.ts | 25 +- .../src/isms/dto/export-isms-document.dto.ts | 13 +- .../api/src/isms/isms-context.service.spec.ts | 230 ++++++--------- apps/api/src/isms/isms-context.service.ts | 125 +++----- .../src/isms/isms-narrative.service.spec.ts | 74 ++--- apps/api/src/isms/isms-narrative.service.ts | 33 +-- .../isms/isms-version.service.export.spec.ts | 183 ++++++++++++ .../api/src/isms/isms-version.service.spec.ts | 194 +++++++++++++ apps/api/src/isms/isms-version.service.ts | 266 ++++++++++++++++++ .../isms/isms.controller.permissions.spec.ts | 6 + apps/api/src/isms/isms.controller.spec.ts | 22 ++ apps/api/src/isms/isms.controller.ts | 15 + apps/api/src/isms/isms.module.ts | 7 +- .../api/src/isms/isms.service.approve.spec.ts | 181 ++++++++++++ ...isms.service.ensure-setup-fallback.spec.ts | 9 +- .../src/isms/isms.service.lifecycle.spec.ts | 135 +-------- apps/api/src/isms/isms.service.spec.ts | 9 +- apps/api/src/isms/isms.service.ts | 59 +++- .../api/src/isms/utils/draft-snapshot.spec.ts | 50 ++++ apps/api/src/isms/utils/draft-snapshot.ts | 28 ++ apps/api/src/isms/utils/export-generator.ts | 4 +- .../api/src/isms/utils/export-payload.spec.ts | 54 ++++ apps/api/src/isms/utils/export-payload.ts | 179 ++++++++++++ .../src/isms/utils/version-snapshot.spec.ts | 78 ----- apps/api/src/isms/utils/version-snapshot.ts | 45 --- .../ContextOfOrganizationClient.test.tsx | 8 +- .../InterestedPartiesClient.test.tsx | 8 +- .../components/IsmsApprovalSection.test.tsx | 4 +- .../isms/components/IsmsApprovalSection.tsx | 27 +- .../components/IsmsControlMappings.test.tsx | 4 +- .../isms/components/IsmsDocumentShell.tsx | 23 ++ .../components/IsmsVersionHistory.test.tsx | 81 ++++++ .../isms/components/IsmsVersionHistory.tsx | 130 +++++++++ .../isms/components/LeadershipClient.test.tsx | 8 +- .../isms/components/LeadershipClient.tsx | 3 +- .../isms/components/ObjectivesClient.test.tsx | 8 +- .../components/RequirementsClient.test.tsx | 8 +- .../isms/components/ScopeClient.test.tsx | 8 +- .../documents/isms/components/ScopeClient.tsx | 5 +- .../components/__test-helpers__/dsMocks.tsx | 19 ++ .../isms/hooks/exportIsmsDocument.ts | 10 +- .../hooks/useIsmsDocumentVersions.test.ts | 73 +++++ .../isms/hooks/useIsmsDocumentVersions.ts | 65 +++++ .../[orgId]/documents/isms/isms-types.ts | 41 ++- .../migration.sql | 60 ++++ packages/db/prisma/schema/auth.prisma | 1 + packages/db/prisma/schema/isms.prisma | 49 +++- 48 files changed, 2066 insertions(+), 628 deletions(-) create mode 100644 apps/api/src/isms/isms-version.service.export.spec.ts create mode 100644 apps/api/src/isms/isms-version.service.spec.ts create mode 100644 apps/api/src/isms/isms-version.service.ts create mode 100644 apps/api/src/isms/isms.service.approve.spec.ts create mode 100644 apps/api/src/isms/utils/draft-snapshot.spec.ts create mode 100644 apps/api/src/isms/utils/draft-snapshot.ts create mode 100644 apps/api/src/isms/utils/export-payload.spec.ts create mode 100644 apps/api/src/isms/utils/export-payload.ts delete mode 100644 apps/api/src/isms/utils/version-snapshot.spec.ts delete mode 100644 apps/api/src/isms/utils/version-snapshot.ts create mode 100644 apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsVersionHistory.test.tsx create mode 100644 apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsVersionHistory.tsx create mode 100644 apps/app/src/app/(app)/[orgId]/documents/isms/hooks/useIsmsDocumentVersions.test.ts create mode 100644 apps/app/src/app/(app)/[orgId]/documents/isms/hooks/useIsmsDocumentVersions.ts create mode 100644 packages/db/prisma/migrations/20260707120000_isms_document_versioning/migration.sql diff --git a/apps/api/src/isms/documents/generate.spec.ts b/apps/api/src/isms/documents/generate.spec.ts index d4eb81cdec..3547f853ec 100644 --- a/apps/api/src/isms/documents/generate.spec.ts +++ b/apps/api/src/isms/documents/generate.spec.ts @@ -40,10 +40,10 @@ function buildTx() { ismsInterestedParty: registerTable(), ismsInterestedPartyRequirement: registerTable(), ismsObjective: registerTable(), - ismsDocument: { findFirst: jest.fn().mockResolvedValue(null) }, - ismsDocumentVersion: { + ismsDocument: { findFirst: jest.fn().mockResolvedValue(null), - create: jest.fn().mockResolvedValue({}), + // CS-701: the draft narrative lives on IsmsDocument, not a version row. + findUnique: jest.fn().mockResolvedValue(null), update: jest.fn().mockResolvedValue({}), }, }; @@ -132,24 +132,27 @@ describe('runDerivation', () => { expect(tx.ismsObjective.createMany).toHaveBeenCalled(); }); - it('creates a version with the derived narrative for isms_scope', async () => { + it('writes the derived narrative onto an empty document draft (isms_scope)', async () => { const tx = buildTx(); await runDerivation({ tx: asTx(tx), type: 'isms_scope', ...baseArgs }); - expect(tx.ismsDocumentVersion.create).toHaveBeenCalled(); - const created = tx.ismsDocumentVersion.create.mock.calls[0][0].data; - expect(created.narrative.certificateScopeSentence).toBeDefined(); + expect(tx.ismsDocument.update).toHaveBeenCalledWith( + expect.objectContaining({ where: { id: 'doc_1' } }), + ); + const updated = tx.ismsDocument.update.mock.calls[0][0].data; + expect(updated.draftNarrative.certificateScopeSentence).toBeDefined(); }); - it('updates the existing version narrative for leadership', async () => { + it('preserves a non-empty existing draft narrative (CS-437 override)', async () => { const tx = buildTx(); - tx.ismsDocumentVersion.findFirst.mockResolvedValue({ id: 'ver_1' }); + tx.ismsDocument.findUnique.mockResolvedValue({ + draftNarrative: { certificateScopeSentence: 'Manual edit' }, + }); await runDerivation({ tx: asTx(tx), type: 'leadership_commitment', ...baseArgs, }); - expect(tx.ismsDocumentVersion.update).toHaveBeenCalledWith( - expect.objectContaining({ where: { id: 'ver_1' } }), - ); + // A regenerate must never clobber the customer's manual narrative. + expect(tx.ismsDocument.update).not.toHaveBeenCalled(); }); }); diff --git a/apps/api/src/isms/documents/generate.ts b/apps/api/src/isms/documents/generate.ts index 4554fbe2b9..4497aa66f1 100644 --- a/apps/api/src/isms/documents/generate.ts +++ b/apps/api/src/isms/documents/generate.ts @@ -174,22 +174,17 @@ async function generateNarrative({ if (!derived) return; const narrative: Prisma.InputJsonValue = JSON.parse(JSON.stringify(derived)); - const latest = await tx.ismsDocumentVersion.findFirst({ - where: { documentId, isLatest: true }, + // The draft narrative lives on IsmsDocument (CS-701). Preserve a non-empty + // existing draft so a regenerate never clobbers the customer's manual edits + // (CS-437 override); seed an absent/empty draft with the derived narrative. + const document = await tx.ismsDocument.findUnique({ + where: { id: documentId }, + select: { draftNarrative: true }, }); - if (latest) { - // Preserve a non-empty narrative so a regenerate never clobbers the - // customer's manual edits (CS-437 override). An absent or empty ({}) value — - // e.g. a snapshot-only version — is still seeded with the derived narrative. - if (hasNarrativeContent(latest.narrative)) return; - await tx.ismsDocumentVersion.update({ - where: { id: latest.id }, - data: { narrative }, - }); - return; - } - await tx.ismsDocumentVersion.create({ - data: { documentId, version: 1, isLatest: true, narrative }, + if (document && hasNarrativeContent(document.draftNarrative)) return; + await tx.ismsDocument.update({ + where: { id: documentId }, + data: { draftNarrative: narrative }, }); } diff --git a/apps/api/src/isms/dto/export-isms-document.dto.ts b/apps/api/src/isms/dto/export-isms-document.dto.ts index 2891e57eff..632b1a2923 100644 --- a/apps/api/src/isms/dto/export-isms-document.dto.ts +++ b/apps/api/src/isms/dto/export-isms-document.dto.ts @@ -1,5 +1,5 @@ -import { ApiProperty } from '@nestjs/swagger'; -import { IsIn } from 'class-validator'; +import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger'; +import { IsIn, IsOptional, IsString } from 'class-validator'; export class ExportIsmsDocumentDto { @ApiProperty({ @@ -9,4 +9,13 @@ export class ExportIsmsDocumentDto { }) @IsIn(['pdf', 'docx']) format!: 'pdf' | 'docx'; + + @ApiPropertyOptional({ + description: + 'Published version to export. Omit to export the current working draft; ' + + 'provide a version id to download exactly what was approved at that version.', + }) + @IsOptional() + @IsString() + versionId?: string; } diff --git a/apps/api/src/isms/isms-context.service.spec.ts b/apps/api/src/isms/isms-context.service.spec.ts index a91d741002..33a0b7e9e4 100644 --- a/apps/api/src/isms/isms-context.service.spec.ts +++ b/apps/api/src/isms/isms-context.service.spec.ts @@ -1,22 +1,19 @@ import { NotFoundException } from '@nestjs/common'; import { db } from '@db'; import { IsmsContextService } from './isms-context.service'; +import type { IsmsVersionService } from './isms-version.service'; import { collectPlatformData } from './documents/data-source'; import { runDerivation } from './documents/generate'; import { diffPlatformSnapshots, parsePlatformSnapshot, } from './documents/snapshot'; -import { buildExportSections } from './documents/registry'; -import { generateIsmsExportFile } from './utils/export-generator'; -import { upsertLatestSnapshotVersion } from './utils/version-snapshot'; +import { updateDraftSnapshot } from './utils/draft-snapshot'; +import { renderLiveExport } from './utils/export-payload'; jest.mock('@db', () => ({ db: { ismsDocument: { findFirst: jest.fn() }, - organization: { findUnique: jest.fn() }, - context: { findMany: jest.fn() }, - ismsProfile: { findUnique: jest.fn() }, $transaction: jest.fn(), }, })); @@ -30,14 +27,11 @@ jest.mock('./documents/snapshot', () => ({ diffPlatformSnapshots: jest.fn(), parsePlatformSnapshot: jest.fn(), })); -jest.mock('./documents/registry', () => ({ - buildExportSections: jest.fn(), +jest.mock('./utils/draft-snapshot', () => ({ + updateDraftSnapshot: jest.fn(), })); -jest.mock('./utils/export-generator', () => ({ - generateIsmsExportFile: jest.fn(), -})); -jest.mock('./utils/version-snapshot', () => ({ - upsertLatestSnapshotVersion: jest.fn(), +jest.mock('./utils/export-payload', () => ({ + renderLiveExport: jest.fn(), })); const mockDb = jest.mocked(db); @@ -45,8 +39,8 @@ const mockCollect = jest.mocked(collectPlatformData); const mockRun = jest.mocked(runDerivation); const mockDiff = jest.mocked(diffPlatformSnapshots); const mockParse = jest.mocked(parsePlatformSnapshot); -const mockBuild = jest.mocked(buildExportSections); -const mockExport = jest.mocked(generateIsmsExportFile); +const mockUpdateDraft = jest.mocked(updateDraftSnapshot); +const mockRenderLive = jest.mocked(renderLiveExport); const snapshot = { organizationName: 'Acme', @@ -66,12 +60,17 @@ const snapshot = { partiesFingerprint: '', }; +const versionService = { + getVersionExport: jest.fn(), + getVersions: jest.fn(), +} as unknown as IsmsVersionService; + describe('IsmsContextService', () => { let service: IsmsContextService; beforeEach(() => { jest.clearAllMocks(); - service = new IsmsContextService(); + service = new IsmsContextService(versionService); }); describe('generate', () => { @@ -89,30 +88,35 @@ describe('IsmsContextService', () => { 'objectives_plan', 'isms_scope', 'leadership_commitment', - ])('dispatches derivation + snapshot for type %s', async (type) => { - (mockDb.ismsDocument.findFirst as jest.Mock) - .mockResolvedValueOnce({ id: 'doc_1', type, frameworkId: 'fw_1' }) - .mockResolvedValueOnce({ id: 'doc_1' }); - mockCollect.mockResolvedValue(snapshot); - const tx = {}; - (mockDb.$transaction as jest.Mock).mockImplementation((cb) => cb(tx)); - - await service.generate(args); - - expect(mockRun).toHaveBeenCalledWith({ - tx, - type, - documentId: 'doc_1', - organizationId: 'org_1', - frameworkId: 'fw_1', - data: snapshot, - }); - expect(upsertLatestSnapshotVersion).toHaveBeenCalledWith({ - tx, - documentId: 'doc_1', - snapshot, - }); - }); + ])( + 'derives + updates the draft snapshot for type %s', + async (type) => { + (mockDb.ismsDocument.findFirst as jest.Mock) + .mockResolvedValueOnce({ id: 'doc_1', type, frameworkId: 'fw_1' }) + .mockResolvedValueOnce({ id: 'doc_1' }); + mockCollect.mockResolvedValue(snapshot); + const tx = {}; + (mockDb.$transaction as jest.Mock).mockImplementation((cb) => cb(tx)); + + await service.generate(args); + + expect(mockRun).toHaveBeenCalledWith({ + tx, + type, + documentId: 'doc_1', + organizationId: 'org_1', + frameworkId: 'fw_1', + data: snapshot, + }); + // CS-701: the drift baseline is the document's draftSnapshot, not a + // version row. + expect(mockUpdateDraft).toHaveBeenCalledWith({ + tx, + documentId: 'doc_1', + snapshot, + }); + }, + ); it('reuses pre-collected data and skips collectPlatformData', async () => { (mockDb.ismsDocument.findFirst as jest.Mock) @@ -137,7 +141,7 @@ describe('IsmsContextService', () => { frameworkId: 'fw_1', data: precollected, }); - expect(upsertLatestSnapshotVersion).toHaveBeenCalledWith({ + expect(mockUpdateDraft).toHaveBeenCalledWith({ tx, documentId: 'doc_1', snapshot: precollected, @@ -153,12 +157,12 @@ describe('IsmsContextService', () => { await expect(service.drift(args)).rejects.toThrow(NotFoundException); }); - it('compares current data against the parsed snapshot by type', async () => { + it('compares current data against the parsed draftSnapshot by type', async () => { (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ id: 'doc_1', type: 'objectives_plan', frameworkId: 'fw_1', - versions: [{ sourceSnapshot: snapshot }], + draftSnapshot: snapshot, }); mockCollect.mockResolvedValue({ ...snapshot, riskCount: 9 }); mockParse.mockReturnValue(snapshot); @@ -175,12 +179,12 @@ describe('IsmsContextService', () => { expect(result).toEqual({ isStale: true, changedSources: ['risks'] }); }); - it('treats a missing snapshot as no baseline', async () => { + it('treats a missing draftSnapshot as no baseline', async () => { (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ id: 'doc_1', type: 'context_of_organization', frameworkId: 'fw_1', - versions: [], + draftSnapshot: null, }); mockCollect.mockResolvedValue(snapshot); mockParse.mockReturnValue(null); @@ -200,109 +204,47 @@ describe('IsmsContextService', () => { }); describe('exportDocument', () => { - it('throws NotFoundException when document missing', async () => { - (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue(null); - await expect( - service.exportDocument({ - documentId: 'doc_1', - organizationId: 'org_1', - dto: { format: 'pdf' }, - }), - ).rejects.toThrow(NotFoundException); - }); + const result = { + fileBuffer: Buffer.from('bytes'), + mimeType: 'application/pdf', + filename: 'doc.pdf', + }; + + it('renders the live draft when no versionId is given', async () => { + mockRenderLive.mockResolvedValue(result); + + const out = await service.exportDocument({ + documentId: 'doc_1', + organizationId: 'org_1', + dto: { format: 'pdf' }, + }); - const buildDocument = (type: string) => ({ - id: 'doc_1', - type, - title: 'Doc', - status: 'approved', - preparedBy: 'Comp AI', - approvedAt: null, - declinedAt: null, - framework: { name: 'ISO 27001' }, - organization: { name: 'Acme', primaryColor: '#004D3D' }, - approver: { user: { name: 'Jane', email: 'jane@acme.io' } }, - contextIssues: [ - { - kind: 'external', - category: 'Regulatory & Legal', - description: 'd', - effect: 'e', - }, - ], - interestedParties: [ - { name: 'Customers', category: 'Customer', needsExpectations: 'n' }, - ], - interestedPartyRequirements: [ - { partyName: 'Customers', requirement: 'r', treatment: 't' }, - ], - objectives: [ - { - objective: 'o', - target: 't', - cadence: 'Annual', - status: 'on_track', - plan: 'p', - measurementMethod: 'm', - }, - ], - versions: [ - { version: 2, narrative: { statement: 's', commitments: [] } }, - ], + expect(mockRenderLive).toHaveBeenCalledWith({ + documentId: 'doc_1', + organizationId: 'org_1', + format: 'pdf', + }); + expect(versionService.getVersionExport).not.toHaveBeenCalled(); + expect(out).toBe(result); }); - it.each([ - ['context_of_organization', 'pdf'], - ['interested_parties_register', 'pdf'], - ['interested_parties_requirements', 'pdf'], - ['objectives_plan', 'pdf'], - ['isms_scope', 'docx'], - ['leadership_commitment', 'docx'], - ] as const)( - 'dispatches export sections for %s (%s)', - async (type, format) => { - (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue( - buildDocument(type), - ); - // The Context document loads the org profile (org + Context Q&A + - // ISMS wizard answers); other types skip it. Stub all three reads. - (mockDb.organization.findUnique as jest.Mock).mockResolvedValue({ - name: 'Acme', - website: 'https://acme.io', - }); - (mockDb.context.findMany as jest.Mock).mockResolvedValue([]); - (mockDb.ismsProfile.findUnique as jest.Mock).mockResolvedValue({ - answers: {}, - }); - mockBuild.mockReturnValue([{ heading: 'H' }]); - mockExport.mockResolvedValue({ - fileBuffer: Buffer.from('bytes'), - mimeType: format === 'pdf' ? 'application/pdf' : 'docx-mime', - filename: `doc.${format}`, - }); + it('routes to the version service when a versionId is given', async () => { + (versionService.getVersionExport as jest.Mock).mockResolvedValue(result); - const result = await service.exportDocument({ - documentId: 'doc_1', - organizationId: 'org_1', - dto: { format }, - }); + const out = await service.exportDocument({ + documentId: 'doc_1', + organizationId: 'org_1', + dto: { format: 'docx', versionId: 'isms_ver_1' }, + }); - expect(mockBuild).toHaveBeenCalledWith( - expect.objectContaining({ type }), - ); - expect(mockExport).toHaveBeenCalledWith( - expect.objectContaining({ - format, - sections: [{ heading: 'H' }], - metadata: expect.objectContaining({ - title: 'Doc', - organizationName: 'Acme', - primaryColor: '#004D3D', - }), - }), - ); - expect(result.fileBuffer).toBeInstanceOf(Buffer); - }, - ); + expect(versionService.getVersionExport).toHaveBeenCalledWith({ + documentId: 'doc_1', + organizationId: 'org_1', + versionId: 'isms_ver_1', + format: 'docx', + }); + expect(mockRenderLive).not.toHaveBeenCalled(); + expect(out).toBe(result); + }); }); }); diff --git a/apps/api/src/isms/isms-context.service.ts b/apps/api/src/isms/isms-context.service.ts index ec3e995ab0..0a48ea8a1b 100644 --- a/apps/api/src/isms/isms-context.service.ts +++ b/apps/api/src/isms/isms-context.service.ts @@ -3,36 +3,41 @@ import { db } from '@db'; import { ExportIsmsDocumentDto } from './dto/export-isms-document.dto'; import { collectPlatformData } from './documents/data-source'; import { runDerivation } from './documents/generate'; -import { loadOrgProfile } from './documents/org-profile'; -import { buildExportSections } from './documents/registry'; import { diffPlatformSnapshots, parsePlatformSnapshot, } from './documents/snapshot'; -import type { DocumentExportInput, IsmsPlatformData } from './documents/types'; -import { - generateIsmsExportFile, - type IsmsExportResult, -} from './utils/export-generator'; -import { buildExportMetadata } from './utils/export-metadata'; -import { upsertLatestSnapshotVersion } from './utils/version-snapshot'; +import type { IsmsPlatformData } from './documents/types'; +import { updateDraftSnapshot } from './utils/draft-snapshot'; +import { renderLiveExport } from './utils/export-payload'; +import type { IsmsExportResult } from './utils/export-generator'; +import { IsmsVersionService } from './isms-version.service'; const DOCUMENT_INCLUDE = { - versions: { where: { isLatest: true }, take: 1 }, + currentVersion: { select: { id: true, version: true, publishedAt: true } }, contextIssues: { orderBy: { position: 'asc' } }, interestedParties: { orderBy: { position: 'asc' } }, interestedPartyRequirements: { orderBy: { position: 'asc' } }, objectives: { orderBy: { position: 'asc' } }, + controlLinks: { + select: { + id: true, + controlId: true, + control: { select: { id: true, name: true } }, + }, + }, } as const; /** * ISMS document derivation, drift detection and export. Dispatches by document * type to the per-document handlers under ./documents. Document lifecycle - * (approve/decline/submit) lives in IsmsService; register CRUD in the register - * services. + * (approve/decline/submit) lives in IsmsService, version history + per-version + * export in IsmsVersionService, and register CRUD in the register services. */ @Injectable() export class IsmsContextService { + constructor(private readonly versionService: IsmsVersionService) {} + async generate({ documentId, organizationId, @@ -70,7 +75,7 @@ export class IsmsContextService { frameworkId: document.frameworkId, data, }); - await upsertLatestSnapshotVersion({ tx, documentId, snapshot: data }); + await updateDraftSnapshot({ tx, documentId, snapshot: data }); }); return this.loadDocument({ documentId, organizationId }); @@ -85,7 +90,7 @@ export class IsmsContextService { }): Promise<{ isStale: boolean; changedSources: string[] }> { const document = await db.ismsDocument.findFirst({ where: { id: documentId, organizationId }, - include: { versions: { where: { isLatest: true }, take: 1 } }, + select: { type: true, frameworkId: true, draftSnapshot: true }, }); if (!document) { throw new NotFoundException('ISMS document not found'); @@ -95,13 +100,15 @@ export class IsmsContextService { organizationId, frameworkId: document.frameworkId, }); - const previous = parsePlatformSnapshot( - document.versions[0]?.sourceSnapshot, - ); + const previous = parsePlatformSnapshot(document.draftSnapshot); return diffPlatformSnapshots({ type: document.type, previous, current }); } + /** + * Export a document. With `versionId`, serve that published version (stored file + * or snapshot re-render); without, render the current working draft on demand. + */ async exportDocument({ documentId, organizationId, @@ -111,83 +118,15 @@ export class IsmsContextService { organizationId: string; dto: ExportIsmsDocumentDto; }): Promise { - const document = await db.ismsDocument.findFirst({ - where: { id: documentId, organizationId }, - include: { - framework: { select: { name: true } }, - organization: { - select: { name: true, website: true, primaryColor: true }, - }, - approver: { select: { user: { select: { name: true, email: true } } } }, - ...DOCUMENT_INCLUDE, - }, - }); - if (!document) { - throw new NotFoundException('ISMS document not found'); + if (dto.versionId) { + return this.versionService.getVersionExport({ + documentId, + organizationId, + versionId: dto.versionId, + format: dto.format, + }); } - - // The Context document (clause 4.1) renders an org overview, mission and - // intended outcomes; other document types don't need the profile. - const orgProfile = - document.type === 'context_of_organization' - ? await loadOrgProfile({ - organizationId, - frameworkId: document.frameworkId, - }) - : undefined; - - const input: DocumentExportInput = { - contextIssues: document.contextIssues.map((issue) => ({ - kind: issue.kind, - category: issue.category, - description: issue.description, - effect: issue.effect, - })), - interestedParties: document.interestedParties.map((party) => ({ - name: party.name, - category: party.category, - needsExpectations: party.needsExpectations, - })), - requirements: document.interestedPartyRequirements.map((row) => ({ - partyName: row.partyName, - requirement: row.requirement, - treatment: row.treatment, - })), - objectives: document.objectives.map((objective) => ({ - objective: objective.objective, - target: objective.target, - cadence: objective.cadence, - status: objective.status, - plan: objective.plan, - measurementMethod: objective.measurementMethod, - })), - narrative: document.versions[0]?.narrative ?? null, - orgProfile, - }; - - const sections = buildExportSections({ type: document.type, input }); - - return generateIsmsExportFile({ - sections, - format: dto.format, - metadata: buildExportMetadata({ - type: document.type, - title: document.title, - frameworkName: document.framework.name || 'ISO 27001', - version: document.versions[0]?.version ?? 1, - status: document.status, - preparedBy: document.preparedBy, - owner: null, - approverName: - document.approver?.user?.name || - document.approver?.user?.email || - null, - approvedAt: document.approvedAt, - declinedAt: document.declinedAt, - organizationName: document.organization.name, - primaryColor: document.organization.primaryColor, - }), - }); + return renderLiveExport({ documentId, organizationId, format: dto.format }); } private async loadDocument({ diff --git a/apps/api/src/isms/isms-narrative.service.spec.ts b/apps/api/src/isms/isms-narrative.service.spec.ts index a1e6db166e..2770acd489 100644 --- a/apps/api/src/isms/isms-narrative.service.spec.ts +++ b/apps/api/src/isms/isms-narrative.service.spec.ts @@ -9,11 +9,6 @@ jest.mock('@db', () => { findUnique: jest.fn(), update: jest.fn(), }, - ismsDocumentVersion: { - findFirst: jest.fn(), - create: jest.fn(), - update: jest.fn(), - }, // Run the callback with the same mock as the transaction client. $transaction: jest.fn((cb: (tx: unknown) => unknown) => cb(db)), }; @@ -36,6 +31,9 @@ describe('IsmsNarrativeService', () => { beforeEach(() => { jest.clearAllMocks(); service = new IsmsNarrativeService(); + (mockDb.ismsDocument.update as jest.Mock).mockResolvedValue({ + id: 'doc_1', + }); }); it('throws NotFoundException when document missing', async () => { @@ -77,14 +75,14 @@ describe('IsmsNarrativeService', () => { ).rejects.toThrow(BadRequestException); }); - it('creates a version when none exists', async () => { + it('writes the validated narrative onto the document draft (CS-701)', async () => { (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ id: 'doc_1', type: 'isms_scope', }); - (mockDb.ismsDocumentVersion.findFirst as jest.Mock).mockResolvedValue(null); - (mockDb.ismsDocumentVersion.create as jest.Mock).mockResolvedValue({ - id: 'ver_1', + // Non-approved: invalidateApprovalIfNeeded is a no-op. + (mockDb.ismsDocument.findUnique as jest.Mock).mockResolvedValue({ + status: 'draft', }); await service.save({ @@ -93,38 +91,20 @@ describe('IsmsNarrativeService', () => { narrative: validScope, }); - expect(mockDb.ismsDocumentVersion.create).toHaveBeenCalledWith( - expect.objectContaining({ - data: expect.objectContaining({ documentId: 'doc_1', version: 1 }), - }), - ); - }); - - it('updates the latest version when present', async () => { - (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ - id: 'doc_1', - type: 'isms_scope', - }); - (mockDb.ismsDocumentVersion.findFirst as jest.Mock).mockResolvedValue({ - id: 'ver_1', - }); - (mockDb.ismsDocumentVersion.update as jest.Mock).mockResolvedValue({ - id: 'ver_1', - }); - - await service.save({ - documentId: 'doc_1', - organizationId: 'org_1', - narrative: validScope, + // The draft narrative lives on IsmsDocument, never on a version row. + expect(mockDb.ismsDocument.update).toHaveBeenCalledTimes(1); + expect(mockDb.ismsDocument.update).toHaveBeenCalledWith({ + where: { id: 'doc_1' }, + data: { + draftNarrative: expect.objectContaining({ + certificateScopeSentence: 'The ISMS covers Acme.', + }), + }, + select: { id: true, draftNarrative: true }, }); - - expect(mockDb.ismsDocumentVersion.update).toHaveBeenCalledWith( - expect.objectContaining({ where: { id: 'ver_1' } }), - ); - expect(mockDb.ismsDocument.update).not.toHaveBeenCalled(); }); - it('reverts an approved document to draft so it needs re-approval', async () => { + it('reverts an approved document to draft, then writes the narrative', async () => { (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ id: 'doc_1', type: 'isms_scope', @@ -132,12 +112,6 @@ describe('IsmsNarrativeService', () => { (mockDb.ismsDocument.findUnique as jest.Mock).mockResolvedValue({ status: 'approved', }); - (mockDb.ismsDocumentVersion.findFirst as jest.Mock).mockResolvedValue({ - id: 'ver_1', - }); - (mockDb.ismsDocumentVersion.update as jest.Mock).mockResolvedValue({ - id: 'ver_1', - }); await service.save({ documentId: 'doc_1', @@ -145,9 +119,19 @@ describe('IsmsNarrativeService', () => { narrative: validScope, }); - expect(mockDb.ismsDocument.update).toHaveBeenCalledWith({ + // First the approval invalidation, then the narrative write — both in-tx. + expect(mockDb.ismsDocument.update).toHaveBeenNthCalledWith(1, { where: { id: 'doc_1' }, data: { status: 'draft', approvedAt: null, approverId: null }, }); + expect(mockDb.ismsDocument.update).toHaveBeenNthCalledWith(2, { + where: { id: 'doc_1' }, + data: { + draftNarrative: expect.objectContaining({ + certificateScopeSentence: 'The ISMS covers Acme.', + }), + }, + select: { id: true, draftNarrative: true }, + }); }); }); diff --git a/apps/api/src/isms/isms-narrative.service.ts b/apps/api/src/isms/isms-narrative.service.ts index fd06a3ceea..6194395abd 100644 --- a/apps/api/src/isms/isms-narrative.service.ts +++ b/apps/api/src/isms/isms-narrative.service.ts @@ -10,8 +10,10 @@ import { invalidateApprovalIfNeeded } from './utils/approval'; /** * Saves the singleton-document narrative (clauses 4.3 ISMS Scope and 5.1 - * Leadership Commitment) into the document's latest version. The payload is - * validated against the per-type Zod schema before persisting. + * Leadership Commitment) onto the document's working DRAFT (`draftNarrative`). + * The payload is validated against the per-type Zod schema before persisting. + * CS-701: the draft narrative lives on IsmsDocument, not the version row, so an + * edit never touches a published version. */ @Injectable() export class IsmsNarrativeService { @@ -51,27 +53,16 @@ export class IsmsNarrativeService { JSON.stringify(parsed.data), ); - // The latest-version read, approval invalidation, and the narrative write - // must all be atomic: reading the latest version outside the transaction - // lets a concurrent save update a stale version or hit the unique - // constraint, and a failed write must not leave the document reverted to - // draft without the new content. + // Approval invalidation and the narrative write must be atomic: a failed + // write must not leave the document reverted to draft without the new + // content. Reverting to draft leaves the published `currentVersion` intact — + // v1 stays live while the draft is edited. return db.$transaction(async (tx) => { - const latest = await tx.ismsDocumentVersion.findFirst({ - where: { documentId, isLatest: true }, - }); - await invalidateApprovalIfNeeded({ tx, documentId }); - - if (latest) { - return tx.ismsDocumentVersion.update({ - where: { id: latest.id }, - data: { narrative: value }, - }); - } - - return tx.ismsDocumentVersion.create({ - data: { documentId, version: 1, isLatest: true, narrative: value }, + return tx.ismsDocument.update({ + where: { id: documentId }, + data: { draftNarrative: value }, + select: { id: true, draftNarrative: true }, }); }); } diff --git a/apps/api/src/isms/isms-version.service.export.spec.ts b/apps/api/src/isms/isms-version.service.export.spec.ts new file mode 100644 index 0000000000..3ee273d6b8 --- /dev/null +++ b/apps/api/src/isms/isms-version.service.export.spec.ts @@ -0,0 +1,183 @@ +import { Logger, NotFoundException } from '@nestjs/common'; +import { db } from '@db'; +import type { AttachmentsService } from '../attachments/attachments.service'; +import { IsmsVersionService } from './isms-version.service'; +import type { IsmsExportSnapshot } from './utils/export-payload'; +import { + parseExportSnapshot, + renderLiveExport, + renderSnapshot, +} from './utils/export-payload'; + +jest.mock('@db', () => ({ + db: { + ismsDocumentVersion: { + findFirst: jest.fn(), + update: jest.fn(), + }, + }, +})); +jest.mock('./utils/export-payload', () => ({ + buildExportInput: jest.fn(() => ({ rows: [] })), + parseExportSnapshot: jest.fn(), + renderLiveExport: jest.fn(), + renderSnapshot: jest.fn(), + resolveOrgProfile: jest.fn(), +})); +jest.mock('./utils/export-metadata', () => ({ + buildExportMetadata: jest.fn(() => ({ version: 0 })), +})); + +const mockDb = jest.mocked(db); +const mockRenderSnapshot = jest.mocked(renderSnapshot); +const mockRenderLive = jest.mocked(renderLiveExport); +const mockParse = jest.mocked(parseExportSnapshot); + +describe('IsmsVersionService export/render', () => { + let service: IsmsVersionService; + const attachments = { + getObjectBuffer: jest.fn(), + uploadBuffer: jest.fn(), + } as unknown as AttachmentsService; + + beforeEach(() => { + jest.clearAllMocks(); + jest.spyOn(Logger.prototype, 'error').mockImplementation(() => undefined); + service = new IsmsVersionService(attachments); + }); + + describe('getVersionExport', () => { + const args = { + documentId: 'doc_1', + organizationId: 'org_1', + versionId: 'isms_ver_1', + format: 'pdf' as const, + }; + + it('throws NotFoundException when the version is missing', async () => { + (mockDb.ismsDocumentVersion.findFirst as jest.Mock).mockResolvedValue( + null, + ); + await expect(service.getVersionExport(args)).rejects.toThrow( + NotFoundException, + ); + }); + + it('serves the stored file from S3 when a rendered key exists', async () => { + (mockDb.ismsDocumentVersion.findFirst as jest.Mock).mockResolvedValue({ + version: 3, + pdfUrl: 'org/isms/doc/v3.pdf', + docxUrl: null, + contentSnapshot: {}, + document: { title: 'Context Doc' }, + }); + const buffer = Buffer.from('stored-pdf'); + (attachments.getObjectBuffer as jest.Mock).mockResolvedValue(buffer); + + const result = await service.getVersionExport(args); + + expect(attachments.getObjectBuffer).toHaveBeenCalledWith( + 'org/isms/doc/v3.pdf', + ); + expect(result).toEqual({ + fileBuffer: buffer, + mimeType: 'application/pdf', + filename: 'context-doc-v3.pdf', + }); + expect(mockRenderSnapshot).not.toHaveBeenCalled(); + }); + + it('re-renders from the content snapshot when no stored file exists', async () => { + (mockDb.ismsDocumentVersion.findFirst as jest.Mock).mockResolvedValue({ + version: 2, + pdfUrl: null, + docxUrl: null, + contentSnapshot: { type: 'x' }, + document: { title: 'Doc' }, + }); + const snapshot = { type: 'x' } as unknown as IsmsExportSnapshot; + mockParse.mockReturnValue(snapshot); + const rendered = { + fileBuffer: Buffer.from('re'), + mimeType: 'application/pdf', + filename: 'doc-v2.pdf', + }; + mockRenderSnapshot.mockResolvedValue(rendered); + + const result = await service.getVersionExport(args); + + expect(attachments.getObjectBuffer).not.toHaveBeenCalled(); + expect(mockRenderSnapshot).toHaveBeenCalledWith(snapshot, 'pdf'); + expect(result).toBe(rendered); + }); + + it('falls back to a live render for a legacy version with no snapshot', async () => { + (mockDb.ismsDocumentVersion.findFirst as jest.Mock).mockResolvedValue({ + version: 1, + pdfUrl: null, + docxUrl: null, + contentSnapshot: null, + document: { title: 'Doc' }, + }); + mockParse.mockReturnValue(null); + const live = { + fileBuffer: Buffer.from('live'), + mimeType: 'application/pdf', + filename: 'doc-v1.pdf', + }; + mockRenderLive.mockResolvedValue(live); + + const result = await service.getVersionExport(args); + + expect(mockRenderSnapshot).not.toHaveBeenCalled(); + expect(mockRenderLive).toHaveBeenCalledWith({ + documentId: 'doc_1', + organizationId: 'org_1', + format: 'pdf', + }); + expect(result).toBe(live); + }); + }); + + describe('publishRenders', () => { + const args = { + organizationId: 'org_1', + documentId: 'doc_1', + versionId: 'isms_ver_1', + version: 2, + snapshot: {} as IsmsExportSnapshot, + }; + + it('uploads both formats and patches the version with the keys', async () => { + mockRenderSnapshot + .mockResolvedValueOnce({ + fileBuffer: Buffer.from('pdf'), + mimeType: 'application/pdf', + filename: 'a.pdf', + }) + .mockResolvedValueOnce({ + fileBuffer: Buffer.from('docx'), + mimeType: 'docx-mime', + filename: 'a.docx', + }); + + await service.publishRenders(args); + + expect(attachments.uploadBuffer).toHaveBeenCalledTimes(2); + const update = (mockDb.ismsDocumentVersion.update as jest.Mock).mock + .calls[0][0]; + expect(update.where).toEqual({ id: 'isms_ver_1' }); + expect(update.data.pdfUrl).toEqual(expect.stringContaining('.pdf')); + expect(update.data.docxUrl).toEqual(expect.stringContaining('.docx')); + }); + + it('swallows render/upload errors and never patches the version', async () => { + mockRenderSnapshot.mockRejectedValue(new Error('render boom')); + + await expect(service.publishRenders(args)).resolves.toBeUndefined(); + + expect(attachments.uploadBuffer).not.toHaveBeenCalled(); + expect(mockDb.ismsDocumentVersion.update).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/apps/api/src/isms/isms-version.service.spec.ts b/apps/api/src/isms/isms-version.service.spec.ts new file mode 100644 index 0000000000..9236a7c921 --- /dev/null +++ b/apps/api/src/isms/isms-version.service.spec.ts @@ -0,0 +1,194 @@ +import { NotFoundException } from '@nestjs/common'; +import { db } from '@db'; +import type { Prisma } from '@db'; +import type { AttachmentsService } from '../attachments/attachments.service'; +import { IsmsVersionService } from './isms-version.service'; +import type { LoadedExportDocument } from './utils/export-payload'; + +// getVersionExport + publishRenders live in isms-version.service.export.spec.ts. +jest.mock('@db', () => ({ + db: { + ismsDocument: { findFirst: jest.fn() }, + ismsDocumentVersion: { + findFirst: jest.fn(), + findMany: jest.fn(), + updateMany: jest.fn(), + create: jest.fn(), + }, + }, +})); +jest.mock('./utils/export-payload', () => ({ + buildExportInput: jest.fn(() => ({ rows: [] })), + resolveOrgProfile: jest.fn(), +})); +jest.mock('./utils/export-metadata', () => ({ + buildExportMetadata: jest.fn(() => ({ version: 0 })), +})); + +const mockDb = jest.mocked(db); + +function asTx(mock: unknown): Prisma.TransactionClient { + return mock as Prisma.TransactionClient; +} + +const buildDocument = ( + over: Partial = {}, +): LoadedExportDocument => + ({ + id: 'doc_1', + type: 'context_of_organization', + title: 'Context Doc', + preparedBy: 'Comp AI', + framework: { name: 'ISO 27001' }, + organization: { name: 'Acme', primaryColor: '#004D3D' }, + approver: { user: { name: 'Jane', email: 'jane@acme.io' } }, + draftNarrative: null, + ...over, + }) as unknown as LoadedExportDocument; + +describe('IsmsVersionService', () => { + let service: IsmsVersionService; + const attachments = { + getObjectBuffer: jest.fn(), + uploadBuffer: jest.fn(), + } as unknown as AttachmentsService; + + beforeEach(() => { + jest.clearAllMocks(); + service = new IsmsVersionService(attachments); + }); + + describe('createPublishedVersion', () => { + const buildTx = (lastVersion: number | null) => ({ + ismsDocumentVersion: { + findFirst: jest + .fn() + .mockResolvedValue( + lastVersion == null ? null : { version: lastVersion }, + ), + updateMany: jest.fn().mockResolvedValue({ count: 1 }), + create: jest.fn().mockResolvedValue({ id: 'isms_ver_new' }), + }, + }); + const now = new Date('2026-07-07T00:00:00.000Z'); + + it('computes the next version, demotes prior latest and creates the row', async () => { + const tx = buildTx(2); + + const result = await service.createPublishedVersion({ + tx: asTx(tx), + document: buildDocument({ draftNarrative: { statement: 's' } }), + memberId: 'mem_1', + now, + snapshotData: { src: true }, + }); + + // next = max(2) + 1 + expect(tx.ismsDocumentVersion.findFirst).toHaveBeenCalledWith( + expect.objectContaining({ orderBy: { version: 'desc' } }), + ); + // one-latest-per-document invariant: prior latest demoted first + expect(tx.ismsDocumentVersion.updateMany).toHaveBeenCalledWith({ + where: { documentId: 'doc_1', isLatest: true }, + data: { isLatest: false }, + }); + const createData = tx.ismsDocumentVersion.create.mock.calls[0][0].data; + expect(createData).toMatchObject({ + documentId: 'doc_1', + version: 3, + isLatest: true, + publishedById: 'mem_1', + publishedAt: now, + }); + expect(createData.narrative).toEqual({ statement: 's' }); + expect(createData.contentSnapshot).toBeDefined(); + expect(createData.sourceSnapshot).toEqual({ src: true }); + expect(result).toEqual({ + versionId: 'isms_ver_new', + version: 3, + snapshot: expect.objectContaining({ type: 'context_of_organization' }), + }); + }); + + it('starts at version 1 when the document has no prior versions', async () => { + const tx = buildTx(null); + + const result = await service.createPublishedVersion({ + tx: asTx(tx), + document: buildDocument(), + memberId: 'mem_1', + now, + snapshotData: {}, + }); + + expect(tx.ismsDocumentVersion.create.mock.calls[0][0].data.version).toBe( + 1, + ); + // Empty draft narrative falls back to {}. + expect(tx.ismsDocumentVersion.create.mock.calls[0][0].data.narrative).toEqual( + {}, + ); + expect(result.version).toBe(1); + }); + }); + + describe('getVersions', () => { + const args = { documentId: 'doc_1', organizationId: 'org_1' }; + + it('throws NotFoundException when the document is missing', async () => { + (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue(null); + await expect(service.getVersions(args)).rejects.toThrow(NotFoundException); + }); + + it('lists published versions, mapping name, flags and isCurrent', async () => { + (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ + currentVersionId: 'isms_ver_2', + }); + (mockDb.ismsDocumentVersion.findMany as jest.Mock).mockResolvedValue([ + { + id: 'isms_ver_2', + version: 2, + publishedAt: new Date('2026-07-07T00:00:00.000Z'), + changelog: 'second', + pdfUrl: 'k.pdf', + docxUrl: null, + publishedBy: { user: { name: 'Jane', email: 'jane@acme.io' } }, + }, + { + id: 'isms_ver_1', + version: 1, + publishedAt: new Date('2026-07-06T00:00:00.000Z'), + changelog: null, + pdfUrl: null, + docxUrl: null, + publishedBy: { user: { name: null, email: 'bob@acme.io' } }, + }, + ]); + + const result = await service.getVersions(args); + + // Only published rows are queried, newest first. + expect(mockDb.ismsDocumentVersion.findMany).toHaveBeenCalledWith( + expect.objectContaining({ + where: { documentId: 'doc_1', publishedAt: { not: null } }, + orderBy: { version: 'desc' }, + }), + ); + expect(result.currentVersionId).toBe('isms_ver_2'); + expect(result.versions[0]).toMatchObject({ + id: 'isms_ver_2', + version: 2, + publishedByName: 'Jane', + hasPdf: true, + hasDocx: false, + isCurrent: true, + }); + // Falls back to email when the name is null; not the current version. + expect(result.versions[1]).toMatchObject({ + publishedByName: 'bob@acme.io', + hasPdf: false, + isCurrent: false, + }); + }); + }); +}); diff --git a/apps/api/src/isms/isms-version.service.ts b/apps/api/src/isms/isms-version.service.ts new file mode 100644 index 0000000000..05aaa3dde3 --- /dev/null +++ b/apps/api/src/isms/isms-version.service.ts @@ -0,0 +1,266 @@ +import { Injectable, Logger, NotFoundException } from '@nestjs/common'; +import { db } from '@db'; +import type { Prisma } from '@db'; +import { AttachmentsService } from '../attachments/attachments.service'; +import { buildExportMetadata } from './utils/export-metadata'; +import { + DOCX_MIME_TYPE, + type IsmsExportFormat, +} from './utils/export-shared'; +import { + sanitizeExportName, + type IsmsExportResult, +} from './utils/export-generator'; +import { + buildExportInput, + parseExportSnapshot, + renderLiveExport, + renderSnapshot, + resolveOrgProfile, + type IsmsExportSnapshot, + type LoadedExportDocument, +} from './utils/export-payload'; + +/** True when a JSON value is a non-empty plain object (a saved narrative). */ +function isNarrativeObject(value: Prisma.JsonValue | null): boolean { + return ( + value != null && + typeof value === 'object' && + !Array.isArray(value) && + Object.keys(value).length > 0 + ); +} + +/** + * ISMS document version lifecycle (CS-701): freeze the working draft into an + * immutable published version at approval, list published-version history, and + * serve a specific version's export (stored file first, snapshot re-render as + * fallback). Document approval/decline live in IsmsService; live-draft export in + * IsmsContextService. + */ +@Injectable() +export class IsmsVersionService { + private readonly logger = new Logger(IsmsVersionService.name); + + constructor(private readonly attachments: AttachmentsService) {} + + /** + * Freeze the current draft into a new published version row inside `tx`. The + * caller sets `IsmsDocument.currentVersionId` to the returned id. Rendering + + * S3 upload happen AFTER the transaction via publishRenders, so a render/upload + * failure never orphans a DB row (mirrors the Policies publish flow). + */ + async createPublishedVersion({ + tx, + document, + memberId, + now, + snapshotData, + changelog, + }: { + tx: Prisma.TransactionClient; + document: LoadedExportDocument; + memberId: string; + now: Date; + snapshotData: unknown; + changelog?: string | null; + }): Promise<{ versionId: string; version: number; snapshot: IsmsExportSnapshot }> { + const nextVersion = await this.nextVersion(tx, document.id); + + const orgProfile = await resolveOrgProfile(document); + const input = buildExportInput({ document, orgProfile }); + const metadata = buildExportMetadata({ + type: document.type, + title: document.title, + frameworkName: document.framework.name || 'ISO 27001', + version: nextVersion, + status: 'approved', + preparedBy: document.preparedBy, + owner: null, + approverName: + document.approver?.user?.name || + document.approver?.user?.email || + null, + approvedAt: now, + declinedAt: null, + organizationName: document.organization.name, + primaryColor: document.organization.primaryColor, + }); + const snapshot: IsmsExportSnapshot = { type: document.type, input, metadata }; + + // Keep the one-latest-per-document invariant (partial unique index). + await tx.ismsDocumentVersion.updateMany({ + where: { documentId: document.id, isLatest: true }, + data: { isLatest: false }, + }); + + const narrative: Prisma.InputJsonValue = isNarrativeObject( + document.draftNarrative, + ) + ? JSON.parse(JSON.stringify(document.draftNarrative)) + : {}; + + const created = await tx.ismsDocumentVersion.create({ + data: { + documentId: document.id, + version: nextVersion, + isLatest: true, + narrative, + contentSnapshot: JSON.parse(JSON.stringify(snapshot)), + sourceSnapshot: JSON.parse(JSON.stringify(snapshotData)), + publishedById: memberId, + publishedAt: now, + changelog: changelog ?? null, + }, + select: { id: true }, + }); + + return { versionId: created.id, version: nextVersion, snapshot }; + } + + /** + * Render both formats and upload them to S3, then store the keys on the version. + * Best-effort: a failure is logged, not thrown — historical export falls back to + * re-rendering from the version's contentSnapshot. + */ + async publishRenders({ + organizationId, + documentId, + versionId, + version, + snapshot, + }: { + organizationId: string; + documentId: string; + versionId: string; + version: number; + snapshot: IsmsExportSnapshot; + }): Promise { + try { + const [pdf, docx] = await Promise.all([ + renderSnapshot(snapshot, 'pdf'), + renderSnapshot(snapshot, 'docx'), + ]); + const base = `${organizationId}/isms/${documentId}/v${version}-${Date.now()}`; + const pdfKey = `${base}.pdf`; + const docxKey = `${base}.docx`; + await Promise.all([ + this.attachments.uploadBuffer(pdfKey, pdf.fileBuffer, pdf.mimeType), + this.attachments.uploadBuffer(docxKey, docx.fileBuffer, docx.mimeType), + ]); + await db.ismsDocumentVersion.update({ + where: { id: versionId }, + data: { pdfUrl: pdfKey, docxUrl: docxKey }, + }); + } catch (err) { + this.logger.error( + `ISMS version ${versionId} render/upload failed; export will fall back to on-demand render`, + err instanceof Error ? err.stack : String(err), + ); + } + } + + /** List a document's published versions (history), newest first. */ + async getVersions({ + documentId, + organizationId, + }: { + documentId: string; + organizationId: string; + }) { + const document = await db.ismsDocument.findFirst({ + where: { id: documentId, organizationId }, + select: { currentVersionId: true }, + }); + if (!document) { + throw new NotFoundException('ISMS document not found'); + } + + const versions = await db.ismsDocumentVersion.findMany({ + where: { documentId, publishedAt: { not: null } }, + orderBy: { version: 'desc' }, + select: { + id: true, + version: true, + publishedAt: true, + changelog: true, + pdfUrl: true, + docxUrl: true, + publishedBy: { select: { user: { select: { name: true, email: true } } } }, + }, + }); + + return { + currentVersionId: document.currentVersionId, + versions: versions.map((v) => ({ + id: v.id, + version: v.version, + publishedAt: v.publishedAt, + changelog: v.changelog, + publishedByName: + v.publishedBy?.user?.name || v.publishedBy?.user?.email || null, + hasPdf: v.pdfUrl != null, + hasDocx: v.docxUrl != null, + isCurrent: v.id === document.currentVersionId, + })), + }; + } + + /** + * Export a specific published version: serve its stored file (byte-identical to + * what was approved), else re-render from its snapshot, else — for a legacy + * migrated version with neither — render from current live data. + */ + async getVersionExport({ + documentId, + organizationId, + versionId, + format, + }: { + documentId: string; + organizationId: string; + versionId: string; + format: IsmsExportFormat; + }): Promise { + const version = await db.ismsDocumentVersion.findFirst({ + where: { id: versionId, documentId, document: { organizationId } }, + select: { + version: true, + pdfUrl: true, + docxUrl: true, + contentSnapshot: true, + document: { select: { title: true } }, + }, + }); + if (!version) { + throw new NotFoundException('ISMS document version not found'); + } + + const key = format === 'pdf' ? version.pdfUrl : version.docxUrl; + if (key) { + const fileBuffer = await this.attachments.getObjectBuffer(key); + return { + fileBuffer, + mimeType: format === 'pdf' ? 'application/pdf' : DOCX_MIME_TYPE, + filename: `${sanitizeExportName(version.document.title)}-v${version.version}.${format}`, + }; + } + + const snapshot = parseExportSnapshot(version.contentSnapshot); + if (snapshot) return renderSnapshot(snapshot, format); + + return renderLiveExport({ documentId, organizationId, format }); + } + + private async nextVersion( + tx: Prisma.TransactionClient, + documentId: string, + ): Promise { + const last = await tx.ismsDocumentVersion.findFirst({ + where: { documentId }, + orderBy: { version: 'desc' }, + select: { version: true }, + }); + return (last?.version ?? 0) + 1; + } +} diff --git a/apps/api/src/isms/isms.controller.permissions.spec.ts b/apps/api/src/isms/isms.controller.permissions.spec.ts index 145432787d..7a9820255a 100644 --- a/apps/api/src/isms/isms.controller.permissions.spec.ts +++ b/apps/api/src/isms/isms.controller.permissions.spec.ts @@ -29,6 +29,9 @@ jest.mock('./isms.service', () => ({ jest.mock('./isms-context.service', () => ({ IsmsContextService: class MockIsmsContextService {}, })); +jest.mock('./isms-version.service', () => ({ + IsmsVersionService: class MockIsmsVersionService {}, +})); jest.mock('./isms-document-control.service', () => ({ IsmsDocumentControlService: class MockIsmsDocumentControlService {}, })); @@ -51,6 +54,9 @@ describe('IsmsController permission metadata', () => { expect(permissionsFor('drift')).toEqual([ { resource: 'evidence', actions: ['read'] }, ]); + expect(permissionsFor('getVersions')).toEqual([ + { resource: 'evidence', actions: ['read'] }, + ]); expect(permissionsFor('exportDocument')).toEqual([ { resource: 'evidence', actions: ['read'] }, ]); diff --git a/apps/api/src/isms/isms.controller.spec.ts b/apps/api/src/isms/isms.controller.spec.ts index fde926f23d..f01ce15224 100644 --- a/apps/api/src/isms/isms.controller.spec.ts +++ b/apps/api/src/isms/isms.controller.spec.ts @@ -8,6 +8,7 @@ import type { AuthContext as AuthContextType } from '../auth/types'; import { IsmsController } from './isms.controller'; import { IsmsService } from './isms.service'; import { IsmsContextService } from './isms-context.service'; +import { IsmsVersionService } from './isms-version.service'; import { IsmsDocumentControlService } from './isms-document-control.service'; const mockResolveRolePermissions = jest.mocked(resolveRolePermissions); @@ -54,6 +55,9 @@ jest.mock('./isms.service', () => ({ jest.mock('./isms-context.service', () => ({ IsmsContextService: class MockIsmsContextService {}, })); +jest.mock('./isms-version.service', () => ({ + IsmsVersionService: class MockIsmsVersionService {}, +})); jest.mock('./isms-document-control.service', () => ({ IsmsDocumentControlService: class MockIsmsDocumentControlService {}, })); @@ -73,6 +77,9 @@ describe('IsmsController', () => { drift: jest.fn(), exportDocument: jest.fn(), }; + const mockVersionService = { + getVersions: jest.fn(), + }; const mockDocumentControlService = { addControls: jest.fn(), removeControl: jest.fn(), @@ -86,6 +93,7 @@ describe('IsmsController', () => { providers: [ { provide: IsmsService, useValue: mockIsmsService }, { provide: IsmsContextService, useValue: mockContextService }, + { provide: IsmsVersionService, useValue: mockVersionService }, { provide: IsmsDocumentControlService, useValue: mockDocumentControlService, @@ -311,6 +319,20 @@ describe('IsmsController', () => { }); }); + it('getVersions delegates to the version service', async () => { + mockVersionService.getVersions.mockResolvedValue({ + currentVersionId: 'isms_ver_1', + versions: [], + }); + + await controller.getVersions('doc_1', 'org_1'); + + expect(mockVersionService.getVersions).toHaveBeenCalledWith({ + documentId: 'doc_1', + organizationId: 'org_1', + }); + }); + it('exportDocument sets headers and sends the buffer', async () => { const fileBuffer = Buffer.from('pdf-data'); mockContextService.exportDocument.mockResolvedValue({ diff --git a/apps/api/src/isms/isms.controller.ts b/apps/api/src/isms/isms.controller.ts index 0cdbbec885..00934ff575 100644 --- a/apps/api/src/isms/isms.controller.ts +++ b/apps/api/src/isms/isms.controller.ts @@ -32,6 +32,7 @@ import { resolveRolePermissions, permissionsGrant } from '../auth/app-access'; import { resolveServiceByName } from '../auth/service-token.config'; import { IsmsService } from './isms.service'; import { IsmsContextService } from './isms-context.service'; +import { IsmsVersionService } from './isms-version.service'; import { IsmsDocumentControlService } from './isms-document-control.service'; import { EnsureIsmsSetupDto } from './dto/ensure-isms-setup.dto'; import { SubmitIsmsForApprovalDto } from './dto/submit-isms-for-approval.dto'; @@ -46,6 +47,7 @@ export class IsmsController { constructor( private readonly ismsService: IsmsService, private readonly contextService: IsmsContextService, + private readonly versionService: IsmsVersionService, private readonly documentControlService: IsmsDocumentControlService, ) {} @@ -222,6 +224,19 @@ export class IsmsController { return this.contextService.drift({ documentId: id, organizationId }); } + @Get('documents/:id/versions') + @RequirePermission('evidence', 'read') + @ApiOperation({ + summary: "List an ISMS document's published version history", + }) + @ApiOkResponse({ description: 'Published versions, newest first' }) + async getVersions( + @Param('id') id: string, + @OrganizationId() organizationId: string, + ) { + return this.versionService.getVersions({ documentId: id, organizationId }); + } + @Post('documents/:id/export') @RequirePermission('evidence', 'read') @ApiOperation({ summary: 'Export an ISMS document as PDF or DOCX' }) diff --git a/apps/api/src/isms/isms.module.ts b/apps/api/src/isms/isms.module.ts index 88a134904a..9d894b737a 100644 --- a/apps/api/src/isms/isms.module.ts +++ b/apps/api/src/isms/isms.module.ts @@ -3,6 +3,7 @@ import { IsmsController } from './isms.controller'; import { IsmsRegistersController } from './isms-registers.controller'; import { IsmsService } from './isms.service'; import { IsmsContextService } from './isms-context.service'; +import { IsmsVersionService } from './isms-version.service'; import { IsmsContextIssueService } from './isms-context-issue.service'; import { IsmsDocumentControlService } from './isms-document-control.service'; import { IsmsInterestedPartyService } from './isms-interested-party.service'; @@ -12,9 +13,11 @@ import { IsmsNarrativeService } from './isms-narrative.service'; import { IsmsProfileController } from './wizard/isms-profile.controller'; import { IsmsProfileService } from './wizard/isms-profile.service'; import { AuthModule } from '../auth/auth.module'; +import { AttachmentsModule } from '../attachments/attachments.module'; @Module({ - imports: [AuthModule], + // AttachmentsModule: S3 access for retaining per-version rendered exports. + imports: [AuthModule, AttachmentsModule], controllers: [ IsmsController, IsmsRegistersController, @@ -23,6 +26,7 @@ import { AuthModule } from '../auth/auth.module'; providers: [ IsmsService, IsmsContextService, + IsmsVersionService, IsmsContextIssueService, IsmsDocumentControlService, IsmsInterestedPartyService, @@ -34,6 +38,7 @@ import { AuthModule } from '../auth/auth.module'; exports: [ IsmsService, IsmsContextService, + IsmsVersionService, IsmsContextIssueService, IsmsDocumentControlService, IsmsInterestedPartyService, diff --git a/apps/api/src/isms/isms.service.approve.spec.ts b/apps/api/src/isms/isms.service.approve.spec.ts new file mode 100644 index 0000000000..cdedf89f6e --- /dev/null +++ b/apps/api/src/isms/isms.service.approve.spec.ts @@ -0,0 +1,181 @@ +import { + BadRequestException, + ForbiddenException, + NotFoundException, +} from '@nestjs/common'; +import { db } from '@db'; +import { IsmsService } from './isms.service'; +import type { IsmsVersionService } from './isms-version.service'; +import { collectPlatformData } from './documents/data-source'; +import { runDerivation } from './documents/generate'; +import { updateDraftSnapshot } from './utils/draft-snapshot'; + +jest.mock('@db', () => ({ + db: { + ismsDocument: { findFirst: jest.fn() }, + member: { findFirst: jest.fn() }, + $transaction: jest.fn(), + }, +})); +jest.mock('./documents/data-source', () => ({ + collectPlatformData: jest.fn(), +})); +jest.mock('./documents/generate', () => ({ + runDerivation: jest.fn(), +})); +jest.mock('./utils/draft-snapshot', () => ({ + updateDraftSnapshot: jest.fn(), +})); + +const mockDb = jest.mocked(db); +const mockCollect = jest.mocked(collectPlatformData); +const mockRunDerivation = jest.mocked(runDerivation); +const mockUpdateDraft = jest.mocked(updateDraftSnapshot); + +const versionService = { + createPublishedVersion: jest.fn(), + publishRenders: jest.fn(), + getVersions: jest.fn(), + getVersionExport: jest.fn(), +} as unknown as IsmsVersionService; + +const platformData = { + organizationName: 'Acme', + frameworkNames: ['ISO 27001'], + vendorCount: 1, + subProcessorCount: 0, + vendorsByCategory: {}, + subProcessorNames: [], + infraVendorNames: [], + memberCount: 1, + membersByDepartment: {}, + deviceCount: 0, + riskCount: 0, + highRiskCount: 0, + hasTrainingProgram: false, + wizardAnswers: {}, + partiesFingerprint: '', +}; + +describe('IsmsService.approve (CS-701 versioning)', () => { + let service: IsmsService; + const args = { + documentId: 'doc_1', + organizationId: 'org_1', + userId: 'usr_1', + }; + + beforeEach(() => { + jest.clearAllMocks(); + service = new IsmsService(versionService); + }); + + it('throws NotFoundException when member not found', async () => { + (mockDb.member.findFirst as jest.Mock).mockResolvedValue(null); + await expect(service.approve(args)).rejects.toThrow(NotFoundException); + }); + + it('throws BadRequestException when document is not pending approval', async () => { + (mockDb.member.findFirst as jest.Mock).mockResolvedValue({ id: 'mem_1' }); + (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ + id: 'doc_1', + status: 'draft', + approverId: 'mem_1', + frameworkId: 'fw_1', + }); + await expect(service.approve(args)).rejects.toThrow(BadRequestException); + }); + + it('throws ForbiddenException when no approver is assigned', async () => { + (mockDb.member.findFirst as jest.Mock).mockResolvedValue({ id: 'mem_1' }); + (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ + id: 'doc_1', + status: 'needs_review', + approverId: null, + frameworkId: 'fw_1', + }); + await expect(service.approve(args)).rejects.toThrow(ForbiddenException); + }); + + it('throws ForbiddenException when not the assigned approver', async () => { + (mockDb.member.findFirst as jest.Mock).mockResolvedValue({ id: 'mem_1' }); + (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ + id: 'doc_1', + status: 'needs_review', + approverId: 'mem_other', + frameworkId: 'fw_1', + }); + await expect(service.approve(args)).rejects.toThrow(ForbiddenException); + }); + + it('freezes a published version, promotes it and marks approved', async () => { + (mockDb.member.findFirst as jest.Mock).mockResolvedValue({ id: 'mem_1' }); + const reloaded = { id: 'doc_1', type: 'context_of_organization' }; + (mockDb.ismsDocument.findFirst as jest.Mock) + .mockResolvedValueOnce({ + id: 'doc_1', + status: 'needs_review', + approverId: 'mem_1', + frameworkId: 'fw_1', + type: 'context_of_organization', + }) + .mockResolvedValueOnce({ id: 'doc_1', status: 'approved' }); + mockCollect.mockResolvedValue(platformData); + const tx = { + ismsDocument: { + findUniqueOrThrow: jest.fn().mockResolvedValue(reloaded), + update: jest.fn().mockResolvedValue({}), + }, + }; + (mockDb.$transaction as jest.Mock).mockImplementation((cb) => cb(tx)); + (versionService.createPublishedVersion as jest.Mock).mockResolvedValue({ + versionId: 'isms_ver_1', + version: 1, + snapshot: {}, + }); + (versionService.publishRenders as jest.Mock).mockResolvedValue(undefined); + + await service.approve(args); + + expect(mockCollect).toHaveBeenCalledWith({ + organizationId: 'org_1', + frameworkId: 'fw_1', + }); + // Re-derives in-tx from the same snapshot: persisted rows and the frozen + // version come from one pass. + expect(mockRunDerivation).toHaveBeenCalledWith({ + tx, + type: 'context_of_organization', + documentId: 'doc_1', + organizationId: 'org_1', + frameworkId: 'fw_1', + data: expect.objectContaining({ organizationName: 'Acme' }), + }); + // The drift baseline is refreshed on the document (CS-701). + expect(mockUpdateDraft).toHaveBeenCalledWith( + expect.objectContaining({ tx, documentId: 'doc_1' }), + ); + // A published version is created from the reloaded (re-derived) document. + expect(versionService.createPublishedVersion).toHaveBeenCalledWith( + expect.objectContaining({ tx, document: reloaded, memberId: 'mem_1' }), + ); + // The document is promoted to the freshly-created version. + expect(tx.ismsDocument.update).toHaveBeenCalledWith({ + where: { id: 'doc_1' }, + data: expect.objectContaining({ + status: 'approved', + declinedAt: null, + currentVersionId: 'isms_ver_1', + }), + }); + // Renders + uploads happen AFTER the transaction (Policies pattern). + expect(versionService.publishRenders).toHaveBeenCalledWith( + expect.objectContaining({ + organizationId: 'org_1', + documentId: 'doc_1', + versionId: 'isms_ver_1', + version: 1, + }), + ); + }); +}); diff --git a/apps/api/src/isms/isms.service.ensure-setup-fallback.spec.ts b/apps/api/src/isms/isms.service.ensure-setup-fallback.spec.ts index ce1dd0a85e..318f0adfe4 100644 --- a/apps/api/src/isms/isms.service.ensure-setup-fallback.spec.ts +++ b/apps/api/src/isms/isms.service.ensure-setup-fallback.spec.ts @@ -1,5 +1,6 @@ import { db } from '@db'; import { IsmsService } from './isms.service'; +import type { IsmsVersionService } from './isms-version.service'; jest.mock('@db', () => ({ db: { @@ -19,12 +20,12 @@ jest.mock('./documents/data-source', () => ({ jest.mock('./documents/generate', () => ({ runDerivation: jest.fn(), })); -jest.mock('./utils/version-snapshot', () => ({ - upsertLatestSnapshotVersion: jest.fn(), -})); const mockDb = jest.mocked(db); +// ensureSetup never touches the version service; a bare stub satisfies the ctor. +const versionService = {} as unknown as IsmsVersionService; + /** Convenience accessor for the first createMany call's `data` array. */ const createManyData = () => (mockDb.ismsDocument.createMany as jest.Mock).mock.calls[0][0].data; @@ -37,7 +38,7 @@ describe('IsmsService ensureSetup fallback to ISMS_TYPE_DEFINITIONS (no template beforeEach(() => { jest.clearAllMocks(); - service = new IsmsService(); + service = new IsmsService(versionService); (mockDb.control.findMany as jest.Mock).mockResolvedValue([]); (mockDb.ismsDocument.createMany as jest.Mock).mockResolvedValue({ count: 0, diff --git a/apps/api/src/isms/isms.service.lifecycle.spec.ts b/apps/api/src/isms/isms.service.lifecycle.spec.ts index 41ea994e3a..80e1311c5e 100644 --- a/apps/api/src/isms/isms.service.lifecycle.spec.ts +++ b/apps/api/src/isms/isms.service.lifecycle.spec.ts @@ -5,10 +5,10 @@ import { } from '@nestjs/common'; import { db } from '@db'; import { IsmsService } from './isms.service'; -import { collectPlatformData } from './documents/data-source'; -import { runDerivation } from './documents/generate'; -import { upsertLatestSnapshotVersion } from './utils/version-snapshot'; +import type { IsmsVersionService } from './isms-version.service'; +// approve() (the CS-701 freeze/version flow) is covered in +// isms.service.approve.spec.ts. jest.mock('@db', () => ({ db: { ismsDocument: { @@ -19,26 +19,22 @@ jest.mock('@db', () => ({ $transaction: jest.fn(), }, })); -jest.mock('./documents/data-source', () => ({ - collectPlatformData: jest.fn(), -})); -jest.mock('./documents/generate', () => ({ - runDerivation: jest.fn(), -})); -jest.mock('./utils/version-snapshot', () => ({ - upsertLatestSnapshotVersion: jest.fn(), -})); const mockDb = jest.mocked(db); -const mockCollect = jest.mocked(collectPlatformData); -const mockRunDerivation = jest.mocked(runDerivation); + +const versionService = { + createPublishedVersion: jest.fn(), + publishRenders: jest.fn(), + getVersions: jest.fn(), + getVersionExport: jest.fn(), +} as unknown as IsmsVersionService; describe('IsmsService document lifecycle', () => { let service: IsmsService; beforeEach(() => { jest.clearAllMocks(); - service = new IsmsService(); + service = new IsmsService(versionService); }); describe('getDocument', () => { @@ -65,7 +61,7 @@ describe('IsmsService document lifecycle', () => { ); }); - it('includes control links with the linked control id and name', async () => { + it('includes the current version and control links', async () => { (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ id: 'doc_1', controlLinks: [], @@ -80,6 +76,8 @@ describe('IsmsService document lifecycle', () => { controlId: true, control: { select: { id: true, name: true } }, }); + // CS-701: the document carries its live/published version for display. + expect(callArgs.include.currentVersion).toBeDefined(); }); }); @@ -121,111 +119,6 @@ describe('IsmsService document lifecycle', () => { }); }); - describe('approve', () => { - const args = { - documentId: 'doc_1', - organizationId: 'org_1', - userId: 'usr_1', - }; - - it('throws NotFoundException when member not found', async () => { - (mockDb.member.findFirst as jest.Mock).mockResolvedValue(null); - await expect(service.approve(args)).rejects.toThrow(NotFoundException); - }); - - it('throws BadRequestException when document is not pending approval', async () => { - (mockDb.member.findFirst as jest.Mock).mockResolvedValue({ id: 'mem_1' }); - (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ - id: 'doc_1', - status: 'draft', - approverId: 'mem_1', - frameworkId: 'fw_1', - }); - await expect(service.approve(args)).rejects.toThrow(BadRequestException); - }); - - it('throws ForbiddenException when no approver is assigned', async () => { - (mockDb.member.findFirst as jest.Mock).mockResolvedValue({ id: 'mem_1' }); - (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ - id: 'doc_1', - status: 'needs_review', - approverId: null, - frameworkId: 'fw_1', - }); - await expect(service.approve(args)).rejects.toThrow(ForbiddenException); - }); - - it('throws ForbiddenException when not the assigned approver', async () => { - (mockDb.member.findFirst as jest.Mock).mockResolvedValue({ id: 'mem_1' }); - (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ - id: 'doc_1', - status: 'needs_review', - approverId: 'mem_other', - frameworkId: 'fw_1', - }); - await expect(service.approve(args)).rejects.toThrow(ForbiddenException); - }); - - it('snapshots data and marks approved', async () => { - (mockDb.member.findFirst as jest.Mock).mockResolvedValue({ id: 'mem_1' }); - (mockDb.ismsDocument.findFirst as jest.Mock) - .mockResolvedValueOnce({ - id: 'doc_1', - status: 'needs_review', - approverId: 'mem_1', - frameworkId: 'fw_1', - type: 'context_of_organization', - }) - .mockResolvedValueOnce({ id: 'doc_1', status: 'approved' }); - mockCollect.mockResolvedValue({ - organizationName: 'Acme', - frameworkNames: ['ISO 27001'], - vendorCount: 1, - subProcessorCount: 0, - vendorsByCategory: {}, - subProcessorNames: [], - infraVendorNames: [], - memberCount: 1, - membersByDepartment: {}, - deviceCount: 0, - riskCount: 0, - highRiskCount: 0, - hasTrainingProgram: false, - wizardAnswers: {}, - partiesFingerprint: '', - }); - const tx = { - ismsDocument: { update: jest.fn().mockResolvedValue({}) }, - }; - (mockDb.$transaction as jest.Mock).mockImplementation((cb) => cb(tx)); - - await service.approve(args); - - expect(mockCollect).toHaveBeenCalledWith({ - organizationId: 'org_1', - frameworkId: 'fw_1', - }); - // Re-derives inside the transaction from the same snapshot, so the - // persisted rows and the snapshot baseline come from one pass. - expect(mockRunDerivation).toHaveBeenCalledWith({ - tx, - type: 'context_of_organization', - documentId: 'doc_1', - organizationId: 'org_1', - frameworkId: 'fw_1', - data: expect.objectContaining({ organizationName: 'Acme' }), - }); - expect(upsertLatestSnapshotVersion).toHaveBeenCalled(); - expect(tx.ismsDocument.update).toHaveBeenCalledWith({ - where: { id: 'doc_1' }, - data: expect.objectContaining({ - status: 'approved', - declinedAt: null, - }), - }); - }); - }); - describe('decline', () => { const args = { documentId: 'doc_1', diff --git a/apps/api/src/isms/isms.service.spec.ts b/apps/api/src/isms/isms.service.spec.ts index d8a1f34410..af4c4e5d0d 100644 --- a/apps/api/src/isms/isms.service.spec.ts +++ b/apps/api/src/isms/isms.service.spec.ts @@ -1,6 +1,7 @@ import { NotFoundException } from '@nestjs/common'; import { db } from '@db'; import { IsmsService } from './isms.service'; +import type { IsmsVersionService } from './isms-version.service'; jest.mock('@db', () => ({ db: { @@ -20,12 +21,12 @@ jest.mock('./documents/data-source', () => ({ jest.mock('./documents/generate', () => ({ runDerivation: jest.fn(), })); -jest.mock('./utils/version-snapshot', () => ({ - upsertLatestSnapshotVersion: jest.fn(), -})); const mockDb = jest.mocked(db); +// ensureSetup never touches the version service; a bare stub satisfies the ctor. +const versionService = {} as unknown as IsmsVersionService; + /** Convenience accessor for the first createMany call's `data` array. */ const createManyData = () => (mockDb.ismsDocument.createMany as jest.Mock).mock.calls[0][0].data; @@ -35,7 +36,7 @@ describe('IsmsService ensureSetup', () => { beforeEach(() => { jest.clearAllMocks(); - service = new IsmsService(); + service = new IsmsService(versionService); (mockDb.control.findMany as jest.Mock).mockResolvedValue([]); (mockDb.ismsDocument.createMany as jest.Mock).mockResolvedValue({ count: 0, diff --git a/apps/api/src/isms/isms.service.ts b/apps/api/src/isms/isms.service.ts index 4d92c33d3f..15dbc19746 100644 --- a/apps/api/src/isms/isms.service.ts +++ b/apps/api/src/isms/isms.service.ts @@ -9,7 +9,9 @@ import { SubmitIsmsForApprovalDto } from './dto/submit-isms-for-approval.dto'; import { deriveControlLinks, resolveDocumentPlans } from './utils/ensure-setup-plan'; import { collectPlatformData } from './documents/data-source'; import { runDerivation } from './documents/generate'; -import { upsertLatestSnapshotVersion } from './utils/version-snapshot'; +import { updateDraftSnapshot } from './utils/draft-snapshot'; +import { EXPORT_DOCUMENT_INCLUDE } from './utils/export-payload'; +import { IsmsVersionService } from './isms-version.service'; /** * ISMS foundational document lifecycle: setup, retrieval and sign-off. Context @@ -18,6 +20,8 @@ import { upsertLatestSnapshotVersion } from './utils/version-snapshot'; */ @Injectable() export class IsmsService { + constructor(private readonly versionService: IsmsVersionService) {} + /** * List the org's ISMS documents, provisioning missing ones first only when the * caller can write (`evidence:update`). Read-only callers never trigger writes. @@ -61,7 +65,9 @@ export class IsmsService { type: doc.type, status: doc.status, requirementId: doc.requirementId, - hasApprovedVersion: doc.status === 'approved', + // A published version exists (the document has been approved at least + // once) — independent of whether the draft has since been edited. + hasApprovedVersion: doc.currentVersionId != null, })), }; } @@ -136,7 +142,11 @@ export class IsmsService { const document = await db.ismsDocument.findFirst({ where: { id: documentId, organizationId }, include: { - versions: { where: { isLatest: true }, take: 1 }, + // The live/published version, for the "Published: vN" display. Full + // history is fetched separately via IsmsVersionService.getVersions. + currentVersion: { + select: { id: true, version: true, publishedAt: true }, + }, contextIssues: { orderBy: { position: 'asc' } }, interestedParties: { orderBy: { position: 'asc' } }, interestedPartyRequirements: { orderBy: { position: 'asc' } }, @@ -204,10 +214,14 @@ export class IsmsService { organizationId, frameworkId: document.frameworkId, }); + const now = new Date(); - await db.$transaction(async (tx) => { - // Re-derive in the same transaction so the persisted rows and the snapshot - // baseline come from one pass (otherwise the approved content can drift). + // Freeze the draft into a new immutable published version and promote it to + // currentVersion. Editing afterwards reverts status to draft but leaves this + // published version live and exportable (CS-701). + const published = await db.$transaction(async (tx) => { + // Re-derive in the same transaction so the persisted rows and the frozen + // snapshot come from one pass (otherwise the approved content can drift). await runDerivation({ tx, type: document.type, @@ -216,11 +230,40 @@ export class IsmsService { frameworkId: document.frameworkId, data: snapshot, }); - await upsertLatestSnapshotVersion({ tx, documentId, snapshot }); + await updateDraftSnapshot({ tx, documentId, snapshot }); + + const reloaded = await tx.ismsDocument.findUniqueOrThrow({ + where: { id: documentId }, + include: EXPORT_DOCUMENT_INCLUDE, + }); + const result = await this.versionService.createPublishedVersion({ + tx, + document: reloaded, + memberId: member.id, + now, + snapshotData: snapshot, + }); + await tx.ismsDocument.update({ where: { id: documentId }, - data: { status: 'approved', approvedAt: new Date(), declinedAt: null }, + data: { + status: 'approved', + approvedAt: now, + declinedAt: null, + currentVersionId: result.versionId, + }, }); + return result; + }); + + // Render + upload the frozen exports outside the transaction (Policies + // pattern) so an S3 hiccup never orphans the published version. + await this.versionService.publishRenders({ + organizationId, + documentId, + versionId: published.versionId, + version: published.version, + snapshot: published.snapshot, }); return this.getDocument({ documentId, organizationId }); diff --git a/apps/api/src/isms/utils/draft-snapshot.spec.ts b/apps/api/src/isms/utils/draft-snapshot.spec.ts new file mode 100644 index 0000000000..ddbaefd9d4 --- /dev/null +++ b/apps/api/src/isms/utils/draft-snapshot.spec.ts @@ -0,0 +1,50 @@ +import type { Prisma } from '@db'; +import { updateDraftSnapshot } from './draft-snapshot'; + +// A fake transaction client: only the ismsDocument.update the unit touches is +// stubbed. No module mock — the unit under test is imported real. +function makeTx() { + const ismsDocument = { update: jest.fn() }; + const tx = { ismsDocument } as unknown as Prisma.TransactionClient; + return { tx, ismsDocument }; +} + +const snapshot = { frameworkNames: ['ISO 27001'], vendorCount: 3 }; + +describe('updateDraftSnapshot', () => { + it('writes the serialized snapshot onto the document draftSnapshot', async () => { + const { tx, ismsDocument } = makeTx(); + + await updateDraftSnapshot({ tx, documentId: 'doc_1', snapshot }); + + expect(ismsDocument.update).toHaveBeenCalledWith({ + where: { id: 'doc_1' }, + data: { draftSnapshot: snapshot }, + }); + }); + + it('serializes the snapshot through JSON, dropping undefined fields', async () => { + const { tx, ismsDocument } = makeTx(); + + await updateDraftSnapshot({ + tx, + documentId: 'doc_2', + snapshot: { keep: 'yes', drop: undefined, nested: { ok: 1 } }, + }); + + const updateArg = ismsDocument.update.mock.calls[0][0]; + expect(updateArg.data.draftSnapshot).toEqual({ + keep: 'yes', + nested: { ok: 1 }, + }); + expect('drop' in updateArg.data.draftSnapshot).toBe(false); + }); + + it('scopes the write to the given document id', async () => { + const { tx, ismsDocument } = makeTx(); + + await updateDraftSnapshot({ tx, documentId: 'doc_3', snapshot }); + + expect(ismsDocument.update.mock.calls[0][0].where).toEqual({ id: 'doc_3' }); + }); +}); diff --git a/apps/api/src/isms/utils/draft-snapshot.ts b/apps/api/src/isms/utils/draft-snapshot.ts new file mode 100644 index 0000000000..5991798468 --- /dev/null +++ b/apps/api/src/isms/utils/draft-snapshot.ts @@ -0,0 +1,28 @@ +import type { Prisma } from '@db'; + +/** + * Persist the derived-data snapshot as the document's DRAFT drift baseline. + * + * CS-701: version rows are now published-only, immutable snapshots, so the live + * drift baseline moved off the version row onto `IsmsDocument.draftSnapshot`. + * Drift is detected by comparing this against the current derived data. Runs on + * the transaction client so it stays atomic with the derivation that produced it. + */ +export async function updateDraftSnapshot({ + tx, + documentId, + snapshot, +}: { + tx: Prisma.TransactionClient; + documentId: string; + snapshot: unknown; +}): Promise { + const draftSnapshot: Prisma.InputJsonValue = JSON.parse( + JSON.stringify(snapshot), + ); + + await tx.ismsDocument.update({ + where: { id: documentId }, + data: { draftSnapshot }, + }); +} diff --git a/apps/api/src/isms/utils/export-generator.ts b/apps/api/src/isms/utils/export-generator.ts index e43ca195c0..3c9115c4b8 100644 --- a/apps/api/src/isms/utils/export-generator.ts +++ b/apps/api/src/isms/utils/export-generator.ts @@ -25,7 +25,7 @@ export async function generateIsmsExportFile({ metadata: IsmsExportMetadata; format: IsmsExportFormat; }): Promise { - const baseName = `${sanitizeName(metadata.title)}-v${metadata.version}`; + const baseName = `${sanitizeExportName(metadata.title)}-v${metadata.version}`; if (format === 'docx') { return { @@ -42,7 +42,7 @@ export async function generateIsmsExportFile({ }; } -function sanitizeName(name: string): string { +export function sanitizeExportName(name: string): string { return (name || 'isms-document') .toLowerCase() .replace(/[^a-z0-9]+/g, '-') diff --git a/apps/api/src/isms/utils/export-payload.spec.ts b/apps/api/src/isms/utils/export-payload.spec.ts new file mode 100644 index 0000000000..5473dd7815 --- /dev/null +++ b/apps/api/src/isms/utils/export-payload.spec.ts @@ -0,0 +1,54 @@ +import { draftVersionNumber, parseExportSnapshot } from './export-payload'; +import type { LoadedExportDocument } from './export-payload'; + +// export-payload imports the db client at module load; a bare stub is enough +// since these pure helpers never touch it. +jest.mock('@db', () => ({ db: {} })); + +const buildDocument = ( + over: Partial = {}, +): LoadedExportDocument => + ({ status: 'draft', currentVersion: null, ...over }) as unknown as LoadedExportDocument; + +describe('draftVersionNumber', () => { + it('returns 1 before anything has been published', () => { + expect(draftVersionNumber(buildDocument({ currentVersion: null }))).toBe(1); + }); + + it('returns the published version when the approved draft is clean', () => { + expect( + draftVersionNumber( + buildDocument({ status: 'approved', currentVersion: { version: 3 } }), + ), + ).toBe(3); + }); + + it('returns the next number while the draft is being edited', () => { + expect( + draftVersionNumber( + buildDocument({ status: 'draft', currentVersion: { version: 3 } }), + ), + ).toBe(4); + }); +}); + +describe('parseExportSnapshot', () => { + it('returns null for null / undefined', () => { + expect(parseExportSnapshot(null)).toBeNull(); + expect(parseExportSnapshot(undefined)).toBeNull(); + }); + + it('returns null for an array', () => { + expect(parseExportSnapshot([1, 2] as never)).toBeNull(); + }); + + it('returns null when required keys are missing', () => { + expect(parseExportSnapshot({ type: 'x', input: {} })).toBeNull(); + expect(parseExportSnapshot({ input: {}, metadata: {} })).toBeNull(); + }); + + it('returns the snapshot when type, input and metadata are present', () => { + const snapshot = { type: 'x', input: { rows: [] }, metadata: { version: 1 } }; + expect(parseExportSnapshot(snapshot)).toBe(snapshot); + }); +}); diff --git a/apps/api/src/isms/utils/export-payload.ts b/apps/api/src/isms/utils/export-payload.ts new file mode 100644 index 0000000000..17e45e7c23 --- /dev/null +++ b/apps/api/src/isms/utils/export-payload.ts @@ -0,0 +1,179 @@ +import { NotFoundException } from '@nestjs/common'; +import { db } from '@db'; +import type { IsmsDocumentType, Prisma } from '@db'; +import { buildExportSections } from '../documents/registry'; +import { loadOrgProfile } from '../documents/org-profile'; +import type { DocumentExportInput, IsmsOrgProfile } from '../documents/types'; +import { buildExportMetadata } from './export-metadata'; +import { + generateIsmsExportFile, + type IsmsExportFormat, + type IsmsExportResult, +} from './export-generator'; +import type { IsmsExportMetadata } from './export-shared'; + +/** + * The immutable content snapshot stored on a published IsmsDocumentVersion. + * Everything needed to re-render a historical version byte-faithfully if its + * stored file is ever missing: the document type, the export input (register + * rows + narrative + org profile) and the export metadata frozen at publish. + */ +export interface IsmsExportSnapshot { + type: IsmsDocumentType; + input: DocumentExportInput; + metadata: IsmsExportMetadata; +} + +/** Prisma include for a document being exported or snapshotted. */ +export const EXPORT_DOCUMENT_INCLUDE = { + framework: { select: { name: true } }, + organization: { select: { name: true, website: true, primaryColor: true } }, + approver: { select: { user: { select: { name: true, email: true } } } }, + currentVersion: { select: { version: true } }, + contextIssues: { orderBy: { position: 'asc' } }, + interestedParties: { orderBy: { position: 'asc' } }, + interestedPartyRequirements: { orderBy: { position: 'asc' } }, + objectives: { orderBy: { position: 'asc' } }, +} satisfies Prisma.IsmsDocumentInclude; + +export type LoadedExportDocument = Prisma.IsmsDocumentGetPayload<{ + include: typeof EXPORT_DOCUMENT_INCLUDE; +}>; + +/** Load a document with everything the export/snapshot builders need. */ +export async function loadExportDocument( + documentId: string, + organizationId: string, +): Promise { + return db.ismsDocument.findFirst({ + where: { id: documentId, organizationId }, + include: EXPORT_DOCUMENT_INCLUDE, + }); +} + +/** The Context document (4.1) renders an org overview; other types don't need it. */ +export async function resolveOrgProfile( + document: LoadedExportDocument, +): Promise { + if (document.type !== 'context_of_organization') return undefined; + return loadOrgProfile({ + organizationId: document.organizationId, + frameworkId: document.frameworkId, + }); +} + +/** Map the loaded document's live rows + draft narrative into export input. */ +export function buildExportInput({ + document, + orgProfile, +}: { + document: LoadedExportDocument; + orgProfile?: IsmsOrgProfile; +}): DocumentExportInput { + return { + contextIssues: document.contextIssues.map((issue) => ({ + kind: issue.kind, + category: issue.category, + description: issue.description, + effect: issue.effect, + })), + interestedParties: document.interestedParties.map((party) => ({ + name: party.name, + category: party.category, + needsExpectations: party.needsExpectations, + })), + requirements: document.interestedPartyRequirements.map((row) => ({ + partyName: row.partyName, + requirement: row.requirement, + treatment: row.treatment, + })), + objectives: document.objectives.map((objective) => ({ + objective: objective.objective, + target: objective.target, + cadence: objective.cadence, + status: objective.status, + plan: objective.plan, + measurementMethod: objective.measurementMethod, + })), + narrative: document.draftNarrative ?? null, + orgProfile, + }; +} + +/** + * The version number to show for the current draft: the published version when + * the draft is clean (approved), the next number when edits are in progress, or + * 1 before anything has been published. + */ +export function draftVersionNumber(document: LoadedExportDocument): number { + const published = document.currentVersion?.version ?? 0; + if (!document.currentVersion) return 1; + return document.status === 'approved' ? published : published + 1; +} + +/** Build the full export snapshot ({type,input,metadata}) for the current draft. */ +export async function buildDraftSnapshot( + document: LoadedExportDocument, +): Promise { + const orgProfile = await resolveOrgProfile(document); + const input = buildExportInput({ document, orgProfile }); + const metadata = buildExportMetadata({ + type: document.type, + title: document.title, + frameworkName: document.framework.name || 'ISO 27001', + version: draftVersionNumber(document), + status: document.status, + preparedBy: document.preparedBy, + owner: null, + approverName: + document.approver?.user?.name || document.approver?.user?.email || null, + approvedAt: document.approvedAt, + declinedAt: document.declinedAt, + organizationName: document.organization.name, + primaryColor: document.organization.primaryColor, + }); + return { type: document.type, input, metadata }; +} + +/** Render a document's current DRAFT to a file (used by the export endpoint). */ +export async function renderLiveExport({ + documentId, + organizationId, + format, +}: { + documentId: string; + organizationId: string; + format: IsmsExportFormat; +}): Promise { + const document = await loadExportDocument(documentId, organizationId); + if (!document) { + throw new NotFoundException('ISMS document not found'); + } + return renderSnapshot(await buildDraftSnapshot(document), format); +} + +/** Render a stored/derived export snapshot to a file. */ +export function renderSnapshot( + snapshot: IsmsExportSnapshot, + format: IsmsExportFormat, +): Promise { + const sections = buildExportSections({ + type: snapshot.type, + input: snapshot.input, + }); + return generateIsmsExportFile({ + sections, + metadata: snapshot.metadata, + format, + }); +} + +/** Parse a stored contentSnapshot JSON back into an export snapshot, or null. */ +export function parseExportSnapshot( + value: Prisma.JsonValue | null | undefined, +): IsmsExportSnapshot | null { + if (!value || typeof value !== 'object' || Array.isArray(value)) return null; + const record = value as Record; + if (!record.type || !record.input || !record.metadata) return null; + return record as unknown as IsmsExportSnapshot; +} diff --git a/apps/api/src/isms/utils/version-snapshot.spec.ts b/apps/api/src/isms/utils/version-snapshot.spec.ts deleted file mode 100644 index b9b1484efc..0000000000 --- a/apps/api/src/isms/utils/version-snapshot.spec.ts +++ /dev/null @@ -1,78 +0,0 @@ -import type { Prisma } from '@db'; -import { upsertLatestSnapshotVersion } from './version-snapshot'; - -// A fake transaction client: only the ismsDocumentVersion methods the unit -// touches are stubbed. No module mock — the unit under test is imported real. -function makeTx() { - const ismsDocumentVersion = { - findFirst: jest.fn(), - update: jest.fn(), - create: jest.fn(), - }; - const tx = { ismsDocumentVersion } as unknown as Prisma.TransactionClient; - return { tx, ismsDocumentVersion }; -} - -const snapshot = { frameworkNames: ['ISO 27001'], vendorCount: 3 }; - -describe('upsertLatestSnapshotVersion', () => { - it('updates the existing latest version with the serialized snapshot (UPDATE branch)', async () => { - const { tx, ismsDocumentVersion } = makeTx(); - ismsDocumentVersion.findFirst.mockResolvedValue({ id: 'ver_existing' }); - - await upsertLatestSnapshotVersion({ - tx, - documentId: 'doc_1', - snapshot, - }); - - expect(ismsDocumentVersion.findFirst).toHaveBeenCalledWith({ - where: { documentId: 'doc_1', isLatest: true }, - }); - expect(ismsDocumentVersion.update).toHaveBeenCalledWith({ - where: { id: 'ver_existing' }, - data: { sourceSnapshot: snapshot }, - }); - expect(ismsDocumentVersion.create).not.toHaveBeenCalled(); - }); - - it('creates version 1 marked latest when none exists (CREATE branch)', async () => { - const { tx, ismsDocumentVersion } = makeTx(); - ismsDocumentVersion.findFirst.mockResolvedValue(null); - - await upsertLatestSnapshotVersion({ - tx, - documentId: 'doc_2', - snapshot, - }); - - expect(ismsDocumentVersion.update).not.toHaveBeenCalled(); - expect(ismsDocumentVersion.create).toHaveBeenCalledWith({ - data: { - documentId: 'doc_2', - version: 1, - isLatest: true, - narrative: {}, - sourceSnapshot: snapshot, - }, - }); - }); - - it('serializes the snapshot through JSON, dropping undefined fields', async () => { - const { tx, ismsDocumentVersion } = makeTx(); - ismsDocumentVersion.findFirst.mockResolvedValue(null); - - await upsertLatestSnapshotVersion({ - tx, - documentId: 'doc_3', - snapshot: { keep: 'yes', drop: undefined, nested: { ok: 1 } }, - }); - - const createArg = ismsDocumentVersion.create.mock.calls[0][0]; - expect(createArg.data.sourceSnapshot).toEqual({ - keep: 'yes', - nested: { ok: 1 }, - }); - expect('drop' in createArg.data.sourceSnapshot).toBe(false); - }); -}); diff --git a/apps/api/src/isms/utils/version-snapshot.ts b/apps/api/src/isms/utils/version-snapshot.ts deleted file mode 100644 index 227a79c060..0000000000 --- a/apps/api/src/isms/utils/version-snapshot.ts +++ /dev/null @@ -1,45 +0,0 @@ -import type { Prisma } from '@db'; - -const EMPTY_NARRATIVE: Prisma.InputJsonValue = {}; - -/** - * Persist the derived-data snapshot onto the document's latest version, creating - * version 1 if none exists. The snapshot is the drift baseline. Serializing - * through JSON keeps it a plain Prisma.InputJsonValue without unsafe casts. The - * existing narrative is preserved (only sourceSnapshot is written). - */ -export async function upsertLatestSnapshotVersion({ - tx, - documentId, - snapshot, -}: { - tx: Prisma.TransactionClient; - documentId: string; - snapshot: unknown; -}): Promise { - const sourceSnapshot: Prisma.InputJsonValue = JSON.parse( - JSON.stringify(snapshot), - ); - - const latest = await tx.ismsDocumentVersion.findFirst({ - where: { documentId, isLatest: true }, - }); - - if (latest) { - await tx.ismsDocumentVersion.update({ - where: { id: latest.id }, - data: { sourceSnapshot }, - }); - return; - } - - await tx.ismsDocumentVersion.create({ - data: { - documentId, - version: 1, - isLatest: true, - narrative: EMPTY_NARRATIVE, - sourceSnapshot, - }, - }); -} diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/ContextOfOrganizationClient.test.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/ContextOfOrganizationClient.test.tsx index a3176b169b..c4dd38c70e 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/ContextOfOrganizationClient.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/ContextOfOrganizationClient.test.tsx @@ -67,6 +67,10 @@ vi.mock('./IsmsControlMappings', () => ({ IsmsControlMappings: () =>
, })); +vi.mock('./IsmsVersionHistory', () => ({ + IsmsVersionHistory: () =>
, +})); + import { ContextOfOrganizationClient } from './ContextOfOrganizationClient'; const ISSUES: IsmsContextIssue[] = [ @@ -106,7 +110,9 @@ function makeDocument(overrides: Partial = {}): IsmsDocument { interestedPartyRequirements: [], objectives: [], controlLinks: [], - versions: [], + draftNarrative: null, + currentVersionId: null, + currentVersion: null, ...overrides, }; } diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/InterestedPartiesClient.test.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/InterestedPartiesClient.test.tsx index 17866d9ea7..7618d876e2 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/InterestedPartiesClient.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/InterestedPartiesClient.test.tsx @@ -92,6 +92,10 @@ vi.mock('./IsmsControlMappings', () => ({ IsmsControlMappings: () =>
, })); +vi.mock('./IsmsVersionHistory', () => ({ + IsmsVersionHistory: () =>
, +})); + import { InterestedPartiesClient } from './InterestedPartiesClient'; const PARTIES: IsmsInterestedParty[] = [ @@ -129,7 +133,9 @@ function makeDocument(overrides: Partial = {}): IsmsDocument { interestedPartyRequirements: [], objectives: [], controlLinks: [], - versions: [], + draftNarrative: null, + currentVersionId: null, + currentVersion: null, ...overrides, }; } diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsApprovalSection.test.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsApprovalSection.test.tsx index c31741c2a5..9e1cccdbe3 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsApprovalSection.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsApprovalSection.test.tsx @@ -105,7 +105,9 @@ function makeDocument(overrides: Partial = {}): IsmsDocument { interestedPartyRequirements: [], objectives: [], controlLinks: [], - versions: [], + draftNarrative: null, + currentVersionId: null, + currentVersion: null, ...overrides, }; } diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsApprovalSection.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsApprovalSection.tsx index c46ab07f27..d5bc90fd50 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsApprovalSection.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsApprovalSection.tsx @@ -75,6 +75,15 @@ export function IsmsApprovalSection({ const approvedDate = formatDate(document.approvedAt); const declinedDate = formatDate(document.declinedAt); + // Versioning context (CS-701): a published version can stay live while the + // draft is edited. `hasDraftChanges` = a published version exists but the + // working draft is no longer approved (edits in progress). + const publishedVersion = document.currentVersion?.version ?? null; + const hasPublishedVersion = publishedVersion != null; + const nextDraftVersion = (publishedVersion ?? 0) + 1; + const hasDraftChanges = + hasPublishedVersion && (isDeclined || (!isApproved && !isPending)); + // The plain submit button is only offered on un-submitted drafts. Pending and // resolved (approved / declined) documents render their own state instead. const showSubmitButton = canManage && !isPending && !isResolved; @@ -98,7 +107,9 @@ export function IsmsApprovalSection({
{isApproved && ( - Approved + + Approved{publishedVersion ? ` · Published as v${publishedVersion}` : ''} + This document was approved by{' '} @@ -109,6 +120,20 @@ export function IsmsApprovalSection({ )} + {hasDraftChanges && ( + + Editing creates a new draft + + Published version{' '} + + v{publishedVersion} + {' '} + stays live and exportable. Your changes are an in-progress draft (v + {nextDraftVersion}) — submit it for approval to publish. + + + )} + {isDeclined && ( Declined diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsControlMappings.test.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsControlMappings.test.tsx index ca860ccc51..269a667b86 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsControlMappings.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsControlMappings.test.tsx @@ -192,7 +192,9 @@ function makeDocument(overrides: Partial = {}): IsmsDocument { interestedPartyRequirements: [], objectives: [], controlLinks: LINKS, - versions: [], + draftNarrative: null, + currentVersionId: null, + currentVersion: null, ...overrides, }; } diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsDocumentShell.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsDocumentShell.tsx index 419474124c..662c061768 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsDocumentShell.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsDocumentShell.tsx @@ -21,10 +21,12 @@ import { toast } from 'sonner'; import { usePermissions } from '@/hooks/use-permissions'; import { useIsmsDocument, type UseIsmsDocumentReturn } from '../hooks/useIsmsDocument'; import { useIsmsDrift } from '../hooks/useIsmsDrift'; +import { useIsmsDocumentVersions } from '../hooks/useIsmsDocumentVersions'; import type { IsmsDocument as IsmsDocumentData } from '../isms-types'; import { DriftBanner } from './DriftBanner'; import { IsmsControlMappings } from './IsmsControlMappings'; import { IsmsApprovalSection, type ApproverOption } from './IsmsApprovalSection'; +import { IsmsVersionHistory } from './IsmsVersionHistory'; import { IsmsPageHeader } from './shared'; /** Arguments passed to the per-document body render-prop. */ @@ -94,6 +96,14 @@ export function IsmsDocumentShell({ handleExport, } = hook; const { isStale, drift, mutateDrift } = useIsmsDrift(documentId); + const { + versions, + isLoading: versionsLoading, + error: versionsError, + downloadingVersionId, + downloadVersion, + mutateVersions, + } = useIsmsDocumentVersions(documentId, organizationId); const [isGenerating, setIsGenerating] = useState(false); @@ -122,6 +132,9 @@ export function IsmsDocumentShell({ const handleApprove = async () => { try { await approve(); + // Approval promotes the draft to a new published version — refresh the + // history list and drift baseline alongside the document itself. + await Promise.all([mutateVersions(), mutateDrift()]); toast.success('Document approved'); } catch (caught) { toast.error(caught instanceof Error ? caught.message : 'Failed to approve'); @@ -244,6 +257,16 @@ export function IsmsDocumentShell({ {document && ( )} + + {document && ( + + )} ); } diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsVersionHistory.test.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsVersionHistory.test.tsx new file mode 100644 index 0000000000..2c31e8b86a --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsVersionHistory.test.tsx @@ -0,0 +1,81 @@ +import { fireEvent, render, screen } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import type { IsmsPublishedVersion } from '../isms-types'; +import { ismsDesignSystemMock, ismsIconsMock, ismsSharedMock } from './__test-helpers__/dsMocks'; + +vi.mock('@trycompai/design-system', () => ismsDesignSystemMock()); +vi.mock('@trycompai/design-system/icons', () => ismsIconsMock()); +vi.mock('./shared', () => ismsSharedMock()); + +import { IsmsVersionHistory } from './IsmsVersionHistory'; + +const VERSIONS: IsmsPublishedVersion[] = [ + { + id: 'v2', + version: 2, + publishedAt: '2026-07-06T10:00:00.000Z', + changelog: 'Updated scope', + publishedByName: 'Alice Admin', + hasPdf: true, + hasDocx: true, + isCurrent: true, + }, + { + id: 'v1', + version: 1, + publishedAt: '2026-06-01T10:00:00.000Z', + changelog: null, + publishedByName: 'Bob Owner', + hasPdf: true, + hasDocx: false, + isCurrent: false, + }, +]; + +const baseProps = { + versions: VERSIONS, + isLoading: false, + error: null, + downloadingVersionId: null, + onDownload: vi.fn(), +}; + +describe('IsmsVersionHistory', () => { + beforeEach(() => vi.clearAllMocks()); + + it('lists every published version with its approver, date and changelog', () => { + render(); + + expect(screen.getByText('v2')).toBeInTheDocument(); + expect(screen.getByText('v1')).toBeInTheDocument(); + // Approver + date + changelog surface on the row subtitle. + expect(screen.getByText(/Alice Admin/)).toBeInTheDocument(); + expect(screen.getByText(/Updated scope/)).toBeInTheDocument(); + expect(screen.getByText(/Bob Owner/)).toBeInTheDocument(); + // Only the current version is marked. + expect(screen.getByText('Current')).toBeInTheDocument(); + }); + + it('downloads a specific version as PDF or DOCX', () => { + render(); + + const pdfButtons = screen.getAllByRole('button', { name: /PDF/i }); + const docxButtons = screen.getAllByRole('button', { name: /DOCX/i }); + // First row is v2 (newest first). + fireEvent.click(pdfButtons[0]); + expect(baseProps.onDownload).toHaveBeenCalledWith('v2', 'pdf'); + + fireEvent.click(docxButtons[0]); + expect(baseProps.onDownload).toHaveBeenCalledWith('v2', 'docx'); + }); + + it('renders an empty state when there are no published versions', () => { + render(); + expect(screen.getByText('No published versions yet')).toBeInTheDocument(); + }); + + it('shows a loading state before the first load resolves', () => { + render(); + expect(screen.getByRole('status')).toBeInTheDocument(); + }); +}); diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsVersionHistory.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsVersionHistory.tsx new file mode 100644 index 0000000000..344f0fecc7 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsVersionHistory.tsx @@ -0,0 +1,130 @@ +'use client'; + +import { + Badge, + Button, + Item, + ItemActions, + ItemContent, + ItemGroup, + ItemMedia, + ItemTitle, + Section, + Spinner, + Text, +} from '@trycompai/design-system'; +import { Document, Download, Time } from '@trycompai/design-system/icons'; +import { IsmsEmptyState } from './shared'; +import type { IsmsExportFormat, IsmsPublishedVersion } from '../isms-types'; + +interface IsmsVersionHistoryProps { + versions: IsmsPublishedVersion[]; + isLoading: boolean; + error: unknown; + /** Version id currently downloading, so its buttons can show a spinner. */ + downloadingVersionId: string | null; + onDownload: (versionId: string, format: IsmsExportFormat) => void | Promise; +} + +/** Format an ISO timestamp as a short human date, or null when unparseable. */ +function formatDate(value: string | null): string | null { + if (!value) return null; + const parsed = new Date(value); + if (Number.isNaN(parsed.getTime())) return null; + return parsed.toLocaleDateString(undefined, { + year: 'numeric', + month: 'short', + day: 'numeric', + }); +} + +/** One published version's "Published {date} by {name}" line, with changelog. */ +function versionSubtitle(version: IsmsPublishedVersion): string { + const date = formatDate(version.publishedAt); + const published = date ? `Published ${date}` : 'Published'; + const by = version.publishedByName ? ` by ${version.publishedByName}` : ''; + const note = version.changelog ? ` — ${version.changelog}` : ''; + return `${published}${by}${note}`; +} + +/** + * Version history for an ISMS document (CS-701): a list of published versions, + * newest first, each with its date, approver and per-version PDF/DOCX download. + * Rendered once inside IsmsDocumentShell so all six documents inherit it. Data is + * owned by the shell (useIsmsDocumentVersions) and passed in as props. + */ +export function IsmsVersionHistory({ + versions, + isLoading, + error, + downloadingVersionId, + onDownload, +}: IsmsVersionHistoryProps) { + return ( +
+ {isLoading && versions.length === 0 ? ( +
+ +
+ ) : error && versions.length === 0 ? ( + + ) : versions.length === 0 ? ( + + ) : ( + + {versions.map((version) => ( + + + + + + {`v${version.version}`} + {version.isCurrent && Current} + + + {versionSubtitle(version)} + + + + + + + + ))} + + )} +
+ ); +} diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/LeadershipClient.test.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/LeadershipClient.test.tsx index 307f5c56bd..e0dd17d495 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/LeadershipClient.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/LeadershipClient.test.tsx @@ -122,6 +122,10 @@ vi.mock('./IsmsControlMappings', () => ({ IsmsControlMappings: () =>
, })); +vi.mock('./IsmsVersionHistory', () => ({ + IsmsVersionHistory: () =>
, +})); + vi.mock('./shared', () => ({ IsmsPageHeader: ({ clause, @@ -169,7 +173,9 @@ function makeDocument(overrides: Partial = {}): IsmsDocument { interestedPartyRequirements: [], objectives: [], controlLinks: [], - versions: [{ id: 'v1', version: 1, isLatest: true, narrative: NARRATIVE }], + draftNarrative: NARRATIVE, + currentVersionId: null, + currentVersion: null, ...overrides, }; } diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/LeadershipClient.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/LeadershipClient.tsx index f52baee3d5..ae89fe368d 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/LeadershipClient.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/LeadershipClient.tsx @@ -21,8 +21,7 @@ interface LeadershipClientProps { function extractNarrative( document: IsmsDocumentData, ): Partial | null { - const versions = Array.isArray(document.versions) ? document.versions : []; - const narrative = versions[0]?.narrative ?? null; + const narrative = document.draftNarrative ?? null; if (!narrative || typeof narrative !== 'object') return null; return narrative as Partial; } diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/ObjectivesClient.test.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/ObjectivesClient.test.tsx index 59098b9fd0..424590c6f0 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/ObjectivesClient.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/ObjectivesClient.test.tsx @@ -67,6 +67,10 @@ vi.mock('./IsmsControlMappings', () => ({ IsmsControlMappings: () =>
, })); +vi.mock('./IsmsVersionHistory', () => ({ + IsmsVersionHistory: () =>
, +})); + import { ObjectivesClient } from './ObjectivesClient'; const OBJECTIVES: IsmsObjective[] = [ @@ -112,7 +116,9 @@ function makeDocument(overrides: Partial = {}): IsmsDocument { interestedPartyRequirements: [], objectives: OBJECTIVES, controlLinks: [], - versions: [], + draftNarrative: null, + currentVersionId: null, + currentVersion: null, ...overrides, }; } diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/RequirementsClient.test.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/RequirementsClient.test.tsx index 123d855946..258f1c7920 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/RequirementsClient.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/RequirementsClient.test.tsx @@ -71,6 +71,10 @@ vi.mock('./IsmsControlMappings', () => ({ IsmsControlMappings: () =>
, })); +vi.mock('./IsmsVersionHistory', () => ({ + IsmsVersionHistory: () =>
, +})); + import { RequirementsClient } from './RequirementsClient'; const REQUIREMENTS: IsmsInterestedPartyRequirement[] = [ @@ -110,7 +114,9 @@ function makeDocument(overrides: Partial = {}): IsmsDocument { interestedPartyRequirements: REQUIREMENTS, objectives: [], controlLinks: [], - versions: [], + draftNarrative: null, + currentVersionId: null, + currentVersion: null, ...overrides, }; } diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/ScopeClient.test.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/ScopeClient.test.tsx index 68f01ec0e9..8ebc910886 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/ScopeClient.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/ScopeClient.test.tsx @@ -118,6 +118,10 @@ vi.mock('./IsmsControlMappings', () => ({ IsmsControlMappings: () =>
, })); +vi.mock('./IsmsVersionHistory', () => ({ + IsmsVersionHistory: () =>
, +})); + vi.mock('./shared', () => ({ IsmsPageHeader: ({ clause, @@ -160,7 +164,9 @@ function makeDocument(overrides: Partial = {}): IsmsDocument { interestedPartyRequirements: [], objectives: [], controlLinks: [], - versions: [{ id: 'v1', version: 1, isLatest: true, narrative: NARRATIVE }], + draftNarrative: NARRATIVE, + currentVersionId: null, + currentVersion: null, ...overrides, }; } diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/ScopeClient.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/ScopeClient.tsx index 29df85dc2d..f3b006621d 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/ScopeClient.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/ScopeClient.tsx @@ -63,12 +63,11 @@ export function ScopeClient(props: ScopeClientProps) { } }; - const versions = Array.isArray(document.versions) ? document.versions : []; - const narrative = toScopeNarrative(versions[0]?.narrative); + const narrative = toScopeNarrative(document.draftNarrative); return (
{children}
, Heading: ({ children }: { children: ReactNode }) =>

{children}

, + Item: ({ children }: { children: ReactNode }) =>
{children}
, + ItemGroup: ({ children }: { children: ReactNode }) =>
{children}
, + ItemMedia: ({ children }: { children?: ReactNode }) =>
{children}
, + ItemContent: ({ children }: { children: ReactNode }) =>
{children}
, + ItemTitle: ({ children }: { children: ReactNode }) =>
{children}
, + ItemActions: ({ children }: { children: ReactNode }) =>
{children}
, Dialog: ({ children }: { children: ReactNode }) =>
{children}
, DialogContent: ({ children }: { children: ReactNode }) =>
{children}
, DialogDescription: ({ children }: { children: ReactNode }) =>

{children}

, @@ -78,6 +84,7 @@ export function ismsIconsMock() { Document: Icon, Download: Icon, Edit: Icon, + Time: Icon, Flag: Icon, ListChecked: Icon, MachineLearningModel: Icon, @@ -90,6 +97,18 @@ export function ismsIconsMock() { export function ismsSharedMock() { return { + IsmsEmptyState: ({ + title, + description, + }: { + title: ReactNode; + description?: ReactNode; + }) => ( +
+

{title}

+ {description ?

{description}

: null} +
+ ), IsmsPageHeader: ({ clause, title, diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/exportIsmsDocument.ts b/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/exportIsmsDocument.ts index 64889e41d2..67ba9ed11f 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/exportIsmsDocument.ts +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/exportIsmsDocument.ts @@ -6,6 +6,11 @@ interface ExportIsmsDocumentParams { documentId: string; format: IsmsExportFormat; organizationId?: string; + /** + * Published version to export. Omit to export the current working draft; provide + * a version id to download exactly what was approved at that version (CS-701). + */ + versionId?: string; } function parseFilename(contentDisposition: string | null, fallback: string): string { @@ -14,11 +19,12 @@ function parseFilename(contentDisposition: string | null, fallback: string): str return match ? match[1] : fallback; } -/** Downloads an ISMS document export (pdf/docx/md) via the API and triggers a browser save. */ +/** Downloads an ISMS document export (pdf/docx) via the API and triggers a browser save. */ export async function exportIsmsDocument({ documentId, format, organizationId, + versionId, }: ExportIsmsDocumentParams): Promise { // Route through the shared apiClient so the request carries the same // credentials + organization header context as every other API call. @@ -26,7 +32,7 @@ export async function exportIsmsDocument({ method: 'POST', organizationId, headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ format }), + body: JSON.stringify(versionId ? { format, versionId } : { format }), }); if (!response.ok) { diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/useIsmsDocumentVersions.test.ts b/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/useIsmsDocumentVersions.test.ts new file mode 100644 index 0000000000..ac2a17d3d3 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/useIsmsDocumentVersions.test.ts @@ -0,0 +1,73 @@ +import { act, renderHook } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { api } from '@/lib/api-client'; + +// Capture the SWR key + fetcher so we can assert the endpoint without firing it +// on mount. `vi.hoisted` keeps the holder available inside the hoisted vi.mock. +const swr = vi.hoisted(() => ({ + key: undefined as unknown, + fetcher: null as null | ((key: unknown) => unknown), +})); + +vi.mock('@/lib/api-client', () => ({ + api: { get: vi.fn() }, +})); + +vi.mock('swr', () => ({ + default: (key: unknown, fetcher: (k: unknown) => unknown) => { + swr.key = key; + swr.fetcher = fetcher; + return { data: undefined, error: undefined, isLoading: false, mutate: vi.fn() }; + }, +})); + +vi.mock('./exportIsmsDocument', () => ({ + exportIsmsDocument: vi.fn().mockResolvedValue(undefined), +})); + +import { useIsmsDocumentVersions } from './useIsmsDocumentVersions'; +import { exportIsmsDocument } from './exportIsmsDocument'; + +const getMock = vi.mocked(api.get); +const exportMock = vi.mocked(exportIsmsDocument); + +const DOC_ID = 'doc_1'; +const ORG_ID = 'org_1'; + +beforeEach(() => { + vi.clearAllMocks(); + swr.key = undefined; + swr.fetcher = null; +}); + +describe('useIsmsDocumentVersions', () => { + it('keys SWR on the document and fetches GET /v1/isms/documents/:id/versions', async () => { + getMock.mockResolvedValue({ data: { currentVersionId: null, versions: [] }, status: 200 }); + + renderHook(() => useIsmsDocumentVersions(DOC_ID, ORG_ID)); + + expect(swr.key).toEqual(['/v1/isms/documents', DOC_ID, 'versions']); + await swr.fetcher?.(['/v1/isms/documents', DOC_ID, 'versions']); + expect(getMock).toHaveBeenCalledWith(`/v1/isms/documents/${DOC_ID}/versions`); + }); + + it('does not fetch when no document id is provided', () => { + renderHook(() => useIsmsDocumentVersions(null, ORG_ID)); + expect(swr.key).toBeNull(); + }); + + it('downloads a specific published version via exportIsmsDocument', async () => { + const { result } = renderHook(() => useIsmsDocumentVersions(DOC_ID, ORG_ID)); + + await act(async () => { + await result.current.downloadVersion('isms_ver_1', 'pdf'); + }); + + expect(exportMock).toHaveBeenCalledWith({ + documentId: DOC_ID, + format: 'pdf', + organizationId: ORG_ID, + versionId: 'isms_ver_1', + }); + }); +}); diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/useIsmsDocumentVersions.ts b/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/useIsmsDocumentVersions.ts new file mode 100644 index 0000000000..7a88488610 --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/useIsmsDocumentVersions.ts @@ -0,0 +1,65 @@ +'use client'; + +import { useState } from 'react'; +import useSWR from 'swr'; +import { api } from '@/lib/api-client'; +import type { IsmsExportFormat, IsmsVersionHistory } from '../isms-types'; +import { exportIsmsDocument } from './exportIsmsDocument'; + +/** + * Published version history for an ISMS document (CS-701). Keyed on + * `['/v1/isms/documents', documentId, 'versions']` so the detail page's history + * section stays in sync. Exposes a `downloadVersion` helper that fetches a + * specific published version's stored export, and `mutateVersions` to revalidate + * after an approval promotes a new version. + */ +export function useIsmsDocumentVersions( + documentId: string | null, + organizationId?: string, +) { + const { data, error, isLoading, mutate } = useSWR( + documentId + ? (['/v1/isms/documents', documentId, 'versions'] as const) + : null, + async ([base, id]: readonly [string, string, string]) => { + const response = await api.get(`${base}/${id}/versions`); + if (response.error || !response.data) { + throw new Error(response.error ?? 'Failed to load version history'); + } + return response.data; + }, + ); + + const [downloadingVersionId, setDownloadingVersionId] = useState( + null, + ); + + const downloadVersion = async ( + versionId: string, + format: IsmsExportFormat, + ): Promise => { + if (!documentId) return; + setDownloadingVersionId(versionId); + try { + await exportIsmsDocument({ + documentId, + format, + organizationId, + versionId, + }); + } finally { + setDownloadingVersionId(null); + } + }; + + return { + history: data ?? null, + versions: data?.versions ?? [], + currentVersionId: data?.currentVersionId ?? null, + error, + isLoading, + downloadingVersionId, + downloadVersion, + mutateVersions: mutate, + }; +} diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/isms-types.ts b/apps/app/src/app/(app)/[orgId]/documents/isms/isms-types.ts index 033193da38..1c160d16d8 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/isms-types.ts +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/isms-types.ts @@ -129,13 +129,29 @@ export interface IsmsControlLink { control: { id: string; name: string }; } -/** Latest version row included on the fetched document. */ -export interface IsmsDocumentVersion { +/** The live/published version summary included on the fetched document (CS-701). */ +export interface IsmsCurrentVersion { id: string; version: number; - isLatest: boolean; - /** Per-type narrative payload (scope / leadership); null for register-only docs. */ - narrative: IsmsScopeNarrative | IsmsLeadershipNarrative | Record | null; + publishedAt: string | null; +} + +/** A published version in the version-history feed (CS-701). */ +export interface IsmsPublishedVersion { + id: string; + version: number; + publishedAt: string | null; + changelog: string | null; + publishedByName: string | null; + hasPdf: boolean; + hasDocx: boolean; + isCurrent: boolean; +} + +/** Response of `GET /v1/isms/documents/:id/versions`. */ +export interface IsmsVersionHistory { + currentVersionId: string | null; + versions: IsmsPublishedVersion[]; } /** Summary row returned by `ensure-setup`. */ @@ -154,8 +170,10 @@ export interface IsmsEnsureSetupResponse { /** * Full document returned by `GET /v1/isms/documents/:id`. Every register array is - * always present (empty when the document type does not use that register), and - * `versions[0]` (when present) carries the singleton narrative. + * always present (empty when the document type does not use that register). The + * editable draft lives on the document itself (register rows + `draftNarrative`); + * `currentVersion` is the live/published version (CS-701). Version history is + * fetched separately via `GET /v1/isms/documents/:id/versions`. */ export interface IsmsDocument { id: string; @@ -170,7 +188,14 @@ export interface IsmsDocument { interestedPartyRequirements: IsmsInterestedPartyRequirement[]; objectives: IsmsObjective[]; controlLinks: IsmsControlLink[]; - versions: IsmsDocumentVersion[]; + /** Working-draft narrative for the singleton documents (Scope, Leadership). */ + draftNarrative: + | IsmsScopeNarrative + | IsmsLeadershipNarrative + | Record + | null; + currentVersionId: string | null; + currentVersion: IsmsCurrentVersion | null; } export interface IsmsDriftResult { diff --git a/packages/db/prisma/migrations/20260707120000_isms_document_versioning/migration.sql b/packages/db/prisma/migrations/20260707120000_isms_document_versioning/migration.sql new file mode 100644 index 0000000000..bce7d29ed6 --- /dev/null +++ b/packages/db/prisma/migrations/20260707120000_isms_document_versioning/migration.sql @@ -0,0 +1,60 @@ +-- ISMS document versioning & history (CS-701) +-- Brings ISMS documents onto the Policies versioning model: a published-version +-- pointer + working-draft fields on the parent, per-version publish metadata + +-- an immutable content snapshot on the version, then a backfill of existing rows. + +-- AlterTable: working-draft holders + current-published-version pointer. +ALTER TABLE "IsmsDocument" ADD COLUMN "currentVersionId" TEXT, +ADD COLUMN "draftNarrative" JSONB, +ADD COLUMN "draftSnapshot" JSONB; + +-- AlterTable: per-version changelog + immutable rendered-content snapshot. +ALTER TABLE "IsmsDocumentVersion" ADD COLUMN "changelog" TEXT, +ADD COLUMN "contentSnapshot" JSONB; + +-- CreateIndex +CREATE UNIQUE INDEX "IsmsDocument_currentVersionId_key" ON "IsmsDocument"("currentVersionId"); + +-- CreateIndex +CREATE INDEX "IsmsDocumentVersion_documentId_publishedAt_idx" ON "IsmsDocumentVersion"("documentId", "publishedAt"); + +-- AddForeignKey +ALTER TABLE "IsmsDocument" ADD CONSTRAINT "IsmsDocument_currentVersionId_fkey" FOREIGN KEY ("currentVersionId") REFERENCES "IsmsDocumentVersion"("id") ON DELETE SET NULL ON UPDATE CASCADE; + +-- AddForeignKey +ALTER TABLE "IsmsDocumentVersion" ADD CONSTRAINT "IsmsDocumentVersion_publishedById_fkey" FOREIGN KEY ("publishedById") REFERENCES "Member"("id") ON DELETE SET NULL ON UPDATE CASCADE; + +-- --------------------------------------------------------------------------- +-- Data backfill: migrate the single-mutable-version model onto published+draft. +-- ISMS is pre-GA (flag-gated), so volumes are tiny; the backfill is defensive. +-- --------------------------------------------------------------------------- + +-- 1. Move the working narrative + drift baseline off the (previously single) +-- version row onto the parent document, which now holds the editable draft. +UPDATE "IsmsDocument" d +SET "draftNarrative" = v."narrative", + "draftSnapshot" = v."sourceSnapshot" +FROM "IsmsDocumentVersion" v +WHERE v."documentId" = d."id" AND v."isLatest" = true; + +-- 2. For APPROVED documents, promote the existing latest row to published v1: +-- stamp publish metadata from the document's approval and point currentVersion +-- at it. contentSnapshot stays NULL -> historical export falls back to an +-- on-demand render (draft == published at migration time, so this is faithful). +UPDATE "IsmsDocumentVersion" v +SET "publishedAt" = COALESCE(d."approvedAt", d."updatedAt"), + "publishedById" = d."approverId" +FROM "IsmsDocument" d +WHERE v."documentId" = d."id" AND v."isLatest" = true AND d."status" = 'approved'; + +UPDATE "IsmsDocument" d +SET "currentVersionId" = v."id" +FROM "IsmsDocumentVersion" v +WHERE v."documentId" = d."id" AND v."isLatest" = true AND d."status" = 'approved'; + +-- 3. For NON-approved documents the latest row was only ever a draft holder; its +-- content now lives on the parent, so drop it. Version rows are henceforth +-- published-only (materialized at approval). +DELETE FROM "IsmsDocumentVersion" v +USING "IsmsDocument" d +WHERE v."documentId" = d."id" AND v."isLatest" = true AND d."status" <> 'approved'; diff --git a/packages/db/prisma/schema/auth.prisma b/packages/db/prisma/schema/auth.prisma index b6d01031c4..0219fc9834 100644 --- a/packages/db/prisma/schema/auth.prisma +++ b/packages/db/prisma/schema/auth.prisma @@ -218,6 +218,7 @@ model Member { approvedPolicies Policy[] @relation("PolicyApprover") // Policies where this member is an approver approvedSOADocuments SOADocument[] @relation("SOADocumentApprover") // SOA documents where this member is an approver approvedIsmsDocuments IsmsDocument[] @relation("IsmsDocumentApprover") // ISMS documents where this member is an approver + publishedIsmsVersions IsmsDocumentVersion[] @relation("IsmsDocumentVersionPublisher") // ISMS document versions this member published risks Risk[] tasks Task[] vendors Vendor[] diff --git a/packages/db/prisma/schema/isms.prisma b/packages/db/prisma/schema/isms.prisma index 536b3b165f..a8ea070346 100644 --- a/packages/db/prisma/schema/isms.prisma +++ b/packages/db/prisma/schema/isms.prisma @@ -50,18 +50,35 @@ model IsmsDocument { title String status IsmsDocumentStatus @default(draft) - // Approval / sign-off (mirrors SOADocument). + // Approval / sign-off (mirrors SOADocument). These track the WORKING DRAFT's + // approval lifecycle; the published version's own approver/date live on the + // IsmsDocumentVersion row (publishedBy/publishedAt). preparedBy String @default("Comp AI") approverId String? approver Member? @relation("IsmsDocumentApprover", fields: [approverId], references: [id], onDelete: SetNull, onUpdate: Cascade) approvedAt DateTime? declinedAt DateTime? + // Versioning (CS-701): mirrors the Policies model (Policy.currentVersionId + + // draftContent). The live register tables + `draftNarrative` are the editable + // DRAFT; `versions` holds immutable PUBLISHED snapshots; `currentVersion` is the + // one that is live and exportable. Editing an approved document reverts `status` + // to draft but never touches `currentVersion`, so v1 stays live while v2 is drafted. + currentVersionId String? @unique + currentVersion IsmsDocumentVersion? @relation("IsmsCurrentVersion", fields: [currentVersionId], references: [id], onDelete: SetNull) + // Working-draft narrative for the singleton documents (Scope 4.3, Leadership 5.1). + // Register documents keep their draft in their own child tables. Replaces the old + // `versions[0].narrative` draft-holder. + draftNarrative Json? + // Drift baseline for the current draft: the derived platform inputs captured at + // the last generate/approve. Replaces the old `versions[0].sourceSnapshot`. + draftSnapshot Json? + createdAt DateTime @default(now()) updatedAt DateTime @updatedAt // Relationships - versions IsmsDocumentVersion[] + versions IsmsDocumentVersion[] @relation("IsmsDocumentVersions") contextIssues IsmsContextIssue[] // 4.1 register interestedParties IsmsInterestedParty[] // 4.2a register interestedPartyRequirements IsmsInterestedPartyRequirement[] // 4.2b/c register @@ -82,25 +99,38 @@ model IsmsDocument { model IsmsDocumentVersion { id String @id @default(dbgenerated("generate_prefixed_cuid('isms_ver'::text)")) documentId String - document IsmsDocument @relation(fields: [documentId], references: [id], onDelete: Cascade) + document IsmsDocument @relation("IsmsDocumentVersions", fields: [documentId], references: [id], onDelete: Cascade) + // Back-relation for IsmsDocument.currentVersion: set when this version is the + // live/published one for its document. + currentForDocument IsmsDocument? @relation("IsmsCurrentVersion") version Int @default(1) isLatest Boolean @default(true) - // Per-section saved narrative + section-level overrides (stored as data, not free - // text, so the model is localisation-ready). + // Frozen published narrative for the singleton documents (Scope, Leadership). + // Empty ({}) for register documents, whose frozen content lives in contentSnapshot. narrative Json - // Snapshot of derived inputs captured at generate / approve. Drift is detected by - // comparing this against the current derived data. + // The full, immutable content snapshot of this published version: the rendered + // export input (register rows + narrative + org profile) plus the export metadata + // captured at publish. Lets a historical version be re-rendered byte-faithfully if + // the stored file is ever missing. Null only for legacy/migrated versions. + contentSnapshot Json? + // Snapshot of derived platform inputs captured at publish, retained for the record + // (the live drift baseline lives on IsmsDocument.draftSnapshot). sourceSnapshot Json? - // Rendered, branded exports. + // Rendered, branded exports retained per published version (S3 keys). Auditors + // sampling an old version download exactly what was approved at the time. pdfUrl String? docxUrl String? - // User/member id who published this version (plain id, like SOAAnswer.createdBy). + // The member who published (approved) this version, and when. `publishedAt != null` + // is the marker of a published version (drafts are not materialized as rows). publishedById String? + publishedBy Member? @relation("IsmsDocumentVersionPublisher", fields: [publishedById], references: [id], onDelete: SetNull) publishedAt DateTime? + // Optional free-text note describing what changed in this version. + changelog String? createdAt DateTime @default(now()) updatedAt DateTime @updatedAt @@ -108,6 +138,7 @@ model IsmsDocumentVersion { @@unique([documentId, version]) @@index([documentId]) @@index([documentId, isLatest]) + @@index([documentId, publishedAt]) } // --- Register: Context of the Organization (clause 4.1) --- From 7484139812981351f60e1b3346d8ef945baf86cd Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Tue, 7 Jul 2026 11:18:59 -0400 Subject: [PATCH 2/5] =?UTF-8?q?fix(isms):=20address=20CS-701=20PR=20review?= =?UTF-8?q?=20=E2=80=94=20export=20integrity,=20download=20UX,=20org=20sco?= =?UTF-8?q?ping?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Claude-Session: https://claude.ai/code/session_012CweXoSSEP89mX93u3Bdcp --- .../api/src/isms/isms-context.service.spec.ts | 47 +++++++++++++- apps/api/src/isms/isms-context.service.ts | 32 +++++++++- .../isms/isms-version.service.export.spec.ts | 57 ++++++++++------- .../api/src/isms/isms-version.service.spec.ts | 39 ++++++++++- apps/api/src/isms/isms-version.service.ts | 64 +++++++++++++------ .../isms/components/IsmsDocumentShell.tsx | 10 +-- .../components/IsmsVersionHistory.test.tsx | 30 +++++++++ .../isms/components/IsmsVersionHistory.tsx | 49 ++++++++------ .../hooks/useIsmsDocumentVersions.test.ts | 10 ++- .../isms/hooks/useIsmsDocumentVersions.ts | 16 ++++- 10 files changed, 279 insertions(+), 75 deletions(-) diff --git a/apps/api/src/isms/isms-context.service.spec.ts b/apps/api/src/isms/isms-context.service.spec.ts index 33a0b7e9e4..1408971fb2 100644 --- a/apps/api/src/isms/isms-context.service.spec.ts +++ b/apps/api/src/isms/isms-context.service.spec.ts @@ -10,6 +10,7 @@ import { } from './documents/snapshot'; import { updateDraftSnapshot } from './utils/draft-snapshot'; import { renderLiveExport } from './utils/export-payload'; +import { invalidateApprovalIfNeeded } from './utils/approval'; jest.mock('@db', () => ({ db: { @@ -33,6 +34,9 @@ jest.mock('./utils/draft-snapshot', () => ({ jest.mock('./utils/export-payload', () => ({ renderLiveExport: jest.fn(), })); +jest.mock('./utils/approval', () => ({ + invalidateApprovalIfNeeded: jest.fn(), +})); const mockDb = jest.mocked(db); const mockCollect = jest.mocked(collectPlatformData); @@ -41,6 +45,7 @@ const mockDiff = jest.mocked(diffPlatformSnapshots); const mockParse = jest.mocked(parsePlatformSnapshot); const mockUpdateDraft = jest.mocked(updateDraftSnapshot); const mockRenderLive = jest.mocked(renderLiveExport); +const mockInvalidate = jest.mocked(invalidateApprovalIfNeeded); const snapshot = { organizationName: 'Acme', @@ -115,6 +120,8 @@ describe('IsmsContextService', () => { documentId: 'doc_1', snapshot, }); + // Regenerating an approved document must revert it to draft. + expect(mockInvalidate).toHaveBeenCalledWith({ tx, documentId: 'doc_1' }); }, ); @@ -210,7 +217,11 @@ describe('IsmsContextService', () => { filename: 'doc.pdf', }; - it('renders the live draft when no versionId is given', async () => { + it('renders the live draft when no versionId is given and a draft is in progress', async () => { + (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ + status: 'draft', + currentVersionId: 'isms_ver_1', + }); mockRenderLive.mockResolvedValue(result); const out = await service.exportDocument({ @@ -228,6 +239,40 @@ describe('IsmsContextService', () => { expect(out).toBe(result); }); + it('serves the published artifact by default for a clean approved document', async () => { + (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ + status: 'approved', + currentVersionId: 'isms_ver_9', + }); + (versionService.getVersionExport as jest.Mock).mockResolvedValue(result); + + const out = await service.exportDocument({ + documentId: 'doc_1', + organizationId: 'org_1', + dto: { format: 'pdf' }, + }); + + expect(versionService.getVersionExport).toHaveBeenCalledWith({ + documentId: 'doc_1', + organizationId: 'org_1', + versionId: 'isms_ver_9', + format: 'pdf', + }); + expect(mockRenderLive).not.toHaveBeenCalled(); + expect(out).toBe(result); + }); + + it('throws NotFoundException when the document is missing', async () => { + (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue(null); + await expect( + service.exportDocument({ + documentId: 'doc_1', + organizationId: 'org_1', + dto: { format: 'pdf' }, + }), + ).rejects.toThrow(NotFoundException); + }); + it('routes to the version service when a versionId is given', async () => { (versionService.getVersionExport as jest.Mock).mockResolvedValue(result); diff --git a/apps/api/src/isms/isms-context.service.ts b/apps/api/src/isms/isms-context.service.ts index 0a48ea8a1b..dc6a50d8d1 100644 --- a/apps/api/src/isms/isms-context.service.ts +++ b/apps/api/src/isms/isms-context.service.ts @@ -8,6 +8,7 @@ import { parsePlatformSnapshot, } from './documents/snapshot'; import type { IsmsPlatformData } from './documents/types'; +import { invalidateApprovalIfNeeded } from './utils/approval'; import { updateDraftSnapshot } from './utils/draft-snapshot'; import { renderLiveExport } from './utils/export-payload'; import type { IsmsExportResult } from './utils/export-generator'; @@ -67,6 +68,10 @@ export class IsmsContextService { })); await db.$transaction(async (tx) => { + // Regenerating changes the working draft, so an approved document must + // revert to draft and be re-approved — matching every other edit path + // (register CRUD, narrative save). Never silently mutate an approved doc. + await invalidateApprovalIfNeeded({ tx, documentId }); await runDerivation({ tx, type: document.type, @@ -106,8 +111,11 @@ export class IsmsContextService { } /** - * Export a document. With `versionId`, serve that published version (stored file - * or snapshot re-render); without, render the current working draft on demand. + * Export a document. + * - With `versionId`: serve that published version (stored file or snapshot). + * - Without: a clean approved document exports its published artifact (exactly + * what was approved); a document with an in-progress draft exports the working + * draft (what the customer is editing), clearly labelled as a draft. */ async exportDocument({ documentId, @@ -126,6 +134,26 @@ export class IsmsContextService { format: dto.format, }); } + + const document = await db.ismsDocument.findFirst({ + where: { id: documentId, organizationId }, + select: { status: true, currentVersionId: true }, + }); + if (!document) { + throw new NotFoundException('ISMS document not found'); + } + + // Approved + no pending draft edits: the published version IS the current + // content — serve the retained artifact so it is byte-identical to approval. + if (document.status === 'approved' && document.currentVersionId) { + return this.versionService.getVersionExport({ + documentId, + organizationId, + versionId: document.currentVersionId, + format: dto.format, + }); + } + return renderLiveExport({ documentId, organizationId, format: dto.format }); } diff --git a/apps/api/src/isms/isms-version.service.export.spec.ts b/apps/api/src/isms/isms-version.service.export.spec.ts index 3ee273d6b8..66c44e448b 100644 --- a/apps/api/src/isms/isms-version.service.export.spec.ts +++ b/apps/api/src/isms/isms-version.service.export.spec.ts @@ -3,11 +3,7 @@ import { db } from '@db'; import type { AttachmentsService } from '../attachments/attachments.service'; import { IsmsVersionService } from './isms-version.service'; import type { IsmsExportSnapshot } from './utils/export-payload'; -import { - parseExportSnapshot, - renderLiveExport, - renderSnapshot, -} from './utils/export-payload'; +import { parseExportSnapshot, renderSnapshot } from './utils/export-payload'; jest.mock('@db', () => ({ db: { @@ -20,7 +16,6 @@ jest.mock('@db', () => ({ jest.mock('./utils/export-payload', () => ({ buildExportInput: jest.fn(() => ({ rows: [] })), parseExportSnapshot: jest.fn(), - renderLiveExport: jest.fn(), renderSnapshot: jest.fn(), resolveOrgProfile: jest.fn(), })); @@ -30,7 +25,6 @@ jest.mock('./utils/export-metadata', () => ({ const mockDb = jest.mocked(db); const mockRenderSnapshot = jest.mocked(renderSnapshot); -const mockRenderLive = jest.mocked(renderLiveExport); const mockParse = jest.mocked(parseExportSnapshot); describe('IsmsVersionService export/render', () => { @@ -43,6 +37,7 @@ describe('IsmsVersionService export/render', () => { beforeEach(() => { jest.clearAllMocks(); jest.spyOn(Logger.prototype, 'error').mockImplementation(() => undefined); + jest.spyOn(Logger.prototype, 'warn').mockImplementation(() => undefined); service = new IsmsVersionService(attachments); }); @@ -111,31 +106,49 @@ describe('IsmsVersionService export/render', () => { expect(result).toBe(rendered); }); - it('falls back to a live render for a legacy version with no snapshot', async () => { + it('falls back to the snapshot when the stored S3 object is unavailable', async () => { (mockDb.ismsDocumentVersion.findFirst as jest.Mock).mockResolvedValue({ - version: 1, - pdfUrl: null, + version: 3, + pdfUrl: 'org/isms/doc/v3.pdf', docxUrl: null, - contentSnapshot: null, + contentSnapshot: { type: 'x' }, document: { title: 'Doc' }, }); - mockParse.mockReturnValue(null); - const live = { - fileBuffer: Buffer.from('live'), + (attachments.getObjectBuffer as jest.Mock).mockRejectedValue( + new Error('NoSuchKey'), + ); + const snapshot = { type: 'x' } as unknown as IsmsExportSnapshot; + mockParse.mockReturnValue(snapshot); + const rendered = { + fileBuffer: Buffer.from('re'), mimeType: 'application/pdf', - filename: 'doc-v1.pdf', + filename: 'doc-v3.pdf', }; - mockRenderLive.mockResolvedValue(live); + mockRenderSnapshot.mockResolvedValue(rendered); const result = await service.getVersionExport(args); - expect(mockRenderSnapshot).not.toHaveBeenCalled(); - expect(mockRenderLive).toHaveBeenCalledWith({ - documentId: 'doc_1', - organizationId: 'org_1', - format: 'pdf', + expect(attachments.getObjectBuffer).toHaveBeenCalledWith( + 'org/isms/doc/v3.pdf', + ); + expect(mockRenderSnapshot).toHaveBeenCalledWith(snapshot, 'pdf'); + expect(result).toBe(rendered); + }); + + it('throws when a version has neither a stored file nor a snapshot (never serves live data)', async () => { + (mockDb.ismsDocumentVersion.findFirst as jest.Mock).mockResolvedValue({ + version: 1, + pdfUrl: null, + docxUrl: null, + contentSnapshot: null, + document: { title: 'Doc' }, }); - expect(result).toBe(live); + mockParse.mockReturnValue(null); + + await expect(service.getVersionExport(args)).rejects.toThrow( + NotFoundException, + ); + expect(mockRenderSnapshot).not.toHaveBeenCalled(); }); }); diff --git a/apps/api/src/isms/isms-version.service.spec.ts b/apps/api/src/isms/isms-version.service.spec.ts index 9236a7c921..ceff4d5764 100644 --- a/apps/api/src/isms/isms-version.service.spec.ts +++ b/apps/api/src/isms/isms-version.service.spec.ts @@ -3,7 +3,11 @@ import { db } from '@db'; import type { Prisma } from '@db'; import type { AttachmentsService } from '../attachments/attachments.service'; import { IsmsVersionService } from './isms-version.service'; -import type { LoadedExportDocument } from './utils/export-payload'; +import type { + IsmsExportSnapshot, + LoadedExportDocument, +} from './utils/export-payload'; +import { parseExportSnapshot } from './utils/export-payload'; // getVersionExport + publishRenders live in isms-version.service.export.spec.ts. jest.mock('@db', () => ({ @@ -20,12 +24,14 @@ jest.mock('@db', () => ({ jest.mock('./utils/export-payload', () => ({ buildExportInput: jest.fn(() => ({ rows: [] })), resolveOrgProfile: jest.fn(), + parseExportSnapshot: jest.fn(() => null), })); jest.mock('./utils/export-metadata', () => ({ buildExportMetadata: jest.fn(() => ({ version: 0 })), })); const mockDb = jest.mocked(db); +const mockParse = jest.mocked(parseExportSnapshot); function asTx(mock: unknown): Prisma.TransactionClient { return mock as Prisma.TransactionClient; @@ -190,5 +196,36 @@ describe('IsmsVersionService', () => { isCurrent: false, }); }); + + it('keeps a version downloadable + attributed via the frozen snapshot when files and member are gone', async () => { + (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ + currentVersionId: 'isms_ver_1', + }); + (mockDb.ismsDocumentVersion.findMany as jest.Mock).mockResolvedValue([ + { + id: 'isms_ver_1', + version: 1, + publishedAt: new Date('2026-07-06T00:00:00.000Z'), + changelog: null, + pdfUrl: null, // stored files absent (e.g. render/upload failed at publish) + docxUrl: null, + contentSnapshot: { type: 'x' }, + publishedBy: null, // approving member later deleted + }, + ]); + mockParse.mockReturnValue({ + metadata: { approverName: 'Frozen Approver' }, + } as unknown as IsmsExportSnapshot); + + const result = await service.getVersions(args); + + // Downloadable via snapshot re-render even without stored files, and the + // approver survives via the frozen snapshot metadata. + expect(result.versions[0]).toMatchObject({ + publishedByName: 'Frozen Approver', + hasPdf: true, + hasDocx: true, + }); + }); }); }); diff --git a/apps/api/src/isms/isms-version.service.ts b/apps/api/src/isms/isms-version.service.ts index 05aaa3dde3..b14416d889 100644 --- a/apps/api/src/isms/isms-version.service.ts +++ b/apps/api/src/isms/isms-version.service.ts @@ -14,7 +14,6 @@ import { import { buildExportInput, parseExportSnapshot, - renderLiveExport, renderSnapshot, resolveOrgProfile, type IsmsExportSnapshot, @@ -186,23 +185,36 @@ export class IsmsVersionService { changelog: true, pdfUrl: true, docxUrl: true, + // The frozen snapshot backs both the approver-name fallback (survives an + // approver being deleted) and download availability (a version is still + // downloadable via snapshot re-render even if its stored file is absent). + contentSnapshot: true, publishedBy: { select: { user: { select: { name: true, email: true } } } }, }, }); return { currentVersionId: document.currentVersionId, - versions: versions.map((v) => ({ - id: v.id, - version: v.version, - publishedAt: v.publishedAt, - changelog: v.changelog, - publishedByName: - v.publishedBy?.user?.name || v.publishedBy?.user?.email || null, - hasPdf: v.pdfUrl != null, - hasDocx: v.docxUrl != null, - isCurrent: v.id === document.currentVersionId, - })), + versions: versions.map((v) => { + const snapshot = parseExportSnapshot(v.contentSnapshot); + const hasSnapshot = snapshot != null; + return { + id: v.id, + version: v.version, + publishedAt: v.publishedAt, + changelog: v.changelog, + // Live member first; fall back to the approver name frozen at publish so + // history keeps an approver even if the member is later removed. + publishedByName: + v.publishedBy?.user?.name || + v.publishedBy?.user?.email || + snapshot?.metadata?.approverName || + null, + hasPdf: v.pdfUrl != null || hasSnapshot, + hasDocx: v.docxUrl != null || hasSnapshot, + isCurrent: v.id === document.currentVersionId, + }; + }), }; } @@ -236,20 +248,34 @@ export class IsmsVersionService { throw new NotFoundException('ISMS document version not found'); } + const mimeType = format === 'pdf' ? 'application/pdf' : DOCX_MIME_TYPE; + const filename = `${sanitizeExportName(version.document.title)}-v${version.version}.${format}`; + + // Prefer the byte-identical stored artifact. If its S3 object is missing or + // temporarily unavailable, fall through to the immutable snapshot rather than + // failing the download. const key = format === 'pdf' ? version.pdfUrl : version.docxUrl; if (key) { - const fileBuffer = await this.attachments.getObjectBuffer(key); - return { - fileBuffer, - mimeType: format === 'pdf' ? 'application/pdf' : DOCX_MIME_TYPE, - filename: `${sanitizeExportName(version.document.title)}-v${version.version}.${format}`, - }; + try { + const fileBuffer = await this.attachments.getObjectBuffer(key); + return { fileBuffer, mimeType, filename }; + } catch (err) { + this.logger.warn( + `Stored export ${key} unavailable for ISMS version ${versionId}; re-rendering from snapshot`, + err instanceof Error ? err.message : String(err), + ); + } } const snapshot = parseExportSnapshot(version.contentSnapshot); if (snapshot) return renderSnapshot(snapshot, format); - return renderLiveExport({ documentId, organizationId, format }); + // Neither a stored file nor a snapshot exists (only possible for a legacy / + // migrated version). Never serve current live data for a specific historical + // version — an auditor must get exactly what was approved, or a clear failure. + throw new NotFoundException( + 'No retained export exists for this document version', + ); } private async nextVersion( diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsDocumentShell.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsDocumentShell.tsx index 662c061768..6e50a82601 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsDocumentShell.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsDocumentShell.tsx @@ -132,13 +132,15 @@ export function IsmsDocumentShell({ const handleApprove = async () => { try { await approve(); - // Approval promotes the draft to a new published version — refresh the - // history list and drift baseline alongside the document itself. - await Promise.all([mutateVersions(), mutateDrift()]); - toast.success('Document approved'); } catch (caught) { toast.error(caught instanceof Error ? caught.message : 'Failed to approve'); + return; } + // Approval already persisted. Refreshing the history list + drift baseline is + // best-effort — a revalidation hiccup must not look like an approval failure + // (which would tempt the user to retry an already-published document). + toast.success('Document approved'); + await Promise.allSettled([mutateVersions(), mutateDrift()]); }; const handleDecline = async () => { diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsVersionHistory.test.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsVersionHistory.test.tsx index 2c31e8b86a..5125201a52 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsVersionHistory.test.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsVersionHistory.test.tsx @@ -69,11 +69,41 @@ describe('IsmsVersionHistory', () => { expect(baseProps.onDownload).toHaveBeenCalledWith('v2', 'docx'); }); + it('only offers formats that are actually available for each version', () => { + render(); + // v1 has hasDocx=false → only v2 exposes a DOCX button (both have PDF). + expect(screen.getAllByRole('button', { name: /PDF/i })).toHaveLength(2); + expect(screen.getAllByRole('button', { name: /DOCX/i })).toHaveLength(1); + }); + + it('shows no download buttons and a note when a version has no retained export', () => { + const noExport: IsmsPublishedVersion[] = [ + { ...VERSIONS[1], id: 'v0', version: 0, hasPdf: false, hasDocx: false }, + ]; + render(); + expect(screen.queryByRole('button', { name: /PDF/i })).not.toBeInTheDocument(); + expect(screen.queryByRole('button', { name: /DOCX/i })).not.toBeInTheDocument(); + expect(screen.getByText('Export unavailable')).toBeInTheDocument(); + }); + it('renders an empty state when there are no published versions', () => { render(); expect(screen.getByText('No published versions yet')).toBeInTheDocument(); }); + it('renders an error state when loading history fails', () => { + render( + , + ); + expect( + screen.getByText("Couldn't load version history"), + ).toBeInTheDocument(); + }); + it('shows a loading state before the first load resolves', () => { render(); expect(screen.getByRole('status')).toBeInTheDocument(); diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsVersionHistory.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsVersionHistory.tsx index 344f0fecc7..38ce641c30 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsVersionHistory.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsVersionHistory.tsx @@ -100,26 +100,35 @@ export function IsmsVersionHistory({ - - + {version.hasPdf && ( + + )} + {version.hasDocx && ( + + )} + {!version.hasPdf && !version.hasDocx && ( + + Export unavailable + + )} ))} diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/useIsmsDocumentVersions.test.ts b/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/useIsmsDocumentVersions.test.ts index ac2a17d3d3..c7f02b1061 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/useIsmsDocumentVersions.test.ts +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/useIsmsDocumentVersions.test.ts @@ -46,9 +46,13 @@ describe('useIsmsDocumentVersions', () => { renderHook(() => useIsmsDocumentVersions(DOC_ID, ORG_ID)); - expect(swr.key).toEqual(['/v1/isms/documents', DOC_ID, 'versions']); - await swr.fetcher?.(['/v1/isms/documents', DOC_ID, 'versions']); - expect(getMock).toHaveBeenCalledWith(`/v1/isms/documents/${DOC_ID}/versions`); + expect(swr.key).toEqual(['/v1/isms/documents', DOC_ID, 'versions', ORG_ID]); + await swr.fetcher?.(['/v1/isms/documents', DOC_ID, 'versions', ORG_ID]); + // Passes the route org (X-Organization-Id), matching the download path. + expect(getMock).toHaveBeenCalledWith( + `/v1/isms/documents/${DOC_ID}/versions`, + ORG_ID, + ); }); it('does not fetch when no document id is provided', () => { diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/useIsmsDocumentVersions.ts b/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/useIsmsDocumentVersions.ts index 7a88488610..03b5496284 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/useIsmsDocumentVersions.ts +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/useIsmsDocumentVersions.ts @@ -19,10 +19,20 @@ export function useIsmsDocumentVersions( ) { const { data, error, isLoading, mutate } = useSWR( documentId - ? (['/v1/isms/documents', documentId, 'versions'] as const) + ? (['/v1/isms/documents', documentId, 'versions', organizationId] as const) : null, - async ([base, id]: readonly [string, string, string]) => { - const response = await api.get(`${base}/${id}/versions`); + async ([base, id, , orgId]: readonly [ + string, + string, + string, + string | undefined, + ]) => { + // Pass the route org so history stays in the same org context as the + // per-version downloads (X-Organization-Id), not the session's active org. + const response = await api.get( + `${base}/${id}/versions`, + orgId, + ); if (response.error || !response.data) { throw new Error(response.error ?? 'Failed to load version history'); } From dfcdc2f20b03065139c165941c785851e336ee18 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Tue, 7 Jul 2026 11:46:46 -0400 Subject: [PATCH 3/5] =?UTF-8?q?fix(isms):=20second=20review=20pass=20?= =?UTF-8?q?=E2=80=94=20default=20export,=20download=20errors,=20tx=20isola?= =?UTF-8?q?tion,=20a11y?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Claude-Session: https://claude.ai/code/session_012CweXoSSEP89mX93u3Bdcp --- apps/api/src/isms/documents/org-profile.ts | 14 ++++-- .../api/src/isms/isms-context.service.spec.ts | 48 ++++++++++--------- apps/api/src/isms/isms-context.service.ts | 14 +++--- apps/api/src/isms/isms-version.service.ts | 4 +- apps/api/src/isms/utils/export-payload.ts | 2 + .../isms/components/IsmsVersionHistory.tsx | 2 + .../hooks/useIsmsDocumentVersions.test.ts | 17 +++++++ .../isms/hooks/useIsmsDocumentVersions.ts | 6 +++ 8 files changed, 73 insertions(+), 34 deletions(-) diff --git a/apps/api/src/isms/documents/org-profile.ts b/apps/api/src/isms/documents/org-profile.ts index be77f5ccdb..446ac9feaf 100644 --- a/apps/api/src/isms/documents/org-profile.ts +++ b/apps/api/src/isms/documents/org-profile.ts @@ -1,4 +1,5 @@ import { db } from '@db'; +import type { Prisma } from '@db'; import { DEFAULT_INTENDED_OUTCOMES } from '../wizard/wizard-defaults'; import { parseStoredAnswers } from '../wizard/wizard-schema'; import type { IsmsKeyValue } from '../utils/export-shared'; @@ -29,23 +30,30 @@ const QUESTIONS = { export async function loadOrgProfile({ organizationId, frameworkId, + client = db, }: { organizationId: string; frameworkId: string; + /** + * DB client to read through. Defaults to the global `db`; the approve flow + * passes its transaction client so the snapshot's org profile reads share the + * same isolation context as the register rows written in the same transaction. + */ + client?: Prisma.TransactionClient; }): Promise { const [organization, contextEntries, profile] = await Promise.all([ - db.organization.findUnique({ + client.organization.findUnique({ where: { id: organizationId }, select: { name: true, website: true }, }), - db.context.findMany({ + client.context.findMany({ where: { organizationId }, // Deterministic ordering so duplicate questions resolve to the same answer // every export; the earliest-created entry wins (id breaks createdAt ties). orderBy: [{ createdAt: 'asc' }, { id: 'asc' }], select: { question: true, answer: true }, }), - db.ismsProfile.findUnique({ + client.ismsProfile.findUnique({ where: { organizationId_frameworkId: { organizationId, frameworkId } }, select: { answers: true }, }), diff --git a/apps/api/src/isms/isms-context.service.spec.ts b/apps/api/src/isms/isms-context.service.spec.ts index 1408971fb2..b7d46147c3 100644 --- a/apps/api/src/isms/isms-context.service.spec.ts +++ b/apps/api/src/isms/isms-context.service.spec.ts @@ -217,10 +217,9 @@ describe('IsmsContextService', () => { filename: 'doc.pdf', }; - it('renders the live draft when no versionId is given and a draft is in progress', async () => { + it('renders the live draft only when the document has no published version', async () => { (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ - status: 'draft', - currentVersionId: 'isms_ver_1', + currentVersionId: null, }); mockRenderLive.mockResolvedValue(result); @@ -239,28 +238,31 @@ describe('IsmsContextService', () => { expect(out).toBe(result); }); - it('serves the published artifact by default for a clean approved document', async () => { - (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ - status: 'approved', - currentVersionId: 'isms_ver_9', - }); - (versionService.getVersionExport as jest.Mock).mockResolvedValue(result); + it.each(['approved', 'draft', 'needs_review'])( + 'serves the published version by default whenever one exists (status %s)', + async (status) => { + (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue({ + status, + currentVersionId: 'isms_ver_9', + }); + (versionService.getVersionExport as jest.Mock).mockResolvedValue(result); - const out = await service.exportDocument({ - documentId: 'doc_1', - organizationId: 'org_1', - dto: { format: 'pdf' }, - }); + const out = await service.exportDocument({ + documentId: 'doc_1', + organizationId: 'org_1', + dto: { format: 'pdf' }, + }); - expect(versionService.getVersionExport).toHaveBeenCalledWith({ - documentId: 'doc_1', - organizationId: 'org_1', - versionId: 'isms_ver_9', - format: 'pdf', - }); - expect(mockRenderLive).not.toHaveBeenCalled(); - expect(out).toBe(result); - }); + expect(versionService.getVersionExport).toHaveBeenCalledWith({ + documentId: 'doc_1', + organizationId: 'org_1', + versionId: 'isms_ver_9', + format: 'pdf', + }); + expect(mockRenderLive).not.toHaveBeenCalled(); + expect(out).toBe(result); + }, + ); it('throws NotFoundException when the document is missing', async () => { (mockDb.ismsDocument.findFirst as jest.Mock).mockResolvedValue(null); diff --git a/apps/api/src/isms/isms-context.service.ts b/apps/api/src/isms/isms-context.service.ts index dc6a50d8d1..cae5156c06 100644 --- a/apps/api/src/isms/isms-context.service.ts +++ b/apps/api/src/isms/isms-context.service.ts @@ -113,9 +113,9 @@ export class IsmsContextService { /** * Export a document. * - With `versionId`: serve that published version (stored file or snapshot). - * - Without: a clean approved document exports its published artifact (exactly - * what was approved); a document with an in-progress draft exports the working - * draft (what the customer is editing), clearly labelled as a draft. + * - Without: serve the current PUBLISHED version whenever one exists — it stays + * live and exportable while a new draft is in progress (CS-701). Only render + * the live draft when the document has never been published yet. */ async exportDocument({ documentId, @@ -137,15 +137,15 @@ export class IsmsContextService { const document = await db.ismsDocument.findFirst({ where: { id: documentId, organizationId }, - select: { status: true, currentVersionId: true }, + select: { currentVersionId: true }, }); if (!document) { throw new NotFoundException('ISMS document not found'); } - // Approved + no pending draft edits: the published version IS the current - // content — serve the retained artifact so it is byte-identical to approval. - if (document.status === 'approved' && document.currentVersionId) { + // A published version is the document's official artifact and remains the + // default export even while an approved document is being re-drafted. + if (document.currentVersionId) { return this.versionService.getVersionExport({ documentId, organizationId, diff --git a/apps/api/src/isms/isms-version.service.ts b/apps/api/src/isms/isms-version.service.ts index b14416d889..d54a0a9926 100644 --- a/apps/api/src/isms/isms-version.service.ts +++ b/apps/api/src/isms/isms-version.service.ts @@ -66,7 +66,9 @@ export class IsmsVersionService { }): Promise<{ versionId: string; version: number; snapshot: IsmsExportSnapshot }> { const nextVersion = await this.nextVersion(tx, document.id); - const orgProfile = await resolveOrgProfile(document); + // Read the org profile through the same transaction so the snapshot's profile + // and the register rows written in this transaction share one point in time. + const orgProfile = await resolveOrgProfile(document, tx); const input = buildExportInput({ document, orgProfile }); const metadata = buildExportMetadata({ type: document.type, diff --git a/apps/api/src/isms/utils/export-payload.ts b/apps/api/src/isms/utils/export-payload.ts index 17e45e7c23..efd9346c33 100644 --- a/apps/api/src/isms/utils/export-payload.ts +++ b/apps/api/src/isms/utils/export-payload.ts @@ -54,11 +54,13 @@ export async function loadExportDocument( /** The Context document (4.1) renders an org overview; other types don't need it. */ export async function resolveOrgProfile( document: LoadedExportDocument, + client?: Prisma.TransactionClient, ): Promise { if (document.type !== 'context_of_organization') return undefined; return loadOrgProfile({ organizationId: document.organizationId, frameworkId: document.frameworkId, + client, }); } diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsVersionHistory.tsx b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsVersionHistory.tsx index 38ce641c30..fd5881252b 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsVersionHistory.tsx +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/components/IsmsVersionHistory.tsx @@ -108,6 +108,7 @@ export function IsmsVersionHistory({ disabled={downloadingVersionId === version.id} onClick={() => void onDownload(version.id, 'pdf')} iconLeft={} + aria-label={`Download v${version.version} as PDF`} > PDF @@ -120,6 +121,7 @@ export function IsmsVersionHistory({ disabled={downloadingVersionId === version.id} onClick={() => void onDownload(version.id, 'docx')} iconLeft={} + aria-label={`Download v${version.version} as DOCX`} > DOCX diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/useIsmsDocumentVersions.test.ts b/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/useIsmsDocumentVersions.test.ts index c7f02b1061..ad7b053fce 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/useIsmsDocumentVersions.test.ts +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/useIsmsDocumentVersions.test.ts @@ -25,11 +25,17 @@ vi.mock('./exportIsmsDocument', () => ({ exportIsmsDocument: vi.fn().mockResolvedValue(undefined), })); +vi.mock('sonner', () => ({ + toast: { error: vi.fn(), success: vi.fn() }, +})); + import { useIsmsDocumentVersions } from './useIsmsDocumentVersions'; import { exportIsmsDocument } from './exportIsmsDocument'; +import { toast } from 'sonner'; const getMock = vi.mocked(api.get); const exportMock = vi.mocked(exportIsmsDocument); +const toastErrorMock = vi.mocked(toast.error); const DOC_ID = 'doc_1'; const ORG_ID = 'org_1'; @@ -74,4 +80,15 @@ describe('useIsmsDocumentVersions', () => { versionId: 'isms_ver_1', }); }); + + it('surfaces a toast when a version download fails', async () => { + exportMock.mockRejectedValueOnce(new Error('boom')); + const { result } = renderHook(() => useIsmsDocumentVersions(DOC_ID, ORG_ID)); + + await act(async () => { + await result.current.downloadVersion('isms_ver_1', 'pdf'); + }); + + expect(toastErrorMock).toHaveBeenCalledWith('boom'); + }); }); diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/useIsmsDocumentVersions.ts b/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/useIsmsDocumentVersions.ts index 03b5496284..7fcf5d30f8 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/useIsmsDocumentVersions.ts +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/useIsmsDocumentVersions.ts @@ -1,6 +1,7 @@ 'use client'; import { useState } from 'react'; +import { toast } from 'sonner'; import useSWR from 'swr'; import { api } from '@/lib/api-client'; import type { IsmsExportFormat, IsmsVersionHistory } from '../isms-types'; @@ -57,6 +58,11 @@ export function useIsmsDocumentVersions( organizationId, versionId, }); + } catch (caught) { + // Surface the failure instead of a silent no-op / unhandled rejection. + toast.error( + caught instanceof Error ? caught.message : 'Failed to download version', + ); } finally { setDownloadingVersionId(null); } From 7afeef39268fc1bd6fc0235f70da293e2ef39bf9 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Tue, 7 Jul 2026 12:30:50 -0400 Subject: [PATCH 4/5] =?UTF-8?q?fix(isms):=20third=20review=20pass=20?= =?UTF-8?q?=E2=80=94=20no=20snapshot-less=20published=20versions,=20downlo?= =?UTF-8?q?ad=20races?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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) Claude-Session: https://claude.ai/code/session_012CweXoSSEP89mX93u3Bdcp --- .../isms/hooks/useIsmsDocumentVersions.ts | 6 ++- .../migration.sql | 37 ++++++++----------- 2 files changed, 21 insertions(+), 22 deletions(-) diff --git a/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/useIsmsDocumentVersions.ts b/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/useIsmsDocumentVersions.ts index 7fcf5d30f8..ae80c58f25 100644 --- a/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/useIsmsDocumentVersions.ts +++ b/apps/app/src/app/(app)/[orgId]/documents/isms/hooks/useIsmsDocumentVersions.ts @@ -64,7 +64,11 @@ export function useIsmsDocumentVersions( caught instanceof Error ? caught.message : 'Failed to download version', ); } finally { - setDownloadingVersionId(null); + // Only clear if this request still owns the indicator — a later download of + // a different version must not have its spinner cleared by an earlier one. + setDownloadingVersionId((current) => + current === versionId ? null : current, + ); } }; diff --git a/packages/db/prisma/migrations/20260707120000_isms_document_versioning/migration.sql b/packages/db/prisma/migrations/20260707120000_isms_document_versioning/migration.sql index bce7d29ed6..3f2b7fb6b1 100644 --- a/packages/db/prisma/migrations/20260707120000_isms_document_versioning/migration.sql +++ b/packages/db/prisma/migrations/20260707120000_isms_document_versioning/migration.sql @@ -37,24 +37,19 @@ SET "draftNarrative" = v."narrative", FROM "IsmsDocumentVersion" v WHERE v."documentId" = d."id" AND v."isLatest" = true; --- 2. For APPROVED documents, promote the existing latest row to published v1: --- stamp publish metadata from the document's approval and point currentVersion --- at it. contentSnapshot stays NULL -> historical export falls back to an --- on-demand render (draft == published at migration time, so this is faithful). -UPDATE "IsmsDocumentVersion" v -SET "publishedAt" = COALESCE(d."approvedAt", d."updatedAt"), - "publishedById" = d."approverId" -FROM "IsmsDocument" d -WHERE v."documentId" = d."id" AND v."isLatest" = true AND d."status" = 'approved'; - -UPDATE "IsmsDocument" d -SET "currentVersionId" = v."id" -FROM "IsmsDocumentVersion" v -WHERE v."documentId" = d."id" AND v."isLatest" = true AND d."status" = 'approved'; - --- 3. For NON-approved documents the latest row was only ever a draft holder; its --- content now lives on the parent, so drop it. Version rows are henceforth --- published-only (materialized at approval). -DELETE FROM "IsmsDocumentVersion" v -USING "IsmsDocument" d -WHERE v."documentId" = d."id" AND v."isLatest" = true AND d."status" <> 'approved'; +-- 2. The pre-CS-701 version row was only ever a mutable draft holder — it has no +-- frozen contentSnapshot, so it cannot serve as an immutable published version +-- (a snapshot-less "current version" would 404 on export). Revert previously +-- approved documents to draft so they are re-approved once to establish a real +-- v1 (with a proper snapshot + retained artifacts), and drop every legacy row. +-- From here on a published version is ONLY created by the approve flow, which +-- always writes a contentSnapshot — so currentVersionId never points at a +-- snapshot-less version. +UPDATE "IsmsDocument" +SET "status" = 'draft', + "approvedAt" = NULL, + "approverId" = NULL, + "declinedAt" = NULL +WHERE "status" = 'approved'; + +DELETE FROM "IsmsDocumentVersion"; From 71cefa780c029ee526be8da5d88c3a388f7ce5bf Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Tue, 7 Jul 2026 13:01:42 -0400 Subject: [PATCH 5/5] =?UTF-8?q?fix(isms):=20migration=20=E2=80=94=20preser?= =?UTF-8?q?ve=20approvals,=20add=20FKs=20after=20legacy=20cleanup?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Claude-Session: https://claude.ai/code/session_012CweXoSSEP89mX93u3Bdcp --- .../migration.sql | 54 +++++++++---------- 1 file changed, 26 insertions(+), 28 deletions(-) diff --git a/packages/db/prisma/migrations/20260707120000_isms_document_versioning/migration.sql b/packages/db/prisma/migrations/20260707120000_isms_document_versioning/migration.sql index 3f2b7fb6b1..bd3d69c426 100644 --- a/packages/db/prisma/migrations/20260707120000_isms_document_versioning/migration.sql +++ b/packages/db/prisma/migrations/20260707120000_isms_document_versioning/migration.sql @@ -1,7 +1,7 @@ -- ISMS document versioning & history (CS-701) -- Brings ISMS documents onto the Policies versioning model: a published-version --- pointer + working-draft fields on the parent, per-version publish metadata + --- an immutable content snapshot on the version, then a backfill of existing rows. +-- pointer + working-draft fields on the parent, and per-version publish metadata + +-- an immutable content snapshot on the version. -- AlterTable: working-draft holders + current-published-version pointer. ALTER TABLE "IsmsDocument" ADD COLUMN "currentVersionId" TEXT, @@ -12,21 +12,10 @@ ADD COLUMN "draftSnapshot" JSONB; ALTER TABLE "IsmsDocumentVersion" ADD COLUMN "changelog" TEXT, ADD COLUMN "contentSnapshot" JSONB; --- CreateIndex -CREATE UNIQUE INDEX "IsmsDocument_currentVersionId_key" ON "IsmsDocument"("currentVersionId"); - --- CreateIndex -CREATE INDEX "IsmsDocumentVersion_documentId_publishedAt_idx" ON "IsmsDocumentVersion"("documentId", "publishedAt"); - --- AddForeignKey -ALTER TABLE "IsmsDocument" ADD CONSTRAINT "IsmsDocument_currentVersionId_fkey" FOREIGN KEY ("currentVersionId") REFERENCES "IsmsDocumentVersion"("id") ON DELETE SET NULL ON UPDATE CASCADE; - --- AddForeignKey -ALTER TABLE "IsmsDocumentVersion" ADD CONSTRAINT "IsmsDocumentVersion_publishedById_fkey" FOREIGN KEY ("publishedById") REFERENCES "Member"("id") ON DELETE SET NULL ON UPDATE CASCADE; - -- --------------------------------------------------------------------------- --- Data backfill: migrate the single-mutable-version model onto published+draft. --- ISMS is pre-GA (flag-gated), so volumes are tiny; the backfill is defensive. +-- Data backfill. Runs BEFORE the new indexes/constraints so the legacy-row +-- cleanup can never trip foreign-key validation. ISMS is pre-GA (flag-gated), +-- so volumes are tiny. -- --------------------------------------------------------------------------- -- 1. Move the working narrative + drift baseline off the (previously single) @@ -37,19 +26,28 @@ SET "draftNarrative" = v."narrative", FROM "IsmsDocumentVersion" v WHERE v."documentId" = d."id" AND v."isLatest" = true; --- 2. The pre-CS-701 version row was only ever a mutable draft holder — it has no +-- 2. The pre-CS-701 version row was only ever a mutable draft holder with no -- frozen contentSnapshot, so it cannot serve as an immutable published version --- (a snapshot-less "current version" would 404 on export). Revert previously --- approved documents to draft so they are re-approved once to establish a real --- v1 (with a proper snapshot + retained artifacts), and drop every legacy row. +-- (a snapshot-less "current version" would 404 on export). Drop every legacy +-- row; its content is preserved on the parent by step 1. +-- +-- Approvals are intentionally preserved: `currentVersionId` stays null, so an +-- approved document keeps its status and simply exports its (unchanged) current +-- content on demand until its next approval captures a real versioned artifact. -- From here on a published version is ONLY created by the approve flow, which -- always writes a contentSnapshot — so currentVersionId never points at a --- snapshot-less version. -UPDATE "IsmsDocument" -SET "status" = 'draft', - "approvedAt" = NULL, - "approverId" = NULL, - "declinedAt" = NULL -WHERE "status" = 'approved'; - +-- snapshot-less version. This DELETE also clears the legacy (always-null) +-- `publishedById` values before the FK below is added. DELETE FROM "IsmsDocumentVersion"; + +-- CreateIndex +CREATE UNIQUE INDEX "IsmsDocument_currentVersionId_key" ON "IsmsDocument"("currentVersionId"); + +-- CreateIndex +CREATE INDEX "IsmsDocumentVersion_documentId_publishedAt_idx" ON "IsmsDocumentVersion"("documentId", "publishedAt"); + +-- AddForeignKey +ALTER TABLE "IsmsDocument" ADD CONSTRAINT "IsmsDocument_currentVersionId_fkey" FOREIGN KEY ("currentVersionId") REFERENCES "IsmsDocumentVersion"("id") ON DELETE SET NULL ON UPDATE CASCADE; + +-- AddForeignKey +ALTER TABLE "IsmsDocumentVersion" ADD CONSTRAINT "IsmsDocumentVersion_publishedById_fkey" FOREIGN KEY ("publishedById") REFERENCES "Member"("id") ON DELETE SET NULL ON UPDATE CASCADE;