From 705c3c08dd3d76096eb0806c7a67e9acfe8159cb Mon Sep 17 00:00:00 2001 From: jubnl Date: Wed, 12 Aug 2026 00:22:32 +0200 Subject: [PATCH] fix(trips): remap cross-links and copy split rows when duplicating a trip Copying a trip nulled budget->reservation and reservation->accommodation links (TEXT accommodation_id vs number-keyed map) and dropped budget_item_members/payers, assignment_participants, budget currency/ exchange_rate/expense_date/ticket_json and reservation url. All links now remap to the copied rows; external_* sync identity is intentionally not copied; the to-do reset is now documented (smoke-test I-01). --- server/src/nest/trips/trips.mcp.ts | 2 +- server/src/nest/trips/trips.service.ts | 63 ++++++++++++++--- server/tests/unit/nest/trips.service.test.ts | 74 ++++++++++++++++++-- 3 files changed, 123 insertions(+), 16 deletions(-) diff --git a/server/src/nest/trips/trips.mcp.ts b/server/src/nest/trips/trips.mcp.ts index a2d6537e0..b4213f5ae 100644 --- a/server/src/nest/trips/trips.mcp.ts +++ b/server/src/nest/trips/trips.mcp.ts @@ -335,7 +335,7 @@ export class TripsMcp { @Tool({ name: 'copy_trip', - description: 'Duplicate a trip (all days, places, itinerary, packing, budget, reservations, day notes). Packing items are reset to unchecked. Returns the new trip.', + description: 'Duplicate a trip (all days, places, itinerary, packing, budget, reservations, day notes). Packing items and to-dos are reset to unchecked. Returns the new trip.', inputSchema: { tripId: z.number().int().positive().describe('Source trip ID to duplicate'), title: z.string().min(1).max(200).optional().describe('Title for the new trip (defaults to source title)'), diff --git a/server/src/nest/trips/trips.service.ts b/server/src/nest/trips/trips.service.ts index 975bbe129..e19f9ee54 100644 --- a/server/src/nest/trips/trips.service.ts +++ b/server/src/nest/trips/trips.service.ts @@ -521,8 +521,10 @@ export class TripsService { /** * Duplicates a trip (all days, places, assignments, accommodations, reservations, * budget, packing bags/items, day notes) into a new trip owned by `newOwnerId`. - * Packing items are reset to unchecked. Budget paid status is cleared. - * Returns the new trip's ID. + * Cross-links are remapped to the copied rows (reservation↔budget item, + * reservation↔accommodation) and split data travels with the copy + * (budget_item_members/payers incl. paid flags, assignment_participants). + * Packing items and to-dos are reset to unchecked. Returns the new trip's ID. */ copy(sourceTripId: string | number, newOwnerId: number, title?: string): number { const src = this.db.prepare('SELECT * FROM trips WHERE id = ?').get(sourceTripId) as any; @@ -590,6 +592,18 @@ export class TripsService { } } + const oldParticipants = this.db.prepare(` + SELECT ap.* FROM assignment_participants ap + JOIN day_assignments da ON da.id = ap.assignment_id + JOIN days d ON d.id = da.day_id + WHERE d.trip_id = ? + `).all(sourceTripId) as any[]; + const insertParticipant = this.db.prepare('INSERT OR IGNORE INTO assignment_participants (assignment_id, user_id) VALUES (?, ?)'); + for (const ap of oldParticipants) { + const newAssignmentId = assignmentMap.get(ap.assignment_id); + if (newAssignmentId) insertParticipant.run(newAssignmentId, ap.user_id); + } + const oldAccom = this.db.prepare('SELECT * FROM day_accommodations WHERE trip_id = ?').all(sourceTripId) as any[]; const accomMap = new Map(); const insertAccom = this.db.prepare(` @@ -607,32 +621,61 @@ export class TripsService { } const oldReservations = this.db.prepare('SELECT * FROM reservations WHERE trip_id = ?').all(sourceTripId) as any[]; + // The external_* / sync_enabled columns are deliberately not copied: the + // duplicate must not inherit the source's external sync identity. + const reservationMap = new Map(); const insertReservation = this.db.prepare(` INSERT INTO reservations (trip_id, day_id, end_day_id, place_id, assignment_id, accommodation_id, title, reservation_time, reservation_end_time, - location, confirmation_number, notes, status, type, metadata, day_plan_position, needs_review) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + location, confirmation_number, notes, url, status, type, metadata, day_plan_position, needs_review) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) `); for (const r of oldReservations) { - insertReservation.run(newTripId, + const rr = insertReservation.run(newTripId, r.day_id ? (dayMap.get(r.day_id) ?? null) : null, // end_day_id is a day reference too (multi-day transport) — remap it like // day_id, otherwise the duplicated trip loses the reservation's end-day link. r.end_day_id ? (dayMap.get(r.end_day_id) ?? null) : null, r.place_id ? (placeMap.get(r.place_id) ?? null) : null, r.assignment_id ? (assignmentMap.get(r.assignment_id) ?? null) : null, - r.accommodation_id ? (accomMap.get(r.accommodation_id) ?? null) : null, + // accommodation_id is a TEXT column, so it reads back as a string — + // coerce before the number-keyed map lookup or the link silently nulls. + r.accommodation_id != null ? (accomMap.get(Number(r.accommodation_id)) ?? null) : null, r.title, r.reservation_time, r.reservation_end_time, - r.location, r.confirmation_number, r.notes, r.status, r.type, + r.location, r.confirmation_number, r.notes, r.url, r.status, r.type, r.metadata, r.day_plan_position, r.needs_review ?? 0); + reservationMap.set(r.id, rr.lastInsertRowid); } const oldBudget = this.db.prepare('SELECT * FROM budget_items WHERE trip_id = ?').all(sourceTripId) as any[]; + const budgetMap = new Map(); const insertBudget = this.db.prepare(` - INSERT INTO budget_items (trip_id, category, name, total_price, persons, days, note, sort_order) - VALUES (?, ?, ?, ?, ?, ?, ?, ?) + INSERT INTO budget_items (trip_id, category, name, total_price, persons, days, note, sort_order, + reservation_id, currency, exchange_rate, expense_date, ticket_json, paid_by_user_id) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) `); for (const b of oldBudget) { - insertBudget.run(newTripId, b.category, b.name, b.total_price, b.persons, b.days, b.note, b.sort_order); + const br = insertBudget.run(newTripId, b.category, b.name, b.total_price, b.persons, b.days, b.note, b.sort_order, + b.reservation_id ? (reservationMap.get(b.reservation_id) ?? null) : null, + b.currency, b.exchange_rate ?? 1, b.expense_date, b.ticket_json, b.paid_by_user_id); + budgetMap.set(b.id, br.lastInsertRowid); + } + + const oldBudgetMembers = this.db.prepare(` + SELECT bm.* FROM budget_item_members bm JOIN budget_items b ON b.id = bm.budget_item_id WHERE b.trip_id = ? + `).all(sourceTripId) as any[]; + const insertBudgetMember = this.db.prepare('INSERT OR IGNORE INTO budget_item_members (budget_item_id, user_id, paid, amount) VALUES (?, ?, ?, ?)'); + for (const bm of oldBudgetMembers) { + const newItemId = budgetMap.get(bm.budget_item_id); + if (newItemId) insertBudgetMember.run(newItemId, bm.user_id, bm.paid ?? 0, bm.amount); + } + + const oldBudgetPayers = this.db.prepare(` + SELECT bp.* FROM budget_item_payers bp JOIN budget_items b ON b.id = bp.budget_item_id WHERE b.trip_id = ? + `).all(sourceTripId) as any[]; + const insertBudgetPayer = this.db.prepare('INSERT OR IGNORE INTO budget_item_payers (budget_item_id, user_id, amount) VALUES (?, ?, ?)'); + for (const bp of oldBudgetPayers) { + const newItemId = budgetMap.get(bp.budget_item_id); + if (newItemId) insertBudgetPayer.run(newItemId, bp.user_id, bp.amount ?? 0); } const oldBags = this.db.prepare('SELECT * FROM packing_bags WHERE trip_id = ?').all(sourceTripId) as any[]; diff --git a/server/tests/unit/nest/trips.service.test.ts b/server/tests/unit/nest/trips.service.test.ts index c05ebf75b..679dee3fa 100644 --- a/server/tests/unit/nest/trips.service.test.ts +++ b/server/tests/unit/nest/trips.service.test.ts @@ -1,5 +1,5 @@ /** - * Unit tests for the DI-native TripsService — TRIP-SVC-001 through TRIP-SVC-058 + * Unit tests for the DI-native TripsService — TRIP-SVC-001 through TRIP-SVC-059 * (001–038 moved 1:1 from the legacy tests/unit/services/tripService.test.ts; * the exportICS cases that duplicated the generateDays 010–012 numbering were * renumbered to 024–026 with the post-fold quirk-fix commit; 040–041 pinned @@ -746,6 +746,68 @@ describe('folded trip CRUD', () => { const secondCopy = svc.copy(trip.id, user.id); expect((testDb.prepare('SELECT title FROM trips WHERE id = ?').get(secondCopy) as any).title).toBe('Origin'); }); + + it('TRIP-SVC-059: copy remaps cross-links and carries splits/participants (smoke-test I-01)', () => { + const { user: owner } = createUser(testDb); + const { user: friend } = createUser(testDb); + const trip = createTrip(testDb, owner.id, { title: 'Linked', start_date: '2025-06-01', end_date: '2025-06-02' }); + const days = getDays(trip.id); + const place = createPlace(testDb, trip.id, { name: 'Hotel Le Test' }); + const assignment = createDayAssignment(testDb, days[0].id, place.id); + testDb.prepare('INSERT INTO assignment_participants (assignment_id, user_id) VALUES (?, ?)').run(assignment.id, owner.id); + + const accomId = Number(testDb.prepare(` + INSERT INTO day_accommodations (trip_id, place_id, start_day_id, end_day_id, check_in, check_out) + VALUES (?, ?, ?, ?, '15:00', '11:00') + `).run(trip.id, place.id, days[0].id, days[1].id).lastInsertRowid); + + const resId = Number(testDb.prepare(` + INSERT INTO reservations (trip_id, day_id, assignment_id, accommodation_id, title, type, url) + VALUES (?, ?, ?, ?, 'Hotel booking', 'hotel', 'https://example.test/booking') + `).run(trip.id, days[0].id, assignment.id, accomId).lastInsertRowid); + + const itemId = Number(testDb.prepare(` + INSERT INTO budget_items (trip_id, category, name, total_price, persons, reservation_id, currency, exchange_rate, expense_date) + VALUES (?, 'Accommodation', 'Hotel', 240, 2, ?, 'JPY', 0.0062, '2025-06-01') + `).run(trip.id, resId).lastInsertRowid); + testDb.prepare('INSERT INTO budget_item_members (budget_item_id, user_id, paid, amount) VALUES (?, ?, 1, 120)').run(itemId, owner.id); + testDb.prepare('INSERT INTO budget_item_members (budget_item_id, user_id, paid, amount) VALUES (?, ?, 0, 120)').run(itemId, friend.id); + testDb.prepare('INSERT INTO budget_item_payers (budget_item_id, user_id, amount) VALUES (?, ?, 240)').run(itemId, owner.id); + testDb.prepare("INSERT INTO todo_items (trip_id, name, checked) VALUES (?, 'Book transfer', 1)").run(trip.id); + + const newTripId = svc.copy(trip.id, owner.id, 'Linked copy'); + + // Budget → reservation link points at the copied reservation, not null / not the old id. + const newItem = testDb.prepare('SELECT * FROM budget_items WHERE trip_id = ?').get(newTripId) as any; + const newRes = testDb.prepare('SELECT * FROM reservations WHERE trip_id = ?').get(newTripId) as any; + expect(newRes.id).not.toBe(resId); + expect(newItem.reservation_id).toBe(newRes.id); + expect(newItem).toMatchObject({ currency: 'JPY', exchange_rate: 0.0062, expense_date: '2025-06-01' }); + + // Reservation → accommodation resolves to the copied accommodation (accommodation_id is TEXT). + const newAccom = testDb.prepare('SELECT * FROM day_accommodations WHERE trip_id = ?').get(newTripId) as any; + expect(newAccom.id).not.toBe(accomId); + expect(Number(newRes.accommodation_id)).toBe(newAccom.id); + expect(newRes.url).toBe('https://example.test/booking'); + + // Splits carried over with per-member paid flags and amounts. + const members = testDb.prepare('SELECT user_id, paid, amount FROM budget_item_members WHERE budget_item_id = ? ORDER BY user_id').all(newItem.id) as any[]; + expect(members).toEqual([ + { user_id: owner.id, paid: 1, amount: 120 }, + { user_id: friend.id, paid: 0, amount: 120 }, + ]); + const payers = testDb.prepare('SELECT user_id, amount FROM budget_item_payers WHERE budget_item_id = ?').all(newItem.id) as any[]; + expect(payers).toEqual([{ user_id: owner.id, amount: 240 }]); + + // Assignment participants copied onto the remapped assignment. + const newAssignment = getAssignments(getDays(newTripId)[0].id)[0]; + const participants = testDb.prepare('SELECT user_id FROM assignment_participants WHERE assignment_id = ?').all(newAssignment.id) as any[]; + expect(participants).toEqual([{ user_id: owner.id }]); + + // To-dos come across but reset to unchecked (documented behaviour). + const todos = testDb.prepare('SELECT name, checked FROM todo_items WHERE trip_id = ?').all(newTripId) as any[]; + expect(todos).toEqual([{ name: 'Book transfer', checked: 0 }]); + }); }); // ── Wrapper helpers (delegating members of the aggregate root) ──────────────── @@ -878,10 +940,12 @@ describe('folded quirk branches', () => { expect(newAccom.place_id).toBe(newPlace.id); expect(newAccom.start_day_id).toBe(newDays[0].id); const newRes = testDb.prepare('SELECT day_id, end_day_id, place_id, accommodation_id FROM reservations WHERE trip_id = ?').get(newTripId) as any; - // Quirk preserved from the legacy copyTripById: reservations.accommodation_id - // is a TEXT column, so the number-keyed accomMap lookup never matches and the - // copied reservation loses its accommodation link (nulled, not remapped). - expect(newRes).toEqual({ day_id: newDays[0].id, end_day_id: newDays[1].id, place_id: newPlace.id, accommodation_id: null }); + // The legacy copyTripById nulled this link (TEXT column vs number-keyed map); + // fixed with smoke-test I-01 — the copy now coerces and remaps it. + expect(newRes.day_id).toBe(newDays[0].id); + expect(newRes.end_day_id).toBe(newDays[1].id); + expect(newRes.place_id).toBe(newPlace.id); + expect(Number(newRes.accommodation_id)).toBe(newAccom.id); expect((testDb.prepare('SELECT COUNT(*) AS n FROM budget_items WHERE trip_id = ?').get(newTripId) as any).n).toBe(1); const newItem = testDb.prepare('SELECT checked, bag_id FROM packing_items WHERE trip_id = ?').get(newTripId) as any; expect(newItem.checked).toBe(0);