fix(documents): tighten cross-port test + refine paths + signing-details coverage
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) <noreply@anthropic.com>
This commit is contained in:
@@ -106,10 +106,14 @@ export const listDocumentsSchema = baseListQuerySchema
|
|||||||
})
|
})
|
||||||
.refine(
|
.refine(
|
||||||
(q) => !(q.folderId !== undefined && (q.entityType !== undefined || q.entityId !== undefined)),
|
(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), {
|
.refine((q) => Boolean(q.entityType) === Boolean(q.entityId), {
|
||||||
message: 'entityType and entityId must be provided together',
|
message: 'entityType and entityId must be provided together',
|
||||||
|
path: ['entityType'],
|
||||||
});
|
});
|
||||||
|
|
||||||
export const uploadSignedSchema = z.object({
|
export const uploadSignedSchema = z.object({
|
||||||
|
|||||||
@@ -31,10 +31,14 @@ export const listFilesSchema = baseListQuerySchema
|
|||||||
})
|
})
|
||||||
.refine(
|
.refine(
|
||||||
(q) => !(q.folderId !== undefined && (q.entityType !== undefined || q.entityId !== undefined)),
|
(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), {
|
.refine((q) => Boolean(q.entityType) === Boolean(q.entityId), {
|
||||||
message: 'entityType and entityId must be provided together',
|
message: 'entityType and entityId must be provided together',
|
||||||
|
path: ['entityType'],
|
||||||
});
|
});
|
||||||
|
|
||||||
export type UploadFileInput = z.infer<typeof uploadFileSchema>;
|
export type UploadFileInput = z.infer<typeof uploadFileSchema>;
|
||||||
|
|||||||
@@ -9,6 +9,8 @@
|
|||||||
* 3. listFilesSchema refine: folderId + entityType together is rejected.
|
* 3. listFilesSchema refine: folderId + entityType together is rejected.
|
||||||
* 4. listFilesSchema refine: entityType without entityId is rejected.
|
* 4. listFilesSchema refine: entityType without entityId is rejected.
|
||||||
* 5. listDocumentsSchema refine: same mutual-exclusion rules.
|
* 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.
|
* Uses makePort / makeClient / makeCompany / makeMembership factory pattern.
|
||||||
*/
|
*/
|
||||||
@@ -16,10 +18,15 @@
|
|||||||
import { describe, it, expect, beforeAll, beforeEach } from 'vitest';
|
import { describe, it, expect, beforeAll, beforeEach } from 'vitest';
|
||||||
|
|
||||||
import { db } from '@/lib/db';
|
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 { user } from '@/lib/db/schema/users';
|
||||||
import { listFilesAggregatedByEntity } from '@/lib/services/files';
|
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 { listFilesSchema } from '@/lib/validators/files';
|
||||||
import { listDocumentsSchema } from '@/lib/validators/documents';
|
import { listDocumentsSchema } from '@/lib/validators/documents';
|
||||||
import { makePort, makeClient, makeCompany, makeMembership } from '../helpers/factories';
|
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 () => {
|
it('does not include other-port files', async () => {
|
||||||
const otherPort = await makePort();
|
const otherPort = await makePort();
|
||||||
const otherClient = await makeClient({ portId: otherPort.id });
|
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);
|
const result = await listFilesAggregatedByEntity(portId, 'client', clientId);
|
||||||
// Groups are only for the correct port — the other-port client's file must not appear
|
// 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));
|
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(result.groups.length).toBeGreaterThan(0);
|
||||||
expect(allFileIds.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);
|
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);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user