From d6c8e054fd10c4905154dbcf87b47dab2324671a Mon Sep 17 00:00:00 2001 From: jubnl Date: Thu, 30 Apr 2026 23:43:38 +0200 Subject: [PATCH] fix: delete linked budget item when accommodation or reservation is deleted (#933) Deleting an accommodation or reservation now removes any budget item linked via reservation_id, preventing orphan entries in the Budget page. Also fixes a pre-existing payload-shape bug where budget:deleted was broadcast with {id} instead of {itemId}, breaking live updates for collaborators when a reservation price was cleared. Tests added: ACCOM-006, RESV-009b, BUDGET-004b. --- server/src/routes/days.ts | 5 ++- server/src/routes/reservations.ts | 7 +++- server/src/services/dayService.ts | 13 ++++-- server/src/services/reservationService.ts | 11 +++-- server/tests/integration/budget.test.ts | 19 +++++++++ server/tests/integration/days.test.ts | 42 +++++++++++++++++++ server/tests/integration/reservations.test.ts | 37 ++++++++++++++++ 7 files changed, 124 insertions(+), 10 deletions(-) diff --git a/server/src/routes/days.ts b/server/src/routes/days.ts index ce967355..659b7096 100644 --- a/server/src/routes/days.ts +++ b/server/src/routes/days.ts @@ -117,10 +117,13 @@ accommodationsRouter.delete('/:id', authenticate, requireTripAccess, (req: Reque if (!dayService.getAccommodation(id, tripId)) return res.status(404).json({ error: 'Accommodation not found' }); - const { linkedReservationId } = dayService.deleteAccommodation(id); + const { linkedReservationId, deletedBudgetItemId } = dayService.deleteAccommodation(id); if (linkedReservationId) { broadcast(tripId, 'reservation:deleted', { reservationId: linkedReservationId }, req.headers['x-socket-id'] as string); } + if (deletedBudgetItemId) { + broadcast(tripId, 'budget:deleted', { itemId: deletedBudgetItemId }, req.headers['x-socket-id'] as string); + } res.json({ success: true }); broadcast(tripId, 'accommodation:deleted', { accommodationId: Number(id) }, req.headers['x-socket-id'] as string); diff --git a/server/src/routes/reservations.ts b/server/src/routes/reservations.ts index c5351caf..52cfdc63 100644 --- a/server/src/routes/reservations.ts +++ b/server/src/routes/reservations.ts @@ -129,7 +129,7 @@ router.put('/:id', authenticate, (req: Request, res: Response) => { const linked = db.prepare('SELECT id FROM budget_items WHERE trip_id = ? AND reservation_id = ?').get(tripId, id) as { id: number } | undefined; if (linked) { deleteBudgetItem(linked.id, tripId); - broadcast(tripId, 'budget:deleted', { id: linked.id }, req.headers['x-socket-id'] as string); + broadcast(tripId, 'budget:deleted', { itemId: linked.id }, req.headers['x-socket-id'] as string); } } @@ -179,12 +179,15 @@ router.delete('/:id', authenticate, (req: Request, res: Response) => { if (!checkPermission('reservation_edit', authReq.user.role, trip.user_id, authReq.user.id, trip.user_id !== authReq.user.id)) return res.status(403).json({ error: 'No permission' }); - const { deleted: reservation, accommodationDeleted } = deleteReservation(id, tripId); + const { deleted: reservation, accommodationDeleted, deletedBudgetItemId } = deleteReservation(id, tripId); if (!reservation) return res.status(404).json({ error: 'Reservation not found' }); if (accommodationDeleted) { broadcast(tripId, 'accommodation:deleted', { accommodationId: reservation.accommodation_id }, req.headers['x-socket-id'] as string); } + if (deletedBudgetItemId) { + broadcast(tripId, 'budget:deleted', { itemId: deletedBudgetItemId }, req.headers['x-socket-id'] as string); + } res.json({ success: true }); broadcast(tripId, 'reservation:deleted', { reservationId: Number(id) }, req.headers['x-socket-id'] as string); diff --git a/server/src/services/dayService.ts b/server/src/services/dayService.ts index 80b773ad..8b3b6361 100644 --- a/server/src/services/dayService.ts +++ b/server/src/services/dayService.ts @@ -292,14 +292,19 @@ export function updateAccommodation(id: string | number, existing: DayAccommodat return getAccommodationWithPlace(Number(id)); } -/** Delete accommodation and its linked reservation. Returns the linked reservation id if one existed. */ -export function deleteAccommodation(id: string | number): { linkedReservationId: number | null } { - // Delete linked reservation +/** Delete accommodation and its linked reservation (and any linked budget item). */ +export function deleteAccommodation(id: string | number): { linkedReservationId: number | null; deletedBudgetItemId: number | null } { const linkedRes = db.prepare('SELECT id FROM reservations WHERE accommodation_id = ?').get(Number(id)) as { id: number } | undefined; + let deletedBudgetItemId: number | null = null; if (linkedRes) { + const linkedBudget = db.prepare('SELECT id FROM budget_items WHERE reservation_id = ?').get(linkedRes.id) as { id: number } | undefined; + if (linkedBudget) { + db.prepare('DELETE FROM budget_items WHERE id = ?').run(linkedBudget.id); + deletedBudgetItemId = linkedBudget.id; + } db.prepare('DELETE FROM reservations WHERE id = ?').run(linkedRes.id); } db.prepare('DELETE FROM day_accommodations WHERE id = ?').run(id); - return { linkedReservationId: linkedRes ? linkedRes.id : null }; + return { linkedReservationId: linkedRes ? linkedRes.id : null, deletedBudgetItemId }; } diff --git a/server/src/services/reservationService.ts b/server/src/services/reservationService.ts index 354a14f7..f78200ca 100644 --- a/server/src/services/reservationService.ts +++ b/server/src/services/reservationService.ts @@ -418,9 +418,9 @@ export function updateReservation(id: string | number, tripId: string | number, return { reservation, accommodationChanged }; } -export function deleteReservation(id: string | number, tripId: string | number): { deleted: { id: number; title: string; type: string; accommodation_id: number | null } | undefined; accommodationDeleted: boolean } { +export function deleteReservation(id: string | number, tripId: string | number): { deleted: { id: number; title: string; type: string; accommodation_id: number | null } | undefined; accommodationDeleted: boolean; deletedBudgetItemId: number | null } { const reservation = db.prepare('SELECT id, title, type, accommodation_id FROM reservations WHERE id = ? AND trip_id = ?').get(id, tripId) as { id: number; title: string; type: string; accommodation_id: number | null } | undefined; - if (!reservation) return { deleted: undefined, accommodationDeleted: false }; + if (!reservation) return { deleted: undefined, accommodationDeleted: false, deletedBudgetItemId: null }; let accommodationDeleted = false; if (reservation.accommodation_id) { @@ -428,6 +428,11 @@ export function deleteReservation(id: string | number, tripId: string | number): accommodationDeleted = true; } + const linkedBudget = db.prepare('SELECT id FROM budget_items WHERE trip_id = ? AND reservation_id = ?').get(tripId, id) as { id: number } | undefined; + if (linkedBudget) { + db.prepare('DELETE FROM budget_items WHERE id = ?').run(linkedBudget.id); + } + db.prepare('DELETE FROM reservations WHERE id = ?').run(id); - return { deleted: reservation, accommodationDeleted }; + return { deleted: reservation, accommodationDeleted, deletedBudgetItemId: linkedBudget ? linkedBudget.id : null }; } diff --git a/server/tests/integration/budget.test.ts b/server/tests/integration/budget.test.ts index 53378f86..7a1854df 100644 --- a/server/tests/integration/budget.test.ts +++ b/server/tests/integration/budget.test.ts @@ -189,6 +189,25 @@ describe('Delete budget item', () => { .set('Cookie', authCookie(user.id)); expect(list.body.items).toHaveLength(0); }); + + it('BUDGET-004b — DELETE budget item does NOT delete its linked reservation', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const reservation = createReservation(testDb, trip.id, { title: 'Hotel Booking', type: 'hotel' }); + + const result = testDb.prepare( + 'INSERT INTO budget_items (trip_id, name, category, total_price, reservation_id) VALUES (?, ?, ?, ?, ?)' + ).run(trip.id, 'Hotel Cost', 'Accommodation', 250, reservation.id); + const itemId = result.lastInsertRowid as number; + + const del = await request(app) + .delete(`/api/trips/${trip.id}/budget/${itemId}`) + .set('Cookie', authCookie(user.id)); + expect(del.status).toBe(200); + + const reservationAfter = testDb.prepare('SELECT id FROM reservations WHERE id = ?').get(reservation.id); + expect(reservationAfter).toBeDefined(); + }); }); // ───────────────────────────────────────────────────────────────────────────── diff --git a/server/tests/integration/days.test.ts b/server/tests/integration/days.test.ts index 26c39f83..408b2d51 100644 --- a/server/tests/integration/days.test.ts +++ b/server/tests/integration/days.test.ts @@ -502,4 +502,46 @@ describe('Accommodations', () => { ).get(reservationBefore.id); expect(reservationAfter).toBeUndefined(); }); + + it('ACCOM-006 — DELETE accommodation also removes its linked budget item (issue #933)', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id, { title: 'Hotel Budget Trip' }); + const day1 = createDay(testDb, trip.id, { date: '2026-11-01' }); + const day2 = createDay(testDb, trip.id, { date: '2026-11-03' }); + const place = createPlace(testDb, trip.id, { name: 'Grand Hotel' }); + + // Create a hotel reservation that creates an accommodation and a linked budget item + const createRes = await request(app) + .post(`/api/trips/${trip.id}/reservations`) + .set('Cookie', authCookie(user.id)) + .send({ + title: 'Grand Hotel Stay', + type: 'hotel', + day_id: day1.id, + create_accommodation: { place_id: place.id, start_day_id: day1.id, end_day_id: day2.id }, + create_budget_entry: { total_price: 450, category: 'Accommodation' }, + }); + expect(createRes.status).toBe(201); + + const accommodationId = testDb.prepare( + 'SELECT id FROM day_accommodations WHERE trip_id = ?' + ).get(trip.id) as any; + expect(accommodationId).toBeDefined(); + + const budgetBefore = testDb.prepare( + 'SELECT id FROM budget_items WHERE trip_id = ?' + ).get(trip.id); + expect(budgetBefore).toBeDefined(); + + // Delete via the accommodation endpoint (the primary bug path) + const delRes = await request(app) + .delete(`/api/trips/${trip.id}/accommodations/${accommodationId.id}`) + .set('Cookie', authCookie(user.id)); + expect(delRes.status).toBe(200); + + const budgetAfter = testDb.prepare( + 'SELECT id FROM budget_items WHERE trip_id = ?' + ).get(trip.id); + expect(budgetAfter).toBeUndefined(); + }); }); diff --git a/server/tests/integration/reservations.test.ts b/server/tests/integration/reservations.test.ts index 63528671..9412871f 100644 --- a/server/tests/integration/reservations.test.ts +++ b/server/tests/integration/reservations.test.ts @@ -452,4 +452,41 @@ describe('Reservation accommodation delete', () => { ).get(accom.id); expect(accomAfter).toBeUndefined(); }); + + it('RESV-009b — DELETE reservation linked to accommodation also removes its linked budget item (issue #933)', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day1 = createDay(testDb, trip.id, { date: '2025-08-01' }); + const day2 = createDay(testDb, trip.id, { date: '2025-08-03' }); + const place = createPlace(testDb, trip.id, { name: 'Seaside Resort' }); + + const createRes = await request(app) + .post(`/api/trips/${trip.id}/reservations`) + .set('Cookie', authCookie(user.id)) + .send({ + title: 'Seaside Resort Stay', + type: 'hotel', + day_id: day1.id, + create_accommodation: { place_id: place.id, start_day_id: day1.id, end_day_id: day2.id }, + create_budget_entry: { total_price: 320, category: 'Accommodation' }, + }); + expect(createRes.status).toBe(201); + const reservationId = createRes.body.reservation.id; + + const budgetBefore = testDb.prepare( + 'SELECT id FROM budget_items WHERE trip_id = ? AND reservation_id = ?' + ).get(trip.id, reservationId); + expect(budgetBefore).toBeDefined(); + + // Delete via the reservation endpoint + const delRes = await request(app) + .delete(`/api/trips/${trip.id}/reservations/${reservationId}`) + .set('Cookie', authCookie(user.id)); + expect(delRes.status).toBe(200); + + const budgetAfter = testDb.prepare( + 'SELECT id FROM budget_items WHERE trip_id = ?' + ).get(trip.id); + expect(budgetAfter).toBeUndefined(); + }); });