diff --git a/server/src/db/repositories/Reservations.repository.ts b/server/src/db/repositories/Reservations.repository.ts index 638943999..460200fea 100644 --- a/server/src/db/repositories/Reservations.repository.ts +++ b/server/src/db/repositories/Reservations.repository.ts @@ -1,7 +1,9 @@ import type { Reservations } from '../entities/Reservations.entity'; import { DayAssignments } from '../entities/DayAssignments.entity'; import { Days } from '../entities/Days.entity'; -import { castIntegerKysely, coalesceParam, columnRef, concatKysely, dayDistance, substringKysely } from '../dialect/sql-functions'; +import { castIntegerKysely, coalesceParam, columnRef, concatKysely, dayDistance, startsWithIsoDateKysely, substringKysely } from '../dialect/sql-functions'; +import { publicReservationExpr, publicStayExists, type ReservationVisibilityKyselyDB } from './_shared/reservation-visibility'; +import type { DayAssignmentRow } from './DayAssignments.repository'; import { TrekRepository } from './_shared/trek-repository'; /** @@ -155,6 +157,420 @@ interface ReservationsByAccommodationKyselyDB { }; } +// --------------------------------------------------------------------------- +// Plan 3d Task 4 (calendar, `listUpcoming`, the visibility-predicate +// consumers incl. `share`, the `public-api` pickup) — additive. `calendar/**` +// and `share/**`'s reads on `day_accommodations`/`reservation_endpoints`/ +// `reservation_day_positions` land here rather than on those tables' own +// repositories: `RoadtripVias`/`RoadtripDayTracks`/`RoadtripPreferences`/ +// `RoadtripDayBoundaries`/`DayAccommodations`/`ReservationEndpoints`/ +// `ReservationTravelers` repositories are Task 6's file set for this window +// (trip copy's additive methods), so a cross-table Kysely read — the SAME +// escape hatch `joinedQuery()`/`getAssignmentTripId`/`findNearestDayId` +// above already use to reach `days`/`places`/`day_accommodations` from this +// file — lands the read here instead of opening a second implementer's file. +// `DayAssignments.repository.ts` is not in Task 6's set, but CL4's read is +// kept alongside its RS20/CL2/CL7 siblings for the same "one file, one +// diff" reason. +// --------------------------------------------------------------------------- + +/** + * CL2 (`CalendarService.buildTripCalendar`'s reservation read) — `SELECT + * r.*, pl.lat AS place_lat, pl.lng AS place_lng, sd.date AS stay_start_date, + * ed.date AS stay_end_date, a.check_in AS stay_check_in, a.check_out AS + * stay_check_out, (SELECT MIN(r2.id) FROM reservations r2 WHERE + * r2.accommodation_id = a.id) AS stay_first_reservation_id, rd.date AS + * day_date, red.date AS end_day_date FROM reservations r LEFT JOIN places + * pl … LEFT JOIN day_accommodations a ON r.accommodation_id = a.id LEFT + * JOIN days sd/ed/rd/red … WHERE r.trip_id = ? AND `, no `ORDER + * BY`. `r.accommodation_id = a.id` and `r2.accommodation_id = a.id` are + * plain column-to-column joins (no `CAST`) — SQLite applies numeric + * affinity to the TEXT side against a genuine INTEGER column on its own + * (§18.1), the same reasoning `joinedQuery()`'s `ap`/`acc_p` joins above + * already rely on; a `CAST` is only needed where one side is a BOUND value + * (RV2, RS20's correlated title subquery). The `stay_first_reservation_id` + * subquery is deliberately **not** filtered by RV1 — inventory §18.2's + * documented, unfixed hole (a staged sibling booking with a lower id can + * win the slot and suppress a public booking's all-day block); kept + * byte-for-byte, pinned by CAL-CL2-HOLE-001 in the repository test. + */ +export interface CalendarReservationRow extends ReservationAllColumnsRow { + place_lat: number | null; + place_lng: number | null; + stay_start_date: string | null; + stay_end_date: string | null; + stay_check_in: string | null; + stay_check_out: string | null; + stay_first_reservation_id: number | null; + day_date: string | null; + end_day_date: string | null; +} + +interface CalendarReservationKyselyDB { + reservations: ReservationJoinKyselyDB['reservations']; + places: { id: number; lat: number | null; lng: number | null }; + day_accommodations: { id: number; start_day_id: number | null; end_day_id: number | null; check_in: string | null; check_out: string | null }; + days: { id: number; date: string | null }; +} + +/** + * 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, + * 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. + */ +export interface CalendarStopRow extends DayAssignmentRow { + place_name: string; + place_address: string | null; + place_lat: number | null; + place_lng: number | null; + effective_time: string | null; + effective_end_time: string | null; +} + +interface CalendarStopsKyselyDB { + day_assignments: { + id: number; day_id: number; place_id: number; order_index: number | null; notes: string | null; + reservation_status: string | null; reservation_notes: string | null; reservation_datetime: string | null; + created_at: string | null; assignment_time: string | null; assignment_end_time: string | null; + leg_transport_mode: string | null; incoming_leg_transport_mode: string | null; end_day: number; accommodation_id: number | null; + }; + places: { id: number; name: string; address: string | null; lat: number | null; lng: number | null; place_time: string | null; end_time: string | null }; +} + +/** + * CL7 (`CalendarService.buildTripCalendar`'s check-in/check-out stay read) + * — `SELECT a.id, a.check_in, a.check_in_end, a.check_out, sd.date AS + * start_date, ed.date AS end_date, p.name AS place_name, p.address AS + * place_address, p.lat AS place_lat, p.lng AS place_lng, (SELECT r.title + * FROM reservations r WHERE r.accommodation_id = a.id AND ORDER + * BY r.id ASC LIMIT 1) AS reservation_title FROM day_accommodations a LEFT + * JOIN days sd … LEFT JOIN days ed … LEFT JOIN places p … WHERE a.trip_id = + * ? AND ORDER BY a.id ASC`. The `reservation_title` subquery is + * a plain column-to-column compare too (no `CAST`), same reasoning as CL2. + * `.where('a.trip_id', ...).where(publicStayExists)` are chained BEFORE the + * `sd`/`ed`/`p` joins below (Kysely renders WHERE after JOIN regardless of + * call order — the AST, not the call sequence, decides clause order) so + * `publicStayExists`'s narrow `ExpressionBuilder<…, 'a'>` signature sees + * exactly the `a` alias it requires (Task 0 review's own finding: a + * differently-joined outer query typechecks as long as the alias stays + * `a`). + */ +export interface CalendarStayRow { + id: number; + check_in: string | null; + check_in_end: string | null; + check_out: string | null; + start_date: string | null; + end_date: string | null; + place_name: string | null; + place_address: string | null; + place_lat: number | null; + place_lng: number | null; + reservation_title: string | null; +} + +interface CalendarStayKyselyDB extends ReservationVisibilityKyselyDB { + day_accommodations: { + id: number; trip_id: number | string; place_id: number | null; start_day_id: number | null; end_day_id: number | null; + check_in: string | null; check_in_end: string | null; check_out: string | null; + }; + reservations: ReservationVisibilityKyselyDB['reservations'] & { title: string }; + days: { id: number; date: string | null }; + places: { id: number; name: string | null; address: string | null; lat: number | null; lng: number | null }; +} + +/** + * RS20 (`ReservationsService.listUpcoming`) — the CTE + `UNION ALL` + * statement, inventory §2e. Ruling (ii): ONE fully typed Kysely statement + * (no `connection.execute()` exception, no JS merge/re-sort — the reviewer's + * prototype matched legacy rows across 5 `(today, now, limit)` settings + * incl. the cross-table `id` tiebreak, and the plan adopted that). GLOB + * through `startsWithIsoDateKysely`; `CAST` through `castIntegerKysely` + * (the correlated title subquery compares a BOUND `a.id` — an outer-query + * value, not a plain column-to-column join, unlike CL2/CL7's `MIN`/title + * subqueries — so the legacy statement casts here too: + * `CAST(res.accommodation_id AS INTEGER) = a.id`); `||` through + * `concatKysely`; `substr` through `substringKysely`. `LIMIT` stays inside + * SQL; the SQLite collation `ORDER BY at_date ASC, COALESCE(at_time, + * '00:00') ASC, id ASC` (the cross-table `id` tiebreak — a stay id and a + * reservation id can collide) is reproduced exactly, never a JS `.sort()`. + * + * Each of the three `entries` arms is `$castTo()`'d before the + * `unionAll` chain: Kysely infers a literal type (`'checkin'`) for + * `eb.val('checkin').as('type')` in arms 2/3 against `r.type: string | + * null` in arm 1, and the three arms' `title`/`reservation_time` sources + * differ in nullability shape too — the union of literal/nullable variants + * doesn't collapse into one column type on its own, the same + * `DayStopRow`/`AssignmentTimeSortKyselyDB` precedent + * (`DayAssignments.repository.ts::listForTimeSort`) uses for its own + * `eb.and([...])`-inferred boolean column. + */ +export interface UpcomingReservationRow { + id: number; + trip_id: number; + title: string; + type: string | null; + status: string | null; + location: string | null; + reservation_time: string | null; + confirmation_number: string | null; + trip_title: string; + trip_cover: string | null; + day_date: string | null; + place_name: string | null; + place_image: string | null; +} + +/** RS20's `entries` CTE row shape — see {@link UpcomingReservationRow}'s docstring for why every arm is cast to this. */ +interface EntriesRow extends UpcomingReservationRow { + at_date: string | null; + at_time: string | null; +} + +interface UpcomingReservationsKyselyDB { + trips: { id: number; user_id: number; title: string; cover_image: string | null; is_archived: number }; + trip_members: { trip_id: number; user_id: number }; + reservations: { + id: number; trip_id: number; title: string; type: string | null; status: string | null; + location: string | null; reservation_time: string | null; confirmation_number: string | null; + day_id: number | null; place_id: number | null; accommodation_id: string | null; + }; + days: { id: number; date: string | null }; + places: { id: number; name: string | null; image_url: string | null }; + day_accommodations: { + id: number; trip_id: number; place_id: number | null; start_day_id: number; end_day_id: number; + check_in: string | null; check_out: string | null; confirmation: string | null; + }; +} + +/** + * `share.service.ts:251` (`publicEndpointsByReservation`) — `SELECT + * e.reservation_id, e.role, e.sequence, e.name, e.code, e.lat, e.lng, + * e.timezone, e.local_date, e.local_time FROM reservation_endpoints e JOIN + * reservations r ON r.id = e.reservation_id WHERE r.trip_id = ? ORDER BY + * e.reservation_id ASC, e.sequence ASC`. Not filtered by RV1 — the legacy + * statement loads every reservation's endpoints regardless of + * `ingest_state`, and the caller only ever looks one up for a reservation + * `share.service.ts`'s OWN filtered read already returned (parity, matching + * `CalendarService.loadEndpointsByTrip`'s identical shape, §4's CL6 note). + */ +export interface ShareEndpointRow { + reservation_id: number; + role: string; + sequence: number; + name: string; + code: string | null; + lat: number; + lng: number; + timezone: string | null; + local_date: string | null; + local_time: string | null; +} + +interface ShareEndpointsKyselyDB { + reservation_endpoints: { + reservation_id: number; role: string; sequence: number; name: string; code: string | null; + lat: number; lng: number; timezone: string | null; local_date: string | null; local_time: string | null; + }; + reservations: { id: number; trip_id: number | string }; +} + +/** + * `share.service.ts:368` (`getSharedTripData`'s day-position read) — + * `SELECT rdp.reservation_id, rdp.day_id, rdp.position FROM + * reservation_day_positions rdp JOIN reservations r ON rdp.reservation_id = + * r.id WHERE r.trip_id = ?`, no `ORDER BY`, no RV1 filter (same reasoning + * as the endpoints read above — the caller only consults `posMap` for a + * reservation its own filtered read already returned). + */ +export interface ShareDayPositionRow { + reservation_id: number; + day_id: number; + position: number; +} + +interface ShareDayPositionsKyselyDB { + reservation_day_positions: { reservation_id: number; day_id: number; position: number }; + reservations: { id: number; trip_id: number | string }; +} + +/** + * `share.service.ts:387` (`getSharedTripData`'s public booking read) — + * `SELECT ${PUBLIC_RESERVATION_COLUMNS} FROM reservations r WHERE r.trip_id + * = ? AND ORDER BY r.reservation_time ASC` — + * `PUBLIC_RESERVATION_COLUMNS` is `id, trip_id, day_id, end_day_id, + * place_id, accommodation_id, title, type, status, location, + * reservation_time, reservation_end_time, notes, url, metadata, + * created_at`, the allow-list a public link may see (never the + * confirmation number, the import trail or who is travelling). + */ +export interface SharePublicReservationRow { + id: number; + trip_id: number; + day_id: number | null; + end_day_id: number | null; + place_id: number | null; + accommodation_id: string | null; + title: string; + type: string | null; + status: string | null; + location: string | null; + reservation_time: string | null; + reservation_end_time: string | null; + notes: string | null; + url: string | null; + metadata: string | null; + created_at: string | null; +} + +interface SharePublicReservationKyselyDB { + reservations: ReservationVisibilityKyselyDB['reservations'] & { + trip_id: number | string; + day_id: number | null; end_day_id: number | null; place_id: number | null; title: string; + type: string | null; status: string | null; location: string | null; + reservation_time: string | null; reservation_end_time: string | null; + notes: string | null; url: string | null; metadata: string | null; created_at: string | null; + }; +} + +/** + * `share.service.ts:401` (`getSharedTripData`'s public stay read) — + * `SELECT ${PUBLIC_ACCOMMODATION_COLUMNS}, p.name as place_name, p.address + * as place_address, p.lat as place_lat, p.lng as place_lng FROM + * day_accommodations a JOIN places p ON a.place_id = p.id WHERE a.trip_id = + * ? AND ` — an INNER `JOIN`, not `LEFT`, unlike CL7: a stay with + * no linked place is silently excluded from a public link (parity, kept + * exactly). `PUBLIC_ACCOMMODATION_COLUMNS` is `id, trip_id, place_id, + * start_day_id, end_day_id, check_in, check_in_end, check_out, notes`. + */ +export interface SharePublicAccommodationRow { + id: number; + trip_id: number; + place_id: number | null; + start_day_id: number | null; + end_day_id: number | null; + check_in: string | null; + check_in_end: string | null; + check_out: string | null; + notes: string | null; + place_name: string | null; + place_address: string | null; + place_lat: number | null; + place_lng: number | null; +} + +interface SharePublicAccommodationKyselyDB extends ReservationVisibilityKyselyDB { + day_accommodations: { + id: number; trip_id: number | string; place_id: number | null; start_day_id: number | null; end_day_id: number | null; + check_in: string | null; check_in_end: string | null; check_out: string | null; notes: string | null; + }; + places: { id: number; name: string | null; address: string | null; lat: number | null; lng: number | null }; +} + +/** + * `public-api.service.ts::reservationsByDay` (Task 5's `// Task 2` pickup) + * — `SELECT day_id, type, title, location, reservation_time, + * reservation_end_time, status, notes FROM reservations WHERE trip_id = ? + * AND day_id IS NOT NULL ORDER BY day_id ASC, reservation_time ASC`. + */ +export interface PublicApiScheduledReservationRow { + day_id: number; + type: string | null; + title: string | null; + location: string | null; + reservation_time: string | null; + reservation_end_time: string | null; + status: string | null; + notes: string | null; +} + +/** + * `public-api.service.ts::buildUnscheduledReservations` (Task 5's `// Task + * 2` pickup) — the same 7 columns minus `day_id`, `WHERE trip_id = ? AND + * day_id IS NULL ORDER BY reservation_time ASC, id ASC`. + */ +export type PublicApiUnscheduledReservationRow = Omit; + +interface PublicApiReservationKyselyDB { + reservations: { + id: number; trip_id: number; day_id: number | null; type: string | null; title: string | null; + location: string | null; reservation_time: string | null; reservation_end_time: string | null; + status: string | null; notes: string | null; + }; +} + +/** + * `public-api.service.ts::buildAccommodations` (Task 5's `// Task 3` + * pickup) — `SELECT p.name, p.address, p.lat, p.lng, ds.date AS start_date, + * de.date AS end_date, a.check_in, a.check_out, a.notes FROM + * day_accommodations a LEFT JOIN places p ON p.id = a.place_id LEFT JOIN + * days ds ON ds.id = a.start_day_id LEFT JOIN days de ON de.id = + * a.end_day_id WHERE a.trip_id = ? ORDER BY ds.date ASC`. + */ +export interface PublicApiAccommodationRow { + name: string | null; + address: string | null; + lat: number | null; + lng: number | null; + start_date: string | null; + end_date: string | null; + check_in: string | null; + check_out: string | null; + notes: string | null; +} + +interface PublicApiAccommodationKyselyDB { + day_accommodations: { id: number; trip_id: number; place_id: number | null; start_day_id: number | null; end_day_id: number | null; check_in: string | null; check_out: string | null; notes: string | null }; + places: { id: number; name: string | null; address: string | null; lat: number | null; lng: number | null }; + days: { id: number; date: string | null }; +} + +/** + * `public-api.service.ts::buildUnplannedPlaces` (Task 5's `// Task 3` + * pickup — "the whole statement stays raw rather than splitting the + * places/day_assignments reads from the one 3d-table predicate", the + * method's own comment) — `SELECT p.name, p.address, p.lat, p.lng, + * p.place_time, p.end_time, p.duration_minutes, p.notes, p.transport_mode, + * c.name AS category FROM places p LEFT JOIN categories c ON c.id = + * p.category_id WHERE p.trip_id = ? AND NOT EXISTS (SELECT 1 FROM + * day_assignments da WHERE da.place_id = p.id) AND NOT EXISTS (SELECT 1 + * FROM day_accommodations a WHERE a.place_id = p.id) ORDER BY + * p.created_at ASC, p.id ASC`. + */ +export interface PublicApiUnplannedPlaceRow { + name: string; + address: string | null; + lat: number | null; + lng: number | null; + place_time: string | null; + end_time: string | null; + duration_minutes: number | null; + notes: string | null; + transport_mode: string | null; + category: string | null; +} + +interface PublicApiUnplannedPlaceKyselyDB { + places: { + id: number; trip_id: number; name: string; address: string | null; lat: number | null; lng: number | null; + place_time: string | null; end_time: string | null; duration_minutes: number | null; notes: string | null; + transport_mode: string | null; category_id: number | null; created_at: string | null; + }; + categories: { id: number; name: string }; + day_assignments: { id: number; place_id: number }; + day_accommodations: { id: number; place_id: number | null }; +} + export class ReservationsRepository extends TrekRepository { private joinedQuery() { return this.kysely() @@ -555,6 +971,375 @@ export class ReservationsRepository extends TrekRepository { .execute(); return rows as { id: number }[]; } + + // --------------------------------------------------------------------------- + // Plan 3d Task 4 — additive (see the file-level comment above the interfaces + // this section's methods return). + // --------------------------------------------------------------------------- + + /** CL2 — see {@link CalendarReservationRow}'s docstring. */ + async listForCalendar(trip_id: number | string): Promise { + const rows = await this.kysely() + .selectFrom('reservations as r') + .leftJoin('places as pl', 'pl.id', 'r.place_id') + .leftJoin('day_accommodations as a', 'a.id', 'r.accommodation_id') + .leftJoin('days as sd', 'sd.id', 'a.start_day_id') + .leftJoin('days as ed', 'ed.id', 'a.end_day_id') + .leftJoin('days as rd', 'rd.id', 'r.day_id') + .leftJoin('days as red', 'red.id', 'r.end_day_id') + .selectAll('r') + .select((eb) => [ + 'pl.lat as place_lat', + 'pl.lng as place_lng', + 'sd.date as stay_start_date', + 'ed.date as stay_end_date', + 'a.check_in as stay_check_in', + 'a.check_out as stay_check_out', + eb + .selectFrom('reservations as r2') + .select((eb2) => eb2.fn.min('r2.id').as('c')) + .whereRef('r2.accommodation_id', '=', 'a.id') + .as('stay_first_reservation_id'), + 'rd.date as day_date', + 'red.date as end_day_date', + ]) + .where('r.trip_id', '=', trip_id) + .where((eb) => publicReservationExpr(eb, 'r.ingest_state')) + .execute(); + return rows as CalendarReservationRow[]; + } + + /** CL4 — see {@link CalendarStopRow}'s docstring. */ + async listCalendarStops(day_id: number): Promise { + const rows = await this.kysely() + .selectFrom('day_assignments as da') + .innerJoin('places as p', 'p.id', 'da.place_id') + .selectAll('da') + .select((eb) => [ + 'p.name as place_name', + 'p.address as place_address', + 'p.lat as place_lat', + 'p.lng as place_lng', + 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.accommodation_id', 'is', null) + .orderBy('da.order_index', 'asc') + .orderBy('da.created_at', 'asc') + .execute(); + return rows as CalendarStopRow[]; + } + + /** CL7 — see {@link CalendarStayRow}'s docstring. */ + async listPublicStaysForCalendar(trip_id: number | string): Promise { + const rows = await this.kysely() + .selectFrom('day_accommodations as a') + .where('a.trip_id', '=', trip_id) + .where((eb) => publicStayExists(eb)) + .leftJoin('days as sd', 'sd.id', 'a.start_day_id') + .leftJoin('days as ed', 'ed.id', 'a.end_day_id') + .leftJoin('places as p', 'p.id', 'a.place_id') + .select((eb) => [ + 'a.id', + 'a.check_in', + 'a.check_in_end', + 'a.check_out', + 'sd.date as start_date', + 'ed.date as end_date', + 'p.name as place_name', + 'p.address as place_address', + 'p.lat as place_lat', + 'p.lng as place_lng', + eb + .selectFrom('reservations as r') + .select('r.title') + .whereRef('r.accommodation_id', '=', 'a.id') + .where((eb2) => publicReservationExpr(eb2, 'r.ingest_state')) + .orderBy('r.id', 'asc') + .limit(1) + .as('reservation_title'), + ]) + .orderBy('a.id', 'asc') + .execute(); + return rows as CalendarStayRow[]; + } + + /** RS20 — see {@link UpcomingReservationRow}'s docstring. */ + async listUpcomingForUser(user_id: number, today: string, now_hhmm: string, limit: number): Promise { + const platform = this.getEntityManager().getPlatform(); + + // The `COALESCE(place name, nameless-stay's-linked-booking title, trip + // title)` title is identical across the check-in and check-out arms + // (both name the STAY, not the moment); it is spelled out in each rather + // than factored into a shared helper, matching `Trips.repository.ts`'s + // `activeTrip` precedent for its own repeated `CASE WHEN` — a helper + // spanning two differently-joined query builders would need the same + // kind of hand-written cross-callback type `_shared/reservation-visibility + // .ts`'s own module docstring explains doesn't hold up. + const rows = await this.kysely() + .with('visible_trips', (qb) => + qb + .selectFrom('trips as t') + .leftJoin('trip_members as tm', (join) => join.onRef('tm.trip_id', '=', 't.id').on('tm.user_id', '=', user_id)) + .select(['t.id', 't.title', 't.cover_image']) + .where((eb) => + eb.and([eb.or([eb('t.user_id', '=', user_id), eb('tm.user_id', 'is not', null)]), eb('t.is_archived', '=', 0)]), + ), + ) + .with('entries', (qb) => { + const bookings = qb + .selectFrom('reservations as r') + .innerJoin('visible_trips as tr', 'tr.id', 'r.trip_id') + .leftJoin('days as d', 'd.id', 'r.day_id') + .leftJoin('places as p', 'p.id', 'r.place_id') + .select((eb) => [ + 'r.id as id', + 'r.trip_id as trip_id', + 'r.title as title', + 'r.type as type', + 'r.status as status', + 'r.location as location', + 'r.reservation_time as reservation_time', + 'r.confirmation_number as confirmation_number', + 'tr.title as trip_title', + 'tr.cover_image as trip_cover', + 'd.date as day_date', + 'p.name as place_name', + 'p.image_url as place_image', + eb + .case() + .when(startsWithIsoDateKysely(platform, eb, 'r.reservation_time')) + .then(substringKysely(platform, eb, 'r.reservation_time', 1, 10)) + .else(eb.ref('d.date')) + .end() + .as('at_date'), + eb + .case() + .when(startsWithIsoDateKysely(platform, eb, 'r.reservation_time')) + .then(substringKysely(platform, eb, 'r.reservation_time', 12)) + .else(eb.ref('r.reservation_time')) + .end() + .as('at_time'), + ]) + .where('r.status', '!=', 'cancelled') + .where((eb) => eb(eb.fn.coalesce(eb.ref('r.type'), eb.val('')), '!=', 'hotel')) + .$castTo(); + + const checkins = qb + .selectFrom('day_accommodations as a') + .innerJoin('visible_trips as tr', 'tr.id', 'a.trip_id') + .innerJoin('days as d', 'd.id', 'a.start_day_id') + .leftJoin('places as p', 'p.id', 'a.place_id') + .select((eb) => [ + 'a.id as id', + 'a.trip_id as trip_id', + eb + .fn.coalesce( + eb.ref('p.name'), + eb + .selectFrom('reservations as res') + .select('res.title') + .where((eb2) => eb2(castIntegerKysely(platform, eb2, 'res.accommodation_id'), '=', eb2.ref('a.id'))) + .where('res.status', '!=', 'cancelled') + .orderBy('res.id', 'asc') + .limit(1), + eb.ref('tr.title'), + ) + .as('title'), + eb.val('checkin').as('type'), + eb.val('confirmed').as('status'), + eb.val(null).as('location'), + eb + .case() + .when('a.check_in', 'is not', null) + .then(concatKysely(platform, eb, { column: 'd.date' }, { value: 'T' }, { column: 'a.check_in' })) + .end() + .as('reservation_time'), + 'a.confirmation as confirmation_number', + 'tr.title as trip_title', + 'tr.cover_image as trip_cover', + 'd.date as day_date', + 'p.name as place_name', + 'p.image_url as place_image', + 'd.date as at_date', + 'a.check_in as at_time', + ]) + .$castTo(); + + const checkouts = qb + .selectFrom('day_accommodations as a') + .innerJoin('visible_trips as tr', 'tr.id', 'a.trip_id') + .innerJoin('days as d', 'd.id', 'a.end_day_id') + .leftJoin('places as p', 'p.id', 'a.place_id') + .select((eb) => [ + 'a.id as id', + 'a.trip_id as trip_id', + eb + .fn.coalesce( + eb.ref('p.name'), + eb + .selectFrom('reservations as res') + .select('res.title') + .where((eb2) => eb2(castIntegerKysely(platform, eb2, 'res.accommodation_id'), '=', eb2.ref('a.id'))) + .where('res.status', '!=', 'cancelled') + .orderBy('res.id', 'asc') + .limit(1), + eb.ref('tr.title'), + ) + .as('title'), + eb.val('checkout').as('type'), + eb.val('confirmed').as('status'), + eb.val(null).as('location'), + eb + .case() + .when('a.check_out', 'is not', null) + .then(concatKysely(platform, eb, { column: 'd.date' }, { value: 'T' }, { column: 'a.check_out' })) + .end() + .as('reservation_time'), + 'a.confirmation as confirmation_number', + 'tr.title as trip_title', + 'tr.cover_image as trip_cover', + 'd.date as day_date', + 'p.name as place_name', + 'p.image_url as place_image', + 'd.date as at_date', + 'a.check_out as at_time', + ]) + .$castTo(); + + return bookings.unionAll(checkins).unionAll(checkouts); + }) + .selectFrom('entries') + .select(['id', 'trip_id', 'title', 'type', 'status', 'location', 'reservation_time', 'confirmation_number', 'trip_title', 'trip_cover', 'day_date', 'place_name', 'place_image']) + .where('at_date', 'is not', null) + .where((eb) => + eb.or([ + eb('at_date', '>', today), + eb.and([eb('at_date', '=', today), eb(eb.fn.coalesce(eb.ref('at_time'), eb.val('23:59')), '>=', now_hhmm)]), + ]), + ) + .orderBy('at_date', 'asc') + .orderBy((eb) => eb.fn.coalesce(eb.ref('at_time'), eb.val('00:00')), 'asc') + .orderBy('id', 'asc') + .limit(limit) + .execute(); + return rows as UpcomingReservationRow[]; + } + + /** `share.service.ts:251` — see {@link ShareEndpointRow}'s docstring. */ + async listEndpointsForShare(trip_id: number | string): Promise { + const rows = await this.kysely() + .selectFrom('reservation_endpoints as e') + .innerJoin('reservations as r', 'r.id', 'e.reservation_id') + .select(['e.reservation_id', 'e.role', 'e.sequence', 'e.name', 'e.code', 'e.lat', 'e.lng', 'e.timezone', 'e.local_date', 'e.local_time']) + .where('r.trip_id', '=', trip_id) + .orderBy('e.reservation_id', 'asc') + .orderBy('e.sequence', 'asc') + .execute(); + return rows as ShareEndpointRow[]; + } + + /** `share.service.ts:368` — see {@link ShareDayPositionRow}'s docstring. */ + async listDayPositionsForShare(trip_id: number | string): Promise { + const rows = await this.kysely() + .selectFrom('reservation_day_positions as rdp') + .innerJoin('reservations as r', 'r.id', 'rdp.reservation_id') + .select(['rdp.reservation_id', 'rdp.day_id', 'rdp.position']) + .where('r.trip_id', '=', trip_id) + .execute(); + return rows as ShareDayPositionRow[]; + } + + /** `share.service.ts:387` — see {@link SharePublicReservationRow}'s docstring. */ + async listPublicForShare(trip_id: number | string): Promise { + const rows = await this.kysely() + .selectFrom('reservations as r') + .select([ + 'r.id', 'r.trip_id', 'r.day_id', 'r.end_day_id', 'r.place_id', 'r.accommodation_id', + 'r.title', 'r.type', 'r.status', 'r.location', 'r.reservation_time', 'r.reservation_end_time', + 'r.notes', 'r.url', 'r.metadata', 'r.created_at', + ]) + .where('r.trip_id', '=', trip_id) + .where((eb) => publicReservationExpr(eb, 'r.ingest_state')) + .orderBy('r.reservation_time', 'asc') + .execute(); + return rows as SharePublicReservationRow[]; + } + + /** `share.service.ts:401` — see {@link SharePublicAccommodationRow}'s docstring. */ + async listPublicAccommodationsForShare(trip_id: number | string): Promise { + const rows = await this.kysely() + .selectFrom('day_accommodations as a') + .where('a.trip_id', '=', trip_id) + .where((eb) => publicStayExists(eb)) + .innerJoin('places as p', 'p.id', 'a.place_id') + .select([ + 'a.id', 'a.trip_id', 'a.place_id', 'a.start_day_id', 'a.end_day_id', + 'a.check_in', 'a.check_in_end', 'a.check_out', 'a.notes', + 'p.name as place_name', 'p.address as place_address', 'p.lat as place_lat', 'p.lng as place_lng', + ]) + .execute(); + return rows as SharePublicAccommodationRow[]; + } + + /** `public-api.service.ts::reservationsByDay` (Task 5's `// Task 2` pickup) — see {@link PublicApiScheduledReservationRow}'s docstring. */ + async listScheduledForPublicApi(trip_id: number): Promise { + const rows = await this.kysely() + .selectFrom('reservations') + .select(['day_id', 'type', 'title', 'location', 'reservation_time', 'reservation_end_time', 'status', 'notes']) + .where('trip_id', '=', trip_id) + .where('day_id', 'is not', null) + .orderBy('day_id', 'asc') + .orderBy('reservation_time', 'asc') + .execute(); + return rows as PublicApiScheduledReservationRow[]; + } + + /** `public-api.service.ts::buildUnscheduledReservations` (Task 5's `// Task 2` pickup) — see {@link PublicApiUnscheduledReservationRow}'s docstring. */ + async listUnscheduledForPublicApi(trip_id: number): Promise { + const rows = await this.kysely() + .selectFrom('reservations') + .select(['type', 'title', 'location', 'reservation_time', 'reservation_end_time', 'status', 'notes']) + .where('trip_id', '=', trip_id) + .where('day_id', 'is', null) + .orderBy('reservation_time', 'asc') + .orderBy('id', 'asc') + .execute(); + return rows as PublicApiUnscheduledReservationRow[]; + } + + /** `public-api.service.ts::buildAccommodations` (Task 5's `// Task 3` pickup) — see {@link PublicApiAccommodationRow}'s docstring. */ + async listAccommodationsForPublicApi(trip_id: number): Promise { + const rows = await this.kysely() + .selectFrom('day_accommodations as a') + .leftJoin('places as p', 'p.id', 'a.place_id') + .leftJoin('days as ds', 'ds.id', 'a.start_day_id') + .leftJoin('days as de', 'de.id', 'a.end_day_id') + .select(['p.name', 'p.address', 'p.lat', 'p.lng', 'ds.date as start_date', 'de.date as end_date', 'a.check_in', 'a.check_out', 'a.notes']) + .where('a.trip_id', '=', trip_id) + .orderBy('ds.date', 'asc') + .execute(); + return rows as PublicApiAccommodationRow[]; + } + + /** `public-api.service.ts::buildUnplannedPlaces` (Task 5's `// Task 3` pickup) — see {@link PublicApiUnplannedPlaceRow}'s docstring. */ + async listUnplannedPlacesForPublicApi(trip_id: number): Promise { + const rows = await this.kysely() + .selectFrom('places as p') + .leftJoin('categories as c', 'c.id', 'p.category_id') + .select((eb) => [ + 'p.name', 'p.address', 'p.lat', 'p.lng', 'p.place_time', 'p.end_time', + 'p.duration_minutes', 'p.notes', 'p.transport_mode', 'c.name as category', + ]) + .where('p.trip_id', '=', trip_id) + .where((eb) => eb.not(eb.exists(eb.selectFrom('day_assignments as da').select('da.id').whereRef('da.place_id', '=', 'p.id')))) + .where((eb) => eb.not(eb.exists(eb.selectFrom('day_accommodations as a').select('a.id').whereRef('a.place_id', '=', 'p.id')))) + .orderBy('p.created_at', 'asc') + .orderBy('p.id', 'asc') + .execute(); + return rows as PublicApiUnplannedPlaceRow[]; + } } /** `SELECT *` — every scalar column of `Reservations`, RS35's shape. */ diff --git a/server/src/db/repositories/_shared/reservation-visibility.ts b/server/src/db/repositories/_shared/reservation-visibility.ts index 73315b1df..99bdfe4cc 100644 --- a/server/src/db/repositories/_shared/reservation-visibility.ts +++ b/server/src/db/repositories/_shared/reservation-visibility.ts @@ -1,5 +1,5 @@ import type { Platform } from '@mikro-orm/core'; -import type { ExpressionBuilder } from 'kysely'; +import type { ExpressionBuilder, ExpressionWrapper, ReferenceExpression, SqlBool } from 'kysely'; import { coalesceParam } from '../../dialect/sql-functions'; /** @@ -7,10 +7,12 @@ import { coalesceParam } from '../../dialect/sql-functions'; * through — ONE source, typed, for `reservations`/`day_accommodations` * (Plan 3d Task 0, R3). * - * Derived from the two legacy SQL-fragment builders this file supersedes + * Derived from the two legacy SQL-fragment builders this file superseded * (`src/nest/reservations/reservation-visibility.ts`'s `publicReservationSql` - * and `publicStaySql`, kept in place — string form — until Tasks 2/4/5 - * convert their own consumers and `share`'s owner is decided, §14.6): + * and `publicStaySql` — DELETED by Plan 3d Task 4, once calendar and + * `share` were the string form's only remaining consumers and both were + * converted in the same commit: no two live implementations of an + * anonymous-surface security predicate, §18.2): * * - **RV1** `publicReservationSql(alias)` — `` COALESCE(${alias}.ingest_state, * 'live') <> 'staged' ``. A reservation is public unless an automated @@ -21,22 +23,28 @@ import { coalesceParam } from '../../dialect/sql-functions'; * `CAST(vr.accommodation_id AS INTEGER) = ${alias}.id` (`accommodation_id` * is TEXT with no FK, §18.1 — some rows read back as `"14.0"`). * - * Two shapes are exported because the two predicates need different query - * APIs: RV1 has no correlated subquery, so it renders as a typed MikroORM - * QueryBuilder filter OBJECT (`publicReservationCondition`) — spreadable - * into any `.where({...})` call at any table alias, exactly like the - * legacy fragment builder's own runtime-alias shape, no compile-time table - * binding needed (rule 23: a typed condition, not a string). RV2's - * NOT EXISTS/EXISTS pair has no MikroORM QueryBuilder expression (no - * relation exists from `day_accommodations` to `reservations` via - * `accommodation_id` — R10 keeps it that way; QB's `$exists` operator - * checks embeddable/JSON path existence, not a correlated SQL subquery), - * so it is built with Kysely's own typed `ExpressionBuilder` - * (`publicStayExists`) — `eb.exists`/`eb.not` (Kysely's `unary('exists'| - * 'not', …)` shortcuts), `eb.cast`, `eb.fn.coalesce`, all fully typed, - * never a `raw()` call or a `sql` tagged template (both banned under - * `src/db/repositories/**` by ESLint's `no-restricted-syntax`, checked - * directly against `server/eslint.config.mjs`). + * THREE shapes are exported, because RV1 alone needs two different query + * APIs and RV2 needs a third that calls back into one of them. RV1 has no + * correlated subquery, so where a caller already holds a MikroORM + * QueryBuilder it renders as a typed filter OBJECT + * (`publicReservationCondition`) — spreadable into any `.where({...})` + * call at any table alias, exactly like the legacy fragment builder's own + * runtime-alias shape, no compile-time table binding needed (rule 23: a + * typed condition, not a string). Where a caller is instead building a raw, + * multi-join statement through `this.kysely()` (CL2/CL7/`share`'s reads — + * no ORM relation exists to join those rows through the QueryBuilder at + * all), RV1 is `publicReservationExpr`, a Kysely expression-builder + * predicate. RV2's NOT EXISTS/EXISTS pair has no MikroORM QueryBuilder + * expression either (no relation exists from `day_accommodations` to + * `reservations` via `accommodation_id` — R10 keeps it that way; QB's + * `$exists` operator checks embeddable/JSON path existence, not a + * correlated SQL subquery), so it is built with Kysely's own typed + * `ExpressionBuilder` (`publicStayExists`) — `eb.exists`/`eb.not` (Kysely's + * `unary('exists'|'not', …)` shortcuts), `eb.cast`, and `publicReservationExpr` + * itself for its own inner RV1 check, all fully typed, never a `raw()` call + * or a `sql` tagged template (both banned under `src/db/repositories/**` by + * ESLint's `no-restricted-syntax`, checked directly against + * `server/eslint.config.mjs`). * * **`publicStayExists` is deliberately narrow, not maximally generic** — * `ReservationVisibilityKyselyDB` fixes the two aliases every known @@ -86,6 +94,30 @@ export function publicReservationCondition(platform: Platform, alias: string) { return { [coalesceParam(platform, `${alias}.ingest_state`, 'live')]: { $ne: 'staged' } }; } +/** + * RV1 as a Kysely predicate (L2, Plan 3d Task 0 review): the SAME + * `COALESCE(, 'live') <> 'staged'` boolean, spelled through the + * expression builder instead of a QB filter object — for the raw-SQL-shaped + * statements CL2/CL7/`share.service.ts`'s reads convert onto (no ORM + * relation joins those rows the QueryBuilder could filter through, so they + * are built with `this.kysely()`, same as `publicStayExists` below). + * `ref` is the caller's own already-aliased `ingest_state` column + * (`'r.ingest_state'`, `'vr.ingest_state'`) — generic over `` + * (unlike `publicStayExists`, this needs no correlated subquery and no + * table-set beyond whatever the caller already has in scope, so nothing + * forces a narrow, non-generic signature the way `publicStayExists`'s + * `ExpressionBuilder` does). + * `publicStayExists` below calls this rather than re-spelling the + * `COALESCE`/`<>` pair a second time — ONE implementation of RV1, not two + * (the Task 0 review's own finding, L2). + */ +export function publicReservationExpr( + eb: ExpressionBuilder, + ref: ReferenceExpression, +): ExpressionWrapper { + return eb(eb.fn.coalesce(ref, eb.val('live')), '<>', eb.val('staged')); +} + /** * `reservations`/`day_accommodations`'s REAL table names, the shape * `publicStayExists` needs. A consumer's own local Kysely `DB` interface @@ -129,8 +161,6 @@ export function publicStayExists(eb: ExpressionBuilder eb2(eb2.fn.coalesce(eb2.ref('vr.ingest_state'), eb2.val('live')), '<>', eb2.val('staged'))), - ), + eb.exists(linkedTo.where((eb2) => publicReservationExpr(eb2, 'vr.ingest_state'))), ]); } diff --git a/server/src/nest/calendar/calendar.module.ts b/server/src/nest/calendar/calendar.module.ts index 10ede6b81..5cf5bc5bf 100644 --- a/server/src/nest/calendar/calendar.module.ts +++ b/server/src/nest/calendar/calendar.module.ts @@ -1,13 +1,23 @@ import { Module } from '@nestjs/common'; +import { MikroOrmModule } from '@mikro-orm/nestjs'; import { CalendarService } from './calendar.service'; import { DatabaseModule } from '../database/database.module'; import { ReservationsModule } from '../reservations/reservations.module'; +import { Trips } from '../../db/entities/Trips.entity'; +import { Days } from '../../db/entities/Days.entity'; +import { DayNotes } from '../../db/entities/DayNotes.entity'; +import { Reservations } from '../../db/entities/Reservations.entity'; /** Calendar export. Imported by trips (the download route) and feeds (the * subscribable URLs); it pulls in neither, which is what lets FeedsModule stop - * importing the whole trips aggregate for an ICS string. */ + * importing the whole trips aggregate for an ICS string. + * + * `MikroOrmModule.forFeature([...])` registers the repositories CL1/CL3/CL5's + * reads (`TripsRepository`/`DaysRepository`/`DayNotesRepository`, all owned + * elsewhere already) and CL2/CL4/CL7's own additive methods on + * `ReservationsRepository` (Plan 3d Task 4) resolve through. */ @Module({ - imports: [DatabaseModule, ReservationsModule], + imports: [DatabaseModule, ReservationsModule, MikroOrmModule.forFeature([Trips, Days, DayNotes, Reservations])], providers: [CalendarService], exports: [CalendarService], }) diff --git a/server/src/nest/calendar/calendar.service.ts b/server/src/nest/calendar/calendar.service.ts index a3071b8b1..df88546f5 100644 --- a/server/src/nest/calendar/calendar.service.ts +++ b/server/src/nest/calendar/calendar.service.ts @@ -1,10 +1,17 @@ import { Injectable } from '@nestjs/common'; -import { DatabaseService } from '../database/database.service'; +import { InjectRepository } from '@mikro-orm/nestjs'; import { ReservationsService } from '../reservations/reservations.service'; -import { publicReservationSql, publicStaySql } from '../reservations/reservation-visibility'; import { addDays } from '../days/days.service'; import { resolveTimeZone } from '../common/timezoneService'; import { NotFoundError } from '../common/domain-errors'; +import { Trips } from '../../db/entities/Trips.entity'; +import type { TripsRepository } from '../../db/repositories/Trips.repository'; +import { Days } from '../../db/entities/Days.entity'; +import type { DaysRepository } from '../../db/repositories/Days.repository'; +import { DayNotes } from '../../db/entities/DayNotes.entity'; +import type { DayNotesRepository } from '../../db/repositories/DayNotes.repository'; +import { Reservations } from '../../db/entities/Reservations.entity'; +import type { ReservationsRepository } from '../../db/repositories/Reservations.repository'; /** The VCALENDAR preamble every TREK calendar starts with, single-trip or merged. */ export const CALENDAR_HEADER = @@ -124,14 +131,13 @@ function buildVTimezone(zone: string, yyyymmdd: string): string { @Injectable() export class CalendarService { constructor( - private readonly dbs: DatabaseService, private readonly reservations: ReservationsService, + @InjectRepository(Trips) private readonly tripsRepo: TripsRepository, + @InjectRepository(Days) private readonly daysRepo: DaysRepository, + @InjectRepository(DayNotes) private readonly dayNotesRepo: DayNotesRepository, + @InjectRepository(Reservations) private readonly reservationsRepo: ReservationsRepository, ) {} - private get db() { - return this.dbs.connection; - } - // ── ICS export ──────────────────────────────────────────────────────────── /** @@ -145,31 +151,14 @@ export class CalendarService { * "END:VEVENT". Handing out the parts removes the need to reassemble them. */ async buildTripCalendar(tripId: string | number): Promise { - const trip = this.db.prepare('SELECT * FROM trips WHERE id = ?').get(tripId) as any; + const trip = await this.tripsRepo.findRaw(tripId); if (!trip) throw new NotFoundError('Trip not found'); // A hotel keeps its dates on the linked stay, not on the reservation: the // booking form writes reservation_time = NULL for type 'hotel' and lets // day_accommodations carry start day, end day and the check-in/out clock. // Joining them here is what lets a stay span its whole range (#1586). - const reservations = this.db - .prepare( - `SELECT r.*, pl.lat AS place_lat, pl.lng AS place_lng, - sd.date AS stay_start_date, ed.date AS stay_end_date, - a.check_in AS stay_check_in, a.check_out AS stay_check_out, - (SELECT MIN(r2.id) FROM reservations r2 - WHERE r2.accommodation_id = a.id) AS stay_first_reservation_id, - rd.date AS day_date, red.date AS end_day_date - FROM reservations r - LEFT JOIN places pl ON r.place_id = pl.id - LEFT JOIN day_accommodations a ON r.accommodation_id = a.id - LEFT JOIN days sd ON a.start_day_id = sd.id - LEFT JOIN days ed ON a.end_day_id = ed.id - LEFT JOIN days rd ON r.day_id = rd.id - LEFT JOIN days red ON r.end_day_id = red.id - WHERE r.trip_id = ? AND ${publicReservationSql('r')}`, - ) - .all(tripId) as any[]; + const reservations = await this.reservationsRepo.listForCalendar(trip.id); const esc = (s: string) => s .replaceAll(/\\/g, '\\\\') @@ -237,7 +226,7 @@ export class CalendarService { } // Days with assignments and notes - const days = this.db.prepare('SELECT * FROM days WHERE trip_id = ? ORDER BY day_number ASC').all(tripId) as any[]; + const days = await this.daysRepo.listByTrip(trip.id); for (const day of days) { if (!day.date) continue; @@ -246,21 +235,9 @@ export class CalendarService { // 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 = this.db.prepare(` - 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 - `).all(day.id) as any[]; + const assignments = await this.reservationsRepo.listCalendarStops(day.id); - const notes = this.db.prepare( - 'SELECT * FROM day_notes WHERE day_id = ? ORDER BY sort_order ASC, created_at ASC' - ).all(day.id) as any[]; + const notes = await this.dayNotesRepo.listByDayIds([day.id]); const timed = assignments.filter(a => a.effective_time); const untimed = assignments.filter(a => !a.effective_time); @@ -601,20 +578,7 @@ export class CalendarService { // keyed by the stay, that emitted the same UID twice and clients then pick one // of the duplicates at random (#1869). Lowest id wins so the title is stable // across exports. - const stays = this.db.prepare(` - SELECT a.id, a.check_in, a.check_in_end, a.check_out, - sd.date AS start_date, ed.date AS end_date, - p.name AS place_name, p.address AS place_address, p.lat AS place_lat, p.lng AS place_lng, - (SELECT r.title FROM reservations r - WHERE r.accommodation_id = a.id AND ${publicReservationSql('r')} - ORDER BY r.id ASC LIMIT 1) AS reservation_title - FROM day_accommodations a - LEFT JOIN days sd ON a.start_day_id = sd.id - LEFT JOIN days ed ON a.end_day_id = ed.id - LEFT JOIN places p ON a.place_id = p.id - WHERE a.trip_id = ? AND ${publicStaySql('a')} - ORDER BY a.id ASC - `).all(tripId) as any[]; + const stays = await this.reservationsRepo.listPublicStaysForCalendar(trip.id); for (const stay of stays) { const name = stay.reservation_title || stay.place_name || 'Accommodation'; diff --git a/server/src/nest/public-api/public-api.module.ts b/server/src/nest/public-api/public-api.module.ts index e4a9f99f5..44cf26a6e 100644 --- a/server/src/nest/public-api/public-api.module.ts +++ b/server/src/nest/public-api/public-api.module.ts @@ -7,6 +7,7 @@ import { TokensModule } from '../tokens/tokens.module'; import { TripMembershipModule } from '../trip-membership/trip-membership.module'; import { RateLimitModule } from '../common/rate-limit.module'; import { Trips } from '../../db/entities/Trips.entity'; +import { Reservations } from '../../db/entities/Reservations.entity'; /** * Public API v1 — the versioned read-only surface for third-party integrations. @@ -26,12 +27,15 @@ import { Trips } from '../../db/entities/Trips.entity'; * class itself. No module edge either way — which is the point. */ @Module({ - // `MikroOrmModule.forFeature([Trips])` registers `TripsRepository` for - // `PublicApiService`'s `@InjectRepository` constructor param (Plan 3d Task 5 - // — its trip reads convert onto the same repository `TripsModule` owns, - // without pulling `TripsModule` itself in, per this module's leaf-module - // constraint above). - imports: [TokensModule, TripMembershipModule, RateLimitModule, MikroOrmModule.forFeature([Trips])], + // `MikroOrmModule.forFeature([Trips, Reservations])` registers + // `TripsRepository` (Plan 3d Task 5 — its trip reads convert onto the same + // repository `TripsModule` owns) and `ReservationsRepository` (Plan 3d + // Task 4's pickup — `reservationsByDay`/`buildAccommodations`/ + // `buildUnplannedPlaces`/`buildUnscheduledReservations`) for + // `PublicApiService`'s `@InjectRepository` constructor params, without + // pulling `TripsModule`/`ReservationsModule` themselves in, per this + // module's leaf-module constraint above. + imports: [TokensModule, TripMembershipModule, RateLimitModule, MikroOrmModule.forFeature([Trips, Reservations])], controllers: [PublicApiController], providers: [PublicApiService, ApiTokenGuard], }) diff --git a/server/src/nest/public-api/public-api.service.ts b/server/src/nest/public-api/public-api.service.ts index 270a701c1..ed180da58 100644 --- a/server/src/nest/public-api/public-api.service.ts +++ b/server/src/nest/public-api/public-api.service.ts @@ -14,6 +14,8 @@ import type { import { InjectRepository } from '@mikro-orm/nestjs'; import { Trips } from '../../db/entities/Trips.entity'; import type { TripsRepository } from '../../db/repositories/Trips.repository'; +import { Reservations } from '../../db/entities/Reservations.entity'; +import type { ReservationsRepository } from '../../db/repositories/Reservations.repository'; import { DatabaseService } from '../database/database.service'; import { TripMembershipService } from '../trip-membership/trip-membership.service'; @@ -42,6 +44,7 @@ export class PublicApiService { private readonly db: DatabaseService, private readonly membership: TripMembershipService, @InjectRepository(Trips) private readonly tripsRepo: TripsRepository, + @InjectRepository(Reservations) private readonly reservationsRepo: ReservationsRepository, ) {} /** Every trip the token's owner may read, newest first, without itineraries. */ @@ -175,17 +178,8 @@ export class PublicApiService { * way to tell that from three flights. */ private async reservationsByDay(tripId: number): Promise> { - // Task 2 — ReservationsRepository owns this table; Task 2 had not landed - // when this task ran (Plan 3d Task 5), so this statement stays raw. - const rows = this.db.all( - `SELECT day_id, type, title, location, reservation_time, reservation_end_time, - status, notes - FROM reservations - WHERE trip_id = ? AND day_id IS NOT NULL - ORDER BY day_id ASC, reservation_time ASC`, - tripId, - ); - return groupBy(rows, (r: ReservationRow) => r.day_id, toReservation); + const rows = await this.reservationsRepo.listScheduledForPublicApi(tripId); + return groupBy(rows as ReservationRow[], (r: ReservationRow) => r.day_id, toReservation); } /** @@ -195,20 +189,7 @@ export class PublicApiService { * TREK day id, and the dates are what it actually needs to match its own nights. */ private async buildAccommodations(tripId: number): Promise { - // Task 3 — DayAccommodationsRepository owns this table; Task 3 had not - // landed when this task ran (Plan 3d Task 5), so this statement stays raw. - const rows = this.db.all( - `SELECT p.name, p.address, p.lat, p.lng, - ds.date AS start_date, de.date AS end_date, - a.check_in, a.check_out, a.notes - FROM day_accommodations a - LEFT JOIN places p ON p.id = a.place_id - LEFT JOIN days ds ON ds.id = a.start_day_id - LEFT JOIN days de ON de.id = a.end_day_id - WHERE a.trip_id = ? - ORDER BY ds.date ASC`, - tripId, - ); + const rows = await this.reservationsRepo.listAccommodationsForPublicApi(tripId); return rows.map((r) => ({ name: r.name ?? null, address: r.address ?? null, @@ -239,24 +220,7 @@ export class PublicApiService { * `accommodations` — listing it twice would read as two different intentions. */ private async buildUnplannedPlaces(tripId: number): Promise { - // Task 3 — the `NOT EXISTS (... day_accommodations ...)` arm reaches a - // table DayAccommodationsRepository owns; Task 3 had not landed when this - // task ran (Plan 3d Task 5). The whole statement stays raw rather than - // splitting the `places`/`day_assignments` reads from the one 3d-table - // predicate — `places`/`day_assignments` are Plan 3c, DONE, but a single - // SQL statement cannot be partially converted. - const rows = this.db.all>( - `SELECT p.name, p.address, p.lat, p.lng, p.place_time, p.end_time, - p.duration_minutes, p.notes, p.transport_mode, - c.name AS category - FROM places p - LEFT JOIN categories c ON c.id = p.category_id - WHERE p.trip_id = ? - AND NOT EXISTS (SELECT 1 FROM day_assignments da WHERE da.place_id = p.id) - AND NOT EXISTS (SELECT 1 FROM day_accommodations a WHERE a.place_id = p.id) - ORDER BY p.created_at ASC, p.id ASC`, - tripId, - ); + const rows = await this.reservationsRepo.listUnplannedPlacesForPublicApi(tripId); return rows.map(toPlace); } @@ -266,16 +230,7 @@ export class PublicApiService { * pinned to. Reporting only day-bound bookings would quietly lose those. */ private async buildUnscheduledReservations(tripId: number): Promise { - // Task 2 — ReservationsRepository owns this table; Task 2 had not landed - // when this task ran (Plan 3d Task 5), so this statement stays raw. - const rows = this.db.all>( - `SELECT type, title, location, reservation_time, reservation_end_time, - status, notes - FROM reservations - WHERE trip_id = ? AND day_id IS NULL - ORDER BY reservation_time ASC, id ASC`, - tripId, - ); + const rows = await this.reservationsRepo.listUnscheduledForPublicApi(tripId); return rows.map(toReservation); } diff --git a/server/src/nest/reservations/reservation-visibility.ts b/server/src/nest/reservations/reservation-visibility.ts deleted file mode 100644 index 7853a2686..000000000 --- a/server/src/nest/reservations/reservation-visibility.ts +++ /dev/null @@ -1,34 +0,0 @@ -/** - * Which reservation rows a viewer without a TREK session may see. - * - * `ingest_state` is 'live' for everything a person put on a trip, and 'staged' - * for a row an automated ingest parked for review. Only the two anonymous - * exports filter on it. The authenticated lists deliberately do not: the - * staging inbox has to see its own rows, or nobody can confirm them. - * - * These are SQL fragments rather than a service. calendar/ and share/ are - * separate modules and neither should have to import the other for a WHERE - * clause. They take the table alias because share.service.ts queries - * `reservations` without one while calendar.service.ts uses `r`. - * - * The COALESCE guards a NULL in the column, not a missing column: a table - * without `ingest_state` fails the query outright rather than returning NULL. - * The migration adds the column NOT NULL DEFAULT 'live', so a NULL should not - * occur; the COALESCE costs nothing and keeps a row that predates a botched - * ALTER visible rather than silently dropping it out of a live calendar feed. - */ -export const publicReservationSql = (alias = 'r'): string => `COALESCE(${alias}.ingest_state, 'live') <> 'staged'`; - -/** - * A stay is public when nothing points at it (somebody added it by hand), or - * when a live booking does. Both halves matter: a plain EXISTS would drop every - * hand-added hotel out of trips that are shared today. - * - * `accommodation_id` is a TEXT column that reads back as "14.0", hence the - * CAST. Same coercion as reservations.service.ts. - */ -export const publicStaySql = (alias = 'a'): string => `( - NOT EXISTS (SELECT 1 FROM reservations vr WHERE CAST(vr.accommodation_id AS INTEGER) = ${alias}.id) - OR EXISTS (SELECT 1 FROM reservations vr WHERE CAST(vr.accommodation_id AS INTEGER) = ${alias}.id - AND ${publicReservationSql('vr')}) - )`; diff --git a/server/src/nest/reservations/reservations.service.ts b/server/src/nest/reservations/reservations.service.ts index b16b1d58d..6980cfa38 100644 --- a/server/src/nest/reservations/reservations.service.ts +++ b/server/src/nest/reservations/reservations.service.ts @@ -132,16 +132,11 @@ export interface UpdateReservationData { needs_review?: boolean; } -/** - * True when `reservation_time` actually carries a date and not just a bare - * `HH:MM`. Both shapes reach this column — the booking form writes the - * datetime-local `YYYY-MM-DDTHH:MM`, while day-anchored rows (the demo seed - * among them) hold only a time — and comparing the two against an ISO - * timestamp as strings silently sorts `'20:00'` above `'2026-…'` while - * dropping `'08:30'` below it, which is how half a trip's bookings vanished - * from the dashboard widget (#1934). - */ -const DATED = "r.reservation_time GLOB '[0-9][0-9][0-9][0-9]-[0-9][0-9]-[0-9][0-9]*'"; +// The "does reservation_time actually carry a date, not just a bare HH:MM" +// question (#1934) that RS20's legacy `DATED` constant used to answer here +// now lives as `startsWithIsoDateKysely` inside +// `ReservationsRepository.listUpcomingForUser` (Plan 3d Task 4) — RS20 was +// this constant's only caller. type AccommodationTimesMeta = { check_in_time?: string | null; @@ -478,89 +473,14 @@ export class ReservationsService { * fields hotels do not use. * * RS20 — Task 4 (calendar + listUpcoming + the visibility predicate - * consumers). Stays raw on `DatabaseService` in this task per the plan's - * own split; the tier ruling (Kysely full statement vs. an explicit - * exception vs. a JS-merge decomposition) is Task 4's, per Task 0's - * concern #4. + * consumers). Ruling (ii): ONE fully typed Kysely statement, in + * `ReservationsRepository.listUpcomingForUser` — see that method's own + * docstring for the CTE/`UNION ALL`/GLOB/CAST shape. */ async listUpcoming(userId: number, limit = 6) { const today = new Date().toISOString().slice(0, 10); const now = new Date().toISOString(); - - const reservations = this.db.all>(` - WITH visible_trips AS ( - SELECT t.id, t.title, t.cover_image - FROM trips t - LEFT JOIN trip_members tm ON tm.trip_id = t.id AND tm.user_id = ? - WHERE (t.user_id = ? OR tm.user_id IS NOT NULL) AND t.is_archived = 0 - ), - -- One row per candidate with its date and time already separated, so the - -- filter and the sort below never have to guess what reservation_time holds. - entries AS ( - SELECT r.id, r.trip_id, r.title, r.type, r.status, r.location, - r.reservation_time, r.confirmation_number, - tr.title as trip_title, tr.cover_image as trip_cover, - d.date as day_date, p.name as place_name, p.image_url as place_image, - CASE WHEN ${DATED} THEN substr(r.reservation_time, 1, 10) ELSE d.date END as at_date, - CASE WHEN ${DATED} THEN substr(r.reservation_time, 12) ELSE r.reservation_time END as at_time - FROM reservations r - JOIN visible_trips tr ON tr.id = r.trip_id - LEFT JOIN days d ON r.day_id = d.id - LEFT JOIN places p ON r.place_id = p.id - WHERE r.status != 'cancelled' - AND COALESCE(r.type, '') != 'hotel' - - UNION ALL - - -- Check-in and check-out as their own moments (#1934). The stay itself is - -- still left out: it covers a range, and a week in one hotel would hold a - -- slot in a six-entry widget for the whole week. Arriving and leaving are - -- points in time, which is exactly what this widget is for, and they are - -- the two the traveller needs reminding of. - SELECT a.id, a.trip_id, - COALESCE(p.name, (SELECT res.title FROM reservations res - WHERE CAST(res.accommodation_id AS INTEGER) = a.id - AND res.status != 'cancelled' - ORDER BY res.id LIMIT 1), tr.title) as title, - 'checkin' as type, 'confirmed' as status, NULL as location, - CASE WHEN a.check_in IS NOT NULL THEN d.date || 'T' || a.check_in END as reservation_time, - a.confirmation as confirmation_number, - tr.title as trip_title, tr.cover_image as trip_cover, - d.date as day_date, p.name as place_name, p.image_url as place_image, - d.date as at_date, a.check_in as at_time - FROM day_accommodations a - JOIN visible_trips tr ON tr.id = a.trip_id - JOIN days d ON d.id = a.start_day_id - LEFT JOIN places p ON p.id = a.place_id - - UNION ALL - - SELECT a.id, a.trip_id, - COALESCE(p.name, (SELECT res.title FROM reservations res - WHERE CAST(res.accommodation_id AS INTEGER) = a.id - AND res.status != 'cancelled' - ORDER BY res.id LIMIT 1), tr.title) as title, - 'checkout' as type, 'confirmed' as status, NULL as location, - CASE WHEN a.check_out IS NOT NULL THEN d.date || 'T' || a.check_out END as reservation_time, - a.confirmation as confirmation_number, - tr.title as trip_title, tr.cover_image as trip_cover, - d.date as day_date, p.name as place_name, p.image_url as place_image, - d.date as at_date, a.check_out as at_time - FROM day_accommodations a - JOIN visible_trips tr ON tr.id = a.trip_id - JOIN days d ON d.id = a.end_day_id - LEFT JOIN places p ON p.id = a.place_id - ) - SELECT id, trip_id, title, type, status, location, reservation_time, - confirmation_number, trip_title, trip_cover, day_date, place_name, place_image - FROM entries - WHERE at_date IS NOT NULL - AND (at_date > ? OR (at_date = ? AND COALESCE(at_time, '23:59') >= ?)) - ORDER BY at_date ASC, COALESCE(at_time, '00:00') ASC, id ASC - LIMIT ? - `, userId, userId, today, today, now.slice(11, 16), limit); - - return reservations; + return await this.reservationsRepo.listUpcomingForUser(userId, today, now.slice(11, 16), limit); } async getReservationWithJoins(id: string | number) { diff --git a/server/src/nest/share/share.module.ts b/server/src/nest/share/share.module.ts index dbd991d10..106676590 100644 --- a/server/src/nest/share/share.module.ts +++ b/server/src/nest/share/share.module.ts @@ -1,4 +1,5 @@ import { Module } from '@nestjs/common'; +import { MikroOrmModule } from '@mikro-orm/nestjs'; import { TripShareController, SharedController } from './share.controller'; import { ShareService } from './share.service'; import { ShareMcp } from './share.mcp'; @@ -9,9 +10,17 @@ import { PlacePhotosModule } from '../place-photos/place-photos.module'; import { StorageModule } from '../storage/storage.module'; import { AuthModule } from '../auth/auth.module'; import { McpSharedModule } from '../mcp-shared/mcp-shared.module'; +import { Reservations } from '../../db/entities/Reservations.entity'; +// `MikroOrmModule.forFeature([Reservations])` registers `ReservationsRepository` +// for `ShareService`'s `@InjectRepository` constructor param (Plan 3d Task 4 — +// its four reads on `reservations`/`reservation_endpoints`/ +// `reservation_day_positions`/`day_accommodations` convert onto the same +// repository `ReservationsModule` owns, without importing that module itself: +// this file stays a leaf the way `PublicApiModule`'s own `Trips`-only +// `forFeature` already does for the same reason). @Module({ - imports: [McpSharedModule, SettingsModule, PermissionsModule, QueryHelpersModule, AuthModule, PlacePhotosModule, StorageModule], + imports: [McpSharedModule, SettingsModule, PermissionsModule, QueryHelpersModule, AuthModule, PlacePhotosModule, StorageModule, MikroOrmModule.forFeature([Reservations])], controllers: [TripShareController, SharedController], providers: [ShareService, ShareMcp], }) diff --git a/server/src/nest/share/share.service.ts b/server/src/nest/share/share.service.ts index ccea7129a..68d9d7721 100644 --- a/server/src/nest/share/share.service.ts +++ b/server/src/nest/share/share.service.ts @@ -1,14 +1,16 @@ import { Injectable } from '@nestjs/common'; import crypto from 'crypto'; +import { InjectRepository } from '@mikro-orm/nestjs'; import { DatabaseService } from '../database/database.service'; import type { TripAccess } from '../database/database.service'; import { PermissionsService } from '../permissions/permissions.service'; import { QueryHelpersService } from '../query-helpers/query-helpers.service'; import { PlacePhotoCacheService } from '../place-photos/place-photo-cache.service'; -import { publicReservationSql, publicStaySql } from '../reservations/reservation-visibility'; import { SettingsService } from '../settings/settings.service'; import { UnitOfWork } from '../database/unit-of-work'; import type { User } from '../../types'; +import { Reservations } from '../../db/entities/Reservations.entity'; +import type { ReservationsRepository } from '../../db/repositories/Reservations.repository'; type Trip = TripAccess; @@ -66,20 +68,12 @@ const PUBLIC_PLACE_COLUMNS = [ 'transport_mode', 'created_at', 'updated_at', ].map(c => `p.${c}`).join(', '); -/** - * What a booking shows the public: when, where, what kind, and the note and - * link the owner attached to it. Never the confirmation number, never the - * import trail, never who is travelling on it. - */ -const PUBLIC_RESERVATION_COLUMNS = [ - 'id', 'trip_id', 'day_id', 'end_day_id', 'place_id', 'accommodation_id', 'title', 'type', 'status', 'location', - 'reservation_time', 'reservation_end_time', 'notes', 'url', 'metadata', 'created_at', -].map(c => `r.${c}`).join(', '); - -/** A stay: which place, which nights, when the desk opens. Not the confirmation. */ -const PUBLIC_ACCOMMODATION_COLUMNS = [ - 'id', 'trip_id', 'place_id', 'start_day_id', 'end_day_id', 'check_in', 'check_in_end', 'check_out', 'notes', -].map(c => `a.${c}`).join(', '); +// What a booking/stay shows the public — never the confirmation number, the +// import trail, or who is travelling — now lives as the exact column list in +// `ReservationsRepository.listPublicForShare`/`listPublicAccommodationsForShare`'s +// own docstrings (Plan 3d Task 4): this file no longer spells the SQL, so +// keeping a second copy of the allow-list here would drift from the one the +// query actually runs. /** * The parts of a booking's metadata that describe the journey rather than the @@ -157,6 +151,7 @@ export class ShareService { private readonly queryHelpers: QueryHelpersService, private readonly photoCache: PlacePhotoCacheService, private readonly uow: UnitOfWork, + @InjectRepository(Reservations) private readonly reservationsRepo: ReservationsRepository, ) {} async verifyTripAccess(tripId: string, userId: number) { @@ -246,12 +241,7 @@ export class ShareService { * to the database. */ private async publicEndpointsByReservation(tripId: number): Promise>>> { - const rows = this.dbs.all<{ reservation_id: number } & Record>( - `SELECT e.reservation_id, e.role, e.sequence, e.name, e.code, e.lat, e.lng, e.timezone, e.local_date, e.local_time - FROM reservation_endpoints e JOIN reservations r ON r.id = e.reservation_id - WHERE r.trip_id = ? ORDER BY e.reservation_id ASC, e.sequence ASC`, - tripId, - ); + const rows = await this.reservationsRepo.listEndpointsForShare(tripId); const out = new Map>>(); for (const { reservation_id, ...endpoint } of rows) { if (!out.has(reservation_id)) out.set(reservation_id, []); @@ -363,12 +353,7 @@ export class ShareService { let reservations: any[] = []; let accommodations: unknown[] = []; if (permissions.share_bookings) { - const dayPositions = this.dbs.all<{ reservation_id: number; day_id: number; position: number }>(` - SELECT rdp.reservation_id, rdp.day_id, rdp.position - FROM reservation_day_positions rdp - JOIN reservations r ON rdp.reservation_id = r.id - WHERE r.trip_id = ? - `, tripId); + const dayPositions = await this.reservationsRepo.listDayPositionsForShare(tripId); const posMap = new Map>(); for (const dp of dayPositions) { @@ -383,10 +368,7 @@ export class ShareService { // link shows what the booking is, not what it would take to change it // (#2320). The endpoints ride along, since the map draws from them. const endpoints = await this.publicEndpointsByReservation(tripId); - reservations = this.dbs.all( - `SELECT ${PUBLIC_RESERVATION_COLUMNS} FROM reservations r - WHERE r.trip_id = ? AND ${publicReservationSql('r')} - ORDER BY r.reservation_time ASC`, tripId) + reservations = (await this.reservationsRepo.listPublicForShare(tripId)) .map((r) => ({ ...r, url: publicHttpUrl(r.url), @@ -395,12 +377,7 @@ export class ShareService { day_positions: posMap.get(r.id) ?? null, })); - accommodations = this.dbs.all(` - SELECT ${PUBLIC_ACCOMMODATION_COLUMNS}, - p.name as place_name, p.address as place_address, p.lat as place_lat, p.lng as place_lng - FROM day_accommodations a JOIN places p ON a.place_id = p.id - WHERE a.trip_id = ? AND ${publicStaySql('a')} - `, tripId); + accommodations = await this.reservationsRepo.listPublicAccommodationsForShare(tripId); } // Packing — a public viewer is neither owner nor recipient, so only Common items diff --git a/server/tests/helpers/mcp-test-controllers.ts b/server/tests/helpers/mcp-test-controllers.ts index 0545ba348..d99131490 100644 --- a/server/tests/helpers/mcp-test-controllers.ts +++ b/server/tests/helpers/mcp-test-controllers.ts @@ -265,7 +265,11 @@ export async function createMcpTestRegistry(): Promise { packingService, reservationsService, collabService, placesService, todoService, new FilesService(dbService, permissionsService, realtimeService, new EphemeralTokenService(), generalStorage, mcpOrm.em), ); - const calendarService = new CalendarService(dbService, reservationsService); + const calendarService = new CalendarService( + reservationsService, + await createTestTripsRepo(dbService.connection), await createTestDaysRepo(dbService.connection), + await createTestDayNotesRepo(dbService.connection), await createTestReservationsRepo(dbService.connection), + ); // The nine addon-gated surfaces read their toggle off an injected service now // rather than off addons.bridge's own instance, so the harness has to supply // one — against the same test DB, which is what makes the `when:` gates @@ -316,7 +320,7 @@ export async function createMcpTestRegistry(): Promise { new SchoolHolidaysMcp(new SchoolHolidaysService(dbService, await createTestUnitOfWork(dbService.connection)), guards), new TripsMcp(tripsService, todoService, collabService, authService, calendarService, membersService, readModelService, addonsService, guards), new TripPromptsMcp(tripsService, readModelService, packingService, addonsService), - new ShareMcp(new ShareService(dbService, new SettingsService(await createTestUnitOfWork(dbService.connection), appSettings, await createTestSettingsRepo(dbService.connection)), permissionsService, queryHelpersService, placePhotoCache, await createTestUnitOfWork(dbService.connection)), authService, guards), + new ShareMcp(new ShareService(dbService, new SettingsService(await createTestUnitOfWork(dbService.connection), appSettings, await createTestSettingsRepo(dbService.connection)), permissionsService, queryHelpersService, placePhotoCache, await createTestUnitOfWork(dbService.connection), await createTestReservationsRepo(dbService.connection)), authService, guards), new FeedsMcp(new FeedsService(await createTestTripsRepo(dbService.connection), usersRepo, calendarService), dbService, new RuntimeEnvService(), guards), new TripInviteMcp(new TripInviteService(dbService, permissionsService, new TripMembershipService(await createTestTripsRepo(dbService.connection), await createTestTripMembersRepo(dbService.connection)), await createTestUnitOfWork(dbService.connection)), dbService, new RuntimeEnvService(), guards, new AuditService(auditLogRepo, usersRepo)), new MapsMcp(mapsService), diff --git a/server/tests/unit/db/repositories/Reservations.repository.test.ts b/server/tests/unit/db/repositories/Reservations.repository.test.ts new file mode 100644 index 000000000..22d4dd138 --- /dev/null +++ b/server/tests/unit/db/repositories/Reservations.repository.test.ts @@ -0,0 +1,386 @@ +/** + * Plan 3d Task 4 — the proof for CL1/CL2/CL4/CL7 (calendar) and RS20 + * (`listUpcoming`): full-key `toEqual()` parity between + * `ReservationsRepository`'s new methods and the exact legacy SQL they + * replace (the legacy text is this file's own oracle — the production + * copies were deleted the same commit these methods were added, per R3 + * "one source"; see `_shared/reservation-visibility.test.ts`'s identical + * reasoning for keeping its own oracle text local). + */ +import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest'; +import { createSnapshotTestDb } from '../../../helpers/db-mock'; +import { resetTestDb } from '../../../helpers/test-db'; +import { + createDay, createDayAccommodation, createPlace, createReservation, createTrip, createUser, addTripMember, +} from '../../../helpers/factories'; +import { + createTestReservationsRepo, createTestTripsRepo, createTestDaysRepo, createTestDayNotesRepo, +} from '../../../helpers/test-uow'; +import type { ReservationsRepository } from '../../../../src/db/repositories/Reservations.repository'; +import type { TripsRepository } from '../../../../src/db/repositories/Trips.repository'; +import type { DaysRepository } from '../../../../src/db/repositories/Days.repository'; +import type { DayNotesRepository } from '../../../../src/db/repositories/DayNotes.repository'; + +const testDb = createSnapshotTestDb(); +let reservationsRepo: ReservationsRepository; +let tripsRepo: TripsRepository; +let daysRepo: DaysRepository; +let dayNotesRepo: DayNotesRepository; + +beforeAll(async () => { + reservationsRepo = await createTestReservationsRepo(testDb); + tripsRepo = await createTestTripsRepo(testDb); + daysRepo = await createTestDaysRepo(testDb); + dayNotesRepo = await createTestDayNotesRepo(testDb); +}); +beforeEach(() => resetTestDb(testDb)); +afterAll(() => testDb.close()); + +// The RV1 fragment CL2/CL7's legacy statements interpolated inline — kept +// here, not imported, for the same "no production copy left to import from" +// reason `_shared/reservation-visibility.test.ts` gives. +const publicReservationSql = (alias = 'r'): string => `COALESCE(${alias}.ingest_state, 'live') <> 'staged'`; +const publicStaySql = (alias = 'a'): string => `( + NOT EXISTS (SELECT 1 FROM reservations vr WHERE CAST(vr.accommodation_id AS INTEGER) = ${alias}.id) + OR EXISTS (SELECT 1 FROM reservations vr WHERE CAST(vr.accommodation_id AS INTEGER) = ${alias}.id + AND ${publicReservationSql('vr')}) + )`; + +describe('ReservationsRepository — CL1 (findRaw, existing method reused by calendar)', () => { + it('CAL-CL1-001: matches SELECT * FROM trips WHERE id = ?', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id, { title: 'Alps' }); + + const typed = await tripsRepo.findRaw(trip.id); + const legacy = testDb.prepare('SELECT * FROM trips WHERE id = ?').get(trip.id); + + expect(typed).toEqual(legacy); + }); +}); + +describe('ReservationsRepository — CL2 (listForCalendar)', () => { + it('CAL-CL2-001: matches the legacy joined statement, live bookings only, staged excluded', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id, { start_date: '2026-06-01', end_date: '2026-06-05' }); + const place = createPlace(testDb, trip.id); + const live = createReservation(testDb, trip.id, { title: 'Live' }); + testDb.prepare('UPDATE reservations SET place_id = ? WHERE id = ?').run(place.id, live.id); + const staged = createReservation(testDb, trip.id, { title: 'Staged' }); + testDb.prepare("UPDATE reservations SET ingest_state = 'staged' WHERE id = ?").run(staged.id); + + const typed = await reservationsRepo.listForCalendar(trip.id); + const legacy = testDb + .prepare( + `SELECT r.*, pl.lat AS place_lat, pl.lng AS place_lng, + sd.date AS stay_start_date, ed.date AS stay_end_date, + a.check_in AS stay_check_in, a.check_out AS stay_check_out, + (SELECT MIN(r2.id) FROM reservations r2 + WHERE r2.accommodation_id = a.id) AS stay_first_reservation_id, + rd.date AS day_date, red.date AS end_day_date + FROM reservations r + LEFT JOIN places pl ON r.place_id = pl.id + LEFT JOIN day_accommodations a ON r.accommodation_id = a.id + LEFT JOIN days sd ON a.start_day_id = sd.id + LEFT JOIN days ed ON a.end_day_id = ed.id + LEFT JOIN days rd ON r.day_id = rd.id + LEFT JOIN days red ON r.end_day_id = red.id + WHERE r.trip_id = ? AND ${publicReservationSql('r')}`, + ) + .all(trip.id); + + expect(typed.map((r) => r.id)).toEqual([live.id]); + expect(typed).toEqual(legacy); + }); + + it('CAL-CL2-002: a "14.0"-shaped accommodation_id join still resolves the stay dates', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id, { start_date: '2026-06-01', end_date: '2026-06-05' }); + const place = createPlace(testDb, trip.id); + const days = testDb.prepare('SELECT id FROM days WHERE trip_id = ? ORDER BY day_number ASC').all(trip.id) as { id: number }[]; + const stay = createDayAccommodation(testDb, trip.id, place.id, days[0].id, days[1].id, { check_in: '15:00', check_out: '11:00' }); + const booking = createReservation(testDb, trip.id, { title: 'Hotel', type: 'hotel' }); + testDb.prepare('UPDATE reservations SET accommodation_id = ? WHERE id = ?').run(`${stay.id}.0`, booking.id); + + const typed = await reservationsRepo.listForCalendar(trip.id); + const legacy = testDb + .prepare( + `SELECT r.*, pl.lat AS place_lat, pl.lng AS place_lng, + sd.date AS stay_start_date, ed.date AS stay_end_date, + a.check_in AS stay_check_in, a.check_out AS stay_check_out, + (SELECT MIN(r2.id) FROM reservations r2 + WHERE r2.accommodation_id = a.id) AS stay_first_reservation_id, + rd.date AS day_date, red.date AS end_day_date + FROM reservations r + LEFT JOIN places pl ON r.place_id = pl.id + LEFT JOIN day_accommodations a ON r.accommodation_id = a.id + LEFT JOIN days sd ON a.start_day_id = sd.id + LEFT JOIN days ed ON a.end_day_id = ed.id + LEFT JOIN days rd ON r.day_id = rd.id + LEFT JOIN days red ON r.end_day_id = red.id + WHERE r.trip_id = ? AND ${publicReservationSql('r')}`, + ) + .all(trip.id); + + expect(typed[0]?.stay_check_in).toBe('15:00'); + expect(typed).toEqual(legacy); + }); + + // Inventory §18.2's documented, unfixed hole: `stay_first_reservation_id`'s + // correlated MIN(r2.id) is NOT filtered by RV1, so a staged sibling booking + // with a LOWER id than the live one still wins the slot. Parity keeps it — + // pinned by name so nobody "fixes" it silently in a later change. + it('CAL-CL2-HOLE-001: a staged sibling with a lower id wins stay_first_reservation_id over the live booking (documented, unfixed)', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id, { start_date: '2026-06-01', end_date: '2026-06-05' }); + const place = createPlace(testDb, trip.id); + const days = testDb.prepare('SELECT id FROM days WHERE trip_id = ? ORDER BY day_number ASC').all(trip.id) as { id: number }[]; + const stay = createDayAccommodation(testDb, trip.id, place.id, days[0].id, days[1].id); + + const stagedFirst = createReservation(testDb, trip.id, { title: 'Staged, lower id', type: 'hotel' }); + testDb.prepare("UPDATE reservations SET accommodation_id = ?, ingest_state = 'staged' WHERE id = ?").run(String(stay.id), stagedFirst.id); + const liveSecond = createReservation(testDb, trip.id, { title: 'Live, higher id', type: 'hotel' }); + testDb.prepare('UPDATE reservations SET accommodation_id = ? WHERE id = ?').run(String(stay.id), liveSecond.id); + + const typed = await reservationsRepo.listForCalendar(trip.id); + const liveRow = typed.find((r) => r.id === liveSecond.id); + + // The visible (live) row reports the STAGED sibling's id as the "first + // reservation" for its own stay — that is the hole, reproduced exactly. + expect(liveRow?.stay_first_reservation_id).toBe(stagedFirst.id); + expect(liveRow?.stay_first_reservation_id).not.toBe(liveSecond.id); + }); +}); + +describe('ReservationsRepository — CL4 (listCalendarStops)', () => { + 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' }); + const day = createDay(testDb, trip.id, { date: '2026-06-01' }); + const place = createPlace(testDb, trip.id, { name: 'Museum' }); + testDb.prepare('INSERT INTO day_assignments (day_id, place_id, order_index) VALUES (?, ?, 0)').run(day.id, place.id); + // A booked-night stop: linked to an accommodation, must be excluded. + const hotelPlace = createPlace(testDb, trip.id, { name: 'Hotel' }); + const accResult = testDb.prepare( + 'INSERT INTO day_accommodations (trip_id, place_id, start_day_id, end_day_id) VALUES (?, ?, ?, ?)', + ).run(trip.id, hotelPlace.id, day.id, day.id); + testDb.prepare('INSERT INTO day_assignments (day_id, place_id, order_index, accommodation_id) VALUES (?, ?, 1, ?)') + .run(day.id, hotelPlace.id, accResult.lastInsertRowid); + + const typed = await reservationsRepo.listCalendarStops(day.id); + const legacy = testDb.prepare(` + 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 + `).all(day.id); + + expect(typed.map((s) => s.place_name)).toEqual(['Museum']); + expect(typed).toEqual(legacy); + }); +}); + +describe('ReservationsRepository — CL7 (listPublicStaysForCalendar)', () => { + it('CAL-CL7-001: matches the legacy statement — a live-linked stay is included, a staged-only one is not', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id, { start_date: '2026-06-01', end_date: '2026-06-06' }); + const place = createPlace(testDb, trip.id); + const days = testDb.prepare('SELECT id FROM days WHERE trip_id = ? ORDER BY day_number ASC').all(trip.id) as { id: number }[]; + + const liveLinked = createDayAccommodation(testDb, trip.id, place.id, days[0].id, days[1].id, { check_in: '15:00', check_out: '11:00' }); + const liveBooking = createReservation(testDb, trip.id, { title: 'Live', type: 'hotel' }); + testDb.prepare('UPDATE reservations SET accommodation_id = ? WHERE id = ?').run(String(liveLinked.id), liveBooking.id); + + const stagedOnly = createDayAccommodation(testDb, trip.id, place.id, days[2].id, days[3].id); + const stagedBooking = createReservation(testDb, trip.id, { title: 'Staged', type: 'hotel' }); + testDb.prepare("UPDATE reservations SET accommodation_id = ?, ingest_state = 'staged' WHERE id = ?").run(String(stagedOnly.id), stagedBooking.id); + + const typed = await reservationsRepo.listPublicStaysForCalendar(trip.id); + const legacy = testDb.prepare(` + SELECT a.id, a.check_in, a.check_in_end, a.check_out, + sd.date AS start_date, ed.date AS end_date, + p.name AS place_name, p.address AS place_address, p.lat AS place_lat, p.lng AS place_lng, + (SELECT r.title FROM reservations r + WHERE r.accommodation_id = a.id AND ${publicReservationSql('r')} + ORDER BY r.id ASC LIMIT 1) AS reservation_title + FROM day_accommodations a + LEFT JOIN days sd ON a.start_day_id = sd.id + LEFT JOIN days ed ON a.end_day_id = ed.id + LEFT JOIN places p ON a.place_id = p.id + WHERE a.trip_id = ? AND ${publicStaySql('a')} + ORDER BY a.id ASC + `).all(trip.id); + + expect(typed.map((s) => s.id)).toEqual([liveLinked.id]); + expect(typed).toEqual(legacy); + }); +}); + +// ── RS20 (listUpcomingForUser) ─────────────────────────────────────────── + +/** The exact legacy statement RS20's `ReservationsService.listUpcoming` ran, kept here as this test's own oracle (§ file header). */ +const LEGACY_LIST_UPCOMING = ` + WITH visible_trips AS ( + SELECT t.id, t.title, t.cover_image + FROM trips t + LEFT JOIN trip_members tm ON tm.trip_id = t.id AND tm.user_id = ? + WHERE (t.user_id = ? OR tm.user_id IS NOT NULL) AND t.is_archived = 0 + ), + entries AS ( + SELECT r.id, r.trip_id, r.title, r.type, r.status, r.location, + r.reservation_time, r.confirmation_number, + tr.title as trip_title, tr.cover_image as trip_cover, + d.date as day_date, p.name as place_name, p.image_url as place_image, + CASE WHEN r.reservation_time GLOB '[0-9][0-9][0-9][0-9]-[0-9][0-9]-[0-9][0-9]*' THEN substr(r.reservation_time, 1, 10) ELSE d.date END as at_date, + CASE WHEN r.reservation_time GLOB '[0-9][0-9][0-9][0-9]-[0-9][0-9]-[0-9][0-9]*' THEN substr(r.reservation_time, 12) ELSE r.reservation_time END as at_time + FROM reservations r + JOIN visible_trips tr ON tr.id = r.trip_id + LEFT JOIN days d ON r.day_id = d.id + LEFT JOIN places p ON r.place_id = p.id + WHERE r.status != 'cancelled' + AND COALESCE(r.type, '') != 'hotel' + + UNION ALL + + SELECT a.id, a.trip_id, + COALESCE(p.name, (SELECT res.title FROM reservations res + WHERE CAST(res.accommodation_id AS INTEGER) = a.id + AND res.status != 'cancelled' + ORDER BY res.id LIMIT 1), tr.title) as title, + 'checkin' as type, 'confirmed' as status, NULL as location, + CASE WHEN a.check_in IS NOT NULL THEN d.date || 'T' || a.check_in END as reservation_time, + a.confirmation as confirmation_number, + tr.title as trip_title, tr.cover_image as trip_cover, + d.date as day_date, p.name as place_name, p.image_url as place_image, + d.date as at_date, a.check_in as at_time + FROM day_accommodations a + JOIN visible_trips tr ON tr.id = a.trip_id + JOIN days d ON d.id = a.start_day_id + LEFT JOIN places p ON p.id = a.place_id + + UNION ALL + + SELECT a.id, a.trip_id, + COALESCE(p.name, (SELECT res.title FROM reservations res + WHERE CAST(res.accommodation_id AS INTEGER) = a.id + AND res.status != 'cancelled' + ORDER BY res.id LIMIT 1), tr.title) as title, + 'checkout' as type, 'confirmed' as status, NULL as location, + CASE WHEN a.check_out IS NOT NULL THEN d.date || 'T' || a.check_out END as reservation_time, + a.confirmation as confirmation_number, + tr.title as trip_title, tr.cover_image as trip_cover, + d.date as day_date, p.name as place_name, p.image_url as place_image, + d.date as at_date, a.check_out as at_time + FROM day_accommodations a + JOIN visible_trips tr ON tr.id = a.trip_id + JOIN days d ON d.id = a.end_day_id + LEFT JOIN places p ON p.id = a.place_id + ) + SELECT id, trip_id, title, type, status, location, reservation_time, + confirmation_number, trip_title, trip_cover, day_date, place_name, place_image + FROM entries + WHERE at_date IS NOT NULL + AND (at_date > ? OR (at_date = ? AND COALESCE(at_time, '23:59') >= ?)) + ORDER BY at_date ASC, COALESCE(at_time, '00:00') ASC, id ASC + LIMIT ? + `; + +function legacyListUpcoming(userId: number, today: string, nowHHMM: string, limit: number) { + return testDb.prepare(LEGACY_LIST_UPCOMING).all(userId, userId, today, today, nowHHMM, limit); +} + +describe('ReservationsRepository — RS20 (listUpcomingForUser)', () => { + /** + * One fully seeded world — owner + member + archived + hidden trips, a + * mixed-type/mixed-shape reservation set, a NAMED stay and a NAMELESS one + * whose only booking's `accommodation_id` is written in the `".0"` + * TEXT shape (§18.1) — run against 5 `(today, now, limit)` settings, + * including one where two bookings tie on `(at_date, at_time)` so the + * `id` tiebreak is load-bearing, and one where `limit` cuts the result + * strictly inside that tie. + */ + it('RS20-001..005: matches the legacy CTE statement across 5 (today, now, limit) settings, incl. the cross-table id tiebreak and a limit straddling a tie', async () => { + const { user: owner } = createUser(testDb); + const { user: stranger } = createUser(testDb); + + const tripOwned = createTrip(testDb, owner.id, { start_date: '2999-01-01', end_date: '2999-01-10' }); + const tripMember = createTrip(testDb, stranger.id, { start_date: '2999-01-01', end_date: '2999-01-05' }); + addTripMember(testDb, tripMember.id, owner.id); + const tripHidden = createTrip(testDb, stranger.id, { start_date: '2999-01-01', end_date: '2999-01-05' }); + const tripArchived = createTrip(testDb, owner.id, { start_date: '2999-01-01', end_date: '2999-01-05' }); + testDb.prepare('UPDATE trips SET is_archived = 1 WHERE id = ?').run(tripArchived.id); + + // Two bookings on the SAME date+time — the (at_date, at_time) tie the + // `id ASC` tiebreak has to resolve, in insertion (id) order. + const tie1 = createReservation(testDb, tripOwned.id, { title: 'Tie 1' }); + testDb.prepare('UPDATE reservations SET reservation_time = ? WHERE id = ?').run('2999-01-02T09:00:00', tie1.id); + const tie2 = createReservation(testDb, tripOwned.id, { title: 'Tie 2' }); + testDb.prepare('UPDATE reservations SET reservation_time = ? WHERE id = ?').run('2999-01-02T09:00:00', tie2.id); + + // Excluded regardless of settings: cancelled, and a hotel-typed booking + // dated through its own reservation_time. + const cancelled = createReservation(testDb, tripOwned.id, { title: 'Cancelled' }); + testDb.prepare("UPDATE reservations SET reservation_time = ?, status = 'cancelled' WHERE id = ?").run('2999-01-03T10:00:00', cancelled.id); + const hotelDated = createReservation(testDb, tripOwned.id, { title: 'Hotel dated', type: 'hotel' }); + testDb.prepare('UPDATE reservations SET reservation_time = ? WHERE id = ?').run('2999-01-03T10:00:00', hotelDated.id); + + // A NULL-typed booking still shows. + const untyped = createReservation(testDb, tripOwned.id, { title: 'Untyped' }); + testDb.prepare('UPDATE reservations SET type = NULL, reservation_time = ? WHERE id = ?').run('2999-01-04T10:00:00', untyped.id); + + // A bare-clock, day-anchored booking. + const bareDay = createDay(testDb, tripOwned.id, { date: '2999-01-05' }); + const bareClock = createReservation(testDb, tripOwned.id, { title: 'Bare clock', day_id: bareDay.id }); + testDb.prepare('UPDATE reservations SET reservation_time = ? WHERE id = ?').run('07:00', bareClock.id); + + // A member-trip booking. + const memberBooking = createReservation(testDb, tripMember.id, { title: 'Member trip' }); + testDb.prepare('UPDATE reservations SET reservation_time = ? WHERE id = ?').run('2999-01-02T12:00:00', memberBooking.id); + + // A hidden-trip booking — must never appear. + const hiddenBooking = createReservation(testDb, tripHidden.id, { title: 'Hidden' }); + testDb.prepare('UPDATE reservations SET reservation_time = ? WHERE id = ?').run('2999-01-02T08:00:00', hiddenBooking.id); + + // A NAMED stay (check-in + check-out). + const namedPlace = createPlace(testDb, tripOwned.id, { name: 'The Plaza' }); + const days = testDb.prepare('SELECT id FROM days WHERE trip_id = ? ORDER BY day_number ASC').all(tripOwned.id) as { id: number }[]; + createDayAccommodation(testDb, tripOwned.id, namedPlace.id, days[5].id, days[7].id, { check_in: '15:00', check_out: '11:00' }); + + // A NAMELESS stay whose only (cancelled-excluded) booking's + // accommodation_id is stored in the ".0" TEXT shape. + const namelessAcc = testDb.prepare( + 'INSERT INTO day_accommodations (trip_id, place_id, start_day_id, end_day_id, check_in, check_out) VALUES (?, NULL, ?, ?, ?, ?)', + ).run(tripOwned.id, days[5].id, days[6].id, '16:00', '10:00'); + const namelessId = namelessAcc.lastInsertRowid as number; + const namelessBooking = createReservation(testDb, tripOwned.id, { title: 'Hotel Ibis', type: 'hotel' }); + testDb.prepare('UPDATE reservations SET accommodation_id = ? WHERE id = ?').run(`${namelessId}.0`, namelessBooking.id); + + const settings: [string, string, number][] = [ + ['2000-01-01', '2000-01-01T00:00', 20], // far past today — everything future-dated shows + ['2999-01-02', '2999-01-02T09:00', 20], // exactly on the tie moment — inclusive, both tie rows stay + ['2999-01-02', '2999-01-02T09:01', 20], // one minute past the tie — both tie rows drop + ['2000-01-01', '2000-01-01T00:00', 3], // straddles the tie: limit cuts inside the tied pair + ['2999-06-01', '2999-06-01T00:00', 20], // nothing left — empty result + ]; + + for (const [today, nowHHMM, limit] of settings) { + const typed = await reservationsRepo.listUpcomingForUser(owner.id, today, nowHHMM, limit); + const legacy = legacyListUpcoming(owner.id, today, nowHHMM, limit); + expect(typed).toEqual(legacy); + } + + // Sanity — the first setting's shape is what the matrix above claims: + // hidden/archived excluded, cancelled/hotel-dated excluded, the tie + // resolves in id order, the nameless stay reads its title off the + // linked ".0" booking. + const baseline = await reservationsRepo.listUpcomingForUser(owner.id, '2000-01-01', '2000-01-01T00:00', 20); + expect(baseline.map((r) => r.title)).not.toContain('Hidden'); + const tieIndex = baseline.findIndex((r) => r.title === 'Tie 1'); + expect(baseline[tieIndex + 1]?.title).toBe('Tie 2'); + expect(baseline.some((r) => r.title === 'Hotel Ibis')).toBe(true); + }); +}); diff --git a/server/tests/unit/db/repositories/_shared/reservation-visibility.test.ts b/server/tests/unit/db/repositories/_shared/reservation-visibility.test.ts index a87d6b61c..6b3d4883f 100644 --- a/server/tests/unit/db/repositories/_shared/reservation-visibility.test.ts +++ b/server/tests/unit/db/repositories/_shared/reservation-visibility.test.ts @@ -6,10 +6,29 @@ import { createDayAccommodation, createPlace, createReservation, createTrip, cre import { Reservations } from '../../../../../src/db/entities/Reservations.entity'; import { publicReservationCondition, + publicReservationExpr, publicStayExists, type ReservationVisibilityKyselyDB, } from '../../../../../src/db/repositories/_shared/reservation-visibility'; -import { publicReservationSql, publicStaySql } from '../../../../../src/nest/reservations/reservation-visibility'; + +/** + * The legacy string fragments this harness proves parity against + * (`src/nest/reservations/reservation-visibility.ts`'s `publicReservationSql`/ + * `publicStaySql`) were DELETED by Plan 3d Task 4 once calendar and `share` + * — their only two consumers — were converted onto the typed predicates + * above in the same commit (no two live implementations of an + * anonymous-surface security check). Kept here, byte-identical to the + * deleted file's own text, as this test's own oracle: the point of a + * string-vs-predicate parity harness is to hold the STRING side fixed and + * independent of the production code it is proving, not to import it from + * production (there is no longer a production copy to import). + */ +const publicReservationSql = (alias = 'r'): string => `COALESCE(${alias}.ingest_state, 'live') <> 'staged'`; +const publicStaySql = (alias = 'a'): string => `( + NOT EXISTS (SELECT 1 FROM reservations vr WHERE CAST(vr.accommodation_id AS INTEGER) = ${alias}.id) + OR EXISTS (SELECT 1 FROM reservations vr WHERE CAST(vr.accommodation_id AS INTEGER) = ${alias}.id + AND ${publicReservationSql('vr')}) + )`; const testDb = createSnapshotTestDb(); let t: TestOrm; @@ -92,6 +111,75 @@ describe('reservation-visibility parity (RV1: publicReservationCondition vs publ }); }); +/** + * RV1's Kysely-expression twin (L2, Plan 3d Task 4) — `publicReservationExpr`, + * the form CL2/CL7/`share.service.ts`'s reads use (statements built with + * `this.kysely()`, not the MikroORM QueryBuilder `publicReservationCondition` + * spreads into). Run through a bare `reservations` selectFrom rather than + * through a repository, since this file has no repository of its own — the + * same "no consumer-facing generic DB" shape `publicStayExists` documents. + */ +interface PublicReservationExprTestDB { + reservations: { id: number; trip_id: number; ingest_state: string | null }; +} + +describe('reservation-visibility parity (RV1 Kysely form: publicReservationExpr vs publicReservationSql)', () => { + it('RESVIS-011: a mix of live/staged reservations — publicReservationExpr and the legacy string fragment select the identical row set', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const live1 = createReservation(testDb, trip.id, { title: 'Live 1' }); + const live2 = createReservation(testDb, trip.id, { title: 'Live 2' }); + const staged = createReservation(testDb, trip.id, { title: 'Staged' }); + testDb.prepare("UPDATE reservations SET ingest_state = 'staged' WHERE id = ?").run(staged.id); + + const typed = await t.em.getKysely() + .selectFrom('reservations as r') + .select('r.id') + .where('r.trip_id', '=', trip.id) + .where((eb) => publicReservationExpr(eb, 'r.ingest_state')) + .orderBy('r.id', 'asc') + .execute(); + const legacy = testDb + .prepare(`SELECT id FROM reservations r WHERE r.trip_id = ? AND ${publicReservationSql('r')} ORDER BY id ASC`) + .all(trip.id) as { id: number }[]; + + expect(typed.map((r) => r.id)).toEqual([live1.id, live2.id]); + expect(typed).toEqual(legacy); + }); + + // Mutation-sensitive: a third `ingest_state` value (neither 'live' nor + // 'staged' — reachable today, no CHECK constraint pins the column to two + // values, `Migration20200101031900...`) must stay PUBLIC. A one-token + // confusion between "hide unless live" (`<> 'live'`, wrong) and "hide only + // staged" (`<> 'staged'`, the actual rule) diverges exactly on this row: + // manually mutated `publicReservationExpr`'s `'staged'` literal to `'live'` + // and re-ran this suite — this case goes red (the row is wrongly hidden) + // while every RESVIS-001/003 live/staged-only case stays green, so it is + // not redundant with them. + it('RESVIS-012 (mutation-sensitive): a third ingest_state value (neither live nor staged) is public — only "staged" hides a row', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const other = createReservation(testDb, trip.id, { title: 'Some other ingest state' }); + testDb.prepare("UPDATE reservations SET ingest_state = 'archived_import' WHERE id = ?").run(other.id); + const staged = createReservation(testDb, trip.id, { title: 'Staged' }); + testDb.prepare("UPDATE reservations SET ingest_state = 'staged' WHERE id = ?").run(staged.id); + + const typed = await t.em.getKysely() + .selectFrom('reservations as r') + .select('r.id') + .where('r.trip_id', '=', trip.id) + .where((eb) => publicReservationExpr(eb, 'r.ingest_state')) + .orderBy('r.id', 'asc') + .execute(); + const legacy = testDb + .prepare(`SELECT id FROM reservations r WHERE r.trip_id = ? AND ${publicReservationSql('r')} ORDER BY id ASC`) + .all(trip.id) as { id: number }[]; + + expect(typed.map((r) => r.id)).toEqual([other.id]); + expect(typed).toEqual(legacy); + }); +}); + /** * `Kysely` typed the way `TrekRepository.kysely()` * types every other repository's Kysely escape hatch — here obtained diff --git a/server/tests/unit/nest/calendar.service.test.ts b/server/tests/unit/nest/calendar.service.test.ts index 43aeb4546..72b04963f 100644 --- a/server/tests/unit/nest/calendar.service.test.ts +++ b/server/tests/unit/nest/calendar.service.test.ts @@ -57,7 +57,7 @@ import { CalendarModule } from '../../../src/nest/calendar/calendar.module'; import { expectRegisteredProvider } from '../../helpers/module-providers'; import { notificationsStub } from '../../helpers/notifications'; import { accommodationsOver } from '../../helpers/accommodations-service'; -import { createTestUnitOfWork, createTestAppSettingsRepo, createTestReservationsRepo, createTestReservationEndpointsRepo, createTestReservationTravelersRepo, createTestReservationDayPositionsRepo, createTestDayAccommodationsRepo, createTestDaysRepo, createTestPlacesRepo, createTestDayAssignmentsRepo, createTestTripMembersRepo, createTestUsersRepo, createTestTripsRepo } from '../../helpers/test-uow'; +import { createTestUnitOfWork, createTestAppSettingsRepo, createTestReservationsRepo, createTestReservationEndpointsRepo, createTestReservationTravelersRepo, createTestReservationDayPositionsRepo, createTestDayAccommodationsRepo, createTestDaysRepo, createTestDayNotesRepo, createTestPlacesRepo, createTestDayAssignmentsRepo, createTestTripMembersRepo, createTestUsersRepo, createTestTripsRepo } from '../../helpers/test-uow'; const dbs = () => new DatabaseService(testDb); @@ -68,8 +68,8 @@ let svc: CalendarService; beforeAll(async () => { budgetSvc = new BudgetService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(dbs().connection)); svc = new CalendarService( - dbs(), new ReservationsService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), budgetSvc, new RealtimeService(), notificationsStub(), new ReservationsReadService(await createTestReservationsRepo(dbs().connection), await createTestReservationEndpointsRepo(dbs().connection), await createTestReservationTravelersRepo(dbs().connection)), await accommodationsOver(dbs()), await createTestUnitOfWork(dbs().connection), await createTestReservationsRepo(dbs().connection), await createTestReservationEndpointsRepo(dbs().connection), await createTestReservationTravelersRepo(dbs().connection), await createTestReservationDayPositionsRepo(dbs().connection), await createTestDayAccommodationsRepo(dbs().connection), await createTestDaysRepo(dbs().connection), await createTestPlacesRepo(dbs().connection), await createTestDayAssignmentsRepo(dbs().connection), await createTestTripMembersRepo(dbs().connection), await createTestUsersRepo(dbs().connection), await createTestTripsRepo(dbs().connection)), + await createTestTripsRepo(dbs().connection), await createTestDaysRepo(dbs().connection), await createTestDayNotesRepo(dbs().connection), await createTestReservationsRepo(dbs().connection), ); }); diff --git a/server/tests/unit/nest/share.service.test.ts b/server/tests/unit/nest/share.service.test.ts index a2a681227..059c601ff 100644 --- a/server/tests/unit/nest/share.service.test.ts +++ b/server/tests/unit/nest/share.service.test.ts @@ -60,7 +60,7 @@ import { ShareService, publicReservationMetadata } from '../../../src/nest/share import { SettingsService } from '../../../src/nest/settings/settings.service'; import { QueryHelpersService } from '../../../src/nest/query-helpers/query-helpers.service'; import type { User } from '../../../src/types'; -import { sharedTestOrm, createTestUnitOfWork, createTestAppSettingsRepo, createTestSettingsRepo, createTestTagsRepo, createTestPlaceRatingsRepo, createTestAssignmentParticipantsRepo } from '../../helpers/test-uow'; +import { sharedTestOrm, createTestUnitOfWork, createTestAppSettingsRepo, createTestSettingsRepo, createTestTagsRepo, createTestPlaceRatingsRepo, createTestAssignmentParticipantsRepo, createTestReservationsRepo } from '../../helpers/test-uow'; import type { TestOrm } from '../../helpers/test-orm'; let svc: ShareService; @@ -81,6 +81,7 @@ beforeAll(async () => { new QueryHelpersService(await createTestTagsRepo(dbs.connection), await createTestPlaceRatingsRepo(dbs.connection), await createTestAssignmentParticipantsRepo(dbs.connection)), photoCacheStub, await createTestUnitOfWork(testDb), + await createTestReservationsRepo(testDb), ); });