mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-22 06:56:35 +00:00
fix(server): plan 3d close-out — legacy text shape for stored accommodation ids restores the date-change restamp; reservation ids parsed once at the gate; parity tests for the reservation repositories
This commit is contained in:
jubnl
@@ -1,5 +1,4 @@
|
||||
import type { ReservationEndpoints } from '../entities/ReservationEndpoints.entity';
|
||||
import { Reservations } from '../entities/Reservations.entity';
|
||||
import { type AssertRowKeys } from './_shared/rows';
|
||||
import { TrekRepository } from './_shared/trek-repository';
|
||||
|
||||
@@ -21,51 +20,6 @@ export interface ReservationEndpointRow {
|
||||
|
||||
const _reservationEndpointRowKeys: AssertRowKeys<ReservationEndpointRow, ReservationEndpoints> = true;
|
||||
|
||||
/**
|
||||
* TP58's `SELECT *` shape — every scalar column of `Reservations`. Declared
|
||||
* here, not on `Reservations.repository.ts` (which has its own, differently
|
||||
* named row type for the same shape, RS35's `ReservationAllColumnsRow`):
|
||||
* Task 4 owns that file for Plan 3d Task 6's tree window, so the
|
||||
* reservations-copy read/write (TP58/TP59) live on this repository instead
|
||||
* — see {@link ReservationEndpointsRepository.listAllForTrip}.
|
||||
*/
|
||||
export interface ReservationCopySourceRow {
|
||||
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;
|
||||
}
|
||||
|
||||
const _reservationCopySourceRowKeys: AssertRowKeys<ReservationCopySourceRow, Reservations> = true;
|
||||
|
||||
/** Kysely typing for TP58 — this repository is templated on `ReservationEndpoints`, which has no relation to walk to `reservations`' own scalar columns, so the read goes through Kysely directly (program rule 23). */
|
||||
interface ReservationsCopyKyselyDB {
|
||||
reservations: ReservationCopySourceRow;
|
||||
}
|
||||
|
||||
export class ReservationEndpointsRepository extends TrekRepository<ReservationEndpoints> {
|
||||
/**
|
||||
* RS3 (`ReservationsService.loadEndpointsByTrip`) — `SELECT e.* FROM
|
||||
@@ -144,93 +98,4 @@ export class ReservationEndpointsRepository extends TrekRepository<ReservationEn
|
||||
async setLocalDate(id: number, local_date: string): Promise<void> {
|
||||
await this.nativeUpdate({ id }, { local_date });
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Plan 3d Task 6 (`trips.service.ts::copy`) — additive. Endpoints/travelers/
|
||||
// day-positions are never copied with a reservation (R7, `copy` never
|
||||
// copied them — kept); these two methods reach the PARENT `reservations`
|
||||
// table only, for TP58/TP59, because Task 4 owns `Reservations
|
||||
// .repository.ts` for this same tree window ("a needed method in the
|
||||
// other's repository is requested via the report, not written").
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* TP58 (`trips.service.ts::copy`'s reservations read) — `SELECT * FROM
|
||||
* reservations WHERE trip_id = ?`, no `ORDER BY` (matching the legacy
|
||||
* statement; the copy loop's own id-remap doesn't depend on read order).
|
||||
*/
|
||||
async listAllForTrip(trip_id: number): Promise<ReservationCopySourceRow[]> {
|
||||
return await this.kysely<ReservationsCopyKyselyDB>()
|
||||
.selectFrom('reservations')
|
||||
.selectAll()
|
||||
.where('trip_id', '=', trip_id)
|
||||
.execute();
|
||||
}
|
||||
|
||||
/**
|
||||
* TP59 (`trips.service.ts::copy`'s reservation INSERT) — `INSERT INTO
|
||||
* reservations (trip_id, day_id, end_day_id, place_id, assignment_id,
|
||||
* accommodation_id, title, reservation_time, reservation_end_time,
|
||||
* location, confirmation_number, notes, url, status, type, metadata,
|
||||
* day_plan_position, needs_review, ingest_state) VALUES (?×19)`. The
|
||||
* `external_*`/`sync_enabled`/`created_at` columns are deliberately
|
||||
* omitted (the legacy statement's own column list never named them — a
|
||||
* copy must not inherit the source's external sync identity).
|
||||
*
|
||||
* `this.getEntityManager().insert(Reservations, …)`, not `this.insert()`:
|
||||
* this repository is templated on `ReservationEndpoints`, so `this.insert`
|
||||
* would write THAT table. `getEntityManager()` is `TrekRepository`'s own
|
||||
* validated override (its class docstring), so this still fails closed
|
||||
* outside a request context/open transaction exactly like every other
|
||||
* write in this file — the escape hatch is safe, not a bypass.
|
||||
*
|
||||
* `accommodation_id` arrives ALREADY formatted as the legacy `'<id>.0'`
|
||||
* TEXT shape (`trips.service.ts`'s own `legacyAccommodationIdText`, the
|
||||
* program's Plan 3d Task 3 `"14.0"` finding): a plain JS number written
|
||||
* here would be inlined as a SQL literal (rule 22) and stored as `'14'`,
|
||||
* not the shape the legacy raw-bound statement produced.
|
||||
*/
|
||||
async insertReservationCopy(input: {
|
||||
trip_id: number;
|
||||
day_id: number | null;
|
||||
end_day_id: number | null;
|
||||
place_id: number | null;
|
||||
assignment_id: number | null;
|
||||
accommodation_id: string | 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 | null;
|
||||
type: string | null;
|
||||
metadata: string | null;
|
||||
day_plan_position: number | null;
|
||||
needs_review: number;
|
||||
ingest_state: string;
|
||||
}): Promise<number> {
|
||||
return await this.getEntityManager().insert(Reservations, {
|
||||
trip: input.trip_id,
|
||||
day: input.day_id,
|
||||
endDay: input.end_day_id,
|
||||
place: input.place_id,
|
||||
assignment: input.assignment_id,
|
||||
accommodation_id: input.accommodation_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,
|
||||
metadata: input.metadata,
|
||||
day_plan_position: input.day_plan_position,
|
||||
needs_review: input.needs_review,
|
||||
ingest_state: input.ingest_state,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
@@ -124,35 +124,42 @@ interface ReservationRestampKyselyDB {
|
||||
/**
|
||||
* Kysely typing for AC37/AC40 (`listIdMetadataByStay`/`listIdsByStay`).
|
||||
*
|
||||
* A bound-parameter `WHERE accommodation_id = ?` on this TEXT column is NOT
|
||||
* the same comparison the legacy statement's own `Number(id)` bind produced
|
||||
* (Task 3 finding, verified empirically against the live driver, not
|
||||
* assumed from the inventory's §18.1 wording): `better-sqlite3` binds every
|
||||
* plain JS number as SQLite REAL, integer-valued or not (`typeof(?)` on a
|
||||
* bound `14` reads back `'real'`) — including through MikroORM's Kysely
|
||||
* dialect — and SQLite's TEXT-affinity conversion of a REAL for a
|
||||
* comparison renders its decimal form (`14` → `'14.0'`), not the plain
|
||||
* integer text `'14'`. A row this repository itself writes through
|
||||
* `em.insert()`/`nativeUpdate()` stores the OTHER shape (`'14'`, no `.0` —
|
||||
* MikroORM inlines the value as a literal in the generated SQL, rule 22,
|
||||
* which SQLite's parser treats as a genuine INTEGER token, not a
|
||||
* REAL-bound parameter) — so a plain `WHERE accommodation_id = ?` bound
|
||||
* with a number would consistently MISS every row this cluster's own
|
||||
* `em.insert()` calls write, not just the documented `"14.0"` edge case.
|
||||
* {@link castIntegerKysely} (SQLF-057, `sql-functions.test.ts`) sidesteps
|
||||
* the whole shape question: `CAST(accommodation_id AS INTEGER) = ?` matches
|
||||
* `'14'` AND `'14.0'` alike (SQLite compares the CAST result against the
|
||||
* bound REAL/INTEGER numerically, not as text), which is what "every
|
||||
* linked booking" (AC37/AC40's own contract — more than one reservation,
|
||||
* written by different call sites with different shapes, can point at one
|
||||
* stay) actually needs, and is R2's own "tested with both `'14'` and
|
||||
* `'14.0'` rows" requirement satisfied structurally rather than by picking
|
||||
* a side.
|
||||
* **Revert to the REAL-bound compare (Plan 3d Task 7 whole-plan review, M3;
|
||||
* corrects this docstring's earlier claim).** A bound-parameter `WHERE
|
||||
* accommodation_id = ?` with a plain JS `number` is EXACTLY the comparison
|
||||
* the legacy statement's own `Number(id)` bind produced: `better-sqlite3`
|
||||
* binds every plain JS number as SQLite REAL, integer-valued or not
|
||||
* (`typeof(?)` on a bound `14` reads back `'real'`) — including through
|
||||
* MikroORM's Kysely dialect — and SQLite's TEXT-affinity conversion of a
|
||||
* REAL for a comparison renders its decimal form (`14` → `'14.0'`), so a
|
||||
* bound `WHERE accommodation_id = ?` matches `'14.0'` ONLY, same as the
|
||||
* legacy statement — never `'14'`. The earlier version of this docstring
|
||||
* claimed the opposite (that `em.insert()`/`nativeUpdate()` calls in this
|
||||
* cluster store the bare `'14'` shape and a REAL-bound compare would
|
||||
* therefore miss them): that was true only because RS28/RS41
|
||||
* (`reservations.service.ts`, outside this repository) used to write
|
||||
* `String(n)` instead of the legacy's REAL-bound shape (H1, the same
|
||||
* review). H1 fixes every write in this cluster to store `'<id>.0'`
|
||||
* (`legacyBoundIntegerText`), so the widening below has no remaining
|
||||
* reason to exist, and parity is law once it doesn't: `CAST(accommodation_id
|
||||
* AS INTEGER) = ?` (via {@link castIntegerKysely}) matches `'14'`, `'14.0'`,
|
||||
* `'14 '` and `'14abc'` alike — a WIDER row set than the legacy's own
|
||||
* `'14.0'`-only compare (measured, task-7-review.md M3) — which is an
|
||||
* unpinned parity break, not a fix, on a read the legacy also bound
|
||||
* `Number(id)` for. `castIntegerKysely` stays in use elsewhere in this file
|
||||
* (RS20/RV2's correlated subqueries) where the legacy statement's OWN
|
||||
* comparison used a `CAST`, not a bound number — this interface's two
|
||||
* consumers are not that case.
|
||||
*/
|
||||
interface ReservationsByAccommodationKyselyDB {
|
||||
reservations: {
|
||||
id: number;
|
||||
accommodation_id: string | null;
|
||||
// `string | number | null`, not just `string` (R2's ruling, the same
|
||||
// widening {@link ReservationRestampKyselyDB} documents for DY23): the
|
||||
// column is TEXT, but the bind below is a plain `number`, matching the
|
||||
// legacy's own `Number(id)` bind — the interface only needs to
|
||||
// type-check that bind, not describe the column's storage type.
|
||||
accommodation_id: string | number | null;
|
||||
metadata: string | null;
|
||||
};
|
||||
}
|
||||
@@ -924,16 +931,16 @@ export class ReservationsRepository extends TrekRepository<Reservations> {
|
||||
* AC37 (`AccommodationsService.updateAccommodation`) — `SELECT id,
|
||||
* metadata FROM reservations WHERE accommodation_id = ?`, every linked
|
||||
* booking (no unique constraint on the column — more than one booking can
|
||||
* point at the same stay, and different write paths can leave it in
|
||||
* either TEXT shape). `castIntegerKysely` (see
|
||||
* {@link ReservationsByAccommodationKyselyDB}'s docstring for why).
|
||||
* point at the same stay). Bound as a plain `number`, matching the
|
||||
* legacy's own `Number(id)` bind byte-for-byte — see
|
||||
* {@link ReservationsByAccommodationKyselyDB}'s docstring (M3, reverted
|
||||
* from the `castIntegerKysely` widening).
|
||||
*/
|
||||
async listIdMetadataByStay(accommodation_id: number): Promise<{ id: number; metadata: string | null }[]> {
|
||||
const platform = this.getEntityManager().getPlatform();
|
||||
const rows = await this.kysely<ReservationsByAccommodationKyselyDB>()
|
||||
.selectFrom('reservations')
|
||||
.select(['id', 'metadata'])
|
||||
.where((eb) => eb(castIntegerKysely(platform, eb, 'accommodation_id'), '=', accommodation_id))
|
||||
.where('accommodation_id', '=', accommodation_id)
|
||||
.execute();
|
||||
return rows as { id: number; metadata: string | null }[];
|
||||
}
|
||||
@@ -960,18 +967,110 @@ export class ReservationsRepository extends TrekRepository<Reservations> {
|
||||
/**
|
||||
* AC40 (`AccommodationsService.deleteAccommodation`) — `SELECT id FROM
|
||||
* reservations WHERE accommodation_id = ?`, ALL linked bookings. Same
|
||||
* `castIntegerKysely` reasoning as {@link listIdMetadataByStay}.
|
||||
* REAL-bound compare as {@link listIdMetadataByStay} (M3).
|
||||
*/
|
||||
async listIdsByStay(accommodation_id: number): Promise<{ id: number }[]> {
|
||||
const platform = this.getEntityManager().getPlatform();
|
||||
const rows = await this.kysely<ReservationsByAccommodationKyselyDB>()
|
||||
.selectFrom('reservations')
|
||||
.select(['id'])
|
||||
.where((eb) => eb(castIntegerKysely(platform, eb, 'accommodation_id'), '=', accommodation_id))
|
||||
.where('accommodation_id', '=', accommodation_id)
|
||||
.execute();
|
||||
return rows as { id: number }[];
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Plan 3d Task 6 (`trips.service.ts::copy`) — moved here from
|
||||
// `ReservationEndpoints.repository.ts` in the Task 7 whole-plan review's
|
||||
// fix wave (item 8/D4, "one repository per table"): Task 4 owned this file
|
||||
// for the Task 6 tree window, so TP58/TP59 (the reservations copy
|
||||
// read/write) landed on `ReservationEndpointsRepository` instead at the
|
||||
// time — a pure move now that the window is closed, same statements,
|
||||
// same parity proof (`Reservations.repository.test.ts`). Endpoints/
|
||||
// travelers/day-positions are never copied with a reservation (R7,
|
||||
// `copy` never copied them — kept as-is on their own repositories).
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* TP58 (`trips.service.ts::copy`'s reservations read) — `SELECT * FROM
|
||||
* reservations WHERE trip_id = ?`, no `ORDER BY` (matching the legacy
|
||||
* statement; the copy loop's own id-remap doesn't depend on read order).
|
||||
* `ReservationAllColumnsRow` (RS35's own `SELECT *` shape) rather than a
|
||||
* second, differently-named copy of the same fields.
|
||||
*/
|
||||
async listAllForTrip(trip_id: number): Promise<ReservationAllColumnsRow[]> {
|
||||
return await this.kysely<{ reservations: ReservationAllColumnsRow }>()
|
||||
.selectFrom('reservations')
|
||||
.selectAll()
|
||||
.where('trip_id', '=', trip_id)
|
||||
.execute();
|
||||
}
|
||||
|
||||
/**
|
||||
* TP59 (`trips.service.ts::copy`'s reservation INSERT) — `INSERT INTO
|
||||
* reservations (trip_id, day_id, end_day_id, place_id, assignment_id,
|
||||
* accommodation_id, title, reservation_time, reservation_end_time,
|
||||
* location, confirmation_number, notes, url, status, type, metadata,
|
||||
* day_plan_position, needs_review, ingest_state) VALUES (?×19)`. The
|
||||
* `external_*`/`sync_enabled`/`created_at` columns are deliberately
|
||||
* omitted (the legacy statement's own column list never named them — a
|
||||
* copy must not inherit the source's external sync identity).
|
||||
*
|
||||
* `this.insert(...)`, not `this.getEntityManager().insert(Reservations,
|
||||
* …)`: this repository IS templated on `Reservations` (unlike
|
||||
* `ReservationEndpointsRepository`, where this method used to live and
|
||||
* needed the escape hatch to reach a table it wasn't templated on) — the
|
||||
* ordinary `TrekRepository.insert` is the right call here.
|
||||
*
|
||||
* `accommodation_id` arrives ALREADY formatted as the legacy `'<id>.0'`
|
||||
* TEXT shape (`row-id.ts`'s `legacyBoundIntegerText`, the program's
|
||||
* Plan 3d Task 3 `"14.0"` finding): a plain JS number written here would
|
||||
* be inlined as a SQL literal (rule 22) and stored as `'14'`, not the
|
||||
* shape the legacy raw-bound statement produced.
|
||||
*/
|
||||
async insertReservationCopy(input: {
|
||||
trip_id: number;
|
||||
day_id: number | null;
|
||||
end_day_id: number | null;
|
||||
place_id: number | null;
|
||||
assignment_id: number | null;
|
||||
accommodation_id: string | 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 | null;
|
||||
type: string | null;
|
||||
metadata: string | null;
|
||||
day_plan_position: number | null;
|
||||
needs_review: number;
|
||||
ingest_state: string;
|
||||
}): Promise<number> {
|
||||
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,
|
||||
accommodation_id: input.accommodation_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,
|
||||
metadata: input.metadata,
|
||||
day_plan_position: input.day_plan_position,
|
||||
needs_review: input.needs_review,
|
||||
ingest_state: input.ingest_state,
|
||||
});
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Plan 3d Task 4 — additive (see the file-level comment above the interfaces
|
||||
// this section's methods return).
|
||||
|
||||
@@ -6,7 +6,7 @@ import { DatabaseService, type PlaceWithTags, type TripAccess } from '../databas
|
||||
import { UnitOfWork } from '../database/unit-of-work';
|
||||
import { PermissionsService } from '../permissions/permissions.service';
|
||||
import { AssignmentsService } from '../assignments/assignments.service';
|
||||
import { toRowId } from '../common/row-id';
|
||||
import { toRowId, legacyBoundIntegerText } from '../common/row-id';
|
||||
import { DayAccommodations } from '../../db/entities/DayAccommodations.entity';
|
||||
import type { DayAccommodationsRepository, DayAccommodationRow } from '../../db/repositories/DayAccommodations.repository';
|
||||
import { DayAssignments } from '../../db/entities/DayAssignments.entity';
|
||||
@@ -55,31 +55,6 @@ export interface AccommodationMirror {
|
||||
* stay row themselves and have to answer with a mirror either way. */
|
||||
export const noStayMirror = (): AccommodationMirror => ({ created: null, moved: null, updated: [], removed: [], stamped: null });
|
||||
|
||||
/**
|
||||
* AC33's `accommodation_id` value, in the exact TEXT shape the TRUE legacy
|
||||
* (pre-ORM) statement stored (Task 3 finding, verified empirically against
|
||||
* the live `better-sqlite3` driver, not assumed): the legacy `INSERT INTO
|
||||
* reservations (…, accommodation_id, …) VALUES (…, ?, …)` bound the new
|
||||
* stay's id — a plain JS `number` from `result.lastInsertRowid` — as a
|
||||
* `?` parameter, and `better-sqlite3` binds every plain JS number as SQLite
|
||||
* REAL regardless of integer-ness (confirmed via `typeof(?)`); SQLite's own
|
||||
* "numeric data inserted into a TEXT column is converted to text first"
|
||||
* rule then renders a REAL's decimal form, `14` → `'14.0'`, not the plain
|
||||
* integer text `'14'`. `accommodations.service.test.ts`'s DAY-SVC-020/023/
|
||||
* 025 etc. read this column back with their OWN plain-number-bound raw SQL
|
||||
* (`testDb.prepare('… WHERE accommodation_id = ?').get(accom.id)`, matching
|
||||
* the same REAL-binding path), so this write must reproduce that exact
|
||||
* shape for those UNCHANGED "proof" tests to keep passing — `String(id)`
|
||||
* (`'14'`, what `MikroORM.em.insert()` would store for either a number or a
|
||||
* string, since it inlines the value as a SQL-text literal, rule 22, a
|
||||
* DIFFERENT shape) does not. `ReservationsRepository`'s own reads of this
|
||||
* column (`listIdMetadataByStay`/`listIdsByStay`) go through
|
||||
* `castIntegerKysely` instead, which matches this shape AND `'14'` alike —
|
||||
* see that file's docstring.
|
||||
*/
|
||||
function legacyBoundIntegerText(id: number): string {
|
||||
return `${id}.0`;
|
||||
}
|
||||
const noMirror = noStayMirror;
|
||||
|
||||
export interface DayAccommodation {
|
||||
|
||||
@@ -92,3 +92,29 @@ export function toRowId(value: unknown): number | null {
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* The TEXT shape the legacy raw-SQL layer actually stored for an
|
||||
* `accommodation_id` value bound as a plain JS number (Plan 3d Task 3/7
|
||||
* finding, verified empirically against the live `better-sqlite3` driver,
|
||||
* not assumed): `better-sqlite3` binds every plain JS `number` as SQLite
|
||||
* REAL regardless of integer-ness (confirmed via `typeof(?)`), and SQLite's
|
||||
* own "numeric value inserted into a TEXT column becomes text" rule then
|
||||
* renders a REAL's decimal form — `14` → `'14.0'`, never the plain integer
|
||||
* text `'14'`. `String(id)` (what a SQL-literal-inlined `em.insert()`,
|
||||
* rule 22, would produce for a bound `number`) renders `'14'` instead — a
|
||||
* DIFFERENT stored shape from the legacy's — and a reader that still
|
||||
* compares against the REAL-bound form (`restampLinkedReservation`/DY23
|
||||
* binds the id as a number through Kysely, which is REAL again) silently
|
||||
* misses every row written that way.
|
||||
*
|
||||
* Use this wherever a `number` id is written into the `accommodation_id`
|
||||
* TEXT column so the stored bytes match what the legacy statement would
|
||||
* have stored for the same input — never `String(id)` for that column.
|
||||
* `ReservationsRepository`'s reads of this column that must match BOTH the
|
||||
* legacy shape and a `String(id)` shape go through `castIntegerKysely`
|
||||
* instead (see that file's docstring); this helper is for WRITES only.
|
||||
*/
|
||||
export function legacyBoundIntegerText(id: number): string {
|
||||
return `${id}.0`;
|
||||
}
|
||||
|
||||
@@ -12,7 +12,7 @@ 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 { toRowId, legacyBoundIntegerText } 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';
|
||||
@@ -733,10 +733,13 @@ export class ReservationsService {
|
||||
const resolvedAssignmentId = await this.resolvedOrNull('day_assignments', assignment_id || null);
|
||||
|
||||
// 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).
|
||||
// the legacy statement bound a plain JS number, which better-sqlite3
|
||||
// binds as SQLite REAL, so the TEXT column stores the `'<id>.0'` shape,
|
||||
// not the plain integer text `String(n)` renders — H1, Plan 3d Task 7
|
||||
// review: this used to be `String(n)`, which stored a DIFFERENT shape
|
||||
// than the legacy and silently broke `restampLinkedReservation`/DY23's
|
||||
// REAL-bound compare. `legacyBoundIntegerText` reproduces the legacy
|
||||
// bytes exactly.
|
||||
const insertedId = await this.reservationsRepo.insertReservation({
|
||||
trip_id: this.rowIdNum(tripId),
|
||||
day_id: resolvedDayId,
|
||||
@@ -752,7 +755,7 @@ export class ReservationsService {
|
||||
url: url || null,
|
||||
status: status || 'pending',
|
||||
type: resolvedType,
|
||||
accommodation_id: resolvedAccommodationId == null ? null : String(resolvedAccommodationId),
|
||||
accommodation_id: resolvedAccommodationId == null ? null : legacyBoundIntegerText(resolvedAccommodationId),
|
||||
metadata: metadata ? JSON.stringify(metadata) : null,
|
||||
needs_review: needs_review ? 1 : 0,
|
||||
});
|
||||
@@ -793,8 +796,16 @@ 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 dayIdNum = this.rowIdNum(dayId);
|
||||
//
|
||||
// M4, Plan 3d Task 7 review: `dayId` parsed with `toRowId` (rule 21),
|
||||
// not `rowIdNum` — `rowIdNum`'s `Number(...)` fallback let a
|
||||
// hex/exponent `dayId` (`0x1`, `1e1`) coerce to a real day and reach
|
||||
// the join, where the legacy raw-bind statement's affinity never
|
||||
// converts a hex string and so matched no row. A miss here is the same
|
||||
// quiet no-op the join already gives a stale id — no write happens.
|
||||
const dayIdNum = toRowId(dayId);
|
||||
const tripIdNum = this.rowIdNum(tripId);
|
||||
if (dayIdNum === null) return;
|
||||
await this.uow.transactional(async () => {
|
||||
for (const item of positions) {
|
||||
// RS31/RS32. position is NOT NULL while the wire contract leaves the value optional.
|
||||
@@ -858,16 +869,32 @@ export class ReservationsService {
|
||||
// `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
|
||||
// always the TEXT column's string; `UpdateReservationData.accommodation_id`
|
||||
// carries the same lie for the INCOMING payload — the wire schema is
|
||||
// `z.union([z.number(), z.string()])`, so a string reaches here despite
|
||||
// the declared `number` type): when the payload names `accommodation_id`
|
||||
// it is USER INPUT and goes through `toRowId` (L2, Plan 3d Task 7
|
||||
// review — `Number('0x1')` used to coerce a hex-spelled link and pass
|
||||
// the existence check below where the legacy raw-bind statement's own
|
||||
// affinity never would; `toRowId` returns `null` for any non-canonical
|
||||
// shape, including a `'1.0'`/`' 1'` string — an ACCEPTED rule-15
|
||||
// narrowing here, since a payload value is untrusted input, unlike the
|
||||
// fallback below); 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);
|
||||
// update, which running it through `toRowId` — or a `Number()`/
|
||||
// `String()` round-trip — would break: `toRowId('14.0')` is `null`,
|
||||
// which would silently drop the link on every edit that doesn't touch
|
||||
// `accommodation_id`; OUR OWN previously-written data is not the input
|
||||
// L2 is about). `accIdForRead(...)` below is the READ-only numeric form
|
||||
// every repository call needs; the final write formats a genuine
|
||||
// `number` through `legacyBoundIntegerText` (H1, Plan 3d Task 7 review —
|
||||
// a bare `String(...)` stored `'<id>'`, not the legacy's REAL-bound
|
||||
// `'<id>.0'`) and passes an already-string value through unchanged, a
|
||||
// no-op.
|
||||
let resolvedAccId: number | string | null = accommodation_id !== undefined
|
||||
? (accommodation_id == null ? null : toRowId(accommodation_id))
|
||||
: (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
|
||||
@@ -987,7 +1014,7 @@ export class ReservationsService {
|
||||
assignment_id: nextAssignmentId,
|
||||
status: status || current.status,
|
||||
type: type || current.type,
|
||||
accommodation_id: resolvedAccId == null ? null : String(resolvedAccId),
|
||||
accommodation_id: resolvedAccId == null ? null : (typeof resolvedAccId === 'number' ? legacyBoundIntegerText(resolvedAccId) : 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),
|
||||
});
|
||||
@@ -1023,8 +1050,16 @@ 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 | null; accommodation_id: string | null } | undefined; accommodationDeleted: boolean; deletedBudgetItemId: number | null }> {
|
||||
const idNum = this.rowIdNum(id);
|
||||
const tripIdNum = this.rowIdNum(tripId);
|
||||
// M4, Plan 3d Task 7 review: parsed ONCE here with `toRowId` (rule 21),
|
||||
// not `rowIdNum` — `rowIdNum`'s `Number(...)` fallback let a hex/exponent
|
||||
// id (`0x1`, `1e1`) reach `findHeaderInTrip` and delete a real row where
|
||||
// the legacy raw-bind statement 404'd. A miss here answers exactly the
|
||||
// `findHeaderInTrip`-miss shape below, without touching the DB.
|
||||
const idNum = toRowId(id);
|
||||
const tripIdNum = toRowId(tripId);
|
||||
if (idNum === null || tripIdNum === null) {
|
||||
return { deleted: undefined, accommodationDeleted: false, deletedBudgetItemId: null };
|
||||
}
|
||||
const removed = await this.uow.transactional(async () => {
|
||||
// RS45
|
||||
const reservation = await this.reservationsRepo.findHeaderInTrip(idNum, tripIdNum);
|
||||
|
||||
@@ -84,13 +84,24 @@ export class RoadtripService {
|
||||
return await this.daysRepo.existsInTrip(dayId, tripId);
|
||||
}
|
||||
|
||||
// L1 (Plan 3d Task 7 whole-plan review): `toRowId`, not `Number()` — a
|
||||
// hex/exponent-spelled id used to coerce to a real row and return its
|
||||
// vias/tracks, where the legacy raw-bind statement's affinity never
|
||||
// converts a hex string and so matched nothing. `TripAccessGuard` already
|
||||
// authorised the SAME hex-spelled id through its own `Number()` (not a
|
||||
// leak, rule-21 seam widening) — a miss here answers the legacy empty
|
||||
// shape, matching every other read path in this class.
|
||||
async listForDay(dayId: string | number): Promise<RoadtripVia[]> {
|
||||
return await this.viasRepo.listForDay(Number(dayId));
|
||||
const dayIdNum = toRowId(dayId);
|
||||
if (dayIdNum === null) return [];
|
||||
return await this.viasRepo.listForDay(dayIdNum);
|
||||
}
|
||||
|
||||
/** Every via of a trip, so the client can route all days without one request per day. */
|
||||
async listForTrip(tripId: string | number): Promise<RoadtripVia[]> {
|
||||
return await this.viasRepo.listForTrip(Number(tripId));
|
||||
const tripIdNum = toRowId(tripId);
|
||||
if (tripIdNum === null) return [];
|
||||
return await this.viasRepo.listForTrip(tripIdNum);
|
||||
}
|
||||
|
||||
async create(dayId: string | number, input: { after_order_index: number; lat: number; lng: number; sequence?: number }): Promise<RoadtripVia> {
|
||||
@@ -162,12 +173,16 @@ export class RoadtripService {
|
||||
* second route for a handful of rows would be a second round trip for nothing.
|
||||
*/
|
||||
async tracksForTrip(tripId: string | number): Promise<RoadtripDayTrack[]> {
|
||||
return await this.tracksRepo.listForTrip(Number(tripId));
|
||||
const tripIdNum = toRowId(tripId);
|
||||
if (tripIdNum === null) return [];
|
||||
return await this.tracksRepo.listForTrip(tripIdNum);
|
||||
}
|
||||
|
||||
/** Whether a place is on this trip, and is a track rather than an ordinary place. RT13 → `PlacesRepository.isTrackInTrip`. */
|
||||
async trackExists(placeId: number, tripId: string | number): Promise<boolean> {
|
||||
return await this.placesRepo.isTrackInTrip(placeId, Number(tripId));
|
||||
const tripIdNum = toRowId(tripId);
|
||||
if (tripIdNum === null) return false;
|
||||
return await this.placesRepo.isTrackInTrip(placeId, tripIdNum);
|
||||
}
|
||||
|
||||
/** Moving a via is the whole edit; where it sits in the chain does not change. */
|
||||
|
||||
@@ -15,7 +15,7 @@ import { RoadtripDayTracks } from '../../db/entities/RoadtripDayTracks.entity';
|
||||
import { RoadtripPreferences } from '../../db/entities/RoadtripPreferences.entity';
|
||||
import { RoadtripDayBoundaries } from '../../db/entities/RoadtripDayBoundaries.entity';
|
||||
import { DayAccommodations } from '../../db/entities/DayAccommodations.entity';
|
||||
import { ReservationEndpoints } from '../../db/entities/ReservationEndpoints.entity';
|
||||
import { Reservations } from '../../db/entities/Reservations.entity';
|
||||
import { MAX_TRIP_DAYS, tripSpanDays, type ActiveTrip, type TrekWsPayload, type TrekWsTripEventName } from '@trek/shared';
|
||||
import { RealtimeService } from '../realtime/realtime.service';
|
||||
import { PermissionsService } from '../permissions/permissions.service';
|
||||
@@ -28,6 +28,7 @@ import { UnsplashService } from '../unsplash/unsplash.service';
|
||||
import { StorageService } from '../storage/storage.service';
|
||||
import { NotFoundError, ValidationError } from '../common/domain-errors';
|
||||
import { UnitOfWork } from '../database/unit-of-work';
|
||||
import { legacyBoundIntegerText } from '../common/row-id';
|
||||
|
||||
export const MS_PER_DAY = 86400000;
|
||||
|
||||
@@ -58,26 +59,6 @@ export function withoutFeedToken<T>(row: T): T {
|
||||
return row;
|
||||
}
|
||||
|
||||
/**
|
||||
* `copy`'s (TP59) `reservations.accommodation_id` write. Mirrors
|
||||
* `AccommodationsService`'s own `legacyBoundIntegerText` (Plan 3d Task 3,
|
||||
* `accommodations.service.ts` — unexported there, so not importable across
|
||||
* this task's file-ownership boundary; reproduced here instead, same
|
||||
* one-line shape): a plain JS number bound as a `?` parameter through
|
||||
* better-sqlite3 is stored as SQLite REAL regardless of integer-ness, and
|
||||
* SQLite's "numeric value inserted into a TEXT column becomes text" rule
|
||||
* then renders `14` as `'14.0'`, not the plain integer text `'14'` a
|
||||
* SQL-literal-inlined `em.insert()` (rule 22) would produce. The legacy raw
|
||||
* `copy` statement bound the new stay's id (a plain number from `accomMap`)
|
||||
* the same way — reproduced explicitly here since the write now goes
|
||||
* through a typed insert (`ReservationEndpointsRepository
|
||||
* .insertReservationCopy`) instead of a raw bound parameter; passing an
|
||||
* already-formatted STRING sidesteps the shape question entirely (SQLite
|
||||
* applies no numeric conversion to a value that already has TEXT affinity).
|
||||
*/
|
||||
function legacyAccommodationIdText(id: number): string {
|
||||
return `${id}.0`;
|
||||
}
|
||||
|
||||
interface CreateTripData {
|
||||
title: string;
|
||||
@@ -257,13 +238,14 @@ export class TripsService {
|
||||
return this.em.getRepository(DayAccommodations);
|
||||
}
|
||||
|
||||
// `ReservationEndpointsRepository`, not `ReservationsRepository`: Task 4
|
||||
// owns `Reservations.repository.ts` for this tree window, so TP58/TP59
|
||||
// (the reservations read/insert) live on a repository this task owns
|
||||
// instead — see `ReservationEndpoints.repository.ts`'s own docstring on
|
||||
// `listAllForTrip`/`insertReservationCopy`.
|
||||
private get reservationEndpointsRepo() {
|
||||
return this.em.getRepository(ReservationEndpoints);
|
||||
// Plan 3d Task 7 whole-plan review (item 8/D4): TP58/TP59 (the
|
||||
// reservations copy read/insert) moved onto `ReservationsRepository` —
|
||||
// the table they read/write — out of `ReservationEndpointsRepository`,
|
||||
// where they used to live only because Task 4 owned this file for the
|
||||
// Task 6 tree window. Same `this.em.getRepository(...)` pattern as every
|
||||
// other getter above.
|
||||
private get reservationsRepo() {
|
||||
return this.em.getRepository(Reservations);
|
||||
}
|
||||
|
||||
async canAccessTrip(tripId: string | number, userId: number) {
|
||||
@@ -814,7 +796,7 @@ export class TripsService {
|
||||
if (newAssignmentId && newAccomId) await this.dayAssignmentsRepo.setAccommodation(newAssignmentId, newAccomId); // TP57
|
||||
}
|
||||
|
||||
const oldReservations = await this.reservationEndpointsRepo.listAllForTrip(Number(sourceTripId)); // TP58
|
||||
const oldReservations = await this.reservationsRepo.listAllForTrip(Number(sourceTripId)); // TP58
|
||||
// The external_* / sync_enabled columns are deliberately not copied: the
|
||||
// duplicate must not inherit the source's external sync identity.
|
||||
const reservationMap = new Map<number, number>();
|
||||
@@ -823,7 +805,7 @@ export class TripsService {
|
||||
// as a string — coerce before the number-keyed map lookup or the
|
||||
// link silently nulls.
|
||||
const newAccomId = r.accommodation_id != null ? (accomMap.get(Number(r.accommodation_id)) ?? null) : null;
|
||||
const newReservationId = await this.reservationEndpointsRepo.insertReservationCopy({
|
||||
const newReservationId = await this.reservationsRepo.insertReservationCopy({
|
||||
trip_id: newTripId,
|
||||
day_id: r.day_id ? (dayMap.get(r.day_id) ?? null) : null,
|
||||
// end_day_id is a day reference too (multi-day transport) — remap it like
|
||||
@@ -832,8 +814,8 @@ export class TripsService {
|
||||
place_id: r.place_id ? (placeMap.get(r.place_id) ?? null) : null,
|
||||
assignment_id: r.assignment_id ? (assignmentMap.get(r.assignment_id) ?? null) : null,
|
||||
// the NEW value is re-formatted to the legacy raw-bound `'<id>.0'`
|
||||
// TEXT shape on the way back out — see `legacyAccommodationIdText`.
|
||||
accommodation_id: newAccomId != null ? legacyAccommodationIdText(newAccomId) : null,
|
||||
// TEXT shape on the way back out — see `legacyBoundIntegerText`.
|
||||
accommodation_id: newAccomId != null ? legacyBoundIntegerText(newAccomId) : null,
|
||||
title: r.title, reservation_time: r.reservation_time, reservation_end_time: r.reservation_end_time,
|
||||
location: r.location, confirmation_number: r.confirmation_number, notes: r.notes, url: r.url, status: r.status, type: r.type,
|
||||
// ingest_state travels with the copy: a staged booking must not turn
|
||||
|
||||
@@ -121,6 +121,16 @@ describe('Roadtrip e2e (real guard chain + temp SQLite)', () => {
|
||||
expect(read).toHaveBeenCalledTimes(1);
|
||||
} finally { read.mockRestore(); }
|
||||
});
|
||||
// L3 (Plan 3d Task 7 review): unlike every other case in this file,
|
||||
// `ChargingService.read` is NOT mocked here — this is the one place CH1
|
||||
// (`PlacesRepository.findChargingProbe`) runs for real, through the whole
|
||||
// guard chain. Place 11 is a real row, just on trip 6, not trip 5 — the
|
||||
// gate answers "Place not found", the same shape a place that never
|
||||
// existed gets, never a leak of trip 6's row.
|
||||
it('CH1 — a place id that belongs to a DIFFERENT trip is "Place not found", not trip 6\'s row (real findChargingProbe, no service mock)', async () => {
|
||||
await request(server).get('/api/trips/5/roadtrip/charging/11').set('Cookie', cookie())
|
||||
.expect(404, { error: 'Place not found' });
|
||||
});
|
||||
it('gates the coordinate lookup like the saved stop and validates before fetching', async () => {
|
||||
const lookup = vi.spyOn(app.get(ChargingService), 'lookup').mockResolvedValue({} as never);
|
||||
const body = { lat: 48.137, lng: 11.575, name: 'Ladepark Nord' };
|
||||
@@ -351,6 +361,34 @@ describe('Roadtrip e2e (real guard chain + temp SQLite)', () => {
|
||||
expect(after.body.vias).toEqual([]);
|
||||
});
|
||||
|
||||
// L1 (Plan 3d Task 7 whole-plan review): `listForDay`/`listForTrip`/
|
||||
// `tracksForTrip` used to bind `Number(tripId)`, which a hex-spelled id
|
||||
// coerces to a real trip — `TripAccessGuard` had already authorised the
|
||||
// SAME id through its own `Number()` (not a leak), but the legacy
|
||||
// raw-bind statement's affinity never converts a hex string, so it
|
||||
// always answered empty. Every one of these three now answers the
|
||||
// legacy empty shape for the same hex id, not the real trip's rows.
|
||||
it('L1 — vias/tracks by a hex-spelled trip id answer the legacy empty shape, not trip 5\'s real rows', async () => {
|
||||
const created = await request(server)
|
||||
.post('/api/trips/5/roadtrip/days/3/vias')
|
||||
.set('Cookie', cookie())
|
||||
.send({ after_order_index: 0, lat: 53, lng: 10 });
|
||||
expect(created.status).toBe(201);
|
||||
|
||||
const hexTripId = '0x' + (5).toString(16);
|
||||
const res = await request(server).get(`/api/trips/${hexTripId}/roadtrip/vias`).set('Cookie', cookie());
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body).toEqual({ vias: [], tracks: [] });
|
||||
|
||||
const hexDayId = '0x' + (3).toString(16);
|
||||
const dayRes = await request(server).get(`/api/trips/5/roadtrip/days/${hexDayId}/vias`).set('Cookie', cookie());
|
||||
// `requireDay` (`toRowId`, unrelated to this fix) already 404s a
|
||||
// hex-spelled day id — asserted here only to document that this route
|
||||
// never reaches `listForDay` with one, so `listForDay`'s own fix is
|
||||
// pinned at the service level instead (see roadtrip.service.test.ts).
|
||||
expect(dayRes.status).toBe(404);
|
||||
});
|
||||
|
||||
it('ROADTRIP-E2E-010: the batch route answers 200 and records the track it was fitted to', async () => {
|
||||
const res = await request(server)
|
||||
.post('/api/trips/5/roadtrip/days/3/vias/batch')
|
||||
|
||||
@@ -121,6 +121,15 @@ describe('airtrail-sync cron tick runs inside a request context', () => {
|
||||
}
|
||||
return originalGetReservation(id, tripId);
|
||||
});
|
||||
// Item 10 (Plan 3d Task 7 review): the tick's own request-context proof
|
||||
// stopped at `getReservation` — `syncOwner` only calls `update` AFTER
|
||||
// that read, when the incoming flight's hash actually differs from the
|
||||
// stored `external_hash` (it does here, 'stale-hash' vs anything real),
|
||||
// so a context regression that only showed up on the WRITE half of this
|
||||
// same tick was invisible to this test. `update` is real here (not
|
||||
// re-mocked), so this also proves it runs to completion without itself
|
||||
// throwing `cannotUseGlobalContext`.
|
||||
const updateSpy = vi.spyOn(reservations, 'update');
|
||||
|
||||
const isEnabledSpy = vi.spyOn(registrar, 'isEnabled').mockReturnValue(true);
|
||||
const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
|
||||
@@ -141,6 +150,7 @@ describe('airtrail-sync cron tick runs inside a request context', () => {
|
||||
expect(caught).toBeUndefined();
|
||||
expect(repoRead).toBeTruthy();
|
||||
expect(getReservationSpy).toHaveBeenCalled();
|
||||
expect(updateSpy).toHaveBeenCalled();
|
||||
|
||||
const suspicious = errSpy.mock.calls
|
||||
.map((args) => args.map(String).join(' '))
|
||||
@@ -151,6 +161,7 @@ describe('airtrail-sync cron tick runs inside a request context', () => {
|
||||
isEnabledSpy.mockRestore();
|
||||
errSpy.mockRestore();
|
||||
getReservationSpy.mockRestore();
|
||||
updateSpy.mockRestore();
|
||||
flightsSpy.mockRestore();
|
||||
credsSpy.mockRestore();
|
||||
syncEnabledSpy.mockRestore();
|
||||
|
||||
@@ -163,4 +163,56 @@ describe('Anonymous ICS feed routes run inside a request context', () => {
|
||||
expect(res.status).toBe(404);
|
||||
expect(res.body).toEqual({ error: 'Feed not found' });
|
||||
});
|
||||
|
||||
/**
|
||||
* M2 (Plan 3d Task 0 review, folded into Task 7's fix wave): SEAM-FEED-001..
|
||||
* 004 only ever drove `/api/feed/trip/…`; the user feed had no valid-token
|
||||
* case at all. `FeedsService.buildUserIcs` catches EVERY per-trip failure
|
||||
* into `// skip failed trips` (`feeds.service.ts:151`) and still answers
|
||||
* 200 with an EMPTY calendar — so a `cannotUseGlobalContext` regression on
|
||||
* this route is invisible to a status/body check the same way SEAM-FEED-003
|
||||
* showed for the trip feed, but WORSE: the trip feed at least degrades to a
|
||||
* 404, while this route stays 200 either way. The assertion that actually
|
||||
* catches it is SEAM-FEED-001's own shape — a genuine repository read
|
||||
* happened, against the real request-scoped `MikroORM`, AND the sentinel
|
||||
* event the stub returns is actually IN the body — never a status check
|
||||
* alone, which cannot tell "the read happened" from "every trip's read was
|
||||
* silently swallowed".
|
||||
*/
|
||||
it('SEAM-FEED-005: GET /api/feed/user/:token.ics for a VALID token performs a real repository read and the resulting event reaches the body, inside the request, against the SAME MikroORM app.get(MikroORM) does', async () => {
|
||||
const orm = app.get(MikroORM);
|
||||
const { user } = createUser(testDb);
|
||||
createTrip(testDb, user.id, { title: 'User-Feed Context-Proof Trip' });
|
||||
const token = randomUUID();
|
||||
testDb.prepare('UPDATE users SET feed_token = ? WHERE id = ?').run(token, user.id);
|
||||
|
||||
let repoRead: unknown;
|
||||
let caught: unknown;
|
||||
const calendar = app.get(CalendarService);
|
||||
const spy = vi.spyOn(calendar, 'buildTripCalendar').mockImplementation(async (tripId: string | number) => {
|
||||
try {
|
||||
repoRead = await orm.em.getRepository(Trips).findOne({ id: Number(tripId) });
|
||||
} catch (e) {
|
||||
caught = e;
|
||||
}
|
||||
return { calName: 'Stub Calendar', filename: 'stub.ics', timezones: new Map(), events: ['BEGIN:VEVENT\r\nSUMMARY:Sentinel Event\r\nEND:VEVENT\r\n'] };
|
||||
});
|
||||
try {
|
||||
const httpApp = app.getHttpAdapter().getInstance() as Application;
|
||||
const res = await request(httpApp).get(`/api/feed/user/${token}.ics`);
|
||||
|
||||
expect(caught).toBeUndefined();
|
||||
expect(repoRead).toBeTruthy();
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.headers['content-type']).toMatch(/text\/calendar/);
|
||||
// NOT a status-only check: this is what distinguishes a genuine read
|
||||
// from the route's own per-trip swallow, which would return the SAME
|
||||
// 200 with an EMPTY calendar (no VEVENT at all) if the read failed
|
||||
// with a request-context error.
|
||||
expect(res.text).toContain('BEGIN:VEVENT');
|
||||
expect(res.text).toContain('Sentinel Event');
|
||||
} finally {
|
||||
spy.mockRestore();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -25,7 +25,7 @@ vi.mock('../../src/websocket', () => ({ broadcast: vi.fn(), broadcastToUser: vi.
|
||||
import { db as testDb } from '../../src/db/database';
|
||||
import { buildApp } from '../../src/bootstrap';
|
||||
import { resetTestDb, resetRateLimits } from '../helpers/test-db';
|
||||
import { createUser, createTrip, createDay, createPlace, createReservation, addTripMember } from '../helpers/factories';
|
||||
import { createUser, createTrip, createDay, createPlace, createReservation, createDayAssignment, addTripMember } from '../helpers/factories';
|
||||
import { authCookie } from '../helpers/auth';
|
||||
|
||||
let nestApp: INestApplication;
|
||||
@@ -265,6 +265,145 @@ describe('Update reservation', () => {
|
||||
expect(accom.check_out).toBe('11:00');
|
||||
expect(accom.confirmation).toBe('HTL-XYZ-999');
|
||||
});
|
||||
|
||||
// L2 (Plan 3d Task 7 whole-plan review): a hex-spelled `accommodation_id`
|
||||
// in the PUT body used to coerce via `Number('0x1')` and pass the
|
||||
// existence check, storing the raw hex string as the link — where the
|
||||
// legacy raw-bind existence check's own affinity never converts a hex
|
||||
// string, so it always stored NULL instead.
|
||||
it('L2 — PUT with a hex-spelled accommodation_id stores NULL, matching the legacy affinity miss', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const day = createDay(testDb, trip.id);
|
||||
const place = createPlace(testDb, trip.id, { name: 'Hotel' });
|
||||
const { accommodation: accom } = (
|
||||
await request(app)
|
||||
.post(`/api/trips/${trip.id}/accommodations`)
|
||||
.set('Cookie', authCookie(user.id))
|
||||
.send({ place_id: place.id, start_day_id: day.id, end_day_id: day.id })
|
||||
).body as { accommodation: { id: number } };
|
||||
const resv = createReservation(testDb, trip.id, { title: 'Flight', type: 'flight' });
|
||||
|
||||
const hexAccId = '0x' + accom.id.toString(16);
|
||||
const res = await request(app)
|
||||
.put(`/api/trips/${trip.id}/reservations/${resv.id}`)
|
||||
.set('Cookie', authCookie(user.id))
|
||||
.send({ accommodation_id: hexAccId });
|
||||
expect(res.status).toBe(200);
|
||||
|
||||
const row = testDb.prepare('SELECT accommodation_id FROM reservations WHERE id = ?').get(resv.id) as { accommodation_id: string | null };
|
||||
expect(row.accommodation_id).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// H1 (Plan 3d Task 7 whole-plan review, live regression) — RS28's
|
||||
// `accommodation_id` write must store the same TEXT shape the legacy
|
||||
// raw-bind statement did (`'<id>.0'`, a REAL-bound number, never `'<id>'`),
|
||||
// because `DaysService.resyncAccommodationDays`'s DY23 restamp
|
||||
// (`ReservationsRepository.restampLinkedReservation`) still compares
|
||||
// against that REAL-bound shape (parity, not a fix — see its own
|
||||
// docstring). A `String(n)` write silently orphans the linked reservation:
|
||||
// the trip's dates change, the day plan moves, and the booking is left
|
||||
// behind with no error anywhere.
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
describe('H1 — a booking on a stay is restamped when the trip\'s dates change (DY23)', () => {
|
||||
it('the accommodation_id RS28 stores is the legacy REAL-bound TEXT shape, and a later date change restamps the linked booking', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id, { start_date: '2026-12-01', end_date: '2026-12-03' });
|
||||
const day1 = testDb.prepare('SELECT id FROM days WHERE trip_id = ? AND date = ?').get(trip.id, '2026-12-01') as { id: number };
|
||||
const day2 = testDb.prepare('SELECT id FROM days WHERE trip_id = ? AND date = ?').get(trip.id, '2026-12-02') as { id: number };
|
||||
const place = createPlace(testDb, trip.id, { name: 'Lighthouse Inn' });
|
||||
|
||||
const createRes = await request(app)
|
||||
.post(`/api/trips/${trip.id}/reservations`)
|
||||
.set('Cookie', authCookie(user.id))
|
||||
.send({
|
||||
title: 'Lighthouse Inn Stay',
|
||||
type: 'hotel',
|
||||
day_id: day1.id,
|
||||
reservation_time: '2026-12-01T10:00',
|
||||
create_accommodation: { place_id: place.id, start_day_id: day1.id, end_day_id: day2.id },
|
||||
});
|
||||
expect(createRes.status).toBe(201);
|
||||
const resvId = createRes.body.reservation.id;
|
||||
|
||||
// Stored-shape assert: the legacy REAL-bound TEXT shape (`'<id>.0'`), not
|
||||
// the SQL-literal-inlined shape (`'<id>'`) `String(n)` used to store.
|
||||
const stored = testDb.prepare('SELECT accommodation_id FROM reservations WHERE id = ?').get(resvId) as { accommodation_id: string };
|
||||
expect(stored.accommodation_id).toMatch(/^\d+\.0$/);
|
||||
|
||||
// Move the whole trip a day later (default date_shift_mode, i.e. NOT
|
||||
// 'shift_all'): days re-date positionally in place, the accommodation
|
||||
// stays glued to its (now re-dated) day rows (#1288), and its linked
|
||||
// reservation must follow — DY23's restamp.
|
||||
const updateRes = await request(app)
|
||||
.put(`/api/trips/${trip.id}`)
|
||||
.set('Cookie', authCookie(user.id))
|
||||
.send({ start_date: '2026-12-02', end_date: '2026-12-04' });
|
||||
expect(updateRes.status).toBe(200);
|
||||
|
||||
const resvAfter = testDb.prepare('SELECT day_id, reservation_time FROM reservations WHERE id = ?').get(resvId) as
|
||||
{ day_id: number; reservation_time: string | null };
|
||||
expect(resvAfter.day_id).toBe(day1.id);
|
||||
// Restamped onto day1's NEW date — red without the fix, where DY23's
|
||||
// REAL-bound compare misses a `String(n)`-shaped accommodation_id and
|
||||
// this stays '2026-12-01T10:00'.
|
||||
expect(resvAfter.reservation_time).toBe('2026-12-02T10:00');
|
||||
});
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// L3 (Plan 3d Task 7 whole-plan review): RS22 (`referencesOutsideTrip`'s
|
||||
// `assignment_id` cross-trip check, `reservations.service.ts:540-541` +
|
||||
// `Reservations.repository.ts::getAssignmentTripId`) had zero hits in the
|
||||
// whole suite (lcov × diff). Pinned here through the real REST route.
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
describe('RS22 — assignment_id foreign-reference check on POST /reservations', () => {
|
||||
it('RS22-own: an assignment_id belonging to the SAME trip is accepted (201)', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const day = createDay(testDb, trip.id);
|
||||
const place = createPlace(testDb, trip.id);
|
||||
const assignment = createDayAssignment(testDb, day.id, place.id);
|
||||
|
||||
const res = await request(app)
|
||||
.post(`/api/trips/${trip.id}/reservations`)
|
||||
.set('Cookie', authCookie(user.id))
|
||||
.send({ title: 'Dinner', type: 'restaurant', assignment_id: assignment.id });
|
||||
expect(res.status).toBe(201);
|
||||
expect(res.body.reservation.assignment_id).toBe(assignment.id);
|
||||
});
|
||||
|
||||
it('RS22-foreign: an assignment_id belonging to a DIFFERENT trip is refused (400 "Not part of this trip: assignment_id")', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const otherTrip = createTrip(testDb, user.id);
|
||||
const otherDay = createDay(testDb, otherTrip.id);
|
||||
const otherPlace = createPlace(testDb, otherTrip.id);
|
||||
const foreignAssignment = createDayAssignment(testDb, otherDay.id, otherPlace.id);
|
||||
|
||||
const res = await request(app)
|
||||
.post(`/api/trips/${trip.id}/reservations`)
|
||||
.set('Cookie', authCookie(user.id))
|
||||
.send({ title: 'Dinner', type: 'restaurant', assignment_id: foreignAssignment.id });
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body).toEqual({ error: 'Not part of this trip: assignment_id' });
|
||||
});
|
||||
|
||||
it('RS22-missing: an assignment_id that resolves to nothing is refused (400 "Unknown reference: assignment_id")', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
|
||||
const res = await request(app)
|
||||
.post(`/api/trips/${trip.id}/reservations`)
|
||||
.set('Cookie', authCookie(user.id))
|
||||
.send({ title: 'Dinner', type: 'restaurant', assignment_id: 999999 });
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body).toEqual({ error: 'Unknown reference: assignment_id' });
|
||||
});
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
@@ -298,6 +437,25 @@ describe('Delete reservation', () => {
|
||||
.set('Cookie', authCookie(user.id));
|
||||
expect(res.status).toBe(404);
|
||||
});
|
||||
|
||||
// M4 (Plan 3d Task 7 review): `remove`'s id parsed with `toRowId`, not the
|
||||
// `Number()`-fallback `rowIdNum` — a hex-spelled id used to coerce to a
|
||||
// real row and delete it, where the legacy raw-bind statement's affinity
|
||||
// never converts a hex string and so 404'd. Rule 21.
|
||||
it('M4 — DELETE …/reservations/0x<id> answers the legacy 404 and deletes nothing', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const resv = createReservation(testDb, trip.id, { title: 'Flight', type: 'flight' });
|
||||
const hexId = '0x' + resv.id.toString(16);
|
||||
|
||||
const res = await request(app)
|
||||
.delete(`/api/trips/${trip.id}/reservations/${hexId}`)
|
||||
.set('Cookie', authCookie(user.id));
|
||||
expect(res.status).toBe(404);
|
||||
|
||||
const row = testDb.prepare('SELECT id FROM reservations WHERE id = ?').get(resv.id);
|
||||
expect(row).toBeDefined();
|
||||
});
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
@@ -318,6 +476,31 @@ describe('Batch update positions', () => {
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.success).toBe(true);
|
||||
});
|
||||
|
||||
// M4 (Plan 3d Task 7 review): `updatePositions`' `dayId` parsed with
|
||||
// `toRowId`, not the `Number()`-fallback `rowIdNum` — a hex-spelled
|
||||
// `day_id` used to coerce to a real day and reach the per-day upsert,
|
||||
// where the legacy raw-bind statement's affinity never converts a hex
|
||||
// string, so the join matched no row (a quiet no-op, not a write). Rule 21.
|
||||
it('M4 — PUT /positions with a hex-spelled day_id is the legacy quiet no-op (no day-scoped row written)', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const day = createDay(testDb, trip.id, { date: '2026-01-01' });
|
||||
const resv = createReservation(testDb, trip.id, { title: 'First', type: 'flight' });
|
||||
const hexDayId = '0x' + day.id.toString(16);
|
||||
|
||||
const res = await request(app)
|
||||
.put(`/api/trips/${trip.id}/reservations/positions`)
|
||||
.set('Cookie', authCookie(user.id))
|
||||
.send({ positions: [{ id: resv.id, day_plan_position: 0 }], day_id: hexDayId });
|
||||
// Same success shape the legacy statement's own no-op miss returns —
|
||||
// this is not a validation error, it silently writes nothing.
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.success).toBe(true);
|
||||
|
||||
const row = testDb.prepare('SELECT * FROM reservation_day_positions WHERE reservation_id = ?').get(resv.id);
|
||||
expect(row).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
@@ -0,0 +1,123 @@
|
||||
/**
|
||||
* DayAccommodationsRepository — parity tests (Plan 3d Task 7 whole-plan
|
||||
* review, item 9/"missing parity tests": this repository had no standalone
|
||||
* test file; its methods were only exercised indirectly through
|
||||
* `accommodations.service.test.ts`/`reservations.service.test.ts`). ONE
|
||||
* seeded world, one `toEqual(<legacy raw>)` test per read method.
|
||||
*/
|
||||
import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest';
|
||||
import { createSnapshotTestDb } from '../../../helpers/db-mock';
|
||||
import { resetTestDb } from '../../../helpers/test-db';
|
||||
import { createTestOrm, type TestOrm } from '../../../helpers/test-orm';
|
||||
import { createDayAccommodation, createPlace, createReservation, createTrip, createUser } from '../../../helpers/factories';
|
||||
import { DayAccommodations } from '../../../../src/db/entities/DayAccommodations.entity';
|
||||
import type { DayAccommodationsRepository } from '../../../../src/db/repositories/DayAccommodations.repository';
|
||||
|
||||
const testDb = createSnapshotTestDb();
|
||||
let t: TestOrm;
|
||||
let repo: DayAccommodationsRepository;
|
||||
|
||||
beforeAll(async () => {
|
||||
t = await createTestOrm(testDb);
|
||||
repo = t.repo(DayAccommodations);
|
||||
});
|
||||
beforeEach(() => { resetTestDb(testDb); t.clear(); });
|
||||
afterAll(async () => { await t.close(); testDb.close(); });
|
||||
|
||||
describe('DayAccommodationsRepository — fully seeded world', () => {
|
||||
// One seeded world for every test below: a trip with two stays —
|
||||
// `named` (a place, a check-in/out, a linked live booking) and `bare`
|
||||
// (no place, no booking) — plus a second trip's stay, to prove scoping.
|
||||
const seed = () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id, { start_date: '2026-09-01', end_date: '2026-09-05' });
|
||||
const otherTrip = createTrip(testDb, user.id, { start_date: '2026-09-01', end_date: '2026-09-02' });
|
||||
const days = testDb.prepare('SELECT id FROM days WHERE trip_id = ? ORDER BY day_number ASC').all(trip.id) as { id: number }[];
|
||||
const place = createPlace(testDb, trip.id, { name: 'The Plaza', description: 'Fancy' });
|
||||
const named = createDayAccommodation(testDb, trip.id, place.id, days[0].id, days[1].id, { check_in: '15:00', check_out: '11:00', confirmation: 'CONF-1' });
|
||||
const booking = createReservation(testDb, trip.id, { title: 'Plaza Stay', type: 'hotel' });
|
||||
testDb.prepare('UPDATE reservations SET accommodation_id = ? WHERE id = ?').run(`${named.id}.0`, booking.id);
|
||||
const bare = createDayAccommodation(testDb, trip.id, place.id, days[2].id, days[3].id);
|
||||
const otherDays = testDb.prepare('SELECT id FROM days WHERE trip_id = ? ORDER BY day_number ASC').all(otherTrip.id) as { id: number }[];
|
||||
const otherPlace = createPlace(testDb, otherTrip.id, { name: 'Elsewhere' });
|
||||
const otherStay = createDayAccommodation(testDb, otherTrip.id, otherPlace.id, otherDays[0].id, otherDays[1].id);
|
||||
return { trip, otherTrip, days, place, named, booking, bare, otherStay };
|
||||
};
|
||||
|
||||
it('AC2 findWithPlace — matches the legacy LEFT JOIN places statement, both with and without a place', async () => {
|
||||
const { named, place } = seed();
|
||||
const legacySql = `
|
||||
SELECT a.*, p.name as place_name, p.address as place_address, p.image_url as place_image, p.lat as place_lat, p.lng as place_lng
|
||||
FROM day_accommodations a LEFT JOIN places p ON a.place_id = p.id WHERE a.id = ?`;
|
||||
const typed = await repo.findWithPlace(named.id);
|
||||
const legacy = testDb.prepare(legacySql).get(named.id);
|
||||
expect(typed).toEqual(legacy);
|
||||
expect(typed?.place_name).toBe(place.name);
|
||||
});
|
||||
|
||||
it('AC3 listForTripWithPlaceAndBooking — matches the legacy LEFT JOIN places/reservations statement, fanning out one row per linked booking', async () => {
|
||||
const { trip, named, bare, booking } = seed();
|
||||
const legacySql = `
|
||||
SELECT a.*, p.name as place_name, p.address as place_address, p.image_url as place_image, p.lat as place_lat, p.lng as place_lng,
|
||||
r.title as reservation_title
|
||||
FROM day_accommodations a LEFT JOIN places p ON a.place_id = p.id LEFT JOIN reservations r ON r.accommodation_id = a.id
|
||||
WHERE a.trip_id = ? ORDER BY a.created_at ASC`;
|
||||
const typed = await repo.listForTripWithPlaceAndBooking(trip.id);
|
||||
const legacy = testDb.prepare(legacySql).all(trip.id);
|
||||
expect(typed).toEqual(legacy);
|
||||
expect(typed.map((r) => r.id)).toEqual([named.id, bare.id]);
|
||||
expect(typed.find((r) => r.id === named.id)?.reservation_title).toBe(booking.title);
|
||||
expect(typed.find((r) => r.id === bare.id)?.reservation_title).toBeNull();
|
||||
});
|
||||
|
||||
it('AC34 findInTrip — matches the legacy statement, scoped by trip (a foreign-trip id misses)', async () => {
|
||||
const { trip, named, otherStay } = seed();
|
||||
const legacy = testDb.prepare('SELECT * FROM day_accommodations WHERE id = ? AND trip_id = ?').get(named.id, trip.id);
|
||||
expect(await repo.findInTrip(named.id, trip.id)).toEqual(legacy);
|
||||
expect(await repo.findInTrip(otherStay.id, trip.id)).toBeUndefined();
|
||||
});
|
||||
|
||||
it('RS37/RS38/RS21 existsInTrip / getCheckIn / getTripId — match the legacy statements', async () => {
|
||||
const { trip, named, otherStay } = seed();
|
||||
expect(await repo.existsInTrip(named.id, trip.id)).toBe(true);
|
||||
expect(await repo.existsInTrip(otherStay.id, trip.id)).toBe(false);
|
||||
expect(await repo.getCheckIn(named.id)).toBe('15:00');
|
||||
expect(await repo.getTripId(named.id)).toBe(trip.id);
|
||||
expect(await repo.getTripId(999999)).toBeUndefined();
|
||||
});
|
||||
|
||||
it('DY19 listStartEndDayNumbers — matches the legacy joined day_number statement', async () => {
|
||||
const { trip, named, bare } = seed();
|
||||
const legacy = testDb.prepare(`
|
||||
SELECT a.id, s.day_number as start_no, e.day_number as end_no FROM day_accommodations a
|
||||
JOIN days s ON s.id = a.start_day_id JOIN days e ON e.id = a.end_day_id WHERE a.trip_id = ?`).all(trip.id);
|
||||
const typed = await repo.listStartEndDayNumbers(trip.id);
|
||||
expect(typed).toEqual(legacy);
|
||||
expect(typed.map((r) => r.id).sort()).toEqual([named.id, bare.id].sort());
|
||||
});
|
||||
|
||||
it('DY20 listForResync — matches the legacy statement, both persist(false) mirror columns present', async () => {
|
||||
const { trip, named } = seed();
|
||||
const legacy = testDb.prepare('SELECT id, start_day_id, end_day_id FROM day_accommodations WHERE trip_id = ?').all(trip.id);
|
||||
const typed = await repo.listForResync(trip.id);
|
||||
expect(typed).toEqual(legacy);
|
||||
expect(typed.find((r) => r.id === named.id)).toMatchObject({ start_day_id: named.start_day_id, end_day_id: named.end_day_id });
|
||||
});
|
||||
|
||||
it('PL16 listForPlace — matches the legacy statement, scoped by trip AND place', async () => {
|
||||
const { trip, place, named, bare } = seed();
|
||||
const legacy = testDb.prepare('SELECT id FROM day_accommodations WHERE trip_id = ? AND place_id = ?').all(trip.id, place.id);
|
||||
const typed = await repo.listForPlace(trip.id, place.id);
|
||||
expect(typed).toEqual(legacy);
|
||||
expect(typed.map((r) => r.id).sort()).toEqual([named.id, bare.id].sort());
|
||||
});
|
||||
|
||||
it('TP55 listAllForTrip — matches the legacy SELECT *, no ORDER BY, scoped by trip', async () => {
|
||||
const { trip, otherTrip, named, bare, otherStay } = seed();
|
||||
const legacy = testDb.prepare('SELECT * FROM day_accommodations WHERE trip_id = ?').all(trip.id);
|
||||
const typed = await repo.listAllForTrip(trip.id);
|
||||
expect(typed).toEqual(legacy);
|
||||
expect(typed.map((r) => r.id).sort()).toEqual([named.id, bare.id].sort());
|
||||
expect(await repo.listAllForTrip(otherTrip.id)).toEqual([{ ...otherStay }]);
|
||||
});
|
||||
});
|
||||
@@ -1084,3 +1084,55 @@ describe('PlacesRepository.listAllForTrip (TP40)', () => {
|
||||
expect(await places.listAllForTrip(trip.id)).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// L3 (Plan 3d Task 7 whole-plan review): CH1 (`findChargingProbe`,
|
||||
// `ChargingService.read`'s trip-scoped gate) had zero coverage of the REAL
|
||||
// repository call anywhere in the suite — every existing `charging.test.ts`/
|
||||
// `roadtrip.e2e.test.ts` case mocks `findChargingProbe` or `ChargingService
|
||||
// .read` itself. Pinned here at the repository level (parity with the legacy
|
||||
// `SELECT name, lat, lng, stop_type FROM places WHERE id = ? AND trip_id =
|
||||
// ?`), and separately at the route level for the foreign-place 404
|
||||
// (`tests/e2e/roadtrip.e2e.test.ts`).
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
describe('PlacesRepository.findChargingProbe (CH1)', () => {
|
||||
it('CH1-001: an own-trip place resolves name/lat/lng/stop_type, matching the legacy statement', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const place = createPlace(testDb, trip.id, { name: 'Ladepark Nord', lat: 48.137, lng: 11.575 });
|
||||
testDb.prepare("UPDATE places SET stop_type = 'charging' WHERE id = ?").run(place.id);
|
||||
|
||||
const typed = await places.findChargingProbe(place.id, trip.id);
|
||||
const legacy = testDb
|
||||
.prepare('SELECT name, lat, lng, stop_type FROM places WHERE id = ? AND trip_id = ?')
|
||||
.get(place.id, trip.id);
|
||||
|
||||
expect(typed).toEqual(legacy);
|
||||
expect(typed).toEqual({ name: 'Ladepark Nord', lat: 48.137, lng: 11.575, stop_type: 'charging' });
|
||||
});
|
||||
|
||||
it('CH1-002: a place belonging to a DIFFERENT trip resolves to undefined, matching the legacy miss', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const otherTrip = createTrip(testDb, user.id);
|
||||
const place = createPlace(testDb, otherTrip.id, { name: 'Theirs' });
|
||||
|
||||
const typed = await places.findChargingProbe(place.id, trip.id);
|
||||
const legacy = testDb
|
||||
.prepare('SELECT name, lat, lng, stop_type FROM places WHERE id = ? AND trip_id = ?')
|
||||
.get(place.id, trip.id);
|
||||
|
||||
expect(typed).toBeUndefined();
|
||||
expect(legacy).toBeUndefined();
|
||||
});
|
||||
|
||||
it('CH1-003: a non-charging place still resolves (the caller, not this query, decides what to do with stop_type)', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const place = createPlace(testDb, trip.id, { name: 'Just a stop' });
|
||||
|
||||
const typed = await places.findChargingProbe(place.id, trip.id);
|
||||
expect(typed).toEqual({ name: 'Just a stop', lat: 48.8566, lng: 2.3522, stop_type: null });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,62 @@
|
||||
/**
|
||||
* ReservationDayPositionsRepository — parity test (Plan 3d Task 7 whole-plan
|
||||
* review, item 9/"missing parity tests": this repository had no standalone
|
||||
* test file). ONE seeded world, one `toEqual(<legacy raw>)` test for the
|
||||
* read method (RS19); RS31 (`upsertScoped`, a write) already has coverage
|
||||
* through `reservations.service.test.ts`'s `updatePositions` cases.
|
||||
*/
|
||||
import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest';
|
||||
import { createSnapshotTestDb } from '../../../helpers/db-mock';
|
||||
import { resetTestDb } from '../../../helpers/test-db';
|
||||
import { createTestOrm, type TestOrm } from '../../../helpers/test-orm';
|
||||
import { createDay, createReservation, createTrip, createUser } from '../../../helpers/factories';
|
||||
import { ReservationDayPositions } from '../../../../src/db/entities/ReservationDayPositions.entity';
|
||||
import type { ReservationDayPositionsRepository } from '../../../../src/db/repositories/ReservationDayPositions.repository';
|
||||
|
||||
const testDb = createSnapshotTestDb();
|
||||
let t: TestOrm;
|
||||
let repo: ReservationDayPositionsRepository;
|
||||
|
||||
beforeAll(async () => {
|
||||
t = await createTestOrm(testDb);
|
||||
repo = t.repo(ReservationDayPositions);
|
||||
});
|
||||
beforeEach(() => { resetTestDb(testDb); t.clear(); });
|
||||
afterAll(async () => { await t.close(); testDb.close(); });
|
||||
|
||||
describe('ReservationDayPositionsRepository — fully seeded world', () => {
|
||||
const seed = () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const day1 = createDay(testDb, trip.id);
|
||||
const day2 = createDay(testDb, trip.id);
|
||||
const multiDay = createReservation(testDb, trip.id, { title: 'Rental car', type: 'car' });
|
||||
testDb.prepare('INSERT INTO reservation_day_positions (reservation_id, day_id, position) VALUES (?, ?, ?)').run(multiDay.id, day1.id, 0);
|
||||
testDb.prepare('INSERT INTO reservation_day_positions (reservation_id, day_id, position) VALUES (?, ?, ?)').run(multiDay.id, day2.id, 1);
|
||||
|
||||
const otherTrip = createTrip(testDb, user.id);
|
||||
const otherDay = createDay(testDb, otherTrip.id);
|
||||
const foreignRes = createReservation(testDb, otherTrip.id, { title: 'Foreign rental', type: 'car' });
|
||||
testDb.prepare('INSERT INTO reservation_day_positions (reservation_id, day_id, position) VALUES (?, ?, ?)').run(foreignRes.id, otherDay.id, 0);
|
||||
|
||||
return { trip, otherTrip, multiDay, day1, day2 };
|
||||
};
|
||||
|
||||
it('RS19 listForTrip — matches the legacy JOIN statement, scoped by trip, both persist(false) mirror columns present', async () => {
|
||||
const { trip, multiDay, day1, day2 } = seed();
|
||||
const legacy = testDb.prepare(`
|
||||
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 = ?`).all(trip.id);
|
||||
const typed = await repo.listForTrip(trip.id);
|
||||
expect(typed).toEqual(legacy);
|
||||
expect(typed.map((r) => [r.reservation_id, r.day_id, r.position]).sort()).toEqual(
|
||||
[[multiDay.id, day1.id, 0], [multiDay.id, day2.id, 1]].sort(),
|
||||
);
|
||||
});
|
||||
|
||||
it('RS19 listForTrip — an empty trip returns an empty array, not a throw', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
expect(await repo.listForTrip(trip.id)).toEqual([]);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,85 @@
|
||||
/**
|
||||
* ReservationEndpointsRepository — parity tests (Plan 3d Task 7 whole-plan
|
||||
* review, item 9/"missing parity tests": this repository had no standalone
|
||||
* test file). ONE seeded world, one `toEqual(<legacy raw>)` test per read
|
||||
* method.
|
||||
*/
|
||||
import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest';
|
||||
import { createSnapshotTestDb } from '../../../helpers/db-mock';
|
||||
import { resetTestDb } from '../../../helpers/test-db';
|
||||
import { createTestOrm, type TestOrm } from '../../../helpers/test-orm';
|
||||
import { createReservation, createTrip, createUser } from '../../../helpers/factories';
|
||||
import { ReservationEndpoints } from '../../../../src/db/entities/ReservationEndpoints.entity';
|
||||
import type { ReservationEndpointsRepository } from '../../../../src/db/repositories/ReservationEndpoints.repository';
|
||||
|
||||
const testDb = createSnapshotTestDb();
|
||||
let t: TestOrm;
|
||||
let repo: ReservationEndpointsRepository;
|
||||
|
||||
beforeAll(async () => {
|
||||
t = await createTestOrm(testDb);
|
||||
repo = t.repo(ReservationEndpoints);
|
||||
});
|
||||
beforeEach(() => { resetTestDb(testDb); t.clear(); });
|
||||
afterAll(async () => { await t.close(); testDb.close(); });
|
||||
|
||||
const insertEndpoint = (
|
||||
reservationId: number,
|
||||
role: string,
|
||||
sequence: number,
|
||||
overrides: Partial<{ name: string; code: string | null; lat: number; lng: number; timezone: string | null; local_time: string | null; local_date: string | null }> = {},
|
||||
) => {
|
||||
testDb.prepare(`
|
||||
INSERT INTO reservation_endpoints (reservation_id, role, sequence, name, code, lat, lng, timezone, local_time, local_date)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`).run(
|
||||
reservationId, role, sequence,
|
||||
overrides.name ?? `${role} endpoint`, overrides.code ?? null, overrides.lat ?? 48.0, overrides.lng ?? 11.0,
|
||||
overrides.timezone ?? null, overrides.local_time ?? null, overrides.local_date ?? null,
|
||||
);
|
||||
};
|
||||
|
||||
describe('ReservationEndpointsRepository — fully seeded world', () => {
|
||||
const seed = () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const flight = createReservation(testDb, trip.id, { title: 'Flight', type: 'flight' });
|
||||
const otherFlight = createReservation(testDb, trip.id, { title: 'Return flight', type: 'flight' });
|
||||
insertEndpoint(flight.id, 'from', 0, { name: 'MUC', code: 'MUC', local_date: '2026-09-01' });
|
||||
insertEndpoint(flight.id, 'to', 1, { name: 'JFK', code: 'JFK', local_date: '2026-09-01' });
|
||||
insertEndpoint(otherFlight.id, 'from', 0, { name: 'JFK', code: 'JFK' });
|
||||
|
||||
const otherTrip = createTrip(testDb, user.id);
|
||||
const foreignFlight = createReservation(testDb, otherTrip.id, { title: 'Foreign', type: 'flight' });
|
||||
insertEndpoint(foreignFlight.id, 'from', 0, { name: 'LHR', code: 'LHR' });
|
||||
|
||||
return { trip, otherTrip, flight, otherFlight, foreignFlight };
|
||||
};
|
||||
|
||||
it('RS3 listForTrip — matches the legacy JOIN reservations statement, ordered by reservation then sequence, scoped by trip', async () => {
|
||||
const { trip, flight, otherFlight } = seed();
|
||||
const legacy = testDb.prepare(`
|
||||
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`).all(trip.id);
|
||||
const typed = await repo.listForTrip(trip.id);
|
||||
expect(typed).toEqual(legacy);
|
||||
expect(typed.map((e) => [e.reservation_id, e.sequence])).toEqual([
|
||||
[flight.id, 0], [flight.id, 1], [otherFlight.id, 0],
|
||||
]);
|
||||
});
|
||||
|
||||
it('RR2 listForReservation — matches the legacy statement, ordered by sequence, scoped to one reservation', async () => {
|
||||
const { flight } = seed();
|
||||
const legacy = testDb.prepare('SELECT * FROM reservation_endpoints WHERE reservation_id = ? ORDER BY sequence').all(flight.id);
|
||||
const typed = await repo.listForReservation(flight.id);
|
||||
expect(typed).toEqual(legacy);
|
||||
expect(typed.map((e) => e.role)).toEqual(['from', 'to']);
|
||||
});
|
||||
|
||||
it('DY17 listIdAndDate — matches the legacy statement', async () => {
|
||||
const { flight } = seed();
|
||||
const legacy = testDb.prepare('SELECT id, local_date FROM reservation_endpoints WHERE reservation_id = ?').all(flight.id);
|
||||
const typed = await repo.listIdAndDate(flight.id);
|
||||
expect(typed).toEqual(legacy);
|
||||
expect(typed.every((r) => r.local_date === '2026-09-01')).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,69 @@
|
||||
/**
|
||||
* ReservationTravelersRepository — parity tests (Plan 3d Task 7 whole-plan
|
||||
* review, item 9/"missing parity tests": this repository had no standalone
|
||||
* test file). ONE seeded world, one `toEqual(<legacy raw>)` test per read
|
||||
* method.
|
||||
*/
|
||||
import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest';
|
||||
import { createSnapshotTestDb } from '../../../helpers/db-mock';
|
||||
import { resetTestDb } from '../../../helpers/test-db';
|
||||
import { createTestOrm, type TestOrm } from '../../../helpers/test-orm';
|
||||
import { createReservation, createTrip, createUser } from '../../../helpers/factories';
|
||||
import { ReservationTravelers } from '../../../../src/db/entities/ReservationTravelers.entity';
|
||||
import type { ReservationTravelersRepository } from '../../../../src/db/repositories/ReservationTravelers.repository';
|
||||
|
||||
const testDb = createSnapshotTestDb();
|
||||
let t: TestOrm;
|
||||
let repo: ReservationTravelersRepository;
|
||||
|
||||
beforeAll(async () => {
|
||||
t = await createTestOrm(testDb);
|
||||
repo = t.repo(ReservationTravelers);
|
||||
});
|
||||
beforeEach(() => { resetTestDb(testDb); t.clear(); });
|
||||
afterAll(async () => { await t.close(); testDb.close(); });
|
||||
|
||||
const legacyListForTrip = (trip_id: number) => testDb.prepare(`
|
||||
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`).all(trip_id);
|
||||
|
||||
const legacyListForReservation = (reservation_id: number) => testDb.prepare(`
|
||||
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 = ?`).all(reservation_id);
|
||||
|
||||
describe('ReservationTravelersRepository — fully seeded world', () => {
|
||||
const seed = () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: named } = createUser(testDb);
|
||||
testDb.prepare('UPDATE users SET display_name = ? WHERE id = ?').run('Named Traveler', named.id);
|
||||
const { user: guest } = createUser(testDb);
|
||||
testDb.prepare('UPDATE users SET is_guest = 1 WHERE id = ?').run(guest.id);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
const flight = createReservation(testDb, trip.id, { title: 'Flight', type: 'flight' });
|
||||
const hotel = createReservation(testDb, trip.id, { title: 'Hotel', type: 'hotel' });
|
||||
testDb.prepare('INSERT INTO reservation_travelers (reservation_id, user_id) VALUES (?, ?)').run(flight.id, owner.id);
|
||||
testDb.prepare('INSERT INTO reservation_travelers (reservation_id, user_id) VALUES (?, ?)').run(flight.id, named.id);
|
||||
testDb.prepare('INSERT INTO reservation_travelers (reservation_id, user_id) VALUES (?, ?)').run(hotel.id, guest.id);
|
||||
return { owner, named, guest, trip, flight, hotel };
|
||||
};
|
||||
|
||||
it('RS6 listForTrip — matches the legacy JOIN statement, COALESCE(display_name, username), ordered by reservation', async () => {
|
||||
const { trip, flight, hotel } = seed();
|
||||
const legacy = legacyListForTrip(trip.id);
|
||||
const typed = await repo.listForTrip(trip.id);
|
||||
expect(typed).toEqual(legacy);
|
||||
expect(typed.map((r) => r.reservation_id)).toEqual([flight.id, flight.id, hotel.id]);
|
||||
expect(typed.find((r) => r.username === 'Named Traveler')).toBeDefined();
|
||||
expect(typed.find((r) => r.is_guest === 1)).toBeDefined();
|
||||
});
|
||||
|
||||
it('RR3 listForReservation — matches the legacy statement (no ORDER BY, no reservation_id column)', async () => {
|
||||
const { flight, owner, named } = seed();
|
||||
const legacy = legacyListForReservation(flight.id);
|
||||
const typed = await repo.listForReservation(flight.id);
|
||||
expect(typed).toEqual(legacy);
|
||||
expect(typed.map((r) => r.user_id).sort((a, b) => a - b)).toEqual([owner.id, named.id].sort((a, b) => a - b));
|
||||
expect((typed[0] as Record<string, unknown>).reservation_id).toBeUndefined();
|
||||
});
|
||||
});
|
||||
@@ -384,3 +384,124 @@ describe('ReservationsRepository — RS20 (listUpcomingForUser)', () => {
|
||||
expect(baseline.some((r) => r.title === 'Hotel Ibis')).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// M3 (Plan 3d Task 7 whole-plan review): AC37/AC40 reverted to the legacy
|
||||
// REAL-bound compare (parity is law — the widening had no reason to exist
|
||||
// once H1 makes every write in this cluster store the legacy `'<id>.0'`
|
||||
// shape). Pinned against the SAME five-shape matrix the review measured.
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
describe('ReservationsRepository — AC37/AC40 (listIdMetadataByStay/listIdsByStay), M3', () => {
|
||||
it('AC37/AC40 match the legacy REAL-bound compare — only the "<id>.0" TEXT shape resolves, not the bare integer, a trailing/leading space or a prefix-numeric value', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id, { start_date: '2026-09-01', end_date: '2026-09-05' });
|
||||
const place = createPlace(testDb, trip.id);
|
||||
const days = testDb.prepare('SELECT id FROM days WHERE trip_id = ? ORDER BY day_number ASC').all(trip.id) as { id: number }[];
|
||||
const stay = createDayAccommodation(testDb, trip.id, place.id, days[0].id, days[1].id);
|
||||
|
||||
// `shapes[1]` ('<id>.0') is the ONLY one the legacy `Number(id)` bind —
|
||||
// and this repository's own reverted compare — ever matched.
|
||||
const shapes = [
|
||||
`${stay.id}`,
|
||||
`${stay.id}.0`,
|
||||
`${stay.id} `,
|
||||
`${stay.id}abc`,
|
||||
` ${stay.id}`,
|
||||
];
|
||||
const ids = shapes.map((shape, i) => {
|
||||
const r = createReservation(testDb, trip.id, { title: `Link ${i}`, type: 'hotel' });
|
||||
testDb.prepare('UPDATE reservations SET accommodation_id = ? WHERE id = ?').run(shape, r.id);
|
||||
return r.id;
|
||||
});
|
||||
|
||||
const typedMeta = await reservationsRepo.listIdMetadataByStay(stay.id);
|
||||
const legacyMeta = testDb.prepare('SELECT id, metadata FROM reservations WHERE accommodation_id = ?').all(stay.id) as { id: number; metadata: string | null }[];
|
||||
expect(typedMeta).toEqual(legacyMeta);
|
||||
expect(typedMeta.map((r) => r.id)).toEqual([ids[1]]);
|
||||
|
||||
const typedIds = await reservationsRepo.listIdsByStay(stay.id);
|
||||
const legacyIds = testDb.prepare('SELECT id FROM reservations WHERE accommodation_id = ?').all(stay.id) as { id: number }[];
|
||||
expect(typedIds).toEqual(legacyIds);
|
||||
expect(typedIds.map((r) => r.id)).toEqual([ids[1]]);
|
||||
});
|
||||
});
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// Plan 3d Task 7 whole-plan review, item 9 ("missing parity tests"): Task 2's
|
||||
// own `ReservationsRepository` reads (`listForTrip`, `findWithJoins`,
|
||||
// `findInTrip`, `listResyncCandidates`, `listForRestamp`) had no
|
||||
// repository-level parity test — they rested on the reservations.service
|
||||
// REAL-LEGACY suite only. ONE seeded world, one `toEqual(<legacy raw>)`
|
||||
// test per method.
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
describe('ReservationsRepository — Task 2 reads (listForTrip / findWithJoins / findInTrip / listResyncCandidates / listForRestamp)', () => {
|
||||
const legacyJoinedSql = `
|
||||
SELECT r.*, 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
|
||||
FROM reservations r
|
||||
LEFT JOIN days d ON d.id = r.day_id
|
||||
LEFT JOIN places p ON p.id = r.place_id
|
||||
LEFT JOIN day_accommodations ap ON ap.id = r.accommodation_id
|
||||
LEFT JOIN places acc_p ON acc_p.id = ap.place_id`;
|
||||
|
||||
const seed = () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id, { start_date: '2026-09-01', end_date: '2026-09-05' });
|
||||
const place = createPlace(testDb, trip.id, { name: 'Museum' });
|
||||
const days = testDb.prepare('SELECT id FROM days WHERE trip_id = ? ORDER BY day_number ASC').all(trip.id) as { id: number }[];
|
||||
const hotelPlace = createPlace(testDb, trip.id, { name: 'Grand Hotel' });
|
||||
const stay = createDayAccommodation(testDb, trip.id, hotelPlace.id, days[0].id, days[1].id);
|
||||
|
||||
// A dated, non-hotel booking — a restamp/resync candidate.
|
||||
const dinner = createReservation(testDb, trip.id, { title: 'Dinner', type: 'restaurant', day_id: days[0].id });
|
||||
testDb.prepare("UPDATE reservations SET reservation_time = ? WHERE id = ?").run('2026-09-01T19:00', dinner.id);
|
||||
|
||||
// A hotel booking linked to the stay above — the joined projection's place/accommodation columns.
|
||||
const hotel = createReservation(testDb, trip.id, { title: 'Grand Hotel Stay', type: 'hotel', day_id: days[0].id });
|
||||
testDb.prepare('UPDATE reservations SET accommodation_id = ?, place_id = ? WHERE id = ?').run(`${stay.id}.0`, place.id, hotel.id);
|
||||
|
||||
return { trip, days, place, hotelPlace, stay, dinner, hotel };
|
||||
};
|
||||
|
||||
it('RS18 listForTrip — matches the legacy joined statement, ordered by reservation_time then created_at', async () => {
|
||||
const { trip } = seed();
|
||||
const legacy = testDb.prepare(`${legacyJoinedSql} WHERE r.trip_id = ? ORDER BY r.reservation_time ASC, r.created_at ASC`).all(trip.id);
|
||||
const typed = await reservationsRepo.listForTrip(trip.id);
|
||||
expect(typed).toEqual(legacy);
|
||||
});
|
||||
|
||||
it('RR1 findWithJoins — matches the legacy joined statement for one row, place/accommodation columns populated', async () => {
|
||||
const { hotel, stay, hotelPlace } = seed();
|
||||
const legacy = testDb.prepare(`${legacyJoinedSql} WHERE r.id = ?`).get(hotel.id);
|
||||
const typed = await reservationsRepo.findWithJoins(hotel.id);
|
||||
expect(typed).toEqual(legacy);
|
||||
expect(typed).toMatchObject({ accommodation_place_id: hotelPlace.id, accommodation_name: hotelPlace.name, accommodation_start_day_id: stay.start_day_id });
|
||||
});
|
||||
|
||||
it('RS35 findInTrip — matches the legacy statement, scoped by trip', async () => {
|
||||
const { trip, dinner } = seed();
|
||||
const legacy = testDb.prepare('SELECT * FROM reservations WHERE id = ? AND trip_id = ?').get(dinner.id, trip.id);
|
||||
expect(await reservationsRepo.findInTrip(dinner.id, trip.id)).toEqual(legacy);
|
||||
expect(await reservationsRepo.findInTrip(999999, trip.id)).toBeUndefined();
|
||||
});
|
||||
|
||||
it('RS12 listResyncCandidates — matches the legacy statement (dated, non-hotel-or-unlinked)', async () => {
|
||||
const { trip, dinner } = seed();
|
||||
const legacy = testDb.prepare(`
|
||||
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`).all(trip.id);
|
||||
const typed = await reservationsRepo.listResyncCandidates(trip.id);
|
||||
expect(typed).toEqual(legacy);
|
||||
expect(typed.map((r) => r.id)).toEqual([dinner.id]);
|
||||
});
|
||||
|
||||
it('DY14 listForRestamp — matches the legacy statement, every reservation of the trip regardless of type', async () => {
|
||||
const { trip, dinner, hotel } = seed();
|
||||
const legacy = testDb.prepare('SELECT id, day_id, end_day_id, reservation_time, reservation_end_time FROM reservations WHERE trip_id = ?').all(trip.id);
|
||||
const typed = await reservationsRepo.listForRestamp(trip.id);
|
||||
expect(typed).toEqual(legacy);
|
||||
expect(typed.map((r) => r.id).sort((a, b) => a - b)).toEqual([dinner.id, hotel.id].sort((a, b) => a - b));
|
||||
});
|
||||
});
|
||||
|
||||
@@ -345,4 +345,66 @@ describe('reservation-visibility parity (RV2: publicStayExists vs publicStaySql)
|
||||
expect(typed.map((r) => r.id)).toEqual([unlinked.id, liveLinked.id]);
|
||||
expect(typed).toEqual(legacy);
|
||||
});
|
||||
|
||||
// M1 (Plan 3d Task 0 review, folded into Task 7's fix wave): the harness
|
||||
// above never made the CAST load-bearing — dropping `eb2.cast(…,
|
||||
// 'integer')` from `publicStayExists` stayed green on all of RESVIS-004..
|
||||
// 009, RESVIS-008 ("14.0") included, because a column-to-column compare
|
||||
// already gets SQLite's own NUMERIC-affinity coercion for a
|
||||
// WHOLLY-numeric TEXT value ('14 ', '14.0', ' 14' all equal 14 without
|
||||
// any CAST — measured). Only a PREFIX-numeric value tells the two
|
||||
// predicates apart: `CAST('14abc' AS INTEGER) = 14` is true, but the bare
|
||||
// affinity compare `'14abc' = 14` is false, since '14abc' is not a
|
||||
// well-formed number and keeps its TEXT affinity.
|
||||
it('RESVIS-013 (mutation-sensitive): a staged-only booking with accommodation_id = "<id>abc" still resolves the link — the stay is hidden (only the CAST recognises a prefix-numeric TEXT value)', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id, { start_date: '2026-09-01', end_date: '2026-09-05' });
|
||||
const place = createPlace(testDb, trip.id);
|
||||
const days = testDb.prepare('SELECT id FROM days WHERE trip_id = ? ORDER BY day_number ASC').all(trip.id) as { id: number }[];
|
||||
const stay = createDayAccommodation(testDb, trip.id, place.id, days[0].id, days[1].id);
|
||||
const booking = createReservation(testDb, trip.id, { title: 'Staged prefix-numeric link', type: 'hotel' });
|
||||
testDb.prepare("UPDATE reservations SET accommodation_id = ?, ingest_state = 'staged' WHERE id = ?").run(`${stay.id}abc`, booking.id);
|
||||
|
||||
const typed = await t.em.getKysely<StayVisibilityTestDB>()
|
||||
.selectFrom('day_accommodations as a')
|
||||
.select('a.id')
|
||||
.where('a.trip_id', '=', trip.id)
|
||||
.where((eb) => publicStayExists(eb))
|
||||
.execute();
|
||||
const legacy = testDb
|
||||
.prepare(`SELECT id FROM day_accommodations a WHERE a.trip_id = ? AND ${publicStaySql('a')}`)
|
||||
.all(trip.id) as { id: number }[];
|
||||
|
||||
// The only link is staged, so the stay must be hidden — the SAME answer
|
||||
// RESVIS-006 gives for a plain-integer-spelled staged link. Dropping the
|
||||
// CAST makes the typed predicate miss the link entirely (a bare
|
||||
// '14abc' = 14 compare is false) and fall into the NOT-EXISTS branch,
|
||||
// wrongly reading the stay as public — this assertion goes red then,
|
||||
// while `typed` still equals `legacy` under the untouched code.
|
||||
expect(typed).toEqual([]);
|
||||
expect(typed).toEqual(legacy);
|
||||
});
|
||||
|
||||
it('RESVIS-014: a staged-only booking with accommodation_id = "<id> " (trailing space) still resolves the link — the stay is hidden (both forms agree; SQLite\'s own affinity already coerces a wholly-numeric padded value)', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id, { start_date: '2026-09-01', end_date: '2026-09-05' });
|
||||
const place = createPlace(testDb, trip.id);
|
||||
const days = testDb.prepare('SELECT id FROM days WHERE trip_id = ? ORDER BY day_number ASC').all(trip.id) as { id: number }[];
|
||||
const stay = createDayAccommodation(testDb, trip.id, place.id, days[0].id, days[1].id);
|
||||
const booking = createReservation(testDb, trip.id, { title: 'Staged padded link', type: 'hotel' });
|
||||
testDb.prepare("UPDATE reservations SET accommodation_id = ?, ingest_state = 'staged' WHERE id = ?").run(`${stay.id} `, booking.id);
|
||||
|
||||
const typed = await t.em.getKysely<StayVisibilityTestDB>()
|
||||
.selectFrom('day_accommodations as a')
|
||||
.select('a.id')
|
||||
.where('a.trip_id', '=', trip.id)
|
||||
.where((eb) => publicStayExists(eb))
|
||||
.execute();
|
||||
const legacy = testDb
|
||||
.prepare(`SELECT id FROM day_accommodations a WHERE a.trip_id = ? AND ${publicStaySql('a')}`)
|
||||
.all(trip.id) as { id: number }[];
|
||||
|
||||
expect(typed).toEqual([]);
|
||||
expect(typed).toEqual(legacy);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -373,6 +373,70 @@ describe('Roadtrip MCP registration and search', () => {
|
||||
});
|
||||
});
|
||||
|
||||
// Item 11 (Plan 3d Task 7 whole-plan review, L5): `updateVia` above is the
|
||||
// only tool in this class with its OWN non-member case — the mutation
|
||||
// coverage reaches every tool (they all gate on the same `findAccessible`
|
||||
// call), but only one had a dedicated test. One per tool, as
|
||||
// `tools-days-accommodations` already does.
|
||||
describe('RoadtripMcp — non-member refusal, one case per tool (Plan 3d Task 7 review, item 11)', () => {
|
||||
function build(findAccessible: boolean) {
|
||||
const service = {
|
||||
dayExists: vi.fn(async () => true),
|
||||
listForDay: vi.fn(async () => []),
|
||||
listForTrip: vi.fn(async () => []),
|
||||
tracksForTrip: vi.fn(async () => []),
|
||||
trackExists: vi.fn(async () => true),
|
||||
create: vi.fn(async () => ({ id: 1 })),
|
||||
createMany: vi.fn(async () => []),
|
||||
reanchor: vi.fn(async () => []),
|
||||
remove: vi.fn(async () => true),
|
||||
move: vi.fn(async () => ({ id: 1 })),
|
||||
broadcast: vi.fn(),
|
||||
};
|
||||
const tripsRepo = { findAccessible: vi.fn(async () => findAccessible) };
|
||||
const guards = { hasTripPermission: vi.fn(async () => true) };
|
||||
const auth = { isDemoUser: vi.fn(async () => false) };
|
||||
const mcp = new RoadtripMcp(service as never, tripsRepo as never, guards as never, auth as never, {} as never);
|
||||
return { mcp, service, tripsRepo };
|
||||
}
|
||||
|
||||
it('list_route_vias refuses a non-member trip (the legacy "not found or access denied" shape)', async () => {
|
||||
const { mcp, service } = build(false);
|
||||
const res = await mcp.listVias({ tripId: 10 }, ctx);
|
||||
expect(res.isError).toBe(true);
|
||||
expect(JSON.stringify(res)).toContain('access denied');
|
||||
expect(service.listForTrip).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('add_route_via refuses a non-member trip', async () => {
|
||||
const { mcp, service } = build(false);
|
||||
const res = await mcp.addVia({ tripId: 10, dayId: 1, after_order_index: 0, lat: 48, lng: 10 }, ctx);
|
||||
expect(res.isError).toBe(true);
|
||||
expect(service.create).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('add_route_vias refuses a non-member trip', async () => {
|
||||
const { mcp, service } = build(false);
|
||||
const res = await mcp.addVias({ tripId: 10, dayId: 1, vias: [] }, ctx);
|
||||
expect(res.isError).toBe(true);
|
||||
expect(service.createMany).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('reanchor_route_vias refuses a non-member trip', async () => {
|
||||
const { mcp, service } = build(false);
|
||||
const res = await mcp.reanchorVias({ tripId: 10, dayId: 1, vias: [] }, ctx);
|
||||
expect(res.isError).toBe(true);
|
||||
expect(service.reanchor).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('remove_route_via refuses a non-member trip', async () => {
|
||||
const { mcp, service } = build(false);
|
||||
const res = await mcp.removeVia({ tripId: 10, dayId: 1, viaId: 4 }, ctx);
|
||||
expect(res.isError).toBe(true);
|
||||
expect(service.remove).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('MCP trip preferences authorization', () => {
|
||||
it('reads shared preferences and refuses inaccessible trips', async () => {
|
||||
const s = setup();
|
||||
@@ -382,6 +446,18 @@ describe('MCP trip preferences authorization', () => {
|
||||
expect((await tool.read({ tripId: 10 }, ctx)).isError).toBe(true);
|
||||
expect((await tool.update({ tripId: 10, settings: {} }, ctx)).isError).toBe(true);
|
||||
});
|
||||
|
||||
// Item 11 (Plan 3d Task 7 whole-plan review, L5): `update_roadtrip_settings`
|
||||
// gets its OWN non-member case — the test above already asserts it as a
|
||||
// second assertion piggybacked on the `read` test, but not in isolation.
|
||||
it('update_roadtrip_settings refuses a non-member trip on its own, without a prior read call', async () => {
|
||||
const s = setup();
|
||||
s.tripsRepo.findAccessible.mockResolvedValue(false);
|
||||
const tool = new RoadtripPreferencesMcp(s.preferences, { isDemoUser: () => false } as never, {} as never, s.tripsRepo as never, { hasTripPermission: () => true } as never);
|
||||
const res = await tool.update({ tripId: 10, settings: { roadtrip_range_km: 50 } }, ctx);
|
||||
expect(res.isError).toBe(true);
|
||||
expect(JSON.stringify(res)).toContain('access denied');
|
||||
});
|
||||
it('rejects demos and readers, but permits a trip editor', async () => {
|
||||
const s = setup();
|
||||
const auth = { isDemoUser: vi.fn(() => true) };
|
||||
|
||||
@@ -281,6 +281,28 @@ describe('RoadtripService', () => {
|
||||
expect(await service.trackExists(999999, f.tripA.id)).toBe(false);
|
||||
});
|
||||
|
||||
// L1 (Plan 3d Task 7 whole-plan review): `listForDay`/`listForTrip`/
|
||||
// `tracksForTrip`/`trackExists` used to bind `Number(id)` directly — a
|
||||
// hex-spelled id coerces to a real row under `Number()`, where the legacy
|
||||
// raw-bind statement's affinity never converts a hex string, so it always
|
||||
// matched nothing. `toRowId` now answers that legacy empty/false shape.
|
||||
it('L1: a hex-spelled day/trip id answers the legacy empty shape on every read path, not the real trip\'s rows', async () => {
|
||||
await service.create(f.dayA1.id, { after_order_index: 0, lat: 53, lng: 10 });
|
||||
await service.createMany(f.dayA1.id, { vias: [], track: { place_id: f.track.id, stray_km: null } });
|
||||
|
||||
const hexDayId = '0x' + f.dayA1.id.toString(16);
|
||||
const hexTripId = '0x' + f.tripA.id.toString(16);
|
||||
|
||||
expect(await service.listForDay(hexDayId)).toEqual([]);
|
||||
expect(await service.listForTrip(hexTripId)).toEqual([]);
|
||||
expect(await service.tracksForTrip(hexTripId)).toEqual([]);
|
||||
expect(await service.trackExists(f.track.id, hexTripId)).toBe(false);
|
||||
|
||||
// Sanity: the canonical id still sees the real rows.
|
||||
expect(await service.listForDay(f.dayA1.id)).toHaveLength(1);
|
||||
expect(await service.tracksForTrip(f.tripA.id)).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('ROADTRIP-SVC-007b: removing one reports whether there was anything to remove', async () => {
|
||||
const via = await service.create(f.dayA1.id, { after_order_index: 0, lat: 53, lng: 10 });
|
||||
|
||||
@@ -303,4 +325,25 @@ describe('RoadtripService', () => {
|
||||
}
|
||||
expect(await service.tracksForTrip(f.tripA.id)).toEqual([]);
|
||||
});
|
||||
|
||||
// R7 (Plan 3d Task 7 whole-plan review, item 12 — flag + pin, not fix, the
|
||||
// MEMBERS-SVC-018 shape): `create`'s `nextSequence` (MAX(sequence)+1) READ
|
||||
// and its `insertVia` WRITE are two separate statements with no lock
|
||||
// between them. Measured under real concurrency (`Promise.allSettled`, no
|
||||
// mocks): both calls succeed — there is no UNIQUE constraint to lose a
|
||||
// race on, unlike `trip_members` — but both land on the SAME `sequence`
|
||||
// (`0`), a genuine duplicate the un-serialized read-then-write leaves
|
||||
// behind. Today's actual outcome, pinned; not a fix.
|
||||
it('R7: two concurrent creates on the same leg both succeed but can land on the SAME sequence (unserialized nextSequence read)', async () => {
|
||||
const results = await Promise.allSettled([
|
||||
service.create(f.dayA1.id, { after_order_index: 0, lat: 1, lng: 1 }),
|
||||
service.create(f.dayA1.id, { after_order_index: 0, lat: 2, lng: 2 }),
|
||||
]);
|
||||
expect(results.every((r) => r.status === 'fulfilled')).toBe(true);
|
||||
const rows = await service.listForDay(f.dayA1.id);
|
||||
expect(rows).toHaveLength(2);
|
||||
// Both landed on the same leg, at the same sequence — the duplicate R7 flags.
|
||||
expect(rows.map((r) => r.after_order_index)).toEqual([0, 0]);
|
||||
expect(rows.map((r) => r.sequence)).toEqual([0, 0]);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user