diff --git a/server/scripts/size-baseline.json b/server/scripts/size-baseline.json index 034ce75db..91ba58ac6 100644 --- a/server/scripts/size-baseline.json +++ b/server/scripts/size-baseline.json @@ -2,7 +2,7 @@ "scripts/db-call-graph.mjs": 1036, "scripts/generate-entities.ts": 2259, "src/db/repositories/Places.repository.ts": 1440, - "src/db/repositories/Reservations.repository.ts": 1676, + "src/db/repositories/Reservations.repository.ts": 1675, "src/db/repositories/Trips.repository.ts": 1121, "src/db/repositories/Users.repository.ts": 1710, "src/nest/atlas/atlas-geo.ts": 1005, @@ -12,7 +12,7 @@ "src/nest/collections/collections.service.ts": 1488, "src/nest/doc-sync/doc-sync.service.ts": 1060, "src/nest/doc-sync/providers/synology-drive.client.ts": 1160, - "src/nest/journey/journey-domain.service.ts": 1804, + "src/nest/journey/journey-domain.service.ts": 1799, "src/nest/maps/maps.service.ts": 3252, "src/nest/places/places.service.ts": 2020, "src/nest/plugins/plugin-runtime.service.ts": 1463, diff --git a/server/src/db/repositories/DayAssignments.repository.ts b/server/src/db/repositories/DayAssignments.repository.ts index 83208d5a4..94480d24e 100644 --- a/server/src/db/repositories/DayAssignments.repository.ts +++ b/server/src/db/repositories/DayAssignments.repository.ts @@ -461,6 +461,30 @@ export class DayAssignmentsRepository extends TrekRepository { await this.nativeUpdate(where, { order_index }); } + /** + * {@link setOrderIndex} for many rows in one statement: `UPDATE + * day_assignments SET order_index = CASE id WHEN ? THEN ? ... END WHERE id IN + * (...) [AND day_id = ?]`. The result is what the writes one by one in the + * given order left: an id listed twice keeps its last index, and with + * `day_id` a row on another day is not touched. + */ + async setOrderIndexes(order: ReadonlyArray<{ id: number; order_index: number }>, day_id?: number): Promise { + const last = new Map(); + for (const { id, order_index } of order) last.set(id, order_index); + const [first, ...rest] = [...last]; + if (!first) return; + let query = this.kysely>() + .updateTable('day_assignments') + .set((eb) => { + let index = eb.case().when('id', '=', first[0]).then(first[1]); + for (const [id, orderIndex] of rest) index = index.when('id', '=', id).then(orderIndex); + return { order_index: index.else(eb.ref('order_index')).end() }; + }) + .where('id', 'in', [...last.keys()]); + if (day_id !== undefined) query = query.where('day_id', '=', day_id); + await query.execute(); + } + /** AS13 (`moveAssignment`'s source-day read) — `SELECT day_id FROM day_assignments WHERE id = ?`. */ async getDayId(id: number): Promise { const platform = this.getEntityManager().getPlatform(); diff --git a/server/src/db/repositories/JourneyEntries.repository.ts b/server/src/db/repositories/JourneyEntries.repository.ts index b99f27934..ed2ab956a 100644 --- a/server/src/db/repositories/JourneyEntries.repository.ts +++ b/server/src/db/repositories/JourneyEntries.repository.ts @@ -115,6 +115,22 @@ export class JourneyEntriesRepository extends TrekRepository { .execute('all', false); } + /** + * JG16/JG73 — the titles of the trips a journey's entries came from, keyed by + * trip id, in one read (the journey reads used to ask once per entry). A trip + * that is gone has no entry, so its entries read a null name as before. + */ + async sourceTripTitles(journeyId: number): Promise> { + const rows = await this.kysely>() + .selectFrom('journey_entries as je') + .innerJoin('trips as t', 't.id', 'je.source_trip_id') + .select(['t.id', 't.title']) + .distinct() + .where('je.journey_id', '=', journeyId) + .execute(); + return new Map(rows.map((r) => [r.id, r.title ?? null])); + } + /** * JS13 (Plan 4 Task 8b relocation) — the public `getPublicJourney` route's * entry list: `SELECT je.* FROM journey_entries je WHERE je.journey_id=? diff --git a/server/src/db/repositories/Reservations.repository.ts b/server/src/db/repositories/Reservations.repository.ts index c1b8abaff..fcad8bd5e 100644 --- a/server/src/db/repositories/Reservations.repository.ts +++ b/server/src/db/repositories/Reservations.repository.ts @@ -171,19 +171,16 @@ export interface CalendarReservationRow extends ReservationAllColumnsRow { type CalendarReservationKyselyDB = Pick; /** - * CL4 (`CalendarService.buildTripCalendar`'s per-day assignment read, - * previously run once per dated day) — `SELECT da.*, p.name as place_name, - * p.address as place_address, p.lat as place_lat, p.lng as place_lng, + * CL4 (`CalendarService.buildTripCalendar`'s assignment read, for every dated + * day at once) — `SELECT da.*, p.name as place_name, p.address as + * place_address, p.lat as place_lat, p.lng as place_lng, * COALESCE(da.assignment_time, p.place_time) as effective_time, * COALESCE(da.assignment_end_time, p.end_time) as effective_end_time FROM - * day_assignments da JOIN places p ON da.place_id = p.id WHERE da.day_id = - * ? AND da.accommodation_id IS NULL ORDER BY da.order_index ASC, - * da.created_at ASC` — the booked-night stop (its own accommodation-linked - * `day_assignments` row) is excluded, matching the legacy comment ("that - * stop is the booking … reading it here as well would put the hotel on the - * day a second time"). Kept per-day (the caller's own loop), not batched — - * result-identical either way per the inventory's own note; per-day is the - * smaller diff against the legacy statement shape. + * day_assignments da JOIN places p ON da.place_id = p.id WHERE da.day_id IN + * (...) AND da.accommodation_id IS NULL ORDER BY da.order_index, da.created_at, + * da.id` — the booked-night stop (its own accommodation-linked row) is + * excluded: it is the booking, and reading it here would put the hotel on the + * day a second time. Each day's rows keep the per-day order; the id settles a tie. */ export interface CalendarStopRow extends DayAssignmentRow { place_name: string; @@ -994,8 +991,9 @@ export class ReservationsRepository extends TrekRepository { return rows as CalendarReservationRow[]; } - /** CL4 — see {@link CalendarStopRow}'s docstring. */ - async listCalendarStops(day_id: number): Promise { + /** CL4 — see {@link CalendarStopRow}'s docstring. No days, no query. */ + async listCalendarStopsForDays(day_ids: number[]): Promise { + if (day_ids.length === 0) return []; const rows = await this.kysely() .selectFrom('day_assignments as da') .innerJoin('places as p', 'p.id', 'da.place_id') @@ -1008,10 +1006,11 @@ export class ReservationsRepository extends TrekRepository { eb.fn.coalesce('da.assignment_time', 'p.place_time').as('effective_time'), eb.fn.coalesce('da.assignment_end_time', 'p.end_time').as('effective_end_time'), ]) - .where('da.day_id', '=', day_id) + .where('da.day_id', 'in', day_ids) .where('da.accommodation_id', 'is', null) .orderBy('da.order_index', 'asc') .orderBy('da.created_at', 'asc') + .orderBy('da.id', 'asc') .execute(); return rows as CalendarStopRow[]; } diff --git a/server/src/nest/assignments/assignments.service.ts b/server/src/nest/assignments/assignments.service.ts index 64061d4f5..e923644a7 100644 --- a/server/src/nest/assignments/assignments.service.ts +++ b/server/src/nest/assignments/assignments.service.ts @@ -325,17 +325,13 @@ export class AssignmentsService { } /** - * AS11 — `DayAssignmentsRepository.setOrderIndex`, day-scoped, one row per - * id, sequentially and in the legacy's own order (not `Promise.all` — the - * program's transaction-ordering rule). + * AS11 — `DayAssignmentsRepository.setOrderIndexes`, day-scoped: every id + * gets its position in one statement, with the result the per-row writes in + * the given order had (an id listed twice keeps its last position). */ async reorderAssignments(dayId: string | number, orderedIds: number[]): Promise { const dayIdNum = toRowId(dayId)!; - await this.uow.transactional(async () => { - for (const [index, id] of orderedIds.entries()) { - await this.dayAssignmentsRepo.setOrderIndex(id, dayIdNum, index); - } - }); + await this.dayAssignmentsRepo.setOrderIndexes(orderedIds.map((id, index) => ({ id, order_index: index })), dayIdNum); } /** @@ -457,11 +453,10 @@ export class AssignmentsService { // with their gaps would put the day notes and bookings that sort between stops in // one place for the writer, who reads the day back, and in another for everyone // else. Only a stop whose key changes is written. AS19 — no day scoping, - // unlike AS11's `reorderAssignments` (`setOrderIndex`'s `day_id` left - // `undefined`), sequentially and in the legacy's own order. - for (const [i, row] of sorted.entries()) { - if (row.order_index !== i) await this.dayAssignmentsRepo.setOrderIndex(row.id, undefined, i); - } + // unlike AS11's `reorderAssignments` (`setOrderIndexes`'s `day_id` left + // `undefined`), in one statement. + const moved = sorted.flatMap((row, i) => (row.order_index !== i ? [{ id: row.id, order_index: i }] : [])); + await this.dayAssignmentsRepo.setOrderIndexes(moved); return { dayId, orderedIds: sorted.map(row => row.id), viasMoved: await this.reanchorVias(dayId, rows, sorted) }; } diff --git a/server/src/nest/calendar/calendar.service.ts b/server/src/nest/calendar/calendar.service.ts index 471ea803a..6532430bc 100644 --- a/server/src/nest/calendar/calendar.service.ts +++ b/server/src/nest/calendar/calendar.service.ts @@ -127,6 +127,17 @@ interface ZoneTransition { } const DAY_MS = 86_400_000; + +/** Rows of several days, by day id; each day keeps the order the rows came in. */ +function groupByDay(rows: T[]): Map { + const byDay = new Map(); + for (const row of rows) { + const list = byDay.get(row.day_id); + if (list) list.push(row); + else byDay.set(row.day_id, [row]); + } + return byDay; +} /** Ten years at most: a span longer than that is a typo, not a trip. */ const MAX_SCAN_DAYS = 3660; @@ -318,19 +329,20 @@ export class CalendarService { events.push(ev); } - // Days with assignments and notes + // Days with assignments and notes, read for all dated days at once. const days = await this.daysRepo.listByTrip(trip.id); + const datedDayIds = days.filter((d) => d.date).map((d) => d.id); + // A booked night puts a stop of its own on its check-in day, so the route + // can reach the hotel. That stop is the booking, not a place the traveller + // planned to visit, and the booking already comes through below as the + // stay block or its check-in and check-out markers. Read here as well it + // would put the hotel on the day a second time. + const stopsByDay = groupByDay(await this.reservationsRepo.listCalendarStopsForDays(datedDayIds)); + const notesByDay = groupByDay(await this.dayNotesRepo.listByDayIds(datedDayIds)); for (const day of days) { if (!day.date) continue; - - // A booked night puts a stop of its own on its check-in day, so the route - // can reach the hotel. That stop is the booking, not a place the traveller - // planned to visit, and the booking already comes through below as the - // stay block or its check-in and check-out markers. Read here as well it - // would put the hotel on the day a second time. - const assignments = await this.reservationsRepo.listCalendarStops(day.id); - - const notes = await this.dayNotesRepo.listByDayIds([day.id]); + const assignments = stopsByDay.get(day.id) ?? []; + const notes = notesByDay.get(day.id) ?? []; const timed = assignments.filter(a => a.effective_time); const untimed = assignments.filter(a => !a.effective_time); diff --git a/server/src/nest/journey/journey-domain.service.ts b/server/src/nest/journey/journey-domain.service.ts index 8ee01f715..d5cacea67 100644 --- a/server/src/nest/journey/journey-domain.service.ts +++ b/server/src/nest/journey/journey-domain.service.ts @@ -115,8 +115,7 @@ export class JourneyDomainService { // R7 — AP1's `this.db.canAccessTrip(...)` becomes a direct call to the // same repository `DatabaseService.canAccessTrip` already delegates to // (Task 0's confirmation: `canAccessTrip` IS `findAccessible`, not - // merely equivalent to it). Also reused by `getJourneyFull`'s per-entry - // `source_trip_name` lookup (JG16, via the already-existing `getTitle`). + // merely equivalent to it). @InjectRepository(Trips) private readonly tripsRepo: TripsRepository, // Plan 3g Task 2 (Part B) — the full photos surface (JG19/JG87-116), // finishing the two sites Task 1 left raw (JG15/JG19) because their @@ -259,16 +258,13 @@ export class JourneyDomainService { // .galleryRead`). const gallery = await this.photosRepo.galleryRead(journeyId); - const enrichedEntries = await Promise.all( - entries.map(async (e) => ({ - ...decodeEntryRow(e), - photos: photosByEntry[e.id] || [], - // JG16 — reuses `TripsRepository.getTitle` (already public, 3c; same - // statement text as this site's legacy `SELECT title FROM trips - // WHERE id = ?`) rather than a new method. - source_trip_name: e.source_trip_id ? await this.tripsRepo.getTitle(e.source_trip_id) : null, - })), - ); + // JG16 — the source trips' titles in one read instead of one per entry. + const tripTitles = await this.entriesRepo.sourceTripTitles(journeyId); + const enrichedEntries = entries.map((e) => ({ + ...decodeEntryRow(e), + photos: photosByEntry[e.id] || [], + source_trip_name: e.source_trip_id ? (tripTitles.get(e.source_trip_id) ?? null) : null, + })); // linked trips (JG17) const trips = await this.journeyTripsRepo.listForJourney(journeyId); @@ -1174,14 +1170,13 @@ export class JourneyDomainService { (photosByEntry[p.entry_id] ||= []).push(p); } - // JG73 — same N+1-by-design shape as JG16, reusing `TripsRepository.getTitle`. - return await Promise.all( - entries.map(async (e) => ({ - ...decodeEntryRow(e), - photos: photosByEntry[e.id] || [], - source_trip_name: e.source_trip_id ? await this.tripsRepo.getTitle(e.source_trip_id) : null, - })), - ); + // JG73 — the source trips' titles in one read, as JG16. + const tripTitles = await this.entriesRepo.sourceTripTitles(journeyId); + return entries.map((e) => ({ + ...decodeEntryRow(e), + photos: photosByEntry[e.id] || [], + source_trip_name: e.source_trip_id ? (tripTitles.get(e.source_trip_id) ?? null) : null, + })); } async createEntry( diff --git a/server/tests/unit/db/repositories/DayAssignments.repository.test.ts b/server/tests/unit/db/repositories/DayAssignments.repository.test.ts index 0cc01427d..714b83653 100644 --- a/server/tests/unit/db/repositories/DayAssignments.repository.test.ts +++ b/server/tests/unit/db/repositories/DayAssignments.repository.test.ts @@ -407,6 +407,32 @@ describe('DayAssignmentsRepository — AS10/AS11/AS13/AS14/AS19 (delete / order expect(await orderOf(a.id)).toBe(3); }); + it('ASSIGNREPO-009b (setOrderIndexes): one statement leaves what the per-row writes left, day scope included', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const otherDay = createDay(testDb, trip.id); + const place = createPlace(testDb, trip.id); + const a = createDayAssignment(testDb, day.id, place.id, { order_index: 0 }); + const b = createDayAssignment(testDb, day.id, place.id, { order_index: 1 }); + const c = createDayAssignment(testDb, day.id, place.id, { order_index: 2 }); + const foreign = createDayAssignment(testDb, otherDay.id, place.id, { order_index: 5 }); + + // b twice: the later index wins, as two writes in a row would leave it; foreign is on another day. + await withRequestContext(t.orm, () => + assignments.setOrderIndexes([{ id: c.id, order_index: 0 }, { id: b.id, order_index: 9 }, { id: a.id, order_index: 2 }, { id: b.id, order_index: 1 }, { id: foreign.id, order_index: 0 }], day.id), + ); + expect([await orderOf(a.id), await orderOf(b.id), await orderOf(c.id), await orderOf(foreign.id)]).toEqual([2, 1, 0, 5]); + + // Unscoped: by id alone, and only the listed rows. + await withRequestContext(t.orm, () => assignments.setOrderIndexes([{ id: foreign.id, order_index: 1 }])); + expect([await orderOf(foreign.id), await orderOf(a.id)]).toEqual([1, 2]); + + // Nothing to write is no statement at all. + await withRequestContext(t.orm, () => assignments.setOrderIndexes([], day.id)); + expect(await orderOf(c.id)).toBe(0); + }); + it('ASSIGNREPO-010 (AS13, getDayId): the assignment\'s day_id, undefined for a missing id', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); diff --git a/server/tests/unit/db/repositories/Reservations.repository.test.ts b/server/tests/unit/db/repositories/Reservations.repository.test.ts index 4854b61ee..3fca19c70 100644 --- a/server/tests/unit/db/repositories/Reservations.repository.test.ts +++ b/server/tests/unit/db/repositories/Reservations.repository.test.ts @@ -185,7 +185,7 @@ describe('ReservationsRepository — CL2 (listForCalendar)', () => { }); }); -describe('ReservationsRepository — CL4 (listCalendarStops)', () => { +describe('ReservationsRepository — CL4 (listCalendarStopsForDays)', () => { it('CAL-CL4-001: matches the legacy per-day statement, excluding a booked-night stop', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id, { start_date: '2026-06-01', end_date: '2026-06-03' }); @@ -197,7 +197,7 @@ describe('ReservationsRepository — CL4 (listCalendarStops)', () => { const accId = await insertRow(await orm(), DayAccommodations, { trip: trip.id, place: hotelPlace.id, startDay: day.id, endDay: day.id }); await insertRow(await orm(), DayAssignments, { day: day.id, place: hotelPlace.id, order_index: 1, accommodation_id: accId }); - const typed = await reservationsRepo.listCalendarStops(day.id); + const typed = await reservationsRepo.listCalendarStopsForDays([day.id]); // test-sql-allow: the legacy statement is the oracle the repository read is held to. const legacy = testDb.prepare(` SELECT da.*, p.name as place_name, p.address as place_address, diff --git a/server/tests/unit/nest/calendar.service.test.ts b/server/tests/unit/nest/calendar.service.test.ts index 4c8194d89..2964f6a4f 100644 --- a/server/tests/unit/nest/calendar.service.test.ts +++ b/server/tests/unit/nest/calendar.service.test.ts @@ -488,6 +488,24 @@ describe('exportICS', () => { expect(ics).toContain('LOCATION:Rue de Rivoli'); }); + it('CAL-016b: read in one go for all days, each day keeps its own stops and notes in their order', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id, { title: 'Two Days' }); + const first = createDay(testDb, trip.id, { date: '2025-06-02', title: 'First' }); + const second = createDay(testDb, trip.id, { date: '2025-06-03', title: 'Second' }); + const place = (name: string) => createPlace(testDb, trip.id, { name }); + createDayAssignment(testDb, first.id, place('Later').id, { order_index: 1 }); + createDayAssignment(testDb, first.id, place('Sooner').id, { order_index: 0 }); + createDayAssignment(testDb, second.id, place('Elsewhere').id, { order_index: 0 }); + createDayNote(testDb, first.id, trip.id, { text: 'second note', sort_order: 1 }); + createDayNote(testDb, first.id, trip.id, { text: 'first note', sort_order: 0 }); + + const ics = (await svc.exportICS(trip.id)).ics.replace(/\r\n /g, ''); + + expect(ics).toContain('SUMMARY:First\r\nDESCRIPTION:• Sooner\\n• Later\\n\\nNotes:\\n• first note\\n• second note\r\n'); + expect(ics).toContain('SUMMARY:Second\r\nDESCRIPTION:• Elsewhere\r\n'); + }); + it('CAL-016: a day without a date is skipped and a titled day uses its own title', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id, { title: 'Mixed Days' }); diff --git a/server/tests/unit/nest/journey-domain.service.test.ts b/server/tests/unit/nest/journey-domain.service.test.ts index c555b20c7..8f00eed8b 100644 --- a/server/tests/unit/nest/journey-domain.service.test.ts +++ b/server/tests/unit/nest/journey-domain.service.test.ts @@ -2729,6 +2729,23 @@ describe('entry enrichment', () => { expect((entry as any).source_trip_name).toBe('Japan 2026'); }); + it('JOURNEY-SVC-ENRICH-001b: entries from two trips each carry their own trip name, in the full read and the list alike', async () => { + const { user } = createUser(testDb); + const a = tripWithPlace(user.id).trip; + const b = tripWithPlace(user.id).trip; + await updateRows(await orm(), Trips, { id: a.id }, { title: 'Alps' }); + await updateRows(await orm(), Trips, { id: b.id }, { title: 'Baltic' }); + const journey = await svc.createJourney(user.id, { title: 'J', trip_ids: [a.id, b.id] }); + await svc.createEntry(journey.id, user.id, { entry_date: '2026-05-01', title: 'Solo' }); + + const names = (rows: Array<{ source_trip_id?: number | null; source_trip_name?: string | null }>) => + rows.map((e) => [e.source_trip_id ?? null, e.source_trip_name]).sort((x, y) => Number(x[0]) - Number(y[0])); + const listed = names((await svc.listEntries(journey.id, user.id))!); + expect(listed).toEqual([[null, null], [a.id, 'Alps'], [b.id, 'Baltic']].sort((x, y) => Number(x[0]) - Number(y[0]))); + const full = (await svc.getJourneyFull(journey.id, user.id))! as unknown as { entries: Array<{ source_trip_id?: number | null; source_trip_name?: string | null }> }; + expect(names(full.entries)).toEqual(listed); + }); + it('JOURNEY-SVC-ENRICH-002: an entry with no trip and no tags gets [] and null, not undefined', async () => { const { user } = createUser(testDb); const journey = await svc.createJourney(user.id, { title: 'J' });