fix(documents): tighten aggregation — filter ended memberships + symmetry
Four follow-ups from Task 8 code review: 1. Aggregation now filters companyMemberships to active rows only (isNull(endDate)) on both client→companies and company→clients joins. Previously a rep who left a company 2y ago would still see that company's files in their aggregated view. Brings this service in line with the 8 other call sites in the codebase that already filter on endDate. 2. Move collectRelatedEntities import to the top of documents.service.ts — was wedged mid-file. 3. listInflightWorkflowsAggregatedByEntity now calls assertEntityInPort for symmetry with the files version. Cross- port reads short-circuit early instead of executing N empty port-scoped queries. 4. Add a cross-port leakage regression test for the workflow projection. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -41,6 +41,7 @@ import {
|
|||||||
type FolderNode,
|
type FolderNode,
|
||||||
type EntityType,
|
type EntityType,
|
||||||
} from '@/lib/services/document-folders.service';
|
} from '@/lib/services/document-folders.service';
|
||||||
|
import { assertEntityInPort, collectRelatedEntities } from '@/lib/services/files';
|
||||||
import type {
|
import type {
|
||||||
CreateDocumentInput,
|
CreateDocumentInput,
|
||||||
UpdateDocumentInput,
|
UpdateDocumentInput,
|
||||||
@@ -1822,8 +1823,6 @@ export async function createFromUpload(
|
|||||||
|
|
||||||
// ─── Aggregated Workflow Projection ───────────────────────────────────────────
|
// ─── Aggregated Workflow Projection ───────────────────────────────────────────
|
||||||
|
|
||||||
import { collectRelatedEntities } from '@/lib/services/files';
|
|
||||||
|
|
||||||
export interface AggregatedWorkflowGroup {
|
export interface AggregatedWorkflowGroup {
|
||||||
label: string;
|
label: string;
|
||||||
source: 'direct' | 'client' | 'company' | 'yacht';
|
source: 'direct' | 'client' | 'company' | 'yacht';
|
||||||
@@ -1844,6 +1843,9 @@ export async function listInflightWorkflowsAggregatedByEntity(
|
|||||||
entityType: 'client' | 'company' | 'yacht',
|
entityType: 'client' | 'company' | 'yacht',
|
||||||
entityId: string,
|
entityId: string,
|
||||||
): Promise<{ groups: AggregatedWorkflowGroup[] }> {
|
): Promise<{ groups: AggregatedWorkflowGroup[] }> {
|
||||||
|
const entityExists = await assertEntityInPort(portId, entityType, entityId);
|
||||||
|
if (!entityExists) return { groups: [] };
|
||||||
|
|
||||||
const related = await collectRelatedEntities(portId, entityType, entityId);
|
const related = await collectRelatedEntities(portId, entityType, entityId);
|
||||||
const groups: AggregatedWorkflowGroup[] = [];
|
const groups: AggregatedWorkflowGroup[] = [];
|
||||||
|
|
||||||
@@ -1910,11 +1912,7 @@ async function fetchWorkflowGroupRows(
|
|||||||
.select()
|
.select()
|
||||||
.from(documents)
|
.from(documents)
|
||||||
.where(
|
.where(
|
||||||
and(
|
and(eq(documents.portId, portId), inArray(documents.status, inflightStatuses), predicate),
|
||||||
eq(documents.portId, portId),
|
|
||||||
inArray(documents.status, inflightStatuses),
|
|
||||||
predicate,
|
|
||||||
),
|
|
||||||
)
|
)
|
||||||
.orderBy(desc(documents.updatedAt))
|
.orderBy(desc(documents.updatedAt))
|
||||||
.limit(WORKFLOW_GROUP_LIMIT);
|
.limit(WORKFLOW_GROUP_LIMIT);
|
||||||
@@ -1923,11 +1921,7 @@ async function fetchWorkflowGroupRows(
|
|||||||
.select({ count: sql<number>`count(*)::int` })
|
.select({ count: sql<number>`count(*)::int` })
|
||||||
.from(documents)
|
.from(documents)
|
||||||
.where(
|
.where(
|
||||||
and(
|
and(eq(documents.portId, portId), inArray(documents.status, inflightStatuses), predicate),
|
||||||
eq(documents.portId, portId),
|
|
||||||
inArray(documents.status, inflightStatuses),
|
|
||||||
predicate,
|
|
||||||
),
|
|
||||||
);
|
);
|
||||||
|
|
||||||
return { rows, total: Number(countRow?.count ?? 0) };
|
return { rows, total: Number(countRow?.count ?? 0) };
|
||||||
|
|||||||
@@ -1,4 +1,4 @@
|
|||||||
import { and, arrayContains, desc, eq, inArray, or, sql } from 'drizzle-orm';
|
import { and, arrayContains, desc, eq, inArray, isNull, or, sql } from 'drizzle-orm';
|
||||||
|
|
||||||
import { db } from '@/lib/db';
|
import { db } from '@/lib/db';
|
||||||
import { files, documents } from '@/lib/db/schema/documents';
|
import { files, documents } from '@/lib/db/schema/documents';
|
||||||
@@ -364,7 +364,7 @@ export async function listFilesAggregatedByEntity(
|
|||||||
return { groups };
|
return { groups };
|
||||||
}
|
}
|
||||||
|
|
||||||
async function assertEntityInPort(
|
export async function assertEntityInPort(
|
||||||
portId: string,
|
portId: string,
|
||||||
entityType: EntityType,
|
entityType: EntityType,
|
||||||
entityId: string,
|
entityId: string,
|
||||||
@@ -415,7 +415,9 @@ export async function collectRelatedEntities(
|
|||||||
companies,
|
companies,
|
||||||
and(eq(companies.id, companyMemberships.companyId), eq(companies.portId, portId)),
|
and(eq(companies.id, companyMemberships.companyId), eq(companies.portId, portId)),
|
||||||
)
|
)
|
||||||
.where(eq(companyMemberships.clientId, entityId));
|
.where(
|
||||||
|
and(eq(companyMemberships.clientId, entityId), isNull(companyMemberships.endDate)),
|
||||||
|
);
|
||||||
|
|
||||||
const directYachts = await db
|
const directYachts = await db
|
||||||
.select({ id: yachts.id, name: yachts.name })
|
.select({ id: yachts.id, name: yachts.name })
|
||||||
@@ -461,7 +463,9 @@ export async function collectRelatedEntities(
|
|||||||
clients,
|
clients,
|
||||||
and(eq(clients.id, companyMemberships.clientId), eq(clients.portId, portId)),
|
and(eq(clients.id, companyMemberships.clientId), eq(clients.portId, portId)),
|
||||||
)
|
)
|
||||||
.where(eq(companyMemberships.companyId, entityId));
|
.where(
|
||||||
|
and(eq(companyMemberships.companyId, entityId), isNull(companyMemberships.endDate)),
|
||||||
|
);
|
||||||
|
|
||||||
const ownedYachts = await db
|
const ownedYachts = await db
|
||||||
.select({ id: yachts.id, name: yachts.name })
|
.select({ id: yachts.id, name: yachts.name })
|
||||||
@@ -561,10 +565,7 @@ export async function applyEntityFkFromFolder<
|
|||||||
if (!payload.folderId) return payload;
|
if (!payload.folderId) return payload;
|
||||||
|
|
||||||
const folder = await db.query.documentFolders.findFirst({
|
const folder = await db.query.documentFolders.findFirst({
|
||||||
where: and(
|
where: and(eq(documentFolders.id, payload.folderId), eq(documentFolders.portId, portId)),
|
||||||
eq(documentFolders.id, payload.folderId),
|
|
||||||
eq(documentFolders.portId, portId),
|
|
||||||
),
|
|
||||||
columns: { systemManaged: true, entityType: true, entityId: true },
|
columns: { systemManaged: true, entityType: true, entityId: true },
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -16,16 +16,11 @@ import { and, eq } from 'drizzle-orm';
|
|||||||
|
|
||||||
import { db } from '@/lib/db';
|
import { db } from '@/lib/db';
|
||||||
import { files, documents, documentFolders } from '@/lib/db/schema/documents';
|
import { files, documents, documentFolders } from '@/lib/db/schema/documents';
|
||||||
|
import { clients } from '@/lib/db/schema/clients';
|
||||||
import { user } from '@/lib/db/schema/users';
|
import { user } from '@/lib/db/schema/users';
|
||||||
import { yachts } from '@/lib/db/schema/yachts';
|
import { yachts } from '@/lib/db/schema/yachts';
|
||||||
import {
|
import { ensureSystemRoots, ensureEntityFolder } from '@/lib/services/document-folders.service';
|
||||||
ensureSystemRoots,
|
import { listFilesAggregatedByEntity, applyEntityFkFromFolder } from '@/lib/services/files';
|
||||||
ensureEntityFolder,
|
|
||||||
} from '@/lib/services/document-folders.service';
|
|
||||||
import {
|
|
||||||
listFilesAggregatedByEntity,
|
|
||||||
applyEntityFkFromFolder,
|
|
||||||
} from '@/lib/services/files';
|
|
||||||
import { listInflightWorkflowsAggregatedByEntity } from '@/lib/services/documents.service';
|
import { listInflightWorkflowsAggregatedByEntity } from '@/lib/services/documents.service';
|
||||||
import { makePort, makeClient, makeCompany, makeYacht, makeMembership } from '../helpers/factories';
|
import { makePort, makeClient, makeCompany, makeYacht, makeMembership } from '../helpers/factories';
|
||||||
|
|
||||||
@@ -133,9 +128,7 @@ describe('files service · listFilesAggregatedByEntity', () => {
|
|||||||
clientId = client.id;
|
clientId = client.id;
|
||||||
|
|
||||||
// Insert 25 files all with clientId
|
// Insert 25 files all with clientId
|
||||||
await Promise.all(
|
await Promise.all(Array.from({ length: 25 }, () => insertFile(portId, { clientId })));
|
||||||
Array.from({ length: 25 }, () => insertFile(portId, { clientId })),
|
|
||||||
);
|
|
||||||
});
|
});
|
||||||
|
|
||||||
it('DIRECTLY ATTACHED group has 20 files and total=25', async () => {
|
it('DIRECTLY ATTACHED group has 20 files and total=25', async () => {
|
||||||
@@ -265,6 +258,16 @@ describe('documents service · listInflightWorkflowsAggregatedByEntity', () => {
|
|||||||
expect(directGroup!.workflows[0]!.status).toBe('sent');
|
expect(directGroup!.workflows[0]!.status).toBe('sent');
|
||||||
expect(directGroup!.workflows[0]!.title).toBe('In-flight Doc');
|
expect(directGroup!.workflows[0]!.title).toBe('In-flight Doc');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('rejects cross-port leakage with defense-in-depth port filter', async () => {
|
||||||
|
const otherPort = await makePort();
|
||||||
|
const [otherClient] = await db
|
||||||
|
.insert(clients)
|
||||||
|
.values({ portId: otherPort.id, fullName: 'Other Port Client' })
|
||||||
|
.returning();
|
||||||
|
const result = await listInflightWorkflowsAggregatedByEntity(portId, 'client', otherClient!.id);
|
||||||
|
expect(result.groups).toEqual([]);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
// ─── applyEntityFkFromFolder ──────────────────────────────────────────────────
|
// ─── applyEntityFkFromFolder ──────────────────────────────────────────────────
|
||||||
|
|||||||
Reference in New Issue
Block a user