From b2384a75230eee0346ab2cbec4cf8d038a5f62c8 Mon Sep 17 00:00:00 2001 From: Maurice Date: Sun, 13 Sep 2026 23:44:50 +0200 Subject: [PATCH] test(places): the delete doubles answer in the shape the service now uses remove and removeMany report what the nights they cancelled took down with them, so the stubs in the controller and plugin-RPC suites report it too. Plus a case for the announcement itself, which had none: place:deleted says nothing about a partner booking or an expense linked by reservation_id. --- .../tests/unit/nest/places.controller.test.ts | 23 +++++++++++++++---- server/tests/unit/places/places.rpc.test.ts | 6 +++-- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/server/tests/unit/nest/places.controller.test.ts b/server/tests/unit/nest/places.controller.test.ts index 60b95e0e3..b9dc565ab 100644 --- a/server/tests/unit/nest/places.controller.test.ts +++ b/server/tests/unit/nest/places.controller.test.ts @@ -242,7 +242,7 @@ describe('PlacesController (parity with the legacy /api/trips/:tripId/places rou expect(removeMany).not.toHaveBeenCalled(); }); it('deletes, fires hooks + broadcasts per deleted id', async () => { - const removeMany = vi.fn().mockReturnValue([1, 2]); const onDeleted = vi.fn(); const broadcast = vi.fn(); + const removeMany = vi.fn().mockReturnValue({ deleted: [1, 2], cancelled: { reservationIds: [], budgetItemIds: [] } }); const onDeleted = vi.fn(); const broadcast = vi.fn(); const scopedIds = vi.fn().mockReturnValue([1, 2]); const s = svc({ removeMany, onDeleted, broadcast, scopedIds } as Partial); expect(await new PlacesController(s, new RuntimeEnvService(), storageStub).bulkDelete(user, '5', { ids: [1, 2] }, 'sock')).toEqual({ deleted: [1, 2], count: 2 }); @@ -253,7 +253,7 @@ describe('PlacesController (parity with the legacy /api/trips/:tripId/places rou // #1745: the hook keys on the place id alone, so an id from another trip // would detach that trip's journey entries even though removeMany skips it. it('fires the journey hook only for ids that belong to the trip, ahead of the delete', async () => { - const removeMany = vi.fn().mockReturnValue([1]); + const removeMany = vi.fn().mockReturnValue({ deleted: [1], cancelled: { reservationIds: [], budgetItemIds: [] } }); const scopedIds = vi.fn().mockReturnValue([1]); const onDeleted = vi.fn(); const s = svc({ removeMany, scopedIds, onDeleted, broadcast: vi.fn() } as Partial); @@ -266,7 +266,7 @@ describe('PlacesController (parity with the legacy /api/trips/:tripId/places rou // #1298: the link is gone once the place is, so the ids have to be read first. it('announces the expenses the deleted places took with them', async () => { - const removeMany = vi.fn().mockReturnValue([1, 2]); + const removeMany = vi.fn().mockReturnValue({ deleted: [1, 2], cancelled: { reservationIds: [], budgetItemIds: [] } }); const linkedExpenseIds = vi.fn().mockReturnValue([77]); const broadcast = vi.fn(); const s = svc({ removeMany, linkedExpenseIds, broadcast } as Partial); @@ -423,14 +423,27 @@ describe('PlacesController (parity with the legacy /api/trips/:tripId/places rou it('DELETE /:id fires the hook then 404 / success', async () => { const onDeleted = vi.fn(); - const remove = vi.fn().mockReturnValue(false); + const remove = vi.fn().mockReturnValue({ deleted: false, cancelled: { reservationIds: [], budgetItemIds: [] } }); expect(await thrownAsync(() => new PlacesController(svc({ remove, onDeleted } as Partial), new RuntimeEnvService(), storageStub).remove(user, '5', '9'))).toEqual({ status: 404, body: { error: 'Place not found' } }); expect(onDeleted).toHaveBeenCalledWith(9); expect(onDeleted.mock.invocationCallOrder[0]).toBeLessThan(remove.mock.invocationCallOrder[0]); - const s = svc({ remove: vi.fn().mockReturnValue(true), broadcast: vi.fn() } as Partial); + const s = svc({ remove: vi.fn().mockReturnValue({ deleted: true, cancelled: { reservationIds: [], budgetItemIds: [] } }), broadcast: vi.fn() } as Partial); expect(await new PlacesController(s, new RuntimeEnvService(), storageStub).remove(user, '5', '9')).toEqual({ success: true }); }); + it('DELETE /:id announces the booking and the expense a cancelled night took down', async () => { + // place:deleted says nothing about either, and an expense linked by + // reservation_id is not one linkedExpenseIds finds. + const broadcast = vi.fn(); + const remove = vi.fn().mockReturnValue({ deleted: true, cancelled: { reservationIds: [12], budgetItemIds: [77] } }); + const s = svc({ remove, broadcast, linkedExpenseIds: vi.fn().mockReturnValue([]) } as Partial); + + await new PlacesController(s, new RuntimeEnvService(), storageStub).remove(user, '5', '9', 'sock'); + + expect(broadcast).toHaveBeenCalledWith('5', 'reservation:deleted', { reservationId: 12 }, 'sock'); + expect(broadcast).toHaveBeenCalledWith('5', 'budget:deleted', { itemId: 77 }, 'sock'); + }); + // #1745: a place on another trip must 404 without the hook ever running — // onPlaceDeleted keys on the place id alone, so it would detach that trip's // journey entries. diff --git a/server/tests/unit/places/places.rpc.test.ts b/server/tests/unit/places/places.rpc.test.ts index 08d54ad32..e215d65d0 100644 --- a/server/tests/unit/places/places.rpc.test.ts +++ b/server/tests/unit/places/places.rpc.test.ts @@ -40,7 +40,8 @@ function build(opts: { canEdit?: boolean; journeyThrows?: boolean } = {}) { create: vi.fn((_t: string, i: Record) => ({ id: 10, ...i })), update: vi.fn((_t: string, id: string) => Promise.resolve(id === '7' ? { id: 7, name: 'updated' } : null)), get: vi.fn((_t: string, id: string) => (id === '7' ? { id: 7 } : undefined)), - remove: vi.fn((_t: string, id: string) => Promise.resolve(id === '7')), + remove: vi.fn((_t: string, id: string) => + Promise.resolve({ deleted: id === '7', cancelled: { reservationIds: [], budgetItemIds: [] } })), linkedExpenseIds: vi.fn(() => []), }; const journey = { @@ -167,7 +168,8 @@ describe('PlacesRpc', () => { const f = build(); // place.get finds it (it exists), but the actual delete comes back false — the // race the null/false-vs-Promise bug used to paper over by always being truthy. - f.places.remove.mockImplementationOnce(() => Promise.resolve(false)); + f.places.remove.mockImplementationOnce(() => + Promise.resolve({ deleted: false, cancelled: { reservationIds: [], budgetItemIds: [] } })); const res = (await f.host('db:write:places').dispatch(req('places.delete', { tripId: 1, placeId: 7 }), 42)) as RpcError; expect(res.ok).toBe(false); expect(res.error.code).toBe('RESOURCE_FORBIDDEN');