refactor(reservations): grouping endpoints and day slots moves into reservations helpers

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;

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<number, ReservationEndpoint[]> {
+  const map = new Map<number, ReservationEndpoint[]>();
+  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<ReservationEndpoint>`
+    // 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<number, Record<number, number>> {
+  const posMap = new Map<number, Record<number, number>>();
+  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,
+  }));
+}

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<Map<number, ReservationEndpoint[]>> {
     const rows: ReservationEndpointRow[] = await this.endpointsRepo.listForTrip(this.rowIdNum(tripId));
-    const map = new Map<number, ReservationEndpoint[]>();
-    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<ReservationEndpoint>`
-      // 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<number, Record<number, number>>();
-    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);

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> = {}): 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],
+    ]);
+  });
+});