k1nq/TREK
1
0
Fork 0
mirror of https://github.com/mauriceboe/TREK.git synced 2026-10-18 21:16:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

Roll TripAccessGuard onto the remaining trip-scoped controllers

Browse Source 
assignments, budget, collab, accommodations, reservations and booking-import
drop their copied requireTrip/requireEdit helpers for the guard. Status codes and
bodies are unchanged.

places is deliberately NOT converted, and that is the finding of this PR rather
than an omission. Its e2e suite documents that the body pipe answers 400 ahead of
the trip 404 — a parity shift somebody made on purpose — and a Nest guard always
runs before the pipe, so converting it silently reverses that ordering. The first
attempt also dropped the permission check on DELETE, which the integration suite
caught as a 200 where a 403 belonged. places keeps its in-handler check, and
README now says why so the next person does not rediscover it the same way.

collab applies the guard per handler for the same reason files does: its note-file
upload route would otherwise be refused before multer had read the body, and a
response sent mid-upload destroys the socket.

Two coverage notes. The per-domain thresholds are regenerated because deleting a
well-covered helper lowers a file's ratio without anything becoming less tested —
the drops are 0.4 to 1.2 points across six domains and match the helpers removed.
And the atlas integration suite gets the same admin1-bundle warm-up the unit suite
got: ATLAS-013 was paying 3.1s for the one-off decompression inside its own 5s
budget, which is why it only ever failed under load. It now runs in 116ms.
This commit is contained in:
Maurice
2026-08-09 19:45:37 +02:00
committed by Maurice
parent 8c0280e287
commit 94221a6d91
14 changed files with 143 additions and 217 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/tests/unit/nest/budget.controller.test.ts
+6 -26
View File
@@ -39,13 +39,10 @@ async function thrownAsync(fn: () => Promise<unknown>): Promise<{ status: number
throw new Error('expected the handler to throw');
}
/** The trip row TripAccessGuard resolves and @Trip() hands to the settlement route. */
const tripRow = { id: 5, user_id: 42, currency: 'USD' } as never;
describe('BudgetController (parity with the legacy /api/trips/:tripId/budget route)', () => {
it('404 when the trip is not accessible', () => {
const svc = makeService({ verifyTripAccess: vi.fn().mockReturnValue(undefined) });
expect(thrown(() => new BudgetController(svc).list(user, '5'))).toEqual({
status: 404, body: { error: 'Trip not found' },
});
});
it('GET / returns items', () => {
const svc = makeService({ list: vi.fn().mockReturnValue([{ id: 1 }]) } as Partial<BudgetService>);
@@ -59,7 +56,8 @@ describe('BudgetController (parity with the legacy /api/trips/:tripId/budget rou
settlement,
} as Partial<BudgetService>);
expect(new BudgetController(svc).perPerson(user, '5')).toEqual({ summary: [{ userId: 1, owes: 10 }] });
expect(new BudgetController(svc).settlement(user, '5')).toEqual({ transfers: [] });
// A trip with no currency set falls back to EUR rather than passing undefined on.
expect(new BudgetController(svc).settlement(user, { id: 5, user_id: 42 } as never, '5')).toEqual({ transfers: [] });
expect(settlement).toHaveBeenLastCalledWith('5', undefined, 'EUR');
});
@@ -69,7 +67,7 @@ describe('BudgetController (parity with the legacy /api/trips/:tripId/budget rou
verifyTripAccess: vi.fn().mockReturnValue({ id: 5, user_id: 1, currency: 'USD' }),
settlement,
} as Partial<BudgetService>);
new BudgetController(svc).settlement(user, '5', 'GBP');
new BudgetController(svc).settlement(user, tripRow, '5', 'GBP');
expect(settlement).toHaveBeenCalledWith('5', 'GBP', 'USD');
});
@@ -79,12 +77,6 @@ describe('BudgetController (parity with the legacy /api/trips/:tripId/budget rou
expect(new BudgetController(svc).listSettlements(user, '5')).toEqual({ settlements: [{ id: 1 }] });
});
it('POST /settlements 403 without budget_edit', async () => {
const svc = makeService({ canEdit: vi.fn().mockReturnValue(false) });
expect(await thrownAsync(() => new BudgetController(svc).createSettlement(user, '5', { from_user_id: 1, to_user_id: 2, amount: 10 }))).toEqual({
status: 403, body: { error: 'No permission' },
});
});
// The legacy 'from_user_id, to_user_id and amount are required' 400s are now
// produced by the global ZodValidationPipe (budget.dto.ts) before the handler
@@ -114,12 +106,6 @@ describe('BudgetController (parity with the legacy /api/trips/:tripId/budget rou
expect(broadcast).toHaveBeenCalledWith('5', 'budget:settlement-deleted', { settlementId: 7 }, 'sock');
});
it('PUT /settlements/:id 403 without budget_edit', async () => {
const svc = makeService({ canEdit: vi.fn().mockReturnValue(false) });
expect(await thrownAsync(() => new BudgetController(svc).updateSettlement(user, '5', '7', { from_user_id: 1, to_user_id: 2, amount: 10 }))).toEqual({
status: 403, body: { error: 'No permission' },
});
});
it('PUT /settlements/:id 404 when missing', async () => {
const svc = makeService({ updateSettlement: vi.fn().mockResolvedValue(null) } as Partial<BudgetService>);
@@ -140,12 +126,6 @@ describe('BudgetController (parity with the legacy /api/trips/:tripId/budget rou
});
describe('POST /', () => {
it('403 without budget_edit', async () => {
const svc = makeService({ canEdit: vi.fn().mockReturnValue(false) });
expect(await thrownAsync(() => new BudgetController(svc).create(user, '5', { name: 'Hotel' }))).toEqual({
status: 403, body: { error: 'No permission' },
});
});
// The legacy 'Name is required' 400 is now produced by the global
// ZodValidationPipe (budgetCreateItemRequestSchema requires name) before