From 5fcfe8b912b7783b0b04df3ef3171a6271834808 Mon Sep 17 00:00:00 2001 From: Maurice Date: Mon, 12 Oct 2026 21:13:57 +0000 Subject: [PATCH] refactor(reservations): grouping endpoints and day slots moves into reservations helpers --- .../repositories/Reservations.repository.ts | 29 +---- .../nest/reservations/reservations.helpers.ts | 60 ++++++++++ .../nest/reservations/reservations.service.ts | 40 +------ .../unit/nest/reservations.helpers.test.ts | 108 ++++++++++++++++++ 4 files changed, 174 insertions(+), 63 deletions(-) create mode 100644 server/src/nest/reservations/reservations.helpers.ts create mode 100644 server/tests/unit/nest/reservations.helpers.test.ts diff --git a/server/src/db/repositories/Reservations.repository.ts b/server/src/db/repositories/Reservations.repository.ts index 08dac3c27..2d42a0b3b 100644 --- a/server/src/db/repositories/Reservations.repository.ts +++ b/server/src/db/repositories/Reservations.repository.ts @@ -20,34 +20,7 @@ import { ROADTRIP_RESERVATION_TYPES } from '@trek/shared'; * folds `accommodation_id` (`Math.trunc(Number(x))`), `day_positions`, * `endpoints` and `travelers` on afterward (unchanged from the legacy split). */ -export interface ReservationJoinRow { - id: number; - trip_id: number; - day_id: number | null; - end_day_id: number | null; - place_id: number | null; - assignment_id: number | null; - title: string; - accommodation_id: string | null; - reservation_time: string | null; - reservation_end_time: string | null; - location: string | null; - confirmation_number: string | null; - notes: string | null; - status: string | null; - type: string | null; - created_at: string | null; - metadata: string | null; - day_plan_position: number | null; - needs_review: number; - external_source: string | null; - external_id: string | null; - external_owner_user_id: number | null; - external_synced_at: string | null; - sync_enabled: number | null; - external_hash: string | null; - url: string | null; - ingest_state: string; +export interface ReservationJoinRow extends ReservationAllColumnsRow { day_number: number | null; place_name: string | null; accommodation_place_id: number | null; diff --git a/server/src/nest/reservations/reservations.helpers.ts b/server/src/nest/reservations/reservations.helpers.ts new file mode 100644 index 000000000..bc8d566ee --- /dev/null +++ b/server/src/nest/reservations/reservations.helpers.ts @@ -0,0 +1,60 @@ +import type { ReservationEndpointRow } from '../../db/repositories/ReservationEndpoints.repository'; +import type { EndpointInput, ReservationEndpoint } from './reservations.service'; + +/** + * Pure helpers of the reservations domain: how a trip's endpoint and per-day + * position rows are grouped by booking for the list, and which endpoint rows a + * save inserts. Nothing here touches the DB, so they stay plain exports rather + * than methods on ReservationsService (the places.helpers.ts split). + */ + +/** A trip's endpoint rows, grouped by the reservation they belong to, in the order read. */ +export function groupEndpointsByReservation(rows: ReservationEndpointRow[]): Map { + const map = new Map(); + for (const r of rows) { + const list = map.get(r.reservation_id) ?? []; + // `role` is TEXT at the DB (unconstrained); `ReservationEndpoint.role` + // narrows it to the three values every writer of this column sends: + // the same widen-on-read the legacy `this.db.all` + // generic parameter asserted without any runtime check. + list.push(r as ReservationEndpoint); + map.set(r.reservation_id, list); + } + return map; +} + +/** Each reservation's slot per day, `{ [day_id]: position }`, keyed by the reservation's id. */ +export function groupDayPositions( + dayPositions: { reservation_id: number; day_id: number; position: number }[], +): Map> { + const posMap = new Map>(); + for (const dp of dayPositions) { + if (!posMap.has(dp.reservation_id)) posMap.set(dp.reservation_id, {}); + posMap.get(dp.reservation_id)![dp.day_id] = dp.position; + } + return posMap; +} + +/** + * The rows a save of a reservation's endpoints inserts, one per kept endpoint + * in the order given. A missing `sequence` is the index after the filter. + */ +export function endpointRows(reservationId: number, endpoints: EndpointInput[]) { + // lat/lng are NOT NULL: an imported transport whose pick-up/return (or station/ + // stop) couldn't be geocoded reaches here with null coords. Skip those rows rather + // than let the INSERT throw and fail the entire booking save; the dates still live + // on reservation_time/reservation_end_time, so the booking lands on its day either way. + const filtered = endpoints.filter((e) => e.lat != null && e.lng != null); + return filtered.map((e, i) => ({ + reservation_id: reservationId, + role: e.role, + sequence: e.sequence ?? i, + name: e.name, + code: e.code ?? null, + lat: e.lat!, + lng: e.lng!, + timezone: e.timezone ?? null, + local_time: e.local_time ?? null, + local_date: e.local_date ?? null, + })); +} diff --git a/server/src/nest/reservations/reservations.service.ts b/server/src/nest/reservations/reservations.service.ts index 03a2863c9..e2ab8cabe 100644 --- a/server/src/nest/reservations/reservations.service.ts +++ b/server/src/nest/reservations/reservations.service.ts @@ -7,6 +7,7 @@ import { RealtimeService } from '../realtime/realtime.service'; import { PermissionsService } from '../permissions/permissions.service'; import { ReservationsReadService, toTraveler } from './reservations-read.service'; import { keepMirroredPrice } from './reservation-metadata'; +import { endpointRows, groupDayPositions, groupEndpointsByReservation } from './reservations.helpers'; import type { Reservation, User } from '../../types'; import { BudgetService } from '../budget/budget.service'; import { typeToCostCategory } from '@trek/shared'; @@ -256,17 +257,7 @@ export class ReservationsService { async loadEndpointsByTrip(tripId: string | number): Promise> { const rows: ReservationEndpointRow[] = await this.endpointsRepo.listForTrip(this.rowIdNum(tripId)); - const map = new Map(); - for (const r of rows) { - const list = map.get(r.reservation_id) ?? []; - // `role` is TEXT at the DB (unconstrained); `ReservationEndpoint.role` - // narrows it to the three values every writer of this column sends — - // the same widen-on-read the legacy `this.db.all` - // generic parameter asserted without any runtime check. - list.push(r as ReservationEndpoint); - map.set(r.reservation_id, list); - } - return map; + return groupEndpointsByReservation(rows); } /** @@ -376,24 +367,8 @@ export class ReservationsService { // database connection is not open"). await this.uow.transactional(async () => { await this.endpointsRepo.deleteForReservation(reservationId); - // lat/lng are NOT NULL: an imported transport whose pick-up/return (or station/ - // stop) couldn't be geocoded reaches here with null coords. Skip those rows rather - // than let the INSERT throw and fail the entire booking save — the dates still live - // on reservation_time/reservation_end_time, so the booking lands on its day either way. - const filtered = endpoints.filter((e) => e.lat != null && e.lng != null); - for (const [i, e] of filtered.entries()) { - await this.endpointsRepo.insertEndpoint({ - reservation_id: reservationId, - role: e.role, - sequence: e.sequence ?? i, - name: e.name, - code: e.code ?? null, - lat: e.lat!, - lng: e.lng!, - timezone: e.timezone ?? null, - local_time: e.local_time ?? null, - local_date: e.local_date ?? null, - }); + for (const row of endpointRows(reservationId, endpoints)) { + await this.endpointsRepo.insertEndpoint(row); } }); } @@ -412,12 +387,7 @@ export class ReservationsService { const reservations: ReservationRow[] = (await this.reservationsRepo.listForTrip(tripId)).map((r) => ({ ...r })); const dayPositions = await this.dayPositionsRepo.listForTrip(this.rowIdNum(tripId)); - - const posMap = new Map>(); - for (const dp of dayPositions) { - if (!posMap.has(dp.reservation_id)) posMap.set(dp.reservation_id, {}); - posMap.get(dp.reservation_id)![dp.day_id] = dp.position; - } + const posMap = groupDayPositions(dayPositions); const endpointsMap = await this.loadEndpointsByTrip(tripId); const travelersMap = await this.loadTravelersByTrip(tripId); diff --git a/server/tests/unit/nest/reservations.helpers.test.ts b/server/tests/unit/nest/reservations.helpers.test.ts new file mode 100644 index 000000000..eb31ae058 --- /dev/null +++ b/server/tests/unit/nest/reservations.helpers.test.ts @@ -0,0 +1,108 @@ +/** + * The pure halves of ReservationsService's endpoint and day position handling + * (reservations.helpers.ts): how a trip's rows are grouped by booking for the + * list, and which endpoint rows a save inserts. The service suites reach them + * through the database; these pin the shapes on their own. + */ +import type { ReservationEndpointRow } from '../../../src/db/repositories/ReservationEndpoints.repository'; +import { + endpointRows, + groupDayPositions, + groupEndpointsByReservation, +} from '../../../src/nest/reservations/reservations.helpers'; +import type { EndpointInput } from '../../../src/nest/reservations/reservations.service'; + +import { describe, expect, it } from 'vitest'; + +const endpointRow = (id: number, reservationId: number, sequence: number): ReservationEndpointRow => ({ + id, + reservation_id: reservationId, + role: sequence === 0 ? 'from' : 'to', + sequence, + name: `Stop ${id}`, + code: null, + lat: 48.1, + lng: 11.5, + timezone: null, + local_time: null, + local_date: null, + created_at: null, +}); + +const input = (name: string, over: Partial = {}): EndpointInput => ({ + role: 'stop', + name, + code: null, + lat: 48.1, + lng: 11.5, + timezone: null, + local_time: null, + local_date: null, + ...over, +}); + +describe('groupEndpointsByReservation', () => { + it('RESV-HELP-001: groups the endpoint rows by booking and keeps the order they were read in', () => { + const rows = [endpointRow(1, 10, 0), endpointRow(2, 20, 0), endpointRow(3, 10, 1)]; + + const grouped = groupEndpointsByReservation(rows); + + expect([...grouped.keys()]).toEqual([10, 20]); + expect(grouped.get(10)?.map((e) => e.id)).toEqual([1, 3]); + expect(grouped.get(20)).toEqual([rows[1]]); + }); + + it('RESV-HELP-002: answers an empty map for a trip without endpoints', () => { + expect(groupEndpointsByReservation([]).size).toBe(0); + }); +}); + +describe('groupDayPositions', () => { + it('RESV-HELP-003: maps each booking to its position on every day it has one', () => { + const grouped = groupDayPositions([ + { reservation_id: 10, day_id: 1, position: 2 }, + { reservation_id: 20, day_id: 1, position: 0 }, + { reservation_id: 10, day_id: 2, position: 3.5 }, + ]); + + expect(grouped.get(10)).toEqual({ 1: 2, 2: 3.5 }); + expect(grouped.get(20)).toEqual({ 1: 0 }); + expect(grouped.has(30)).toBe(false); + }); +}); + +describe('endpointRows', () => { + it('RESV-HELP-004: builds the insert row of an endpoint, with null for each optional field it leaves out', () => { + const endpoint = { role: 'from', name: 'Berlin Hbf', code: 'BER', lat: 52.5, lng: 13.4 } as EndpointInput; + + expect(endpointRows(7, [endpoint])).toEqual([ + { + reservation_id: 7, + role: 'from', + sequence: 0, + name: 'Berlin Hbf', + code: 'BER', + lat: 52.5, + lng: 13.4, + timezone: null, + local_time: null, + local_date: null, + }, + ]); + }); + + it('RESV-HELP-005: skips an endpoint without coordinates and numbers the rest after the skip', () => { + // An imported station the geocoder could not place arrives with null coordinates. + const rows = endpointRows(7, [ + input('Unplaced', { lat: null }), + input('No longitude', { lng: null }), + input('Munich Hbf'), + input('Vienna Hbf', { sequence: 9 }), + ]); + + expect(rows.map((r) => [r.name, r.sequence])).toEqual([ + ['Munich Hbf', 0], + ['Vienna Hbf', 9], + ]); + }); +});