From c9f0bdc687f41db2485825c686254dd87469b518 Mon Sep 17 00:00:00 2001 From: Matt Date: Mon, 11 May 2026 12:13:27 +0200 Subject: [PATCH] fix(documents): tighten cross-port test + refine paths + signing-details coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three follow-ups from Task 9 code review: 1. Cross-port isolation test now explicitly asserts the other-port file's id is absent from the aggregated result (previously only checked .length > 0, which would pass even with leakage). 2. Refine errors now carry path fields so frontend field-level error display can target the right form input (matches createDocumentSchema pattern in the same validators module). 3. Add a service-composition test for the signing-details route's workflow+signers+events shape — closes the coverage gap for the thin Promise.all combinator. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/lib/validators/documents.ts | 6 +- src/lib/validators/files.ts | 6 +- .../files-folder-aggregation.test.ts | 91 ++++++++++++++++++- 3 files changed, 97 insertions(+), 6 deletions(-) diff --git a/src/lib/validators/documents.ts b/src/lib/validators/documents.ts index 78de2916..02de6de8 100644 --- a/src/lib/validators/documents.ts +++ b/src/lib/validators/documents.ts @@ -106,10 +106,14 @@ export const listDocumentsSchema = baseListQuerySchema }) .refine( (q) => !(q.folderId !== undefined && (q.entityType !== undefined || q.entityId !== undefined)), - { message: 'folderId is mutually exclusive with entityType/entityId' }, + { + message: 'folderId is mutually exclusive with entityType/entityId', + path: ['folderId'], + }, ) .refine((q) => Boolean(q.entityType) === Boolean(q.entityId), { message: 'entityType and entityId must be provided together', + path: ['entityType'], }); export const uploadSignedSchema = z.object({ diff --git a/src/lib/validators/files.ts b/src/lib/validators/files.ts index f669512c..f74f7db5 100644 --- a/src/lib/validators/files.ts +++ b/src/lib/validators/files.ts @@ -31,10 +31,14 @@ export const listFilesSchema = baseListQuerySchema }) .refine( (q) => !(q.folderId !== undefined && (q.entityType !== undefined || q.entityId !== undefined)), - { message: 'folderId is mutually exclusive with entityType/entityId' }, + { + message: 'folderId is mutually exclusive with entityType/entityId', + path: ['folderId'], + }, ) .refine((q) => Boolean(q.entityType) === Boolean(q.entityId), { message: 'entityType and entityId must be provided together', + path: ['entityType'], }); export type UploadFileInput = z.infer; diff --git a/tests/integration/files-folder-aggregation.test.ts b/tests/integration/files-folder-aggregation.test.ts index 21908d16..014f47c7 100644 --- a/tests/integration/files-folder-aggregation.test.ts +++ b/tests/integration/files-folder-aggregation.test.ts @@ -9,6 +9,8 @@ * 3. listFilesSchema refine: folderId + entityType together is rejected. * 4. listFilesSchema refine: entityType without entityId is rejected. * 5. listDocumentsSchema refine: same mutual-exclusion rules. + * 6. signing-details route service composition: getDocumentById + + * listDocumentSigners + listDocumentEvents compose into the expected shape. * * Uses makePort / makeClient / makeCompany / makeMembership factory pattern. */ @@ -16,10 +18,15 @@ import { describe, it, expect, beforeAll, beforeEach } from 'vitest'; import { db } from '@/lib/db'; -import { files, documents } from '@/lib/db/schema/documents'; +import { files, documents, documentSigners, documentEvents } from '@/lib/db/schema/documents'; import { user } from '@/lib/db/schema/users'; import { listFilesAggregatedByEntity } from '@/lib/services/files'; -import { listInflightWorkflowsAggregatedByEntity } from '@/lib/services/documents.service'; +import { + listInflightWorkflowsAggregatedByEntity, + getDocumentById, + listDocumentSigners, + listDocumentEvents, +} from '@/lib/services/documents.service'; import { listFilesSchema } from '@/lib/validators/files'; import { listDocumentsSchema } from '@/lib/validators/documents'; import { makePort, makeClient, makeCompany, makeMembership } from '../helpers/factories'; @@ -117,14 +124,15 @@ describe('GET /api/v1/files?entityType=client&entityId=… — service layer', ( it('does not include other-port files', async () => { const otherPort = await makePort(); const otherClient = await makeClient({ portId: otherPort.id }); - await insertFile(otherPort.id, { clientId: otherClient.id }); + const otherFile = await insertFile(otherPort.id, { clientId: otherClient.id }); const result = await listFilesAggregatedByEntity(portId, 'client', clientId); // Groups are only for the correct port — the other-port client's file must not appear const allFileIds = result.groups.flatMap((g) => g.files.map((f) => (f as { id: string }).id)); - // Just verifying groups are non-empty for our port and don't reference cross-port data expect(result.groups.length).toBeGreaterThan(0); expect(allFileIds.length).toBeGreaterThan(0); + // Explicit cross-port isolation assertion — leakage would cause this to fail + expect(allFileIds).not.toContain(otherFile.id); }); }); @@ -256,3 +264,78 @@ describe('listDocumentsSchema refine rules', () => { expect(result.success).toBe(true); }); }); + +// ─── Signing-details route service composition ──────────────────────────────── + +describe('signing-details route service composition', () => { + let portId: string; + let docId: string; + + beforeEach(async () => { + const port = await makePort(); + portId = port.id; + + const client = await makeClient({ portId }); + + const [doc] = await db + .insert(documents) + .values({ + portId, + clientId: client.id, + documentType: 'contract', + title: 'Signing Details Test', + status: 'sent', + createdBy: TEST_USER_ID, + }) + .returning(); + docId = doc!.id; + + // Insert one signer + await db.insert(documentSigners).values({ + documentId: docId, + signerName: 'Alice Signer', + signerEmail: 'alice@example.com', + signerRole: 'client', + signingOrder: 1, + status: 'pending', + }); + + // Insert one event + await db.insert(documentEvents).values({ + documentId: docId, + eventType: 'sent', + }); + }); + + it('returns workflow, signers, and events for a document', async () => { + const [doc, signers, events] = await Promise.all([ + getDocumentById(docId, portId), + listDocumentSigners(docId, portId), + listDocumentEvents(docId, portId), + ]); + + expect(doc).toBeDefined(); + expect(doc.id).toBe(docId); + expect(signers).toBeInstanceOf(Array); + expect(events).toBeInstanceOf(Array); + expect(signers.length).toBeGreaterThanOrEqual(1); + expect(events.length).toBeGreaterThanOrEqual(1); + }); + + it('signers carry expected fields', async () => { + const signers = await listDocumentSigners(docId, portId); + const signer = signers[0]!; + expect(signer).toHaveProperty('signerName', 'Alice Signer'); + expect(signer).toHaveProperty('signerEmail', 'alice@example.com'); + expect(signer).toHaveProperty('signerRole', 'client'); + expect(signer).toHaveProperty('signingOrder', 1); + expect(signer).toHaveProperty('status', 'pending'); + }); + + it('events carry expected fields', async () => { + const events = await listDocumentEvents(docId, portId); + const event = events[0]!; + expect(event).toHaveProperty('eventType', 'sent'); + expect(event).toHaveProperty('documentId', docId); + }); +});