diff --git a/server/src/db/dialect/sql-functions.ts b/server/src/db/dialect/sql-functions.ts index fe7ec5e1e..cb9d85526 100644 --- a/server/src/db/dialect/sql-functions.ts +++ b/server/src/db/dialect/sql-functions.ts @@ -1,5 +1,6 @@ import { raw, type Platform, type RawQueryFragment } from '@mikro-orm/core'; import { SqlitePlatform } from '@mikro-orm/sql'; +import type { Expression, ExpressionBuilder, ExpressionWrapper, ReferenceExpression, SqlBool, StringReference } from 'kysely'; /** * The only place a repository may spell a database function. @@ -500,3 +501,131 @@ export function dayDistance(platform: Platform, ref: string, isoDate: string): R return unsupported(platform); } +// --------------------------------------------------------------------------- +// Plan 3d Task 0 review (H1/L1) — the five helpers above return a MikroORM +// `RawQueryFragment`. Handed into a Kysely statement, a `RawQueryFragment` +// stringifies through its own `[Symbol.toPrimitive]('string')` coercion into +// the literal text `?` (its parameter-placeholder marker, meant for the +// MikroORM QueryBuilder's OWN parameter interpolation, not Kysely's) — Kysely +// binds it as an actual `?` value parameter and the statement fails at +// execution (verified directly, not assumed: `db.selectFrom(...).where(..., +// startsWithIsoDate(platform, 'col'))` throws `SqliteError: near "?": syntax +// error` the first time it runs). Task 2 absorbed this finding (its report, +// H1/L1) — these are genuinely separate functions, not a wrapper over the +// ones above, because a Kysely `Expression` and a MikroORM `RawQueryFragment` +// are different types with no conversion between them. Each takes the +// caller's own `ExpressionBuilder` (the same "no consumer-facing +// generic DB" shape `_shared/reservation-visibility.ts`'s `publicStayExists` +// documents is the only one that survives `tsc` for a shared Kysely helper — +// unlike that one, these ARE fully generic over ``, because they only +// ever call `eb.fn`/`eb.cast`/`eb(...)`, none of which hit the QueryBuilder +// method-chaining inference limit that forced `publicStayExists` to fix its +// shape). Platform-dispatched and fail-closed like every helper above; each +// has an SQLF-0xx test pinning its compiled `{sql, parameters}` AND a +// raw-statement equivalence on seeded rows (`tests/unit/db/dialect/sql-functions.test.ts`). +// --------------------------------------------------------------------------- + +/** + * The Kysely-expression twin of {@link startsWithIsoDate}: ` GLOB + * '[0-9][0-9][0-9][0-9]-[0-9][0-9]-[0-9][0-9]*'`, spelled through SQLite's + * function form (`glob(X, Y)` is exactly `Y GLOB X` — the inventory's own + * §19 finding, since Kysely's `ComparisonOperator` union has no `GLOB` + * entry). Returns a boolean `Expression`, usable in a `.where()` + * or inside a `CASE WHEN` condition. Same digit-class pattern as the + * MikroORM version — NOT the wider `'????-??-??*'` shorthand (a `?` in + * SQLite GLOB matches any character, letters included). + */ +export function startsWithIsoDateKysely( + platform: Platform, + eb: ExpressionBuilder, + ref: ReferenceExpression, +): ExpressionWrapper { + if (platform instanceof SqlitePlatform) { + return eb.fn('glob', [eb.val('[0-9][0-9][0-9][0-9]-[0-9][0-9]-[0-9][0-9]*'), ref]); + } + return unsupported(platform); +} + +/** + * The Kysely-expression twin of {@link substring}: `substr(, + * [, ])`, 1-based on both ends. `start`/`length` are + * validated integers, the same guard `substring()` above applies, and bound + * as genuine parameters here (`eb.val`) rather than spelled into the SQL + * text — Kysely's `fn()` takes `ReferenceExpression`s, not a raw text + * fragment, so there is no equivalent "spell a constant into the SQL" + * shape to match; a bound integer literal renders identically. + */ +export function substringKysely( + platform: Platform, + eb: ExpressionBuilder, + ref: ReferenceExpression, + start: number, + length?: number, +): ExpressionWrapper { + if (!Number.isInteger(start) || start < 1) { + throw new Error(`sql-functions: substringKysely needs a 1-based integer start, got ${start}`); + } + if (length !== undefined && (!Number.isInteger(length) || length < 0)) { + throw new Error(`sql-functions: substringKysely needs a non-negative integer length, got ${length}`); + } + if (platform instanceof SqlitePlatform) { + const args: ReferenceExpression[] = length === undefined + ? [ref, eb.val(start)] + : [ref, eb.val(start), eb.val(length)]; + return eb.fn('substr', args); + } + return unsupported(platform); +} + +/** A part of a `concatKysely()` expression — a column reference, a bound value, or a nested Kysely `Expression` (composes freely, unlike `concat()`'s MikroORM `RawQueryFragment` form). */ +export type KyselyConcatPart = + | { column: StringReference } + | { value: string } + | { expression: Expression }; + +/** + * The Kysely-expression twin of {@link concat}: ` || || …`, + * SQLite's/Postgres's string concatenation operator, built through the + * expression builder's own binary-operator call (`eb(lhs, '||', rhs)`) so + * it composes with {@link substringKysely}/{@link castIntegerKysely} — + * exactly the composition `concat()`'s MikroORM form cannot do (its + * docstring explains why). + */ +export function concatKysely( + platform: Platform, + eb: ExpressionBuilder, + ...parts: readonly KyselyConcatPart[] +): ExpressionWrapper { + if (parts.length < 2) { + throw new Error(`sql-functions: concatKysely needs at least two parts, got ${parts.length}`); + } + if (!(platform instanceof SqlitePlatform)) return unsupported(platform); + const operand = (part: KyselyConcatPart): Expression => + 'column' in part ? eb.ref(part.column).$castTo() : 'value' in part ? eb.val(part.value) : part.expression; + let acc: Expression = operand(parts[0]!); + for (const part of parts.slice(1)) { + acc = eb(acc, '||', operand(part)); + } + // The loop above always runs at least once (the `parts.length < 2` guard + // above throws otherwise), so `acc` is always the result of the `eb(...)` + // binary-operator call — a real `ExpressionWrapper`, not the bare + // `Expression` interface `operand()`'s own return type declares. + return acc as ExpressionWrapper; +} + +/** + * The Kysely-expression twin of {@link castInteger}: `CAST( AS + * INTEGER)`, for `reservations.accommodation_id` (TEXT) compared against an + * INTEGER column inside a Kysely statement (RS20/RV2's join shape — a + * correlated `EXISTS`/scalar-subquery context the MikroORM QueryBuilder + * cannot express, per the inventory's own T6 ruling). + */ +export function castIntegerKysely( + platform: Platform, + eb: ExpressionBuilder, + ref: ReferenceExpression, +): ExpressionWrapper { + if (platform instanceof SqlitePlatform) return eb.cast(ref, 'integer'); + return unsupported(platform); +} + diff --git a/server/src/db/repositories/DayAccommodations.repository.ts b/server/src/db/repositories/DayAccommodations.repository.ts index 9594441b6..224fce9fd 100644 --- a/server/src/db/repositories/DayAccommodations.repository.ts +++ b/server/src/db/repositories/DayAccommodations.repository.ts @@ -1,4 +1,149 @@ import type { DayAccommodations } from '../entities/DayAccommodations.entity'; +import { coalesceParam, columnRef } from '../dialect/sql-functions'; import { TrekRepository } from './_shared/trek-repository'; -export class DayAccommodationsRepository extends TrekRepository {} +/** + * Plan 3d Task 2 (`ReservationsService`'s `day_accommodations` writes — RS27, + * RS29, RS30, RS37–RS40, RS42, RS43, RS46, RS47 — plus the RS21 dispatch + * union's `'day_accommodations'` arm). This is the reservations-owned SUBSET + * of the repository: `days.service.ts` DY19/DY20/DY22 and `places.service.ts` + * PL16 (Plan 3d Task 3) add methods here too — the class is shared, per the + * inventory's own note (§7) — Task 3 appends rather than replaces. + * + * `day_accommodations` has no MikroORM relation to `reservations` + * (`reservations.accommodation_id` is a bare `p.text().nullable()` column, + * no FK — Task 0's R10 finding) — every method here that only reaches this + * table stays on the QueryBuilder/`nativeUpdate`/`insert`, never Kysely: the + * TEXT-vs-INTEGER join lives in `ReservationsRepository`, not here. + */ +export class DayAccommodationsRepository extends TrekRepository { + /** + * RS27/RS40 (`ReservationsService.createInTx`/`updateInTx`'s auto-create + * branch) — `INSERT INTO day_accommodations (trip_id, place_id, + * start_day_id, end_day_id, check_in, check_out, confirmation) VALUES + * (?×7)`. Seven columns only — no `check_in_end`, no `notes` (unlike + * `AccommodationsService`'s own AC30, a 9-column insert — a DIFFERENT + * method, D4). The caller passes already-decided values (the `|| null` + * coercions stay in the service); `em.insert()` returns the generated PK. + */ + async insertBookingStay(input: { + trip_id: number | string; + place_id: number | null; + start_day_id: number; + end_day_id: number; + check_in: string | null; + check_out: string | null; + confirmation: string | null; + }): Promise { + return await this.insert({ + trip: input.trip_id, + place: input.place_id, + startDay: input.start_day_id, + endDay: input.end_day_id, + check_in: input.check_in, + check_out: input.check_out, + confirmation: input.confirmation, + }); + } + + /** + * RS29/RS42 — `UPDATE day_accommodations SET check_in = COALESCE(?, + * check_in), check_in_end = COALESCE(?, check_in_end), check_out = + * COALESCE(?, check_out) WHERE id = ?`. `coalesceParam` (value-side, a + * bound `?`) through `qb().update()` — `nativeUpdate`'s `EntityData` + * shape doesn't accept a raw fragment value (`DayAssignmentsRepository + * .shiftOrderFrom`'s precedent). + */ + async patchTimes(id: number, check_in: string | null, check_in_end: string | null, check_out: string | null): Promise { + const platform = this.getEntityManager().getPlatform(); + await this.qb() + .update({ + check_in: coalesceParam(platform, 'check_in', check_in), + check_in_end: coalesceParam(platform, 'check_in_end', check_in_end), + check_out: coalesceParam(platform, 'check_out', check_out), + }) + .where({ id }) + .execute('run'); + } + + /** RS30/RS43 — `UPDATE day_accommodations SET confirmation = COALESCE(?, confirmation) WHERE id = ?`. */ + async patchConfirmation(id: number, confirmation: string | null): Promise { + const platform = this.getEntityManager().getPlatform(); + await this.qb() + .update({ confirmation: coalesceParam(platform, 'confirmation', confirmation) }) + .where({ id }) + .execute('run'); + } + + /** + * RS37/RS46 — `SELECT id FROM day_accommodations WHERE id = ? AND trip_id + * = ?`. `id`/`trip_id: number` (typed filter, program rule 23): both + * callers (`updateInTx`'s re-link scoping check, `remove`'s own-stay + * check) pass the SAME `toRowId`-parsed trip id the service method itself + * resolved once at its gate. + */ + async existsInTrip(id: number, trip_id: number): Promise { + const row = await this.qb('a') + .select(['a.id']) + .where({ id, trip: trip_id }) + .execute<{ id: number } | undefined>('get', false); + return !!row; + } + + /** RS38 — `SELECT check_in FROM day_accommodations WHERE id = ?`. */ + async getCheckIn(id: number): Promise { + const row = await this.qb('a') + .select(['a.check_in']) + .where({ id }) + .execute<{ check_in: string | null } | undefined>('get', false); + return row?.check_in; + } + + /** + * RS39 — `UPDATE day_accommodations SET place_id = ?, start_day_id = ?, + * end_day_id = ?, check_in = ?, check_out = ?, confirmation = ? WHERE id = + * ?`. Six columns (no `check_in_end`/`notes`, unlike `AccommodationsService + * .moveStay`'s own write) — the caller's already-decided values, written + * verbatim (the `PlacesRepository.updatePlace`/PL11 precedent: no SQL-side + * COALESCE here, the legacy statement itself has none for these columns). + */ + async updateFromBooking(id: number, write: { + place_id: number | null; + start_day_id: number; + end_day_id: number; + check_in: string | null; + check_out: string | null; + confirmation: string | null; + }): Promise { + await this.nativeUpdate({ id }, { + place: write.place_id, + startDay: write.start_day_id, + endDay: write.end_day_id, + check_in: write.check_in, + check_out: write.check_out, + confirmation: write.confirmation, + }); + } + + /** RS47 — `DELETE FROM day_accommodations WHERE id = ? AND trip_id = ?`, run AFTER `accommodations.dropStayStops` (ordering load-bearing, kept in the service). `id`/`trip_id: number` (typed filter, program rule 23; rule 21 — the SAME `toRowId`-parsed trip id `existsInTrip`'s gate read used). */ + async deleteInTrip(id: number, trip_id: number): Promise { + await this.nativeDelete({ id, trip: trip_id }); + } + + /** + * RS21's `'day_accommodations'` dispatch arm (`ReservationsService + * .referencesOutsideTrip`'s `elsewhere('day_accommodations', + * data.accommodation_id)` — `SELECT trip_id FROM day_accommodations WHERE + * id = ?`). A missing row is not an offender (the caller's own `!!row && + * String(row.trip_id) !== String(tripId)` check, preserved in the + * service) — this returns `undefined` on a miss, never throws. + */ + async getTripId(id: number): Promise { + const platform = this.getEntityManager().getPlatform(); + const row = await this.qb('a') + .select([columnRef(platform, 'a.trip_id').as('trip_id')]) + .where({ id }) + .execute<{ trip_id: number } | undefined>('get', false); + return row?.trip_id; + } +} diff --git a/server/src/db/repositories/ReservationDayPositions.repository.ts b/server/src/db/repositories/ReservationDayPositions.repository.ts index 13fce5320..0ac75b16d 100644 --- a/server/src/db/repositories/ReservationDayPositions.repository.ts +++ b/server/src/db/repositories/ReservationDayPositions.repository.ts @@ -1,4 +1,79 @@ import type { ReservationDayPositions } from '../entities/ReservationDayPositions.entity'; +import { columnRef } from '../dialect/sql-functions'; import { TrekRepository } from './_shared/trek-repository'; -export class ReservationDayPositionsRepository extends TrekRepository {} +/** `reservation_day_positions`/`reservations`/`days`'s shape for RS31's Kysely upsert. */ +interface ReservationDayPositionsKyselyDB { + reservation_day_positions: { + reservation_id: number; + day_id: number; + position: number; + }; + reservations: { + id: number; + // `number | string`: the legacy statement raw-binds `tripId` exactly as + // the caller received it (D4's T5 escape hatch) — the Kysely column type + // here is widened to accept both without narrowing which non-canonical + // ids match, never actually persisted (this table has no `trip_id` + // column of its own). + trip_id: number | string; + }; + days: { + id: number; + trip_id: number | string; + }; +} + +export class ReservationDayPositionsRepository extends TrekRepository { + /** + * RS19 (`ReservationsService.list`) — `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 = ?`. The + * fold into `{[day_id]: position}` per reservation stays in the service. + */ + async listForTrip(trip_id: number): Promise<{ reservation_id: number; day_id: number; position: number }[]> { + const platform = this.getEntityManager().getPlatform(); + // `rdp.reservation_id`/`rdp.day_id` are `persist(false)` mirrors of the + // `reservation`/`day` relations — bare-selecting them alongside an + // active JOIN on `rdp.reservation` (aliased `r`) is the exact trap + // `DayAssignmentsRepository.assignmentWithPlaceSelect`'s `category_id` + // docstring documents (the column silently aliases off the JOINED + // entity instead); `columnRef` selects the literal physical column. + return this.qb('rdp') + .join('rdp.reservation', 'r') + .select([columnRef(platform, 'rdp.reservation_id').as('reservation_id'), columnRef(platform, 'rdp.day_id').as('day_id'), 'rdp.position']) + .where({ 'r.trip': trip_id }) + .execute<{ reservation_id: number; day_id: number; position: number }[]>('all', false); + } + + /** + * RS31 (`ReservationsService.updatePositions`'s day branch) — `INSERT OR + * REPLACE INTO reservation_day_positions (reservation_id, day_id, + * position) SELECT r.id, d.id, ? FROM reservations r JOIN days d ON + * d.trip_id = r.trip_id WHERE r.id = ? AND d.id = ? AND r.trip_id = ?`. + * + * Kysely `insertInto().orReplace().columns().expression(selectFrom…)` + * (the MikroORM QueryBuilder's `insertFrom` has no `OR REPLACE` flag — + * `.onConflict().merge()` is semantically different, a real UPDATE rather + * than REPLACE's delete-then-insert). The trip scoping lives IN the + * SELECT (a foreign `reservation_id`/`day_id` materialises zero rows to + * insert, never a foreign-key error) — preserved exactly, not hoisted + * into a pre-check. + */ + async upsertScoped(trip_id: number | string, reservation_id: number, day_id: number, position: number): Promise { + await this.kysely() + .insertInto('reservation_day_positions') + .orReplace() + .columns(['reservation_id', 'day_id', 'position']) + .expression((eb) => + eb + .selectFrom('reservations as r') + .innerJoin('days as d', 'd.trip_id', 'r.trip_id') + .select(['r.id as reservation_id', 'd.id as day_id', eb.val(position).as('position')]) + .where('r.id', '=', reservation_id) + .where('d.id', '=', day_id) + .where('r.trip_id', '=', trip_id), + ) + .execute(); + } +} diff --git a/server/src/db/repositories/ReservationEndpoints.repository.ts b/server/src/db/repositories/ReservationEndpoints.repository.ts index 57d3bcca9..ed1a2eca9 100644 --- a/server/src/db/repositories/ReservationEndpoints.repository.ts +++ b/server/src/db/repositories/ReservationEndpoints.repository.ts @@ -1,4 +1,101 @@ import type { ReservationEndpoints } from '../entities/ReservationEndpoints.entity'; +import { type AssertRowKeys } from './_shared/rows'; import { TrekRepository } from './_shared/trek-repository'; -export class ReservationEndpointsRepository extends TrekRepository {} +/** A `reservation_endpoints` row exactly as `r.*`/`SELECT *` read it (every scalar column). */ +export interface ReservationEndpointRow { + id: number; + reservation_id: number; + role: string; + sequence: number; + name: string; + code: string | null; + lat: number; + lng: number; + timezone: string | null; + local_time: string | null; + local_date: string | null; + created_at: string | null; +} + +const _reservationEndpointRowKeys: AssertRowKeys = true; + +export class ReservationEndpointsRepository extends TrekRepository { + /** + * RS3 (`ReservationsService.loadEndpointsByTrip`) — `SELECT e.* FROM + * reservation_endpoints e JOIN reservations r ON e.reservation_id = r.id + * WHERE r.trip_id = ? ORDER BY e.reservation_id, e.sequence`. The + * per-reservation grouping into a `Map` stays in the service (JS, not SQL). + */ + async listForTrip(trip_id: number): Promise { + return this.qb('e') + .join('e.reservation', 'r') + .select(['e.*']) + .where({ 'r.trip': trip_id }) + .orderBy({ 'e.reservation': 'asc', 'e.sequence': 'asc' }) + .execute('all', false); + } + + /** + * RR2 (`ReservationsReadService.loadEndpoints`) — `SELECT * FROM + * reservation_endpoints WHERE reservation_id = ? ORDER BY sequence`. + */ + async listForReservation(reservation_id: number): Promise { + return this.qb('e') + .select(['e.*']) + .where({ reservation: reservation_id }) + .orderBy({ sequence: 'asc' }) + .execute('all', false); + } + + /** RS16 (`ReservationsService.saveEndpoints`'s replace-all) — `DELETE FROM reservation_endpoints WHERE reservation_id = ?`. */ + async deleteForReservation(reservation_id: number): Promise { + await this.nativeDelete({ reservation: reservation_id }); + } + + /** + * RS17 — `INSERT INTO reservation_endpoints (reservation_id, role, + * sequence, name, code, lat, lng, timezone, local_time, local_date) VALUES + * (?×10)`, one row per call (the legacy `prepare` + per-row `run`). The + * lat/lng-null skip and the `sequence ?? i` (index AFTER the filter) stay + * in the service — this writes exactly the row it is handed. + */ + async insertEndpoint(input: { + reservation_id: number; + role: string; + sequence: number; + name: string; + code: string | null; + lat: number; + lng: number; + timezone: string | null; + local_time: string | null; + local_date: string | null; + }): Promise { + return await this.insert({ + reservation: input.reservation_id, + role: input.role, + sequence: input.sequence, + name: input.name, + code: input.code, + lat: input.lat, + lng: input.lng, + timezone: input.timezone, + local_time: input.local_time, + local_date: input.local_date, + }); + } + + /** DY17 (`DaysService.restampReservationDates`) — `SELECT id, local_date FROM reservation_endpoints WHERE reservation_id = ?`. */ + async listIdAndDate(reservation_id: number): Promise<{ id: number; local_date: string | null }[]> { + return this.qb('e') + .select(['e.id', 'e.local_date']) + .where({ reservation: reservation_id }) + .execute<{ id: number; local_date: string | null }[]>('all', false); + } + + /** DY18 — `UPDATE reservation_endpoints SET local_date = ? WHERE id = ?`. */ + async setLocalDate(id: number, local_date: string): Promise { + await this.nativeUpdate({ id }, { local_date }); + } +} diff --git a/server/src/db/repositories/ReservationTravelers.repository.ts b/server/src/db/repositories/ReservationTravelers.repository.ts index 802477e5d..c72e39de7 100644 --- a/server/src/db/repositories/ReservationTravelers.repository.ts +++ b/server/src/db/repositories/ReservationTravelers.repository.ts @@ -1,4 +1,77 @@ import type { ReservationTravelers } from '../entities/ReservationTravelers.entity'; +import { coalesce, columnRef } from '../dialect/sql-functions'; import { TrekRepository } from './_shared/trek-repository'; -export class ReservationTravelersRepository extends TrekRepository {} +/** A traveler row as RS6/RR3 project it (before `toTraveler()`'s `avatar_url` add). */ +export interface TravelerJoinRow { + reservation_id: number; + user_id: number; + username: string; + avatar: string | null; + is_guest: number | null; +} + +export class ReservationTravelersRepository extends TrekRepository { + /** + * RS6 (`ReservationsService.loadTravelersByTrip`) — `SELECT + * rt.reservation_id, rt.user_id, COALESCE(u.display_name, u.username) AS + * username, u.avatar, u.is_guest FROM reservation_travelers rt JOIN + * reservations r ON rt.reservation_id = r.id JOIN users u ON rt.user_id = + * u.id WHERE r.trip_id = ? ORDER BY rt.reservation_id`. + */ + async listForTrip(trip_id: number): Promise { + const platform = this.getEntityManager().getPlatform(); + // `rt.reservation_id`/`rt.user_id` are `persist(false)` mirrors of the + // `reservation`/`user` relations, both ALSO actively joined here + // (aliased `r`/`u`) — the exact trap `DayAssignmentsRepository + // .assignmentWithPlaceSelect`'s `category_id` docstring documents + // (a bare select silently aliases off the joined entity instead); + // `columnRef` selects the literal physical columns. + return this.qb('rt') + .join('rt.reservation', 'r') + .join('rt.user', 'u') + .select([columnRef(platform, 'rt.reservation_id').as('reservation_id'), columnRef(platform, 'rt.user_id').as('user_id'), coalesce(platform, 'u.display_name', 'u.username').as('username'), 'u.avatar', 'u.is_guest']) + .where({ 'r.trip': trip_id }) + .orderBy({ 'rt.reservation': 'asc' }) + .execute('all', false); + } + + /** + * RR3 (`ReservationsReadService.loadTravelers`) — `SELECT rt.user_id, + * COALESCE(u.display_name, u.username) AS username, u.avatar, u.is_guest + * FROM reservation_travelers rt JOIN users u ON rt.user_id = u.id WHERE + * rt.reservation_id = ?` — **no ORDER BY** (unlike `listForTrip`), matching + * the legacy statement's own row order (SQLite's). + */ + async listForReservation(reservation_id: number): Promise[]> { + const platform = this.getEntityManager().getPlatform(); + return this.qb('rt') + .join('rt.user', 'u') + .select([columnRef(platform, 'rt.user_id').as('user_id'), coalesce(platform, 'u.display_name', 'u.username').as('username'), 'u.avatar', 'u.is_guest']) + .where({ reservation: reservation_id }) + .execute[]>('all', false); + } + + /** RS8 (`ReservationsService.setReservationTravelers`'s replace-all) — `DELETE FROM reservation_travelers WHERE reservation_id = ?`. */ + async deleteForReservation(reservation_id: number): Promise { + await this.nativeDelete({ reservation: reservation_id }); + } + + /** + * RS9 — `INSERT OR IGNORE INTO reservation_travelers (reservation_id, + * user_id) VALUES (?, ?)`, run once per id. `upsertMany` with + * `onConflictAction: 'ignore'` on the table's real unique key + * (`UNIQUE(reservation_id, user_id)`, `ReservationTravelers.entity.ts`'s + * `uniques`), named by entity property (`reservation`/`user`, the + * `AssignmentParticipantsRepository.insertIgnore` precedent). The caller + * roster-scopes and dedupes the ids before calling; the empty-array guard + * here is defensive, matching the service's own guard. + */ + async insertIgnore(reservation_id: number, user_ids: number[]): Promise { + if (user_ids.length === 0) return; + await this.upsertMany( + user_ids.map((user_id) => ({ reservation: reservation_id, user: user_id })), + { onConflictFields: ['reservation', 'user'], onConflictAction: 'ignore' }, + ); + } +} diff --git a/server/src/db/repositories/Reservations.repository.ts b/server/src/db/repositories/Reservations.repository.ts index 52e920a2a..65117e372 100644 --- a/server/src/db/repositories/Reservations.repository.ts +++ b/server/src/db/repositories/Reservations.repository.ts @@ -1,4 +1,458 @@ import type { Reservations } from '../entities/Reservations.entity'; +import { DayAssignments } from '../entities/DayAssignments.entity'; +import { Days } from '../entities/Days.entity'; +import { columnRef, concatKysely, dayDistance, substringKysely } from '../dialect/sql-functions'; import { TrekRepository } from './_shared/trek-repository'; -export class ReservationsRepository extends TrekRepository {} +/** + * RS18/RR1's shared joined projection — `SELECT r.*, d.day_number, p.name as + * place_name, r.assignment_id, ap.place_id as accommodation_place_id, + * acc_p.name as accommodation_name, ap.start_day_id as + * accommodation_start_day_id, ap.end_day_id as accommodation_end_day_id FROM + * reservations r LEFT JOIN days d ON r.day_id = d.id LEFT JOIN places p ON + * r.place_id = p.id LEFT JOIN day_accommodations ap ON r.accommodation_id = + * ap.id LEFT JOIN places acc_p ON ap.place_id = acc_p.id`. The service + * folds `accommodation_id` (`Math.trunc(Number(x))`), `day_positions`, + * `endpoints` and `travelers` on afterward (unchanged from the legacy split). + */ +export interface ReservationJoinRow { + id: number; + trip_id: number; + day_id: number | null; + end_day_id: number | null; + place_id: number | null; + assignment_id: number | null; + title: string; + accommodation_id: string | null; + reservation_time: string | null; + reservation_end_time: string | null; + location: string | null; + confirmation_number: string | null; + notes: string | null; + status: string | null; + type: string | null; + created_at: string | null; + metadata: string | null; + day_plan_position: number | null; + needs_review: number; + external_source: string | null; + external_id: string | null; + external_owner_user_id: number | null; + external_synced_at: string | null; + sync_enabled: number | null; + external_hash: string | null; + url: string | null; + ingest_state: string; + day_number: number | null; + place_name: string | null; + accommodation_place_id: number | null; + accommodation_name: string | null; + accommodation_start_day_id: number | null; + accommodation_end_day_id: number | null; +} + +/** + * Kysely typing for the RS18/RR1 join (`joinedQuery` below) — `reservations + * .accommodation_id` is a bare `p.text().nullable()` column with no FK to + * `day_accommodations` (§18.1 of the inventory), so the QueryBuilder's + * relation-path `.join()` cannot express `LEFT JOIN day_accommodations ap ON + * r.accommodation_id = ap.id` at all — the same escape hatch + * `DayAssignmentsRepository.effectiveStart`/`TripsRepository.tripSelectQuery` + * use. `id`/`trip_id` stay `number | string` (raw-bind, D4's T5 seam): a + * Kysely `.where(col, '=', value)` call is a TYPED condition regardless of + * how loosely its value is typed (program rule 23 bans a raw SQL-text + * condition string, not a loosely-typed bound value), so this join keeps the + * legacy statement's exact raw-bind flexibility. + */ +interface ReservationJoinKyselyDB { + reservations: { + id: number | string; + trip_id: number | string; + day_id: number | null; + end_day_id: number | null; + place_id: number | null; + assignment_id: number | null; + title: string; + accommodation_id: string | null; + reservation_time: string | null; + reservation_end_time: string | null; + location: string | null; + confirmation_number: string | null; + notes: string | null; + status: string | null; + type: string | null; + created_at: string | null; + metadata: string | null; + day_plan_position: number | null; + needs_review: number; + external_source: string | null; + external_id: string | null; + external_owner_user_id: number | null; + external_synced_at: string | null; + sync_enabled: number | null; + external_hash: string | null; + url: string | null; + ingest_state: string; + }; + days: { id: number; day_number: number }; + places: { id: number; name: string }; + day_accommodations: { id: number; place_id: number | null; start_day_id: number; end_day_id: number }; +} + +/** + * Kysely typing for `restampLinkedReservation` (DY23). `accommodation_id: + * string | number | null` — R2's ruling: the legacy statement binds `accId` + * (a plain JS `number`, `stay.id`) straight into the `:accId` named param + * compared against the TEXT column, so this method's caller passes the SAME + * number, unconverted, and the interface is widened only so that bind + * type-checks (SQLite applies its own TEXT-affinity conversion at compare + * time regardless of which JS type bound the value — see the method's own + * docstring). + */ +interface ReservationRestampKyselyDB { + reservations: { + id: number; + accommodation_id: string | number | null; + type: string | null; + day_id: number | null; + reservation_time: string | null; + }; +} + +export class ReservationsRepository extends TrekRepository { + private joinedQuery() { + return this.kysely() + .selectFrom('reservations as r') + .leftJoin('days as d', 'd.id', 'r.day_id') + .leftJoin('places as p', 'p.id', 'r.place_id') + .leftJoin('day_accommodations as ap', 'ap.id', 'r.accommodation_id') + .leftJoin('places as acc_p', 'acc_p.id', 'ap.place_id') + .selectAll('r') + .select([ + 'd.day_number as day_number', + 'p.name as place_name', + 'ap.place_id as accommodation_place_id', + 'acc_p.name as accommodation_name', + 'ap.start_day_id as accommodation_start_day_id', + 'ap.end_day_id as accommodation_end_day_id', + ]); + } + + /** + * RS18 (`ReservationsService.list`) — the joined projection for a whole + * trip, `ORDER BY r.reservation_time ASC, r.created_at ASC`. + */ + async listForTrip(trip_id: number | string): Promise { + const rows = await this.joinedQuery() + .where('r.trip_id', '=', trip_id) + .orderBy('r.reservation_time', 'asc') + .orderBy('r.created_at', 'asc') + .execute(); + return rows as ReservationJoinRow[]; + } + + /** + * RR1 (`ReservationsReadService.getReservationWithJoins`) — the SAME + * projection for one reservation, no `ORDER BY` (a single row). + */ + async findWithJoins(id: number | string): Promise { + const row = await this.joinedQuery().where('r.id', '=', id).executeTakeFirst(); + return row as ReservationJoinRow | undefined; + } + + /** + * RS35 (`ReservationsService.getReservation`) — `SELECT * FROM + * reservations WHERE id = ? AND trip_id = ?`, the trip-scoping guard every + * write path re-reads through. `id`/`trip_id: number` (rule 21 — the + * SERVICE parses both ONCE via `toRowId` before calling this, and every + * later write in the same method reuses that same parsed value, never a + * second, independent conversion). + */ + async findInTrip(id: number, trip_id: number): Promise { + return this.qb('r') + .select(['r.*']) + .where({ id, trip: trip_id }) + .execute('get', false); + } + + /** RS45 (`ReservationsService.remove`'s pre-image read) — `SELECT id, title, type, accommodation_id FROM reservations WHERE id = ? AND trip_id = ?`. */ + async findHeaderInTrip(id: number, trip_id: number): Promise<{ id: number; title: string; type: string | null; accommodation_id: string | null } | undefined> { + return this.qb('r') + .select(['r.id', 'r.title', 'r.type', 'r.accommodation_id']) + .where({ id, trip: trip_id }) + .execute<{ id: number; title: string; type: string | null; accommodation_id: string | null } | undefined>('get', false); + } + + /** + * RS12 (`ReservationsService.resyncReservationDays`) — `SELECT id, + * reservation_time, reservation_end_time, day_id, end_day_id FROM + * reservations WHERE trip_id = ? AND (type != 'hotel' OR accommodation_id + * IS NULL) AND reservation_time IS NOT NULL`. + */ + async listResyncCandidates(trip_id: number): Promise<{ + id: number; reservation_time: string | null; reservation_end_time: string | null; + day_id: number | null; end_day_id: number | null; + }[]> { + const platform = this.getEntityManager().getPlatform(); + return this.qb('r') + .select(['r.id', 'r.reservation_time', 'r.reservation_end_time', columnRef(platform, 'r.day_id').as('day_id'), columnRef(platform, 'r.end_day_id').as('end_day_id')]) + .where({ + trip: trip_id, + $or: [{ type: { $ne: 'hotel' } }, { accommodation_id: null }], + reservation_time: { $ne: null }, + }) + .execute<{ id: number; reservation_time: string | null; reservation_end_time: string | null; day_id: number | null; end_day_id: number | null }[]>('all', false); + } + + /** RS13 (`resyncReservationDays`'s per-row write) — `UPDATE reservations SET day_id = ?, end_day_id = ? WHERE id = ?`. */ + async setDays(id: number, day_id: number | null, end_day_id: number | null): Promise { + await this.nativeUpdate({ id }, { day: day_id, endDay: end_day_id }); + } + + /** + * RS28 (`ReservationsService.createInTx`) — `INSERT INTO reservations + * (trip_id, day_id, end_day_id, place_id, assignment_id, title, + * reservation_time, reservation_end_time, location, confirmation_number, + * notes, url, status, type, accommodation_id, metadata, needs_review) + * VALUES (?×17)`. `sync_enabled`/`ingest_state`/`day_plan_position` + * deliberately NOT named, so the column's own DB-level `DEFAULT` applies + * (`sync_enabled INTEGER DEFAULT 1`, `Migration20200101021800_…`; + * `ingest_state` `.default('live')`) — the caller has already resolved + * every value (the `|| null` coercions, `resolvedOrNull`, the numeric + * `accommodation_id` stringified for the TEXT column, R2). + */ + async insertReservation(input: { + trip_id: number; + day_id: number | null; + end_day_id: number | null; + place_id: number | null; + assignment_id: number | null; + title: string; + reservation_time: string | null; + reservation_end_time: string | null; + location: string | null; + confirmation_number: string | null; + notes: string | null; + url: string | null; + status: string; + type: string; + accommodation_id: string | null; + metadata: string | null; + needs_review: number; + }): Promise { + return await this.insert({ + trip: input.trip_id, + day: input.day_id, + endDay: input.end_day_id, + place: input.place_id, + assignment: input.assignment_id, + title: input.title, + reservation_time: input.reservation_time, + reservation_end_time: input.reservation_end_time, + location: input.location, + confirmation_number: input.confirmation_number, + notes: input.notes, + url: input.url, + status: input.status, + type: input.type, + accommodation_id: input.accommodation_id, + metadata: input.metadata, + needs_review: input.needs_review, + }); + } + + /** + * RS41 (`ReservationsService.updateInTx`) — the 16-column `UPDATE + * reservations SET … WHERE id = ?`. The `PlacesRepository.updatePlace`/PL11 + * precedent: the four legacy `COALESCE(?, col)` keep-if-null columns + * (`title`, `status`, `type`, `needs_review`) are resolved to their FINAL + * value by the SERVICE first (it already holds the pre-image via the + * `current` parameter every caller passes), so this writes exactly what it + * is handed — no SQL-side COALESCE. `id: number` (rule 21 — the SAME + * `toRowId`-parsed value `findInTrip`'s gate read used). + */ + async updateReservation(id: number, write: { + title: string; + reservation_time: string | null; + reservation_end_time: string | null; + location: string | null; + confirmation_number: string | null; + notes: string | null; + url: string | null; + day_id: number | null; + end_day_id: number | null; + place_id: number | null; + assignment_id: number | null; + status: string; + type: string; + accommodation_id: string | null; + metadata: string | null; + needs_review: number; + }): Promise { + await this.nativeUpdate({ id }, { + title: write.title, + reservation_time: write.reservation_time, + reservation_end_time: write.reservation_end_time, + location: write.location, + confirmation_number: write.confirmation_number, + notes: write.notes, + url: write.url, + day: write.day_id, + endDay: write.end_day_id, + place: write.place_id, + assignment: write.assignment_id, + status: write.status, + type: write.type, + accommodation_id: write.accommodation_id, + metadata: write.metadata, + needs_review: write.needs_review, + }); + } + + /** + * RS33 (`updatePositions`'s legacy/global branch) — `UPDATE reservations + * SET day_plan_position = ? WHERE id = ? AND trip_id = ?`. `position: + * number | null`, never `undefined` (R8 — the SERVICE maps an absent + * `day_plan_position` to `null` explicitly before calling, so + * `nativeUpdate` always WRITES the column instead of silently skipping it). + */ + async setDayPlanPosition(id: number, trip_id: number, position: number | null): Promise { + await this.nativeUpdate({ id, trip: trip_id }, { day_plan_position: position }); + } + + /** RS50 (`remove`) — `DELETE FROM reservations WHERE id = ?`, unscoped (the caller already proved trip access via `findHeaderInTrip`). */ + async deleteById(id: number): Promise { + await this.nativeDelete({ id }); + } + + /** DY14 (`DaysService.restampReservationDates`) — `SELECT id, day_id, end_day_id, reservation_time, reservation_end_time FROM reservations WHERE trip_id = ?`. */ + async listForRestamp(trip_id: number): Promise<{ + id: number; day_id: number | null; end_day_id: number | null; + reservation_time: string | null; reservation_end_time: string | null; + }[]> { + const platform = this.getEntityManager().getPlatform(); + return this.qb('r') + .select(['r.id', columnRef(platform, 'r.day_id').as('day_id'), columnRef(platform, 'r.end_day_id').as('end_day_id'), 'r.reservation_time', 'r.reservation_end_time']) + .where({ trip: trip_id }) + .execute<{ id: number; day_id: number | null; end_day_id: number | null; reservation_time: string | null; reservation_end_time: string | null }[]>('all', false); + } + + /** DY15 — `UPDATE reservations SET reservation_time = ? WHERE id = ?`. */ + async setReservationTime(id: number, reservation_time: string): Promise { + await this.nativeUpdate({ id }, { reservation_time }); + } + + /** DY16 — `UPDATE reservations SET reservation_end_time = ? WHERE id = ?`. */ + async setReservationEndTime(id: number, reservation_end_time: string): Promise { + await this.nativeUpdate({ id }, { reservation_end_time }); + } + + /** + * DY23 (`DaysService.resyncAccommodationDays`) — the named-param `UPDATE + * reservations SET day_id = :dayId, reservation_time = CASE WHEN + * reservation_time IS NULL THEN :date ELSE :date || SUBSTR(reservation_time, + * 11) END WHERE accommodation_id = :accId AND type = 'hotel'`. Kysely (the + * `CASE WHEN … ELSE … || SUBSTR(...)` shape has no typed-filter + * equivalent — the T6 tier per the inventory's own ruling), through + * `concatKysely`/`substringKysely` (`sql-functions.ts`'s Kysely-expression + * twins of `concat`/`substring` — the MikroORM `RawQueryFragment` forms + * render as a literal `?` token inside a Kysely statement and throw at + * execution, Task 0 review H1). `accommodation_id` (TEXT) compared against + * `accId` (a bound NUMBER, exactly as the legacy statement bound it — R2, + * never stringified here, so the SAME SQLite TEXT-affinity conversion the + * legacy statement triggered still applies, including its own miss on a + * `"14.0"`-shaped row — parity, not a fix). + */ + async restampLinkedReservation(accommodation_id: number, day_id: number, date: string): Promise { + const platform = this.getEntityManager().getPlatform(); + await this.kysely() + .updateTable('reservations') + .set((eb) => ({ + day_id, + reservation_time: eb + .case() + .when('reservation_time', 'is', null) + .then(eb.val(date)) + .else(concatKysely(platform, eb, { value: date }, { expression: substringKysely(platform, eb, 'reservation_time', 11) })) + .end(), + })) + .where('accommodation_id', '=', accommodation_id) + .where('type', '=', 'hotel') + .execute(); + } + + /** + * RS22's dispatch (`ReservationsService.referencesOutsideTrip`'s + * `assignment_id` branch) — `SELECT d.trip_id FROM day_assignments da JOIN + * days d ON da.day_id = d.id WHERE da.id = ?`. Reached through + * `DayAssignmentsRepository`'s own relation join (`da.day` — the same + * cross-repository-read shape `DaysRepository.isEmptyDay`/ + * `TripMembersRepository.rosterUserIds` already use for a table this + * repository does not own), never a new method on that file + * (`DayAssignmentsRepository`/`DaysRepository` are Task 1's + * additive-method territory per the file-ownership split). + */ + async getAssignmentTripId(assignment_id: number): Promise { + const platform = this.getEntityManager().getPlatform(); + const dayAssignments = this.getEntityManager().getRepository(DayAssignments); + const row = await dayAssignments + .qb('da') + .join('da.day', 'd') + .select([columnRef(platform, 'd.trip_id').as('trip_id')]) + .where({ id: assignment_id }) + .execute<{ trip_id: number } | undefined>('get', false); + return row?.trip_id; + } + + /** + * RS11 (`resolveDayIdFromTime`'s nearest-day fallback) — `SELECT id FROM + * days WHERE trip_id = ? ORDER BY ABS(JULIANDAY(date) - JULIANDAY(?)) ASC, + * date ASC LIMIT 1`. `dayDistance`, through `DaysRepository`'s own QB (the + * same cross-repository-read shape as `getAssignmentTripId` above — + * `DaysRepository` stays Task 1's additive-method territory; no new + * method is added there). + */ + async findNearestDayId(trip_id: number, iso_date: string): Promise { + const platform = this.getEntityManager().getPlatform(); + const days = this.getEntityManager().getRepository(Days); + const row = await days + .qb('d') + .select(['d.id']) + .where({ trip: trip_id }) + .orderBy([{ [dayDistance(platform, 'd.date', iso_date)]: 'asc' }, { date: 'asc' }]) + .limit(1) + .execute<{ id: number } | undefined>('get', false); + return row?.id; + } +} + +/** `SELECT *` — every scalar column of `Reservations`, RS35's shape. */ +export interface ReservationAllColumnsRow { + id: number; + trip_id: number; + day_id: number | null; + end_day_id: number | null; + place_id: number | null; + assignment_id: number | null; + title: string; + accommodation_id: string | null; + reservation_time: string | null; + reservation_end_time: string | null; + location: string | null; + confirmation_number: string | null; + notes: string | null; + status: string | null; + type: string | null; + created_at: string | null; + metadata: string | null; + day_plan_position: number | null; + needs_review: number; + external_source: string | null; + external_id: string | null; + external_owner_user_id: number | null; + external_synced_at: string | null; + sync_enabled: number | null; + external_hash: string | null; + url: string | null; + ingest_state: string; +} diff --git a/server/src/nest/days/days.module.ts b/server/src/nest/days/days.module.ts index 3c101a6b4..761d964b6 100644 --- a/server/src/nest/days/days.module.ts +++ b/server/src/nest/days/days.module.ts @@ -15,6 +15,8 @@ import { Days } from '../../db/entities/Days.entity'; import { DayAssignments } from '../../db/entities/DayAssignments.entity'; import { DayNotes } from '../../db/entities/DayNotes.entity'; import { Trips } from '../../db/entities/Trips.entity'; +import { Reservations } from '../../db/entities/Reservations.entity'; +import { ReservationEndpoints } from '../../db/entities/ReservationEndpoints.entity'; /** * Days (S6 — Phase 2 trip sub-domain), mounted at /api/trips/:tripId/days. @@ -25,12 +27,15 @@ import { Trips } from '../../db/entities/Trips.entity'; * Day notes used to live here too, with their own full file set; they are their * own domain now (day-notes/). * - * `MikroOrmModule.forFeature([Days, DayAssignments, DayNotes, Trips])` - * registers `DaysRepository`/`DayAssignmentsRepository`/`DayNotesRepository`/ - * `TripsRepository` for `DaysService`'s `@InjectRepository` constructor - * params (Plan 3c Task 2) — the same forFeature + `@InjectRepository` - * wiring every converted domain copies (`trip-membership.module.ts`'s - * precedent for pulling in a repository this module doesn't otherwise own). + * `MikroOrmModule.forFeature([Days, DayAssignments, DayNotes, Trips, + * Reservations, ReservationEndpoints])` registers `DaysRepository`/ + * `DayAssignmentsRepository`/`DayNotesRepository`/`TripsRepository`/ + * `ReservationsRepository`/`ReservationEndpointsRepository` for + * `DaysService`'s `@InjectRepository` constructor params (Plan 3c Task 2; + * the last two added by Plan 3d Task 2 for DY14–DY18/DY23) — the same + * forFeature + `@InjectRepository` wiring every converted domain copies + * (`trip-membership.module.ts`'s precedent for pulling in a repository this + * module doesn't otherwise own). */ @Module({ imports: [ @@ -41,7 +46,7 @@ import { Trips } from '../../db/entities/Trips.entity'; AuthModule, RealtimeModule, PluginGuardsModule, - MikroOrmModule.forFeature([Days, DayAssignments, DayNotes, Trips]), + MikroOrmModule.forFeature([Days, DayAssignments, DayNotes, Trips, Reservations, ReservationEndpoints]), ], controllers: [DaysController], providers: [DaysService, DaysMcp, DaysRpc], diff --git a/server/src/nest/days/days.service.ts b/server/src/nest/days/days.service.ts index 3603d41a7..74d6cef5d 100644 --- a/server/src/nest/days/days.service.ts +++ b/server/src/nest/days/days.service.ts @@ -17,6 +17,10 @@ import { DayNotes } from '../../db/entities/DayNotes.entity'; import type { DayNotesRepository } from '../../db/repositories/DayNotes.repository'; 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 { ReservationEndpoints } from '../../db/entities/ReservationEndpoints.entity'; +import type { ReservationEndpointsRepository } from '../../db/repositories/ReservationEndpoints.repository'; type Trip = TripAccess; @@ -114,6 +118,12 @@ export class DaysService { @InjectRepository(DayAssignments) private readonly dayAssignmentsRepo: DayAssignmentsRepository, @InjectRepository(DayNotes) private readonly dayNotesRepo: DayNotesRepository, @InjectRepository(Trips) private readonly tripsRepo: TripsRepository, + // Plan 3d Task 2 (DY14–DY18, DY23): `restampReservationDates`/ + // `resyncAccommodationDays`'s `reservations`/`reservation_endpoints` + // statements convert onto these two — the only new constructor params + // this task adds (`day_accommodations` stays raw here, Task 3's own). + @InjectRepository(Reservations) private readonly reservationsRepo: ReservationsRepository, + @InjectRepository(ReservationEndpoints) private readonly reservationEndpointsRepo: ReservationEndpointsRepository, ) {} async verifyTripAccess(tripId: string | number, userId: number) { @@ -336,9 +346,11 @@ export class DaysService { * date (time-of-day preserved). Transport endpoints (flight legs) shift by the * same per-booking day delta so multi-leg timing stays internally consistent. * - * DY14–DY18 — Plan 3d (`reservations`/`reservation_endpoints`): stays raw on - * `DatabaseService` inside the caller's `uow.transactional` block, per the - * Plan 3b `UserCleanupService` precedent for cross-domain writes. + * DY14–DY18 — Plan 3d Task 2: converted onto `ReservationsRepository + * .listForRestamp`/`setReservationTime`/`setReservationEndTime` and + * `ReservationEndpointsRepository.listIdAndDate`/`setLocalDate`, inside the + * caller's `uow.transactional` block (unchanged — repository methods never + * open their own transaction, program brief item 7). * * `tripId: number` (Task 9 fix wave, H2): every caller now passes the ONE * `toRowId`-parsed value its own entry point already computed, never the @@ -353,34 +365,22 @@ export class DaysService { newDateById: Map, ): Promise { // DY14 — Plan 3d - const reservations = this.db.all<{ - id: number; day_id: number | null; end_day_id: number | null; - reservation_time: string | null; reservation_end_time: string | null; - }>( - 'SELECT id, day_id, end_day_id, reservation_time, reservation_end_time FROM reservations WHERE trip_id = ?', - tripId - ); - - // DY15 — Plan 3d - const setTime = this.db.prepare('UPDATE reservations SET reservation_time = ? WHERE id = ?'); - // DY16 — Plan 3d - const setEndTime = this.db.prepare('UPDATE reservations SET reservation_end_time = ? WHERE id = ?'); - // DY17 — Plan 3d - const endpoints = this.db.prepare('SELECT id, local_date FROM reservation_endpoints WHERE reservation_id = ?'); - // DY18 — Plan 3d - const setEndpointDate = this.db.prepare('UPDATE reservation_endpoints SET local_date = ? WHERE id = ?'); + const reservations = await this.reservationsRepo.listForRestamp(tripId); for (const r of reservations) { if (r.day_id != null && r.reservation_time) { const oldDate = oldDateById.get(r.day_id); const newDate = newDateById.get(r.day_id); if (oldDate && newDate && oldDate !== newDate) { - setTime.run(withDatePart(r.reservation_time, newDate), r.id); + // DY15 — Plan 3d + await this.reservationsRepo.setReservationTime(r.id, withDatePart(r.reservation_time, newDate)); // Shift each transport leg's local_date by the same number of days. const delta = dayDelta(oldDate, newDate); if (delta !== 0) { - for (const ep of endpoints.all(r.id) as { id: number; local_date: string | null }[]) { - if (ep.local_date) setEndpointDate.run(addDays(ep.local_date, delta), ep.id); + // DY17 — Plan 3d + for (const ep of await this.reservationEndpointsRepo.listIdAndDate(r.id)) { + // DY18 — Plan 3d + if (ep.local_date) await this.reservationEndpointsRepo.setLocalDate(ep.id, addDays(ep.local_date, delta)); } } } @@ -389,7 +389,8 @@ export class DaysService { const oldDate = oldDateById.get(r.end_day_id); const newDate = newDateById.get(r.end_day_id); if (oldDate && newDate && oldDate !== newDate) { - setEndTime.run(withDatePart(r.reservation_end_time, newDate), r.id); + // DY16 — Plan 3d + await this.reservationsRepo.setReservationEndTime(r.id, withDatePart(r.reservation_end_time, newDate)); } } } @@ -408,6 +409,10 @@ export class DaysService { * docstring for why binding the raw route string here was the live bug. */ private async assertNoInvertedAccommodation(tripId: number): Promise { + // DY19 — Plan 3d (marker normalised to the `// — Plan 3d` shape + // every other survivor in this file uses, per the Task 0 review carry + // item — this task does not own `day_accommodations`, so the statement + // itself is untouched) const spans = this.db.all<{ id: number; start_no: number; end_no: number }>(` SELECT a.id, s.day_number AS start_no, e.day_number AS end_no FROM day_accommodations a @@ -432,13 +437,14 @@ export class DaysService { * whole trip still shifts everything together. The linked hotel reservation follows * its accommodation's start day in both branches. * - * DY20/DY22/DY23 — Plan 3d (`day_accommodations`/`reservations`) stay raw; + * DY20/DY22 — Plan 3d (`day_accommodations`) stay raw, Task 3's own table; * DY21 (`DaysRepository.findByTripAndDate`) and DY25 * (`DaysRepository.findById`) convert — both root on `days`, which this * plan owns. DY24 (the `day_assignments` stop that follows its booking) * converts via `DayAssignmentsRepository.reanchorToDay`, a Kysely * statement proven to join this method's ambient transaction by rollback - * (see the task report). + * (see the task report). DY23 (`reservations`) converts onto + * `ReservationsRepository.restampLinkedReservation` — Plan 3d Task 2. * * `tripId: number` (Task 9 fix wave, H2): callers now pass their own * `toRowId`-parsed value; DY20's raw bind uses it too (previously the raw @@ -458,13 +464,6 @@ export class DaysService { // DY22 — Plan 3d const updateStay = this.db.prepare('UPDATE day_accommodations SET start_day_id = ?, end_day_id = ? WHERE id = ?'); - // DY23 — Plan 3d - const restampLinkedRes = this.db.prepare(` - UPDATE reservations SET day_id = :dayId, - reservation_time = CASE WHEN reservation_time IS NULL THEN :date - ELSE :date || SUBSTR(reservation_time, 11) END - WHERE accommodation_id = :accId AND type = 'hotel' - `); for (const stay of stays) { const oldStartDate = prevDateByDayId.get(stay.start_day_id); @@ -489,7 +488,8 @@ export class DaysService { // DY25 const startDay = await this.daysRepo.findById(stay.start_day_id); if (startDay?.date) { - restampLinkedRes.run({ dayId: stay.start_day_id, date: startDay.date, accId: stay.id }); + // DY23 — Plan 3d + await this.reservationsRepo.restampLinkedReservation(stay.id, stay.start_day_id, startDay.date); } } } diff --git a/server/src/nest/reservations/reservations-read.module.ts b/server/src/nest/reservations/reservations-read.module.ts index 20cd0e1ca..fbe16c350 100644 --- a/server/src/nest/reservations/reservations-read.module.ts +++ b/server/src/nest/reservations/reservations-read.module.ts @@ -1,12 +1,24 @@ import { Module } from '@nestjs/common'; +import { MikroOrmModule } from '@mikro-orm/nestjs'; import { ReservationsReadService } from './reservations-read.service'; +import { Reservations } from '../../db/entities/Reservations.entity'; +import { ReservationEndpoints } from '../../db/entities/ReservationEndpoints.entity'; +import { ReservationTravelers } from '../../db/entities/ReservationTravelers.entity'; /** * A leaf on purpose (the trip-membership precedent): AirtrailCoreModule needs * the single-reservation hydration reads without dragging ReservationsModule * in — which would re-close the cycle that airtrail.bridge used to dodge. + * + * `MikroOrmModule.forFeature([Reservations, ReservationEndpoints, + * ReservationTravelers])` registers `ReservationsRepository`/ + * `ReservationEndpointsRepository`/`ReservationTravelersRepository` for + * `ReservationsReadService`'s `@InjectRepository` constructor params (Plan + * 3d Task 2, R11) — the entity classes only, not `ReservationsModule` + * itself, so this module stays the same leaf it always was. */ @Module({ + imports: [MikroOrmModule.forFeature([Reservations, ReservationEndpoints, ReservationTravelers])], providers: [ReservationsReadService], exports: [ReservationsReadService], }) diff --git a/server/src/nest/reservations/reservations-read.service.ts b/server/src/nest/reservations/reservations-read.service.ts index dc2a7777d..4544c0a0f 100644 --- a/server/src/nest/reservations/reservations-read.service.ts +++ b/server/src/nest/reservations/reservations-read.service.ts @@ -1,7 +1,13 @@ import { Injectable } from '@nestjs/common'; -import { DatabaseService } from '../database/database.service'; +import { InjectRepository } from '@mikro-orm/nestjs'; import { avatarUrl } from '../common/avatarUrl'; import type { ReservationRow, ReservationEndpoint, ReservationTraveler } from './reservations.service'; +import { Reservations } from '../../db/entities/Reservations.entity'; +import type { ReservationsRepository } from '../../db/repositories/Reservations.repository'; +import { ReservationEndpoints } from '../../db/entities/ReservationEndpoints.entity'; +import type { ReservationEndpointsRepository } from '../../db/repositories/ReservationEndpoints.repository'; +import { ReservationTravelers } from '../../db/entities/ReservationTravelers.entity'; +import type { ReservationTravelersRepository } from '../../db/repositories/ReservationTravelers.repository'; /** The one traveler projection every reservation read shares (avatar_url included). */ export function toTraveler(r: { user_id: number; username: string; avatar: string | null; is_guest?: number | null }): ReservationTraveler { @@ -19,56 +25,47 @@ export function toTraveler(r: { user_id: number; username: string; avatar: strin * delegates, so there is exactly one copy of each query. * * Renamed from `ReservationsReadRepository` (Plan 3d Task 0, R11 — inventory - * §18.3): this is a plain Nest `@Injectable` over `DatabaseService.prepare`, - * not a MikroORM repository extending `TrekRepository`, so it must not carry - * the `Repository` suffix — that name is reserved for the ORM layer's "one - * repository per entity" convention (D4). Its three statements (RR1-RR3) - * still convert to `ReservationsRepository`/`ReservationEndpointsRepository`/ - * `ReservationTravelersRepository` methods in a later Plan 3d task (per §18.3 - * option (a), which then deletes this class and turns - * `ReservationsReadModule` into a plain `MikroOrmModule.forFeature([...])` - * leaf, the `TripMembershipModule` precedent) — Task 0 converts no SQL site, - * so only the rename happens here, with zero behaviour change. + * §18.3): this is a plain Nest `@Injectable`, not a MikroORM repository + * extending `TrekRepository`, so it must not carry the `Repository` suffix — + * that name is reserved for the ORM layer's "one repository per entity" + * convention (D4). Its three statements (RR1-RR3) now delegate to + * `ReservationsRepository.findWithJoins`/`ReservationEndpointsRepository + * .listForReservation`/`ReservationTravelersRepository.listForReservation` + * (Plan 3d Task 2, §18.3 option (b) — the class stays a thin facade rather + * than being deleted: `AirtrailCoreModule`/its own test suites reach it by + * this exact public shape, and Task 0's rename already committed to keeping + * it). Zero behaviour change beyond the SQL leaving this file. */ @Injectable() export class ReservationsReadService { - constructor(private readonly db: DatabaseService) {} + constructor( + @InjectRepository(Reservations) private readonly reservationsRepo: ReservationsRepository, + @InjectRepository(ReservationEndpoints) private readonly endpointsRepo: ReservationEndpointsRepository, + @InjectRepository(ReservationTravelers) private readonly travelersRepo: ReservationTravelersRepository, + ) {} async getReservationWithJoins(id: string | number): Promise { - const row = this.db.get(` - SELECT r.*, d.day_number, p.name as place_name, r.assignment_id, - ap.place_id as accommodation_place_id, acc_p.name as accommodation_name, - ap.start_day_id as accommodation_start_day_id, ap.end_day_id as accommodation_end_day_id - FROM reservations r - LEFT JOIN days d ON r.day_id = d.id - LEFT JOIN places p ON r.place_id = p.id - LEFT JOIN day_accommodations ap ON r.accommodation_id = ap.id - LEFT JOIN places acc_p ON ap.place_id = acc_p.id - WHERE r.id = ? - `, id); + // RR1 + const row = await this.reservationsRepo.findWithJoins(id); if (!row) return undefined; - row.endpoints = await this.loadEndpoints(row.id); - row.travelers = await this.loadTravelers(row.id); + const result: ReservationRow = { ...row }; + result.endpoints = await this.loadEndpoints(row.id); + result.travelers = await this.loadTravelers(row.id); // accommodation_id is a TEXT column; the integer FK reads back as a numeric // string (e.g. "14.0"). Normalize to an int so clients can parse it. - row.accommodation_id = row.accommodation_id == null ? null : Math.trunc(Number(row.accommodation_id)); - return row; + result.accommodation_id = row.accommodation_id == null ? null : Math.trunc(Number(row.accommodation_id)); + return result; } async loadEndpoints(reservationId: number): Promise { - return this.db.all( - 'SELECT * FROM reservation_endpoints WHERE reservation_id = ? ORDER BY sequence', - reservationId - ); + // RR2 — no ORDER BY difference: `ReservationEndpointsRepository + // .listForReservation` already orders by `sequence` (RR2's own shape). + return (await this.endpointsRepo.listForReservation(reservationId)) as ReservationEndpoint[]; } async loadTravelers(reservationId: number | string): Promise { - const rows = this.db.all(` - SELECT rt.user_id, COALESCE(u.display_name, u.username) AS username, u.avatar, u.is_guest - FROM reservation_travelers rt - JOIN users u ON rt.user_id = u.id - WHERE rt.reservation_id = ? - `, reservationId); + // RR3 — no ORDER BY (unlike RS6), matching the legacy statement's own row order. + const rows = await this.travelersRepo.listForReservation(Number(reservationId)); return rows.map(r => toTraveler(r)); } } diff --git a/server/src/nest/reservations/reservations.module.ts b/server/src/nest/reservations/reservations.module.ts index c4aa1dd3f..29f2e5bce 100644 --- a/server/src/nest/reservations/reservations.module.ts +++ b/server/src/nest/reservations/reservations.module.ts @@ -1,4 +1,5 @@ import { Module } from '@nestjs/common'; +import { MikroOrmModule } from '@mikro-orm/nestjs'; import { ReservationsReadModule } from './reservations-read.module'; import { AirtrailCoreModule } from '../integrations/airtrail-core.module'; import { BudgetModule } from '../budget/budget.module'; @@ -16,12 +17,33 @@ import { AssignmentsModule } from '../assignments/assignments.module'; import { AccommodationsModule } from '../accommodations/accommodations.module'; import { NotificationsModule } from '../notifications/notifications.module'; import { McpSharedModule } from '../mcp-shared/mcp-shared.module'; +import { Reservations } from '../../db/entities/Reservations.entity'; +import { ReservationEndpoints } from '../../db/entities/ReservationEndpoints.entity'; +import { ReservationTravelers } from '../../db/entities/ReservationTravelers.entity'; +import { ReservationDayPositions } from '../../db/entities/ReservationDayPositions.entity'; +import { DayAccommodations } from '../../db/entities/DayAccommodations.entity'; +import { Days } from '../../db/entities/Days.entity'; +import { Places } from '../../db/entities/Places.entity'; +import { DayAssignments } from '../../db/entities/DayAssignments.entity'; +import { TripMembers } from '../../db/entities/TripMembers.entity'; +import { Users } from '../../db/entities/Users.entity'; +import { Trips } from '../../db/entities/Trips.entity'; /** * Reservations + accommodations domain (S5 — Phase 2 trip sub-domain). * Mounts: /api/trips/:tripId/reservations, /accommodations, and the cross-trip * /api/reservations/upcoming dashboard feed. ReservationsMcp carries the * decorator-registered MCP tools + resource for the domain. + * + * `MikroOrmModule.forFeature([...])` registers `ReservationsRepository`/ + * `ReservationEndpointsRepository`/`ReservationTravelersRepository`/ + * `ReservationDayPositionsRepository`/`DayAccommodationsRepository` (this + * task's own) plus `DaysRepository`/`PlacesRepository`/ + * `DayAssignmentsRepository`/`TripMembersRepository`/`UsersRepository`/ + * `TripsRepository` (owned elsewhere, reached the same way + * `AssignmentsDomainModule` reaches `Days`/`Places`/`TripMembers` — the + * entity classes only, never the owning module) for `ReservationsService`'s + * `@InjectRepository` constructor params (Plan 3d Task 2). */ @Module({ // DaysModule: ReservationsMcp injects DaysService for its nine getDay calls. @@ -29,7 +51,10 @@ import { McpSharedModule } from '../mcp-shared/mcp-shared.module'; // AccommodationsModule: a hotel booking writes its own day_accommodations row, and // that stay owes the day plan the same stop one entered under Days does. No edge // back — accommodations reaches neither days nor reservations (ACC-002). - imports: [McpSharedModule, NotificationsModule, DaysModule, AssignmentsModule, AccommodationsModule, PermissionsModule, BudgetModule, AuthModule, RealtimeModule, PluginGuardsModule, ReservationsReadModule, AirtrailCoreModule], + imports: [ + McpSharedModule, NotificationsModule, DaysModule, AssignmentsModule, AccommodationsModule, PermissionsModule, BudgetModule, AuthModule, RealtimeModule, PluginGuardsModule, ReservationsReadModule, AirtrailCoreModule, + MikroOrmModule.forFeature([Reservations, ReservationEndpoints, ReservationTravelers, ReservationDayPositions, DayAccommodations, Days, Places, DayAssignments, TripMembers, Users, Trips]), + ], controllers: [ReservationsController, UpcomingReservationsController], providers: [ReservationsService, ReservationsMcp, ReservationsRpc], // For in-container consumers (ReservationsRpc, TripsService, BookingImportService). diff --git a/server/src/nest/reservations/reservations.service.ts b/server/src/nest/reservations/reservations.service.ts index 36a5bc42a..b16b1d58d 100644 --- a/server/src/nest/reservations/reservations.service.ts +++ b/server/src/nest/reservations/reservations.service.ts @@ -1,4 +1,5 @@ import { BadRequestException, Injectable } from '@nestjs/common'; +import { InjectRepository } from '@mikro-orm/nestjs'; import { DatabaseService, type TripAccess } from '../database/database.service'; import { UnitOfWork } from '../database/unit-of-work'; import type { TrekWsPayload, TrekWsTripEventName } from '@trek/shared'; @@ -11,6 +12,29 @@ import { BudgetService } from '../budget/budget.service'; import { typeToCostCategory } from '@trek/shared'; import { NotificationsService } from '../notifications/notifications.service'; import { AccommodationsService, noStayMirror, type AccommodationMirror } from '../accommodations/accommodations.service'; +import { toRowId } from '../common/row-id'; +import { Reservations } from '../../db/entities/Reservations.entity'; +import type { ReservationsRepository } from '../../db/repositories/Reservations.repository'; +import { ReservationEndpoints as ReservationEndpointsEntity } from '../../db/entities/ReservationEndpoints.entity'; +import type { ReservationEndpointsRepository, ReservationEndpointRow } from '../../db/repositories/ReservationEndpoints.repository'; +import { ReservationTravelers as ReservationTravelersEntity } from '../../db/entities/ReservationTravelers.entity'; +import type { ReservationTravelersRepository } from '../../db/repositories/ReservationTravelers.repository'; +import { ReservationDayPositions } from '../../db/entities/ReservationDayPositions.entity'; +import type { ReservationDayPositionsRepository } from '../../db/repositories/ReservationDayPositions.repository'; +import { DayAccommodations } from '../../db/entities/DayAccommodations.entity'; +import type { DayAccommodationsRepository } from '../../db/repositories/DayAccommodations.repository'; +import { Days } from '../../db/entities/Days.entity'; +import type { DaysRepository } from '../../db/repositories/Days.repository'; +import { Places } from '../../db/entities/Places.entity'; +import type { PlacesRepository } from '../../db/repositories/Places.repository'; +import { DayAssignments } from '../../db/entities/DayAssignments.entity'; +import type { DayAssignmentsRepository } from '../../db/repositories/DayAssignments.repository'; +import { TripMembers } from '../../db/entities/TripMembers.entity'; +import type { TripMembersRepository } from '../../db/repositories/TripMembers.repository'; +import { Users } from '../../db/entities/Users.entity'; +import type { UsersRepository } from '../../db/repositories/Users.repository'; +import { Trips } from '../../db/entities/Trips.entity'; +import type { TripsRepository } from '../../db/repositories/Trips.repository'; type Trip = TripAccess; type BudgetEntry = { total_price?: number; category?: string } | undefined; @@ -153,12 +177,40 @@ export class ReservationsService { private readonly reads: ReservationsReadService, private readonly accommodations: AccommodationsService, private readonly uow: UnitOfWork, + @InjectRepository(Reservations) private readonly reservationsRepo: ReservationsRepository, + @InjectRepository(ReservationEndpointsEntity) private readonly endpointsRepo: ReservationEndpointsRepository, + @InjectRepository(ReservationTravelersEntity) private readonly travelersRepo: ReservationTravelersRepository, + @InjectRepository(ReservationDayPositions) private readonly dayPositionsRepo: ReservationDayPositionsRepository, + @InjectRepository(DayAccommodations) private readonly dayAccommodationsRepo: DayAccommodationsRepository, + @InjectRepository(Days) private readonly daysRepo: DaysRepository, + @InjectRepository(Places) private readonly placesRepo: PlacesRepository, + @InjectRepository(DayAssignments) private readonly dayAssignmentsRepo: DayAssignmentsRepository, + @InjectRepository(TripMembers) private readonly tripMembersRepo: TripMembersRepository, + @InjectRepository(Users) private readonly usersRepo: UsersRepository, + @InjectRepository(Trips) private readonly tripsRepo: TripsRepository, ) {} async verifyTripAccess(tripId: string | number, userId: number) { return await this.db.canAccessTrip(tripId, userId); } + /** + * `tripId` reaches every read/write below only downstream of + * `TripAccessGuard` (REST) or an equivalent MCP/RPC check, which already + * resolved it to a real trip with its own `Number()` coercion — this + * mirrors that same coercion for repository calls whose typed filters + * require a genuine `number` (program rule 23). `Number.isFinite`, not a + * bare `Number(...)`: MikroORM inlines every bound parameter into the SQL + * text (rule 22), so a stray `NaN` would render as the bare token `NaN` + * and throw at prepare time (rule 15) rather than simply matching no + * rows — `-1` never matches a real (positive, autoincrement) trip id, so + * an already-impossible tripId here degrades to "no rows", never a 500. + */ + private rowIdNum(value: string | number): number { + const n = Number(value); + return Number.isFinite(n) ? n : -1; + } + async canEdit(trip: Trip, user: User): Promise { return this.permissions.checkPermission('reservation_edit', user.role, trip.user_id, user.id, trip.user_id !== user.id); } @@ -191,17 +243,19 @@ export class ReservationsService { // reaches nothing in this direction — and it hid the edge while handing the // send a second NotificationsService built outside the container. try { - const actor = this.db.get<{ email: string }>('SELECT email FROM users WHERE id = ?', actorId); - if (!actor) return; - const trip = this.db.get<{ title: string }>('SELECT title FROM trips WHERE id = ?', tripId); + // RS1 + const actorEmail = await this.usersRepo.getEmail(actorId); + if (!actorEmail) return; + // RS2 + const tripTitle = await this.tripsRepo.getTitle(tripId); this.notifications.send({ event: 'booking_change', actorId, scope: 'trip', targetId: Number(tripId), params: { - trip: trip?.title || 'Untitled', - actor: actor.email, + trip: tripTitle || 'Untitled', + actor: actorEmail, booking, type: type || 'booking', tripId: String(tripId), @@ -213,39 +267,34 @@ export class ReservationsService { } async loadEndpointsByTrip(tripId: string | number): Promise> { - const rows = this.db.all(` - SELECT e.* FROM reservation_endpoints e - JOIN reservations r ON e.reservation_id = r.id - WHERE r.trip_id = ? - ORDER BY e.reservation_id, e.sequence - `, tripId); + // RS3 + const rows: ReservationEndpointRow[] = await this.endpointsRepo.listForTrip(this.rowIdNum(tripId)); const map = new Map(); for (const r of rows) { - const list = map.get(r.reservation_id!) ?? []; - list.push(r); - map.set(r.reservation_id!, list); + const list = map.get(r.reservation_id) ?? []; + // `role` is TEXT at the DB (unconstrained); `ReservationEndpoint.role` + // narrows it to the three values every writer of this column sends — + // the same widen-on-read the legacy `this.db.all` + // generic parameter asserted without any runtime check. + list.push(r as ReservationEndpoint); + map.set(r.reservation_id, list); } return map; } - /** Users assignable on a trip: its members (guests included) plus the owner. */ + /** + * Users assignable on a trip: its members (guests included) plus the + * owner. RS4+RS5 — `TripMembersRepository.rosterUserIds` (the same two + * reads, merged through the same `Set`, per the inventory's §18.9 ruling + * that the two are provably identical). + */ private async assignableUserIds(tripId: string | number): Promise> { - const members = this.db.all<{ user_id: number }>('SELECT user_id FROM trip_members WHERE trip_id = ?', tripId); - const ids = new Set(members.map(m => m.user_id)); - const owner = this.db.get<{ user_id: number }>('SELECT user_id FROM trips WHERE id = ?', tripId); - if (owner) ids.add(owner.user_id); - return ids; + return this.tripMembersRepo.rosterUserIds(tripId); } async loadTravelersByTrip(tripId: string | number): Promise> { - const rows = this.db.all(` - SELECT rt.reservation_id, rt.user_id, COALESCE(u.display_name, u.username) AS username, u.avatar, u.is_guest - FROM reservation_travelers rt - JOIN reservations r ON rt.reservation_id = r.id - JOIN users u ON rt.user_id = u.id - WHERE r.trip_id = ? - ORDER BY rt.reservation_id - `, tripId); + // RS6 + const rows = await this.travelersRepo.listForTrip(this.rowIdNum(tripId)); const map = new Map(); for (const row of rows) { const list = map.get(row.reservation_id) ?? []; @@ -265,14 +314,15 @@ export class ReservationsService { * can't leak a cross-trip user. #1517. */ async setReservationTravelers(reservationId: number | string, tripId: string | number, userIds: number[]): Promise { + // RS7: the roster is computed BEFORE the transaction opens (unchanged). const allowed = await this.assignableUserIds(tripId); const ids = [...new Set(userIds)].filter(uid => allowed.has(uid)); + const reservationIdNum = this.rowIdNum(reservationId); await this.uow.transactional(async () => { - this.db.run('DELETE FROM reservation_travelers WHERE reservation_id = ?', reservationId); - if (ids.length > 0) { - const insert = this.db.prepare('INSERT OR IGNORE INTO reservation_travelers (reservation_id, user_id) VALUES (?, ?)'); - for (const uid of ids) insert.run(reservationId, uid); - } + // RS8 + await this.travelersRepo.deleteForReservation(reservationIdNum); + // RS9 + await this.travelersRepo.insertIgnore(reservationIdNum, ids); }); } @@ -296,18 +346,17 @@ export class ReservationsService { if (!time) return null; const datePart = time.slice(0, 10); if (!/^\d{4}-\d{2}-\d{2}$/.test(datePart)) return null; - const exact = this.db.get<{ id: number }>('SELECT id FROM days WHERE trip_id = ? AND date = ? LIMIT 1', tripId, datePart); + // RS10 + const exact = await this.daysRepo.findByTripAndDate(this.rowIdNum(tripId), datePart); if (exact) return exact.id; // Fallback: clamp to the nearest day in the trip so an imported booking whose // exact date has no day row (or sits just outside the span) still lands on a day. // Skipped by callers (e.g. resyncReservationDays) that must leave a booking whose // date now falls outside the range untouched instead of snapping it to an edge day. if (!clampToNearest) return null; - const nearest = this.db.get<{ id: number }>( - 'SELECT id FROM days WHERE trip_id = ? ORDER BY ABS(JULIANDAY(date) - JULIANDAY(?)) ASC, date ASC LIMIT 1', - tripId, datePart - ); - return nearest?.id ?? null; + // RS11 + const nearestId = await this.reservationsRepo.findNearestDayId(this.rowIdNum(tripId), datePart); + return nearestId ?? null; } // After a trip's date range changes, generateDays positionally re-dates the day rows @@ -319,16 +368,8 @@ export class ReservationsService { // resyncAccommodationDays re-anchors the accommodation span and its linked reservation; // unlinked dated hotels (e.g. imported ones) re-anchor like any other booking. async resyncReservationDays(tripId: string | number): Promise { - const rows = this.db.all<{ - id: number; reservation_time: string | null; reservation_end_time: string | null; - day_id: number | null; end_day_id: number | null; - }>( - `SELECT id, reservation_time, reservation_end_time, day_id, end_day_id - FROM reservations - WHERE trip_id = ? AND (type != 'hotel' OR accommodation_id IS NULL) AND reservation_time IS NOT NULL`, - tripId - ); - const update = this.db.prepare('UPDATE reservations SET day_id = ?, end_day_id = ? WHERE id = ?'); + // RS12 — read BEFORE the transaction (inside TripsService.updateTrip's outer tx), unchanged. + const rows = await this.reservationsRepo.listResyncCandidates(this.rowIdNum(tripId)); await this.uow.transactional(async () => { for (const r of rows) { const newDayId = await this.resolveDayIdFromTime(tripId, r.reservation_time, false); @@ -337,7 +378,8 @@ export class ReservationsService { ? ((await this.resolveDayIdFromTime(tripId, r.reservation_end_time, false)) ?? r.end_day_id) : r.end_day_id; if (newDayId !== r.day_id || newEndDayId !== r.end_day_id) { - update.run(newDayId, newEndDayId, r.id); + // RS13 + await this.reservationsRepo.setDays(r.id, newDayId, newEndDayId); } } }); @@ -352,43 +394,46 @@ export class ReservationsService { // bound its transaction lazily per call for the same reason ("The // database connection is not open"). await this.uow.transactional(async () => { - this.db.run('DELETE FROM reservation_endpoints WHERE reservation_id = ?', reservationId); - const insert = this.db.prepare(` - INSERT INTO reservation_endpoints (reservation_id, role, sequence, name, code, lat, lng, timezone, local_time, local_date) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?) - `); + // RS16 + await this.endpointsRepo.deleteForReservation(reservationId); // lat/lng are NOT NULL: an imported transport whose pick-up/return (or station/ // stop) couldn't be geocoded reaches here with null coords. Skip those rows rather // than let the INSERT throw and fail the entire booking save — the dates still live // on reservation_time/reservation_end_time, so the booking lands on its day either way. - endpoints - .filter((e) => e.lat != null && e.lng != null) - .forEach((e, i) => { - insert.run(reservationId, e.role, e.sequence ?? i, e.name, e.code ?? null, e.lat, e.lng, e.timezone ?? null, e.local_time ?? null, e.local_date ?? null); + const filtered = endpoints.filter((e) => e.lat != null && e.lng != null); + for (const [i, e] of filtered.entries()) { + // RS17 + await this.endpointsRepo.insertEndpoint({ + reservation_id: reservationId, + role: e.role, + sequence: e.sequence ?? i, + name: e.name, + code: e.code ?? null, + lat: e.lat!, + lng: e.lng!, + timezone: e.timezone ?? null, + local_time: e.local_time ?? null, + local_date: e.local_date ?? null, }); + } }); } async list(tripId: string | number) { - const reservations = this.db.all(` - SELECT r.*, d.day_number, p.name as place_name, r.assignment_id, - ap.place_id as accommodation_place_id, acc_p.name as accommodation_name, - ap.start_day_id as accommodation_start_day_id, ap.end_day_id as accommodation_end_day_id - FROM reservations r - LEFT JOIN days d ON r.day_id = d.id - LEFT JOIN places p ON r.place_id = p.id - LEFT JOIN day_accommodations ap ON r.accommodation_id = ap.id - LEFT JOIN places acc_p ON ap.place_id = acc_p.id - WHERE r.trip_id = ? - ORDER BY r.reservation_time ASC, r.created_at ASC - `, tripId); + // RS18. `ReservationRow`'s `status`/`type` are declared non-nullable + // (every write path here always supplies a value — `status || 'pending'` + // / `type || 'other'`, RS28/RS41) even though the physical columns are + // nullable; the repository's honest `ReservationJoinRow` type keeps them + // `string | null`. Neither the legacy raw row nor this one coerces a + // genuine NULL at read time — spreading into a fresh object literal + // (assignable to `ReservationRow`'s index signature the same way a + // literal always is) changes only the compile-time claim, never a + // runtime value, so a row a write path here never produces (a + // hand-edited NULL) still reads back as `null` on the wire. + const reservations: ReservationRow[] = (await this.reservationsRepo.listForTrip(tripId)).map((r) => ({ ...r })); - const dayPositions = this.db.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); + // RS19 + const dayPositions = await this.dayPositionsRepo.listForTrip(this.rowIdNum(tripId)); const posMap = new Map>(); for (const dp of dayPositions) { @@ -431,6 +476,12 @@ export class ReservationsService { * once and never restamps it, so moving the stay left the widget showing the * old date. Both halves of the report come from reading a hotel's date off * 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. */ async listUpcoming(userId: number, limit = 6) { const today = new Date().toISOString().slice(0, 10); @@ -536,32 +587,45 @@ export class ReservationsService { // and refusing the whole write instead would make such a booking // permanently uneditable. Only a row that exists in a DIFFERENT trip is one // the caller must not have reached for. - const elsewhere = (table: 'days' | 'places' | 'day_accommodations', id: unknown) => { - const row = this.db.get<{ trip_id: number }>(`SELECT trip_id FROM ${table} WHERE id = ?`, id); - return !!row && String(row.trip_id) !== String(tripId); + // + // RS21 — a CLOSED union of table names dispatched to typed repository + // calls (never an interpolated identifier): `DaysRepository.findById`, + // the inherited `PlacesRepository.findOne` and + // `DayAccommodationsRepository.getTripId`. R1: this was a SYNCHRONOUS + // closure used as a boolean in the legacy code — now genuinely async, so + // every call site below `await`s it; the async-closure mutation harness + // (`tests/integration/async-closure-truthiness-guards.test.ts`, + // RS21-001/RS21-002) proves a dropped `await` goes red. + const elsewhere = async (table: 'days' | 'places' | 'day_accommodations', id: number): Promise => { + let rowTripId: number | undefined; + if (table === 'days') { + rowTripId = (await this.daysRepo.findById(id))?.trip_id; + } else if (table === 'places') { + rowTripId = (await this.placesRepo.findOne({ id }))?.trip_id; + } else { + rowTripId = await this.dayAccommodationsRepo.getTripId(id); + } + return rowTripId !== undefined && String(rowTripId) !== String(tripId); }; const check = (field: string, offending: boolean) => { if (offending) offenders.push(field); }; - if (data.day_id != null) check('day_id', elsewhere('days', data.day_id)); - if (data.end_day_id != null) check('end_day_id', elsewhere('days', data.end_day_id)); - if (data.place_id != null) check('place_id', elsewhere('places', data.place_id)); - if (data.accommodation_id != null) check('accommodation_id', elsewhere('day_accommodations', data.accommodation_id)); + if (data.day_id != null) check('day_id', await elsewhere('days', data.day_id)); + if (data.end_day_id != null) check('end_day_id', await elsewhere('days', data.end_day_id)); + if (data.place_id != null) check('place_id', await elsewhere('places', data.place_id)); + if (data.accommodation_id != null) check('accommodation_id', await elsewhere('day_accommodations', data.accommodation_id)); if (data.assignment_id != null) { // An assignment belongs to a trip through its day, so the join is the - // lookup — same shape as the MCP tool's getAssignmentForTrip. - const row = this.db.get<{ trip_id: number }>( - 'SELECT d.trip_id FROM day_assignments da JOIN days d ON da.day_id = d.id WHERE da.id = ?', - data.assignment_id, - ); - check('assignment_id', !!row && String(row.trip_id) !== String(tripId)); + // lookup — same shape as the MCP tool's getAssignmentForTrip. RS22. + const rowTripId = await this.reservationsRepo.getAssignmentTripId(data.assignment_id); + check('assignment_id', rowTripId !== undefined && String(rowTripId) !== String(tripId)); } const acc = data.create_accommodation; if (acc) { - if (acc.place_id != null) check('create_accommodation.place_id', elsewhere('places', acc.place_id)); - if (acc.start_day_id != null) check('create_accommodation.start_day_id', elsewhere('days', acc.start_day_id)); - if (acc.end_day_id != null) check('create_accommodation.end_day_id', elsewhere('days', acc.end_day_id)); + if (acc.place_id != null) check('create_accommodation.place_id', await elsewhere('places', acc.place_id)); + if (acc.start_day_id != null) check('create_accommodation.start_day_id', await elsewhere('days', acc.start_day_id)); + if (acc.end_day_id != null) check('create_accommodation.end_day_id', await elsewhere('days', acc.end_day_id)); } return offenders; @@ -589,19 +653,24 @@ export class ReservationsService { */ async unresolvedReferences(tripId: string | number, data: CreateReservationData | UpdateReservationData): Promise { const offenders: string[] = []; - const onTrip = (table: 'days' | 'places', id: unknown) => - !!this.db.get(`SELECT id FROM ${table} WHERE id = ? AND trip_id = ?`, id, tripId); + // RS23 — a CLOSED union ('days' | 'places') dispatched to the existing + // typed `existsInTrip` methods. R1: this closure has NO typed + // intermediate the way `check()` gives `elsewhere` above — `tsc` does + // NOT catch a dropped `await` here (Task 0's finding), so every call + // site below is `await`ed deliberately and the async-closure mutation + // harness (RS23-001) is the only net. + const onTrip = async (table: 'days' | 'places', id: number): Promise => { + const tripIdNum = this.rowIdNum(tripId); + return table === 'days' ? this.daysRepo.existsInTrip(id, tripIdNum) : this.placesRepo.existsInTrip(id, tripIdNum); + }; - if (data.day_id && !onTrip('days', data.day_id)) offenders.push('day_id'); - if (data.end_day_id && !onTrip('days', data.end_day_id)) offenders.push('end_day_id'); - if (data.place_id && !onTrip('places', data.place_id)) offenders.push('place_id'); + if (data.day_id && !(await onTrip('days', data.day_id))) offenders.push('day_id'); + if (data.end_day_id && !(await onTrip('days', data.end_day_id))) offenders.push('end_day_id'); + if (data.place_id && !(await onTrip('places', data.place_id))) offenders.push('place_id'); if (data.assignment_id) { // An assignment belongs to a trip through its day, the same join the - // other guard walks. - const row = this.db.get( - 'SELECT da.id FROM day_assignments da JOIN days d ON da.day_id = d.id WHERE da.id = ? AND d.trip_id = ?', - data.assignment_id, tripId, - ); + // other guard walks. RS24. + const row = await this.dayAssignmentsRepo.findInTrip(data.assignment_id, tripId); if (!row) offenders.push('assignment_id'); } @@ -619,10 +688,17 @@ export class ReservationsService { return offenders; } - /** Is there still a row behind this id? Existence only — which trip it sits - * on is the guards' question, and they answer it before the write. */ - private async referenceExists(table: 'days' | 'places' | 'day_assignments', id: unknown): Promise { - return !!this.db.get(`SELECT id FROM ${table} WHERE id = ?`, id); + /** + * Is there still a row behind this id? Existence only — which trip it sits + * on is the guards' question, and they answer it before the write. RS25 — + * a CLOSED union dispatched to each entity's own inherited `findOne` + * (never an interpolated identifier); no new method on `Days`/`Places`/ + * `DayAssignments` repositories. + */ + private async referenceExists(table: 'days' | 'places' | 'day_assignments', id: number): Promise { + if (table === 'days') return (await this.daysRepo.findOne({ id })) !== null; + if (table === 'places') return (await this.placesRepo.findOne({ id })) !== null; + return (await this.dayAssignmentsRepo.findOne({ id })) !== null; } /** @@ -658,8 +734,12 @@ export class ReservationsService { private async requireResolvableStay(acc: CreateAccommodation): Promise { const missing: string[] = []; if (acc.place_id && !(await this.referenceExists('places', acc.place_id))) missing.push('place_id'); - if (!(await this.referenceExists('days', acc.start_day_id))) missing.push('start_day_id'); - if (!(await this.referenceExists('days', acc.end_day_id))) missing.push('end_day_id'); + // Both days are already known to be set where this is called (the + // docstring above) — every caller guards on `start_day_id && end_day_id` + // before calling, so the non-null assertion here names a real invariant, + // not a widened type. + if (!(await this.referenceExists('days', acc.start_day_id!))) missing.push('start_day_id'); + if (!(await this.referenceExists('days', acc.end_day_id!))) missing.push('end_day_id'); if (missing.length > 0) { throw new BadRequestException(`Unknown reference: ${missing.map((field) => `create_accommodation.${field}`).join(', ')}`); } @@ -695,11 +775,16 @@ export class ReservationsService { const { place_id: accPlaceId, start_day_id, end_day_id, check_in, check_out, confirmation: accConf } = create_accommodation; if (start_day_id && end_day_id) { await this.requireResolvableStay(create_accommodation); - const accResult = this.db.run( - 'INSERT INTO day_accommodations (trip_id, place_id, start_day_id, end_day_id, check_in, check_out, confirmation) VALUES (?, ?, ?, ?, ?, ?, ?)', - tripId, accPlaceId || null, start_day_id, end_day_id, check_in || null, check_out || null, accConf || confirmation_number || null - ); - resolvedAccommodationId = Number(accResult.lastInsertRowid); + // RS27 + resolvedAccommodationId = await this.dayAccommodationsRepo.insertBookingStay({ + trip_id: tripId, + place_id: accPlaceId || null, + start_day_id, + end_day_id, + check_in: check_in || null, + check_out: check_out || null, + confirmation: accConf || confirmation_number || null, + }); accommodationCreated = true; // Same night, same day header, so the same stop the road trip draws for a // night entered under Days. Without it the hotel booked on this form is the @@ -727,31 +812,33 @@ export class ReservationsService { const resolvedPlaceId = await this.resolvedOrNull('places', place_id || null); const resolvedAssignmentId = await this.resolvedOrNull('day_assignments', assignment_id || null); - const result = this.db.run(` - INSERT INTO reservations (trip_id, day_id, end_day_id, place_id, assignment_id, title, reservation_time, reservation_end_time, location, confirmation_number, notes, url, status, type, accommodation_id, metadata, needs_review) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) - `, - tripId, - resolvedDayId, - resolvedEndDayId, - resolvedPlaceId, - resolvedAssignmentId, + // RS28. `accommodation_id` bound as the TEXT column's string form (R2 — + // the legacy statement bound a plain JS number and let better-sqlite3's + // driver / SQLite's own TEXT-affinity conversion at write time store it + // as text; `String(n)` on a normal integer id renders the identical + // bytes, so this is byte-parity, not a behaviour change). + const insertedId = await this.reservationsRepo.insertReservation({ + trip_id: this.rowIdNum(tripId), + day_id: resolvedDayId, + end_day_id: resolvedEndDayId, + place_id: resolvedPlaceId, + assignment_id: resolvedAssignmentId, title, - reservation_time || null, - reservation_end_time || null, - location || null, - confirmation_number || null, - notes || null, - url || null, - status || 'pending', - resolvedType, - resolvedAccommodationId, - metadata ? JSON.stringify(metadata) : null, - needs_review ? 1 : 0 - ); + reservation_time: reservation_time || null, + reservation_end_time: reservation_end_time || null, + location: location || null, + confirmation_number: confirmation_number || null, + notes: notes || null, + url: url || null, + status: status || 'pending', + type: resolvedType, + accommodation_id: resolvedAccommodationId == null ? null : String(resolvedAccommodationId), + metadata: metadata ? JSON.stringify(metadata) : null, + needs_review: needs_review ? 1 : 0, + }); if (endpoints && endpoints.length > 0) { - await this.saveEndpoints(Number(result.lastInsertRowid), endpoints); + await this.saveEndpoints(insertedId, endpoints); } // Sync check-in/out to accommodation if linked. Keyed off the RESOLVED id @@ -761,21 +848,19 @@ export class ReservationsService { if (resolvedAccommodationId && metadata) { const meta = (typeof metadata === 'string' ? JSON.parse(metadata) : metadata) as AccommodationTimesMeta; if (meta.check_in_time || meta.check_in_end_time || meta.check_out_time) { - this.db.run( - 'UPDATE day_accommodations SET check_in = COALESCE(?, check_in), check_in_end = COALESCE(?, check_in_end), check_out = COALESCE(?, check_out) WHERE id = ?', - meta.check_in_time || null, meta.check_in_end_time || null, meta.check_out_time || null, resolvedAccommodationId + // RS29 + await this.dayAccommodationsRepo.patchTimes( + resolvedAccommodationId, meta.check_in_time || null, meta.check_in_end_time || null, meta.check_out_time || null, ); } if (confirmation_number) { - this.db.run( - 'UPDATE day_accommodations SET confirmation = COALESCE(?, confirmation) WHERE id = ?', - confirmation_number, resolvedAccommodationId - ); + // RS30 + await this.dayAccommodationsRepo.patchConfirmation(resolvedAccommodationId, confirmation_number); } } // The row was just inserted, so the re-select can't miss (legacy typed this any). - const reservation = (await this.getReservationWithJoins(Number(result.lastInsertRowid)))!; + const reservation = (await this.getReservationWithJoins(insertedId))!; return { reservation, accommodationCreated, stayMirror }; } @@ -788,32 +873,45 @@ export class ReservationsService { // agree on it. Doing that in the statement rather than as a pre-check also // makes a stale id a quiet no-op instead of a foreign-key error surfacing // as a 500. - const stmt = this.db.prepare(` - INSERT OR REPLACE INTO reservation_day_positions (reservation_id, day_id, position) - SELECT r.id, d.id, ? - FROM reservations r - JOIN days d ON d.trip_id = r.trip_id - WHERE r.id = ? AND d.id = ? AND r.trip_id = ? - `); + const dayIdNum = this.rowIdNum(dayId); + const tripIdNum = this.rowIdNum(tripId); await this.uow.transactional(async () => { for (const item of positions) { - // position is NOT NULL while the wire contract leaves the value optional. - stmt.run(item.day_plan_position ?? 0, item.id, dayId, tripId); + // RS31/RS32. position is NOT NULL while the wire contract leaves the value optional. + await this.dayPositionsRepo.upsertScoped(tripIdNum, item.id, dayIdNum, item.day_plan_position ?? 0); } }); } else { // Legacy: update global position - const stmt = this.db.prepare('UPDATE reservations SET day_plan_position = ? WHERE id = ? AND trip_id = ?'); + const tripIdNum = this.rowIdNum(tripId); await this.uow.transactional(async () => { for (const item of positions) { - stmt.run(item.day_plan_position, item.id, tripId); + // RS33/RS34. `?? null`, never `undefined` (R8) — a `nativeUpdate` + // partial SKIPS a column whose value is `undefined`, which would + // leave the row's old position in place instead of clearing it, + // unlike the legacy statement's own bind of `undefined` (which + // better-sqlite3 writes as SQL NULL). + await this.reservationsRepo.setDayPlanPosition(item.id, tripIdNum, item.day_plan_position ?? null); } }); } } - async getReservation(id: string | number, tripId: string | number) { - return this.db.get('SELECT * FROM reservations WHERE id = ? AND trip_id = ?', id, tripId); + async getReservation(id: string | number, tripId: string | number): Promise { + // RS35 — the trip-scoping guard every write path re-reads through. Both + // ids parsed ONCE here (rule 21); a miss reads as `undefined`, matching + // the legacy raw-bind miss. + const idNum = toRowId(id); + const tripIdNum = toRowId(tripId); + if (idNum === null || tripIdNum === null) return undefined; + const row = await this.reservationsRepo.findInTrip(idNum, tripIdNum); + if (!row) return undefined; + // `status`/`type` are NOT NULL in every row a write path here ever + // produces (`status || 'pending'` / `type || 'other'`, RS28/RS41); the + // fallback only guards the physical column's own nullability for a row + // this service never wrote, the same defensive shape `resolvedType` + // already applies at every call site that reads `current.type`. + return { ...row, status: row.status ?? 'pending', type: row.type ?? 'other' }; } /** The accommodation upsert, the reservation update, the endpoint replace @@ -835,12 +933,27 @@ export class ReservationsService { let accommodationChanged = false; let stayMirror = noStayMirror(); - // Update or create accommodation for hotel reservations - let resolvedAccId: number | null = accommodation_id !== undefined ? (accommodation_id || null) : (current.accommodation_id ?? null); + // Update or create accommodation for hotel reservations. + // + // `resolvedAccId` deliberately keeps BOTH shapes alive, exactly as the + // legacy code's own type lie did at runtime (`Reservation + // .accommodation_id` claimed `number` while the raw row underneath was + // always the TEXT column's string): when the payload names + // `accommodation_id` it is a genuine `number` (`UpdateReservationData`'s + // declared type); when it falls back to `current.accommodation_id` it is + // the UNCONVERTED string this reservation already stored (R2 — a + // `"14.0"`-shaped value must round-trip byte-identical through a no-op + // update, which a `Number()`/`String()` round-trip would truncate to + // `"14"`). `accIdForRead(...)` below is the READ-only numeric form every + // repository call needs; the final write still binds `resolvedAccId` + // itself (via `String(...)`, a no-op on an already-string value). + let resolvedAccId: number | string | null = accommodation_id !== undefined ? (accommodation_id || null) : (current.accommodation_id ?? null); + const accIdForRead = (v: number | string | null): number | null => (v == null ? null : Number(v)); if (resolvedAccId) { // Scoped to the trip on purpose: an id belonging to someone else's trip // must read as absent here, not as an accommodation to write through to. - const accExists = this.db.get('SELECT id FROM day_accommodations WHERE id = ? AND trip_id = ?', resolvedAccId, tripId); + // RS37 + const accExists = await this.dayAccommodationsRepo.existsInTrip(accIdForRead(resolvedAccId)!, this.rowIdNum(tripId)); if (!accExists) resolvedAccId = null; } if (type === 'hotel' && create_accommodation) { @@ -848,23 +961,35 @@ export class ReservationsService { if (start_day_id && end_day_id) { await this.requireResolvableStay(create_accommodation); if (resolvedAccId) { - const prior = this.db.get<{ check_in: string | null }>('SELECT check_in FROM day_accommodations WHERE id = ?', resolvedAccId); - this.db.run( - 'UPDATE day_accommodations SET place_id = ?, start_day_id = ?, end_day_id = ?, check_in = ?, check_out = ?, confirmation = ? WHERE id = ?', - accPlaceId || null, start_day_id, end_day_id, check_in || null, check_out || null, accConf || confirmation_number || null, resolvedAccId - ); + const resolvedAccIdNum = accIdForRead(resolvedAccId)!; + // RS38 + const priorCheckIn = await this.dayAccommodationsRepo.getCheckIn(resolvedAccIdNum); + // RS39 + await this.dayAccommodationsRepo.updateFromBooking(resolvedAccIdNum, { + place_id: accPlaceId || null, + start_day_id, + end_day_id, + check_in: check_in || null, + check_out: check_out || null, + confirmation: accConf || confirmation_number || null, + }); // The stay just moved. Its stop moves with it, or it is left sitting on a // day nobody sleeps there any more, hidden from the day list because it // still carries this booking's id and stranded in the middle of the drive. - stayMirror = await this.accommodations.moveStayStop(resolvedAccId, accPlaceId || null, start_day_id, check_in, { - checkInChanged: (check_in || null) !== (prior?.check_in ?? null), + stayMirror = await this.accommodations.moveStayStop(resolvedAccIdNum, accPlaceId || null, start_day_id, check_in, { + checkInChanged: (check_in || null) !== (priorCheckIn ?? null), }); } else if (accPlaceId) { - const accResult = this.db.run( - 'INSERT INTO day_accommodations (trip_id, place_id, start_day_id, end_day_id, check_in, check_out, confirmation) VALUES (?, ?, ?, ?, ?, ?, ?)', - tripId, accPlaceId, start_day_id, end_day_id, check_in || null, check_out || null, accConf || confirmation_number || null - ); - resolvedAccId = Number(accResult.lastInsertRowid); + // RS40 + resolvedAccId = await this.dayAccommodationsRepo.insertBookingStay({ + trip_id: tripId, + place_id: accPlaceId, + start_day_id, + end_day_id, + check_in: check_in || null, + check_out: check_out || null, + confirmation: accConf || confirmation_number || null, + }); stayMirror = await this.accommodations.attachStayStop(resolvedAccId, accPlaceId, start_day_id, check_in); } accommodationChanged = true; @@ -919,65 +1044,53 @@ export class ReservationsService { const nextPlaceId = await this.resolvedOrNull('places', place_id !== undefined ? (place_id || null) : (current.place_id ?? null)); const nextAssignmentId = await this.resolvedOrNull('day_assignments', assignment_id !== undefined ? (assignment_id || null) : (current.assignment_id ?? null)); - this.db.run(` - UPDATE reservations SET - title = COALESCE(?, title), - reservation_time = ?, - reservation_end_time = ?, - location = ?, - confirmation_number = ?, - notes = ?, - url = ?, - day_id = ?, - end_day_id = ?, - place_id = ?, - assignment_id = ?, - status = COALESCE(?, status), - type = COALESCE(?, type), - accommodation_id = ?, - metadata = ?, - needs_review = COALESCE(?, needs_review) - WHERE id = ? - `, - title || null, - nextReservationTime, - nextReservationEndTime, - location !== undefined ? (location || null) : current.location, - confirmation_number !== undefined ? (confirmation_number || null) : current.confirmation_number, - notes !== undefined ? (notes || null) : current.notes, - url !== undefined ? (url || null) : (current as Reservation & { url?: string | null }).url, - nextDayId, - nextEndDayId, - nextPlaceId, - nextAssignmentId, - status || null, - type || null, - resolvedAccId, - nextMetadata !== undefined ? (nextMetadata ? JSON.stringify(nextMetadata) : null) : current.metadata, - needs_review === undefined ? null : (needs_review ? 1 : 0), - id - ); + // RS41. The `PlacesRepository.updatePlace`/PL11 precedent: the four + // legacy `COALESCE(?, col)` keep-if-null columns (`title`/`status`/ + // `type`/`needs_review`) are resolved to their FINAL value here, in JS, + // against the `current` pre-image this method already holds — nothing + // else in this same transaction touches those four columns between the + // read and this write, so `current`'s snapshot is exactly what the + // legacy statement's own SQL-side `COALESCE` against the live row would + // have read. + const idNum = this.rowIdNum(id); + await this.reservationsRepo.updateReservation(idNum, { + title: title || current.title, + reservation_time: nextReservationTime, + reservation_end_time: nextReservationEndTime, + location: location !== undefined ? (location || null) : (current.location ?? null), + confirmation_number: confirmation_number !== undefined ? (confirmation_number || null) : (current.confirmation_number ?? null), + notes: notes !== undefined ? (notes || null) : (current.notes ?? null), + url: url !== undefined ? (url || null) : ((current as Reservation & { url?: string | null }).url ?? null), + day_id: nextDayId, + end_day_id: nextEndDayId, + place_id: nextPlaceId, + assignment_id: nextAssignmentId, + status: status || current.status, + type: type || current.type, + accommodation_id: resolvedAccId == null ? null : String(resolvedAccId), + metadata: nextMetadata !== undefined ? (nextMetadata ? JSON.stringify(nextMetadata) : null) : (current.metadata ?? null), + needs_review: needs_review === undefined ? (current.needs_review ?? 0) : (needs_review ? 1 : 0), + }); if (endpoints !== undefined) { - await this.saveEndpoints(Number(id), endpoints); + await this.saveEndpoints(idNum, endpoints); } // Sync check-in/out to accommodation if linked const resolvedMeta = nextMetadata !== undefined ? nextMetadata : (current.metadata ? JSON.parse(current.metadata as string) : null); if (resolvedAccId && resolvedMeta) { const meta = (typeof resolvedMeta === 'string' ? JSON.parse(resolvedMeta) : resolvedMeta) as AccommodationTimesMeta; + const resolvedAccIdNum = accIdForRead(resolvedAccId)!; if (meta.check_in_time || meta.check_in_end_time || meta.check_out_time) { - this.db.run( - 'UPDATE day_accommodations SET check_in = COALESCE(?, check_in), check_in_end = COALESCE(?, check_in_end), check_out = COALESCE(?, check_out) WHERE id = ?', - meta.check_in_time || null, meta.check_in_end_time || null, meta.check_out_time || null, resolvedAccId + // RS42 + await this.dayAccommodationsRepo.patchTimes( + resolvedAccIdNum, meta.check_in_time || null, meta.check_in_end_time || null, meta.check_out_time || null, ); } const resolvedConf = confirmation_number !== undefined ? confirmation_number : current.confirmation_number; if (resolvedConf) { - this.db.run( - 'UPDATE day_accommodations SET confirmation = COALESCE(?, confirmation) WHERE id = ?', - resolvedConf, resolvedAccId - ); + // RS43 + await this.dayAccommodationsRepo.patchConfirmation(resolvedAccIdNum, resolvedConf); } } @@ -989,11 +1102,12 @@ export class ReservationsService { /** The accommodation + budget-item + reservation deletes are one logical * cascade — all-or-nothing. */ - async remove(id: string | number, tripId: string | number): Promise<{ deleted: { id: number; title: string; type: string; accommodation_id: number | null } | undefined; accommodationDeleted: boolean; deletedBudgetItemId: number | null }> { + async remove(id: string | number, tripId: string | number): Promise<{ deleted: { id: number; title: string; type: string | null; accommodation_id: string | null } | undefined; accommodationDeleted: boolean; deletedBudgetItemId: number | null }> { + const idNum = this.rowIdNum(id); + const tripIdNum = this.rowIdNum(tripId); const removed = await this.uow.transactional(async () => { - const reservation = this.db.get<{ id: number; title: string; type: string; accommodation_id: number | null }>( - 'SELECT id, title, type, accommodation_id FROM reservations WHERE id = ? AND trip_id = ?', id, tripId - ); + // RS45 + const reservation = await this.reservationsRepo.findHeaderInTrip(idNum, tripIdNum); if (!reservation) return { deleted: undefined, accommodationDeleted: false, deletedBudgetItemId: null, stayMirror: noStayMirror() }; let accommodationDeleted = false; @@ -1003,26 +1117,34 @@ export class ReservationsService { // before referencesOutsideTrip existed can still carry a foreign // accommodation_id, and the cascade must not follow it. The stops go by // accommodation id alone, which is exactly the reach that guard denies. - const ownStay = this.db.get<{ id: number }>( - 'SELECT id FROM day_accommodations WHERE id = ? AND trip_id = ?', reservation.accommodation_id, tripId, - ); + // accommodation_id is TEXT (R2); Number(...) here is a READ-time + // normalisation only, the same `Math.trunc(Number(x))`-style + // coercion every other reader of this column already applies — the + // WRITE-side raw string (`reservation.accommodation_id`) still ships + // unconverted in the `accommodation:deleted` broadcast payload below. + const accIdNum = Number(reservation.accommodation_id); + // RS46 + const ownStay = await this.dayAccommodationsRepo.existsInTrip(accIdNum, tripIdNum); if (ownStay) { // Released before the row goes, not after: the release looks the stops up // by accommodation id, and that pointer is cleared the moment the stay is // deleted. Reversed, the stop stands with nothing left to remove it, and // the day list hides it for carrying a booking id. - stayMirror = await this.accommodations.dropStayStops(reservation.accommodation_id); - this.db.run('DELETE FROM day_accommodations WHERE id = ? AND trip_id = ?', reservation.accommodation_id, tripId); + stayMirror = await this.accommodations.dropStayStops(accIdNum); + // RS47 + await this.dayAccommodationsRepo.deleteInTrip(accIdNum, tripIdNum); accommodationDeleted = true; } } + // RS48/RS49 — stay raw, Plan 3e's `budget_items` table. const linkedBudget = this.db.get<{ id: number }>('SELECT id FROM budget_items WHERE trip_id = ? AND reservation_id = ?', tripId, id); if (linkedBudget) { this.db.run('DELETE FROM budget_items WHERE id = ?', linkedBudget.id); } - this.db.run('DELETE FROM reservations WHERE id = ?', id); + // RS50 + await this.reservationsRepo.deleteById(idNum); return { deleted: reservation, accommodationDeleted, deletedBudgetItemId: linkedBudget ? linkedBudget.id : null, stayMirror }; }); const { stayMirror, ...answer } = removed; diff --git a/server/src/types.ts b/server/src/types.ts index 2306f2fa0..ae3fb00cf 100644 --- a/server/src/types.ts +++ b/server/src/types.ts @@ -218,7 +218,14 @@ export interface Reservation { notes?: string | null; status: string; type: string; - accommodation_id?: number | null; + /** + * TEXT at the DB, not INTEGER (Plan 3d inventory §18.1 — no FK, some rows + * read back as `"14.0"`): every reader normalises it, so this stays a + * string here rather than the `DayAssignment.accommodation_id` field + * above's genuine INTEGER shape. Do not "fix" this to `number` — that + * field is a different column on a different table. + */ + accommodation_id?: string | null; metadata?: string | null; needs_review?: number; endpoints?: ReservationEndpoint[]; diff --git a/server/tests/helpers/mcp-test-controllers.ts b/server/tests/helpers/mcp-test-controllers.ts index a038c58c4..40d6282a4 100644 --- a/server/tests/helpers/mcp-test-controllers.ts +++ b/server/tests/helpers/mcp-test-controllers.ts @@ -121,6 +121,10 @@ import { createTestGooglePlacePhotoMetaRepo, createTestPlacesRepo, createTestRoadtripViasRepo, createTestRoadtripDayTracksRepo, createTestReservationsRepo, createTestReservationEndpointsRepo, + createTestReservationTravelersRepo, + createTestReservationDayPositionsRepo, + createTestDayAccommodationsRepo, + createTestUsersRepo, } from './test-uow'; import { createTestOrm } from './test-orm'; import { AppSettings } from '../../src/db/entities/AppSettings.entity'; @@ -232,7 +236,7 @@ export async function createMcpTestRegistry(): Promise { await createTestTripsRepo(dbService.connection), ); // Built after it: a hotel booking writes the stay's day stop through this one. - const reservationsService = new ReservationsService(dbService, permissionsService, budgetService, realtimeService, notificationsStub(), new ReservationsReadService(dbService), accommodationsService, await createTestUnitOfWork(dbService.connection)); + const reservationsService = new ReservationsService(dbService, permissionsService, budgetService, realtimeService, notificationsStub(), new ReservationsReadService(await createTestReservationsRepo(dbService.connection), await createTestReservationEndpointsRepo(dbService.connection), await createTestReservationTravelersRepo(dbService.connection)), accommodationsService, await createTestUnitOfWork(dbService.connection), await createTestReservationsRepo(dbService.connection), await createTestReservationEndpointsRepo(dbService.connection), await createTestReservationTravelersRepo(dbService.connection), await createTestReservationDayPositionsRepo(dbService.connection), await createTestDayAccommodationsRepo(dbService.connection), await createTestDaysRepo(dbService.connection), await createTestPlacesRepo(dbService.connection), await createTestDayAssignmentsRepo(dbService.connection), await createTestTripMembersRepo(dbService.connection), await createTestUsersRepo(dbService.connection), await createTestTripsRepo(dbService.connection)); const membersService = new TripMembersService(dbService, budgetService, new UserCleanupService(dbService, budgetService, await createTestUnitOfWork(dbService.connection), usersRepo), permissionsService, realtimeService, notificationsStub(), await createTestUnitOfWork(dbService.connection), await createTestTripsRepo(dbService.connection), await createTestTripMembersRepo(dbService.connection), usersRepo); const tripsService = new TripsService( dbService, diff --git a/server/tests/helpers/plugin-host.ts b/server/tests/helpers/plugin-host.ts index 892fd8c1f..c125c58bb 100644 --- a/server/tests/helpers/plugin-host.ts +++ b/server/tests/helpers/plugin-host.ts @@ -79,6 +79,10 @@ import { createTestGooglePlacePhotoMetaRepo, createTestPlacesRepo, createTestRoadtripViasRepo, createTestReservationsRepo, createTestReservationEndpointsRepo, + createTestReservationTravelersRepo, + createTestReservationDayPositionsRepo, + createTestDayAccommodationsRepo, + createTestUsersRepo, } from './test-uow'; import { AppSettings } from '../../src/db/entities/AppSettings.entity'; import { AuditLog } from '../../src/db/entities/AuditLog.entity'; @@ -154,7 +158,7 @@ export async function createPluginRpcHostFactory(dbs: DatabaseService): Promise< await createTestTripsRepo(dbs.connection), ); // After accommodations: a hotel booking writes the stay's day stop through it. - const reservations = new ReservationsService(dbs, permissions, budget, realtime, notificationsStub(), new ReservationsReadService(dbs), accommodations, await createTestUnitOfWork(dbs.connection)); + const reservations = new ReservationsService(dbs, permissions, budget, realtime, notificationsStub(), new ReservationsReadService(await createTestReservationsRepo(dbs.connection), await createTestReservationEndpointsRepo(dbs.connection), await createTestReservationTravelersRepo(dbs.connection)), accommodations, 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)); const trips = new TripsService(dbs, reservations, days, permissions, budget, vacay, realtime, unsplash, generalStorage, await createTestUnitOfWork(dbs.connection), (await sharedTestOrm(dbs.connection)).em); const members = new TripMembersService(dbs, budget, new UserCleanupService(dbs, budget, await createTestUnitOfWork(dbs.connection), usersRepo), permissions, realtime, notificationsStub(), await createTestUnitOfWork(dbs.connection), await createTestTripsRepo(dbs.connection), await createTestTripMembersRepo(dbs.connection), usersRepo); const guards = new PluginGuards(dbs, permissions, addons); diff --git a/server/tests/integration/dayReorder.test.ts b/server/tests/integration/dayReorder.test.ts index 5b5e44f53..f18f075ed 100644 --- a/server/tests/integration/dayReorder.test.ts +++ b/server/tests/integration/dayReorder.test.ts @@ -29,6 +29,8 @@ import { createTestUnitOfWork, createTestAppSettingsRepo, createTestDaysRepo, createTestDayAssignmentsRepo, createTestDayNotesRepo, createTestTripsRepo, createTestTagsRepo, createTestPlaceRatingsRepo, createTestAssignmentParticipantsRepo, + createTestReservationsRepo, + createTestReservationEndpointsRepo, } from '../helpers/test-uow'; let svc: DaysService; @@ -43,6 +45,8 @@ beforeAll(async () => { await createTestDayAssignmentsRepo(testDb), await createTestDayNotesRepo(testDb), await createTestTripsRepo(testDb), + await createTestReservationsRepo(testDb), + await createTestReservationEndpointsRepo(testDb), ); }); const reorderDays = async (tripId: number, orderedIds: number[]) => await svc.reorder(tripId, orderedIds); diff --git a/server/tests/unit/db/dialect/sql-functions.test.ts b/server/tests/unit/db/dialect/sql-functions.test.ts index 240a28613..87e1cbc29 100644 --- a/server/tests/unit/db/dialect/sql-functions.test.ts +++ b/server/tests/unit/db/dialect/sql-functions.test.ts @@ -1,5 +1,6 @@ import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest'; import { Platform } from '@mikro-orm/core'; +import { expressionBuilder } from 'kysely'; import { createSnapshotTestDb } from '../../../helpers/db-mock'; import { resetTestDb } from '../../../helpers/test-db'; import { createTestOrm, type TestOrm } from '../../../helpers/test-orm'; @@ -9,11 +10,13 @@ import { absDifference, caseWhenEquals, castInteger, + castIntegerKysely, coalesce, coalesceParam, columnIncrementedBy, columnRef, concat, + concatKysely, countAll, countAllRef, currentTimestamp, @@ -27,10 +30,22 @@ import { minOf, nowMinusDays, startsWithIsoDate, + startsWithIsoDateKysely, substring, + substringKysely, trim, } from '../../../../src/db/dialect/sql-functions'; +/** The `users` columns the Kysely-expression tests below read/write, narrowed the same way every other Kysely-typed repository method in this program declares its own `TDB`. */ +interface UsersKyselyDB { + users: { + id: number; + username: string; + display_name: string | null; + created_at: string | null; + }; +} + const testDb = createSnapshotTestDb(); let t: TestOrm; @@ -507,6 +522,8 @@ describe('sql-functions (sqlite)', () => { ['2026-09-21 13:05:09', true], ['not-a-date', false], ['2026-1-1', false], // not zero-padded — the legacy pattern is exactly 4-2-2 digit classes, not a wildcard shorthand + ['abcd-ef-gh', false], // Task 0 review L1 — the wider '????-??-??*' shorthand WOULD match this (a `?` matches any char); the digit-class pattern must not + ['yyyy-mm-dd', false], // Task 0 review L1 — same reason, letters where the pattern requires digits ['', false], [null, false], ]; @@ -686,4 +703,182 @@ describe('sql-functions (sqlite)', () => { const foreign = new FakePlatform(); expect(() => dayDistance(foreign, 'u.date', '2026-01-01')).toThrow(/no implementation for platform FakePlatform/); }); + + // Plan 3d Task 0 review (H1/L1): the five helpers above return a MikroORM + // RawQueryFragment, which stringifies to a literal `?` token and throws at + // execution when handed into a Kysely statement — verified directly below + // (the whole reason these Kysely-native twins exist). Task 2 absorbed this + // finding; DY23 (`ReservationsRepository.restampLinkedReservation`) is the + // first consumer. + + it('SQLF-048: a MikroORM RawQueryFragment cannot be coerced to a plain string outside the MikroORM QueryBuilder that produced it — the H1 finding, proven directly ([Symbol.toPrimitive]("string") returns an internal Symbol key the QueryBuilder recognises to re-substitute the real SQL text; nothing else, Kysely included, understands that key, which is why the Kysely-native twins below exist instead of reusing this fragment)', () => { + const platform = t.em.getPlatform(); + const fragment = startsWithIsoDate(platform, 'username'); + expect(typeof fragment[Symbol.toPrimitive]('string')).toBe('symbol'); + expect(() => `${fragment}`).toThrow(TypeError); + }); + + it('SQLF-049: startsWithIsoDateKysely matches the same shapes as startsWithIsoDate (dates, non-dates, letters-in-place-of-digits, empty, NULL)', async () => { + const platform = t.em.getPlatform(); + const cases: Array<[string | null, boolean]> = [ + ['2026-09-21', true], + ['2026-09-21T13:05:09Z', true], + ['not-a-date', false], + ['2026-1-1', false], + ['abcd-ef-gh', false], + ['yyyy-mm-dd', false], + ['', false], + [null, false], + ]; + for (const [value, expected] of cases) { + const { user } = createUser(testDb); + testDb.prepare('UPDATE users SET display_name = ? WHERE id = ?').run(value, user.id); + + const row = await t.em.getKysely() + .selectFrom('users') + .select((eb) => [startsWithIsoDateKysely(platform, eb, 'display_name').as('matched')]) + .where('id', '=', user.id) + .executeTakeFirstOrThrow(); + expect(Boolean(row.matched)).toBe(expected); + + const raw = testDb + .prepare(`SELECT (display_name GLOB '[0-9][0-9][0-9][0-9]-[0-9][0-9]-[0-9][0-9]*') as matched FROM users WHERE id = ?`) + .get(user.id) as { matched: number | null }; + expect(Boolean(raw.matched)).toBe(expected); + } + }); + + it('SQLF-050: an unknown platform fails closed for startsWithIsoDateKysely', () => { + class FakePlatform extends Platform {} + const foreign = new FakePlatform(); + const eb = expressionBuilder(); + expect(() => startsWithIsoDateKysely(foreign, eb, 'username')).toThrow(/no implementation for platform FakePlatform/); + }); + + it('SQLF-051: substringKysely(ref, start) — the two-arg form — matches substr(col, N)', async () => { + const { user } = createUser(testDb, { username: 'kysely-iso-ts' }); + testDb.prepare('UPDATE users SET display_name = ? WHERE id = ?').run('2026-09-21T13:05:09Z', user.id); + const platform = t.em.getPlatform(); + + const row = await t.em.getKysely() + .selectFrom('users') + .select((eb) => [substringKysely(platform, eb, 'display_name', 12).as('n')]) + .where('id', '=', user.id) + .executeTakeFirstOrThrow(); + expect(row.n).toBe('13:05:09Z'); + + const expected = testDb.prepare('SELECT substr(display_name, 12) as n FROM users WHERE id = ?').get(user.id) as { n: string }; + expect(row.n).toBe(expected.n); + }); + + it('SQLF-052: substringKysely(ref, start, length) — the three-arg form — matches substr(col, N, L)', async () => { + const { user } = createUser(testDb, { username: 'kysely-iso-ts-2' }); + testDb.prepare('UPDATE users SET display_name = ? WHERE id = ?').run('2026-09-21T13:05:09Z', user.id); + const platform = t.em.getPlatform(); + + const row = await t.em.getKysely() + .selectFrom('users') + .select((eb) => [substringKysely(platform, eb, 'display_name', 1, 10).as('n')]) + .where('id', '=', user.id) + .executeTakeFirstOrThrow(); + expect(row.n).toBe('2026-09-21'); + + const expected = testDb.prepare('SELECT substr(display_name, 1, 10) as n FROM users WHERE id = ?').get(user.id) as { n: string }; + expect(row.n).toBe(expected.n); + }); + + it('SQLF-053: substringKysely rejects a non-positive start or a negative length', () => { + const platform = t.em.getPlatform(); + const eb = expressionBuilder(); + expect(() => substringKysely(platform, eb, 'username', 0)).toThrow(/1-based integer start/); + expect(() => substringKysely(platform, eb, 'username', 1.5)).toThrow(/1-based integer start/); + expect(() => substringKysely(platform, eb, 'username', 1, -1)).toThrow(/non-negative integer length/); + expect(() => substringKysely(platform, eb, 'username', 1, 1.5)).toThrow(/non-negative integer length/); + }); + + it('SQLF-054: concatKysely renders || || substringKysely(...), composing a nested Kysely expression (the shape concat() cannot do)', async () => { + const { user } = createUser(testDb, { username: 'concat-me' }); + testDb.prepare('UPDATE users SET display_name = ? WHERE id = ?').run('2026-09-21T13:05', user.id); + const platform = t.em.getPlatform(); + + const row = await t.em.getKysely() + .selectFrom('users') + .select((eb) => [ + concatKysely(platform, eb, { column: 'username' }, { value: '@' }, { expression: substringKysely(platform, eb, 'display_name', 12) }).as('n'), + ]) + .where('id', '=', user.id) + .executeTakeFirstOrThrow(); + expect(row.n).toBe('concat-me@13:05'); + + const expected = testDb + .prepare("SELECT username || ? || substr(display_name, 12) as n FROM users WHERE id = ?") + .get('@', user.id) as { n: string }; + expect(row.n).toBe(expected.n); + }); + + it('SQLF-055: concatKysely rejects fewer than two parts', () => { + const platform = t.em.getPlatform(); + const eb = expressionBuilder(); + expect(() => concatKysely(platform, eb, { column: 'username' })).toThrow(/at least two parts/); + expect(() => concatKysely(platform, eb)).toThrow(/at least two parts/); + }); + + it('SQLF-056: an unknown platform fails closed for concatKysely and substringKysely', () => { + class FakePlatform extends Platform {} + const foreign = new FakePlatform(); + const eb = expressionBuilder(); + expect(() => concatKysely(foreign, eb, { column: 'username' }, { value: 'x' })).toThrow(/no implementation for platform FakePlatform/); + expect(() => substringKysely(foreign, eb, 'username', 1)).toThrow(/no implementation for platform FakePlatform/); + }); + + it('SQLF-057: castIntegerKysely matches both "14" and "14.0" TEXT rows against a bound INTEGER value (the accommodation_id shape, §18.1)', async () => { + const platform = t.em.getPlatform(); + const { user: exact } = createUser(testDb); + testDb.prepare('UPDATE users SET display_name = ? WHERE id = ?').run('14', exact.id); + const { user: dotZero } = createUser(testDb); + testDb.prepare('UPDATE users SET display_name = ? WHERE id = ?').run('14.0', dotZero.id); + const { user: other } = createUser(testDb); + testDb.prepare('UPDATE users SET display_name = ? WHERE id = ?').run('15', other.id); + + const rows = await t.em.getKysely() + .selectFrom('users') + .select('id') + .where('id', 'in', [exact.id, dotZero.id, other.id]) + .where((eb) => eb(castIntegerKysely(platform, eb, 'display_name'), '=', 14)) + .orderBy('id', 'asc') + .execute(); + expect(rows.map((r) => r.id)).toEqual([exact.id, dotZero.id]); + + const expected = testDb + .prepare('SELECT id FROM users WHERE id IN (?, ?, ?) AND CAST(display_name AS INTEGER) = ? ORDER BY id ASC') + .all(exact.id, dotZero.id, other.id, 14); + expect(rows).toEqual(expected); + }); + + it('SQLF-058: an unknown platform fails closed for castIntegerKysely', () => { + class FakePlatform extends Platform {} + const foreign = new FakePlatform(); + const eb = expressionBuilder(); + expect(() => castIntegerKysely(foreign, eb, 'display_name')).toThrow(/no implementation for platform FakePlatform/); + }); + + it('SQLF-059: DY23\'s exact shape compiles to the expected SQL text (compiled {sql, parameters} capture)', () => { + const platform = t.em.getPlatform(); + const compiled = t.em.getKysely() + .updateTable('users') + .set((eb) => ({ + username: eb + .case() + .when('username', 'is', null) + .then(eb.val('2026-09-21')) + .else(concatKysely(platform, eb, { value: '2026-09-21' }, { expression: substringKysely(platform, eb, 'created_at', 11) })) + .end(), + })) + .where('id', '=', 1) + .compile(); + expect(compiled.sql).toBe( + 'update "users" set "username" = case when "username" is null then ? else ? || substr("created_at", ?) end where "id" = ?', + ); + expect(compiled.parameters).toEqual(['2026-09-21', '2026-09-21', 11, 1]); + }); }); diff --git a/server/tests/unit/nest/calendar.service.test.ts b/server/tests/unit/nest/calendar.service.test.ts index b82262395..43aeb4546 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 } from '../../helpers/test-uow'; +import { createTestUnitOfWork, createTestAppSettingsRepo, createTestReservationsRepo, createTestReservationEndpointsRepo, createTestReservationTravelersRepo, createTestReservationDayPositionsRepo, createTestDayAccommodationsRepo, createTestDaysRepo, createTestPlacesRepo, createTestDayAssignmentsRepo, createTestTripMembersRepo, createTestUsersRepo, createTestTripsRepo } from '../../helpers/test-uow'; const dbs = () => new DatabaseService(testDb); @@ -69,7 +69,7 @@ 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(dbs()), await accommodationsOver(dbs()), await createTestUnitOfWork(dbs().connection)), + 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)), ); }); diff --git a/server/tests/unit/nest/days.service.test.ts b/server/tests/unit/nest/days.service.test.ts index 821c246ba..9547aee68 100644 --- a/server/tests/unit/nest/days.service.test.ts +++ b/server/tests/unit/nest/days.service.test.ts @@ -68,6 +68,8 @@ import { createTestUnitOfWork, createTestAppSettingsRepo, createTestDatabaseService, createTestDaysRepo, createTestDayAssignmentsRepo, createTestDayNotesRepo, createTestTripsRepo, createTestTagsRepo, createTestPlaceRatingsRepo, createTestAssignmentParticipantsRepo, + createTestReservationsRepo, + createTestReservationEndpointsRepo, } from '../../helpers/test-uow'; let svc: DaysService; @@ -83,6 +85,8 @@ beforeAll(async () => { await createTestDayAssignmentsRepo(testDb), await createTestDayNotesRepo(testDb), await createTestTripsRepo(testDb), + await createTestReservationsRepo(testDb), + await createTestReservationEndpointsRepo(testDb), ); }); let accommodations: Awaited>; @@ -839,6 +843,8 @@ describe('DaysService.canEdit', () => { await createTestDayAssignmentsRepo(testDb), await createTestDayNotesRepo(testDb), await createTestTripsRepo(testDb), + await createTestReservationsRepo(testDb), + await createTestReservationEndpointsRepo(testDb), ); const trip = { id: 1, user_id: 1 } as never; diff --git a/server/tests/unit/nest/reservations.service.test.ts b/server/tests/unit/nest/reservations.service.test.ts index 6d758a329..27a8beac3 100644 --- a/server/tests/unit/nest/reservations.service.test.ts +++ b/server/tests/unit/nest/reservations.service.test.ts @@ -75,12 +75,12 @@ const bridge = { import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; import { notificationsStub } from '../../helpers/notifications'; import { makeAccommodationsService } from '../../helpers/accommodations-service'; -import { createTestUnitOfWork, createTestDatabaseService } from '../../helpers/test-uow'; +import { createTestUnitOfWork, createTestDatabaseService, createTestReservationsRepo, createTestReservationEndpointsRepo, createTestReservationTravelersRepo, createTestReservationDayPositionsRepo, createTestDayAccommodationsRepo, createTestDaysRepo, createTestPlacesRepo, createTestDayAssignmentsRepo, createTestTripMembersRepo, createTestUsersRepo, createTestTripsRepo } from '../../helpers/test-uow'; let svc: ReservationsService; beforeAll(async () => { const dbs = await createTestDatabaseService(testDb); - svc = new ReservationsService(dbs, permissionsStub, budget as unknown as BudgetService, new RealtimeService(), notificationsStub(notif.send), new ReservationsReadService(dbs), await makeAccommodationsService(testDb), await createTestUnitOfWork(testDb)); + svc = new ReservationsService(dbs, permissionsStub, budget as unknown as BudgetService, new RealtimeService(), notificationsStub(notif.send), new ReservationsReadService(await createTestReservationsRepo(testDb), await createTestReservationEndpointsRepo(testDb), await createTestReservationTravelersRepo(testDb)), await makeAccommodationsService(testDb), await createTestUnitOfWork(testDb), await createTestReservationsRepo(testDb), await createTestReservationEndpointsRepo(testDb), await createTestReservationTravelersRepo(testDb), await createTestReservationDayPositionsRepo(testDb), await createTestDayAccommodationsRepo(testDb), await createTestDaysRepo(testDb), await createTestPlacesRepo(testDb), await createTestDayAssignmentsRepo(testDb), await createTestTripMembersRepo(testDb), await createTestUsersRepo(testDb), await createTestTripsRepo(testDb)); }); beforeAll(() => { createTables(testDb); runMigrations(testDb); }); @@ -959,8 +959,15 @@ describe('ReservationsService — quirk fixes (post-fold)', () => { const { trip } = ownerTrip({ start_date: '2030-05-01', end_date: '2030-05-02' }); const place = createPlace(testDb, trip.id); const days = testDb.prepare('SELECT id FROM days WHERE trip_id = ? ORDER BY day_number').all(trip.id) as { id: number }[]; - // sequence is an unbindable object -> the endpoint INSERT throws mid-write. - const badEndpoints = [{ role: 'from', name: 'A', code: null, lat: 1, lng: 2, timezone: null, local_time: null, local_date: null, sequence: {} }]; + // Plan 3d Task 2 (§15a): `sequence: {}` (an unbindable object) no longer + // throws once RS17 goes through `em.insert()` — MikroORM serialises it + // instead of handing it to a raw bind. A repository-level spy + // reproduces the same "the write mid-transaction fails" shape the fault + // injection existed to prove, still inside the SAME `uow.transactional` + // this test's rollback assertion depends on. + const endpointsRepo = await createTestReservationEndpointsRepo(testDb); + const spy = vi.spyOn(endpointsRepo, 'insertEndpoint').mockRejectedValueOnce(new Error('boom')); + const badEndpoints = [{ role: 'from', name: 'A', code: null, lat: 1, lng: 2, timezone: null, local_time: null, local_date: null }]; await expect(svc.create(String(trip.id), { title: 'Hotel', type: 'hotel', create_accommodation: { place_id: place.id, start_day_id: days[0].id, end_day_id: days[1].id }, @@ -968,15 +975,19 @@ describe('ReservationsService — quirk fixes (post-fold)', () => { } as never)).rejects.toThrow(); expect(testDb.prepare('SELECT COUNT(*) as c FROM reservations WHERE trip_id = ?').get(trip.id)).toEqual({ c: 0 }); expect(testDb.prepare('SELECT COUNT(*) as c FROM day_accommodations WHERE trip_id = ?').get(trip.id)).toEqual({ c: 0 }); + spy.mockRestore(); }); it('RESV-FIX-002: update is atomic — a failing endpoint save rolls back the field update', async () => { const { trip } = ownerTrip(); const res = createReservation(testDb, trip.id, { title: 'Old' }); const current = (await svc.getReservation(String(res.id), String(trip.id)))!; - const badEndpoints = [{ role: 'from', name: 'A', code: null, lat: 1, lng: 2, timezone: null, local_time: null, local_date: null, sequence: {} }]; + const endpointsRepo = await createTestReservationEndpointsRepo(testDb); + const spy = vi.spyOn(endpointsRepo, 'insertEndpoint').mockRejectedValueOnce(new Error('boom')); + const badEndpoints = [{ role: 'from', name: 'A', code: null, lat: 1, lng: 2, timezone: null, local_time: null, local_date: null }]; await expect(svc.update(String(res.id), String(trip.id), { title: 'New', endpoints: badEndpoints } as never, current)).rejects.toThrow(); expect(testDb.prepare('SELECT title FROM reservations WHERE id = ?').get(res.id)).toEqual({ title: 'Old' }); + spy.mockRestore(); }); }); diff --git a/server/tests/unit/nest/trip-read-model.service.test.ts b/server/tests/unit/nest/trip-read-model.service.test.ts index a805c9ff5..7f88d8643 100644 --- a/server/tests/unit/nest/trip-read-model.service.test.ts +++ b/server/tests/unit/nest/trip-read-model.service.test.ts @@ -84,6 +84,11 @@ import { createTestDaysRepo, createTestDayAssignmentsRepo, createTestDayNotesRepo, createTestTripsRepo, createTestTripMembersRepo, createTestTagsRepo, createTestPlaceRatingsRepo, createTestAssignmentParticipantsRepo, createTestGooglePlacePhotoMetaRepo, createTestPlacesRepo, createTestCategoriesRepo, + createTestReservationsRepo, + createTestReservationEndpointsRepo, + createTestReservationTravelersRepo, + createTestReservationDayPositionsRepo, + createTestDayAccommodationsRepo, } from '../../helpers/test-uow'; // Real sibling services over the same in-memory DB — the aggregation runs the @@ -123,6 +128,8 @@ beforeAll(async () => { await createTestDayAssignmentsRepo(dbs().connection), await createTestDayNotesRepo(dbs().connection), await createTestTripsRepo(dbs().connection), + await createTestReservationsRepo(dbs().connection), + await createTestReservationEndpointsRepo(dbs().connection), ); placesSvc = new PlacesService( dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), @@ -146,7 +153,7 @@ const buildReadModel = async (tripsRepo: TripsRepository, roster: TripMembersSer new TripReadModelService( tripsRepo, roster, daysSvc, accommodationsSvc, budgetSvc, new PackingService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), notificationsStub(), await createTestUnitOfWork(dbs().connection)), - new ReservationsService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), budgetSvc, new RealtimeService(), notificationsStub(), new ReservationsReadService(dbs()), await accommodationsOver(dbs()), await createTestUnitOfWork(dbs().connection)), + 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)), new CollabService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), notificationsStub(), makeStorageFixture('').storage, new RateLimitService(), await createTestUnitOfWork(dbs().connection)), placesSvc, new TodoService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), await createTestUnitOfWork(dbs().connection)), diff --git a/server/tests/unit/nest/trips.service.test.ts b/server/tests/unit/nest/trips.service.test.ts index ce0a6141e..7facb611d 100644 --- a/server/tests/unit/nest/trips.service.test.ts +++ b/server/tests/unit/nest/trips.service.test.ts @@ -93,6 +93,11 @@ import { createTestDaysRepo, createTestDayAssignmentsRepo, createTestDayNotesRepo, createTestTripsRepo, createTestTripMembersRepo, createTestTagsRepo, createTestPlaceRatingsRepo, createTestAssignmentParticipantsRepo, createTestGooglePlacePhotoMetaRepo, createTestPlacesRepo, createTestCategoriesRepo, + createTestReservationsRepo, + createTestReservationEndpointsRepo, + createTestReservationTravelersRepo, + createTestReservationDayPositionsRepo, + createTestDayAccommodationsRepo, } from '../../helpers/test-uow'; // Real sibling services over the same in-memory DB — updateTrip's date-shift @@ -156,6 +161,8 @@ beforeAll(async () => { await createTestDayAssignmentsRepo(dbs().connection), await createTestDayNotesRepo(dbs().connection), await createTestTripsRepo(dbs().connection), + await createTestReservationsRepo(dbs().connection), + await createTestReservationEndpointsRepo(dbs().connection), ); placesSvc = new PlacesService( dbs(), @@ -178,7 +185,7 @@ beforeAll(async () => { ); svc = new TripsService( dbs(), - new ReservationsService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), budgetSvc, new RealtimeService(), notificationsStub(), new ReservationsReadService(dbs()), accommodationsSvc, await createTestUnitOfWork(dbs().connection)), + 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)), accommodationsSvc, 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)), daysSvc, new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), budgetSvc, @@ -193,7 +200,7 @@ beforeAll(async () => { readModelSvc = new TripReadModelService( await createTestTripsRepo(dbs().connection), membersSvc, daysSvc, accommodationsSvc, budgetSvc, new PackingService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), notificationsStub(), await createTestUnitOfWork(dbs().connection)), - new ReservationsService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), budgetSvc, new RealtimeService(), notificationsStub(), new ReservationsReadService(dbs()), accommodationsSvc, await createTestUnitOfWork(dbs().connection)), + 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)), accommodationsSvc, 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)), new CollabService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), notificationsStub(), coversFx.storage, new RateLimitService(), await createTestUnitOfWork(dbs().connection)), placesSvc, new TodoService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), await createTestUnitOfWork(dbs().connection)), diff --git a/server/tests/unit/services/airtrailImport.test.ts b/server/tests/unit/services/airtrailImport.test.ts index 2ca25b81f..c49b07033 100644 --- a/server/tests/unit/services/airtrailImport.test.ts +++ b/server/tests/unit/services/airtrailImport.test.ts @@ -23,7 +23,7 @@ import type { AirtrailClient } from '../../../src/nest/integrations/airtrail.cli import type { AirtrailService } from '../../../src/nest/integrations/airtrail.service'; import { notificationsStub } from '../../helpers/notifications'; import { accommodationsOver } from '../../helpers/accommodations-service'; -import { createTestUnitOfWork, createTestAppSettingsRepo } from '../../helpers/test-uow'; +import { createTestUnitOfWork, createTestAppSettingsRepo, createTestReservationsRepo, createTestReservationEndpointsRepo, createTestReservationTravelersRepo, createTestReservationDayPositionsRepo, createTestDayAccommodationsRepo, createTestDaysRepo, createTestPlacesRepo, createTestDayAssignmentsRepo, createTestTripMembersRepo, createTestUsersRepo, createTestTripsRepo } from '../../helpers/test-uow'; // The client and the per-user credentials are the only stubs; the reservation // writes go through the real service against the real test DB, as before. They @@ -44,8 +44,12 @@ async function makeImportService(): Promise { new BudgetService(dbs(), permissions, new ExchangeRatesService(), realtime, await createTestUnitOfWork(dbs().connection)), realtime, notificationsStub(), - new ReservationsReadService(dbs()), + 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), ), { listFlights } as unknown as AirtrailClient, {