From d7f5bc64c1a9fb1893456dce938bfcd2783f7913 Mon Sep 17 00:00:00 2001 From: jubnl Date: Wed, 23 Sep 2026 05:31:36 +0200 Subject: [PATCH] refactor(days): days and day notes through repositories --- .../repositories/DayAssignments.repository.ts | 223 +++++++++++++- .../db/repositories/DayNotes.repository.ts | 12 + server/src/db/repositories/Days.repository.ts | 94 ++++++ .../src/db/repositories/Trips.repository.ts | 10 + server/src/nest/days/days.module.ts | 23 +- server/src/nest/days/days.service.ts | 276 ++++++++++-------- server/tests/e2e/days.e2e.test.ts | 94 ++---- server/tests/helpers/mcp-test-controllers.ts | 17 +- server/tests/helpers/plugin-host.ts | 17 +- server/tests/helpers/test-uow.ts | 44 +++ server/tests/integration/dayReorder.test.ts | 17 +- .../DayAssignments.repository.test.ts | 266 +++++++++++++++++ .../repositories/DayNotes.repository.test.ts | 37 ++- .../db/repositories/Days.repository.test.ts | 103 +++++++ .../db/repositories/Trips.repository.test.ts | 15 + server/tests/unit/nest/days.service.test.ts | 29 +- .../unit/nest/trip-read-model.service.test.ts | 17 +- server/tests/unit/nest/trips.service.test.ts | 17 +- 18 files changed, 1104 insertions(+), 207 deletions(-) create mode 100644 server/tests/unit/db/repositories/DayAssignments.repository.test.ts diff --git a/server/src/db/repositories/DayAssignments.repository.ts b/server/src/db/repositories/DayAssignments.repository.ts index 331380225..1a4aa8c18 100644 --- a/server/src/db/repositories/DayAssignments.repository.ts +++ b/server/src/db/repositories/DayAssignments.repository.ts @@ -1,4 +1,225 @@ +import type { Platform } from '@mikro-orm/core'; import type { DayAssignments } from '../entities/DayAssignments.entity'; +import { coalesce, columnRef } from '../dialect/sql-functions'; import { TrekRepository } from './_shared/trek-repository'; -export class DayAssignmentsRepository extends TrekRepository {} +/** + * The DY1/DY3/AS1/AS3 assignment-with-place projection row (Plan 3c Task 2's + * report is the contract for Task 3, which consumes this unchanged): + * + * ```sql + * SELECT da.*, p.id as place_id, p.name as place_name, p.description as place_description, + * p.lat, p.lng, p.address, p.category_id, p.price, p.currency as place_currency, + * COALESCE(da.assignment_time, p.place_time) as place_time, + * COALESCE(da.assignment_end_time, p.end_time) as end_time, + * p.duration_minutes, p.notes as place_notes, + * p.image_url, p.transport_mode, p.google_place_id, p.google_ftid, p.osm_id, p.amap_poi_id, + * p.website, p.phone, p.stop_type, p.fill_percent, + * c.name as category_name, c.color as category_color, c.icon as category_icon + * FROM day_assignments da + * JOIN places p ON da.place_id = p.id + * LEFT JOIN categories c ON p.category_id = c.id + * WHERE + * [ORDER BY da.order_index ASC, da.created_at ASC] + * ``` + * + * `da.*` selects every scalar column of `DayAssignments` (`place_id` among + * them, the physical FK column) and then `p.id as place_id` selects it again + * under the same output key — the legacy statement did this too, and + * better-sqlite3's row object keeps the LAST column with a given name, so + * `place_id` on the wire is always `p.id`, not `da.place_id` (they agree by + * the JOIN condition regardless). Nullability here is the physical column's, + * not `AssignmentRow`'s (types.ts) narrower claims (rule 16) — a place with + * no category genuinely nulls `category_name`/`category_color`/ + * `category_icon` through the LEFT JOIN even though `Categories.name` itself + * is NOT NULL. + */ +export interface AssignmentWithPlaceRow { + id: number; + day_id: number; + place_id: number; + order_index: number | null; + notes: string | null; + reservation_status: string | null; + reservation_notes: string | null; + reservation_datetime: string | null; + assignment_time: string | null; + assignment_end_time: string | null; + end_day: number; + leg_transport_mode: string | null; + incoming_leg_transport_mode: string | null; + accommodation_id: number | null; + created_at: string | null; + place_name: string; + place_description: string | null; + lat: number | null; + lng: number | null; + address: string | null; + category_id: number | null; + price: number | null; + place_currency: string | null; + place_time: string | null; + end_time: string | null; + duration_minutes: number | null; + place_notes: string | null; + image_url: string | null; + transport_mode: string | null; + google_place_id: string | null; + google_ftid: string | null; + osm_id: string | null; + amap_poi_id: string | null; + website: string | null; + phone: string | null; + stop_type: string | null; + fill_percent: number | null; + category_name: string | null; + category_color: string | null; + category_icon: string | null; +} + +/** + * `day_assignments`'s shape as DY24's `em.getKysely()` call needs it (an + * explicit `TDB` type argument, per `WebauthnChallengesRepository + * .claimChallenge`'s docstring — entity-metadata inference is not what a + * hand-written statement wants), narrowed to the columns `reanchorToDay` + * reads or writes. + */ +interface DayAssignmentsKyselyDB { + day_assignments: { + id: number; + day_id: number; + order_index: number | null; + accommodation_id: number | null; + }; +} + +export class DayAssignmentsRepository extends TrekRepository { + /** The DY1/DY3/AS1/AS3 projection's SELECT list, shared by all three query shapes below. */ + private assignmentWithPlaceSelect(platform: Platform) { + return [ + 'da.*', + 'p.id as place_id', + 'p.name as place_name', + 'p.description as place_description', + 'p.lat', + 'p.lng', + 'p.address', + // `category_id` is `persist(false)` on `Places` — a bare `'p.category_id'` + // string selects nothing (not a real property name), and `'p.category'` + // (the owning relation) is ALSO the joined alias here (`c`), so MikroORM + // absorbs the select into the join and returns `c__id`, not `category_id` + // (`McpTokensRepository.listAllWithUsername`'s `t.user`/`user_id` + // docstring is the same trap, one join away from this one). `columnRef` + // selects the literal physical column, keyed `category_id`. + columnRef(platform, 'p.category_id'), + 'p.price', + 'p.currency as place_currency', + coalesce(platform, 'da.assignment_time', 'p.place_time').as('place_time'), + coalesce(platform, 'da.assignment_end_time', 'p.end_time').as('end_time'), + 'p.duration_minutes', + 'p.notes as place_notes', + 'p.image_url', + 'p.transport_mode', + 'p.google_place_id', + 'p.google_ftid', + 'p.osm_id', + 'p.amap_poi_id', + 'p.website', + 'p.phone', + 'p.stop_type', + 'p.fill_percent', + 'c.name as category_name', + 'c.color as category_color', + 'c.icon as category_icon', + ] as const; + } + + /** + * AS1 (`assignments.service.ts::getAssignmentWithPlace`, Task 3): the + * projection filtered to one assignment, no ORDER BY (a single row). + */ + async findWithPlaceAndCategory(id: number): Promise { + const platform = this.getEntityManager().getPlatform(); + const row = await this.qb('da') + .join('da.place', 'p') + .leftJoin('p.category', 'c') + .select(this.assignmentWithPlaceSelect(platform)) + .where({ 'da.id': id }) + .execute('get', false); + return row ?? null; + } + + /** + * DY1 (`days.service.ts::getAssignmentsForDay`) and AS3 + * (`assignments.service.ts::listDayAssignments`, Task 3): the projection + * for one day, `ORDER BY da.order_index ASC, da.created_at ASC`. + */ + async listForDay(day_id: number): Promise { + const platform = this.getEntityManager().getPlatform(); + return await this.qb('da') + .join('da.place', 'p') + .leftJoin('p.category', 'c') + .select(this.assignmentWithPlaceSelect(platform)) + .where({ 'da.day': day_id }) + .orderBy({ 'da.order_index': 'asc', 'da.created_at': 'asc' }) + .execute('all', false); + } + + /** + * DY3 (`days.service.ts::list`): the same projection for several days at + * once (`WHERE da.day_id IN (...)`), same ORDER BY. Empty-array + * short-circuit before any query, as the legacy dynamic-`IN` builder did. + */ + async listWithPlaceAndCategory(day_ids: number[]): Promise { + if (day_ids.length === 0) return []; + const platform = this.getEntityManager().getPlatform(); + return await this.qb('da') + .join('da.place', 'p') + .leftJoin('p.category', 'c') + .select(this.assignmentWithPlaceSelect(platform)) + .where({ 'da.day': { $in: day_ids } }) + .orderBy({ 'da.order_index': 'asc', 'da.created_at': 'asc' }) + .execute('all', false); + } + + /** + * DY24 (`days.service.ts::resyncAccommodationDays`'s `moveStayStop`, + * inventory §18.2) — a correlated scalar subquery in an UPDATE SET clause + * the QueryBuilder cannot express: + * + * ```sql + * UPDATE day_assignments + * SET day_id = :dayId, + * order_index = COALESCE((SELECT MAX(order_index) FROM day_assignments WHERE day_id = :dayId), -1) + 1 + * WHERE accommodation_id = :accId + * ``` + * + * `this.kysely()` (`_shared/trek-repository.ts`), not + * `this.getEntityManager().getKysely()` directly — see + * `WebauthnChallengesRepository.claimChallenge`'s docstring for why. + * Proven to join the caller's ambient `uow.transactional` block by + * rollback, not by doc comment (ROLLBACK proof in the repository test and + * the task report): a call inside a transaction that then throws leaves + * the row un-moved; inside one that commits, the row is moved. + */ + async reanchorToDay(accommodation_id: number, day_id: number): Promise { + await this.kysely() + .updateTable('day_assignments') + .set((eb) => ({ + day_id, + order_index: eb( + eb.fn.coalesce( + eb + .selectFrom('day_assignments as da2') + .select((eb2) => eb2.fn.max('da2.order_index').as('m')) + .where('da2.day_id', '=', day_id), + eb.val(-1), + ), + '+', + eb.val(1), + ), + })) + .where('accommodation_id', '=', accommodation_id) + .execute(); + } +} diff --git a/server/src/db/repositories/DayNotes.repository.ts b/server/src/db/repositories/DayNotes.repository.ts index a78457dba..bcbf7f714 100644 --- a/server/src/db/repositories/DayNotes.repository.ts +++ b/server/src/db/repositories/DayNotes.repository.ts @@ -62,4 +62,16 @@ export class DayNotesRepository extends TrekRepository { } return toRow(inserted) as DayNoteRow; } + + /** + * DY4 (`days.service.ts::list`) — `` SELECT * FROM day_notes WHERE day_id + * IN (${dayPlaceholders}) ORDER BY sort_order ASC, created_at ASC ``. + * Empty-array short-circuit before any query, as the legacy dynamic-`IN` + * builder did. + */ + async listByDayIds(day_ids: number[]): Promise { + if (day_ids.length === 0) return []; + const notes = await this.find({ day: { $in: day_ids } }, { orderBy: { sort_order: 'asc', created_at: 'asc' } }); + return notes.map((n) => toRow(n) as DayNoteRow); + } } diff --git a/server/src/db/repositories/Days.repository.ts b/server/src/db/repositories/Days.repository.ts index d96185d52..da0e59a81 100644 --- a/server/src/db/repositories/Days.repository.ts +++ b/server/src/db/repositories/Days.repository.ts @@ -15,6 +15,19 @@ export interface DayRow { const _dayRowKeys: AssertRowKeys = true; +/** The DY26/DY29 reorder/insert projection: `SELECT id, day_number, date FROM days WHERE trip_id = ? ORDER BY day_number`. */ +export interface DayOrderRow { + id: number; + day_number: number; + date: string | null; +} + +/** The DY21 re-anchor lookup: `SELECT id, day_number FROM days WHERE trip_id = ? AND date = ? LIMIT 1`. */ +export interface DayIdAndNumberRow { + id: number; + day_number: number; +} + export class DaysRepository extends TrekRepository { /** `SELECT * FROM days WHERE trip_id = ? ORDER BY day_number ASC` */ async listByTrip(trip_id: number): Promise { @@ -84,4 +97,85 @@ export class DaysRepository extends TrekRepository { } return toRow(inserted) as DayRow; } + + /** + * `SELECT * FROM days WHERE id = ?` — DY7/DY10/DY12/DY32/DY36's shared + * re-select, one method for all five sites (DY32/DY36 read after their + * transaction resolves, so this is a fresh statement against committed + * rows, no `refresh` needed). + */ + async findById(id: number): Promise { + const day = await this.findOne({ id }); + return day ? (toRow(day) as DayRow) : undefined; + } + + /** + * DY9 — `UPDATE days SET notes = ?, title = ? WHERE id = ?`. The presence + * sentinel that decides `notes`/`title` (the legacy's asymmetric `||` + * vs. `??` coercion) stays in `DaysService.update`; this takes the final, + * already-decided values and writes them verbatim. + */ + async updateNotesAndTitle(id: number, notes: string | null, title: string | null): Promise { + await this.nativeUpdate({ id }, { notes, title }); + } + + /** DY11 — `UPDATE days SET default_transport_mode = ? WHERE id = ?` (#1281). */ + async setDefaultTransportMode(id: number, mode: string | null): Promise { + await this.nativeUpdate({ id }, { default_transport_mode: mode }); + } + + /** DY13 — `DELETE FROM days WHERE id = ?`, unscoped: the caller proved trip access. */ + async deleteById(id: number): Promise { + await this.nativeDelete({ id }); + } + + /** + * DY26/DY29 — `SELECT id, day_number, date FROM days WHERE trip_id = ? + * ORDER BY day_number`, the read `reorder`/`insert` both take before + * opening their `uow.transactional` block (same text, two legacy methods, + * one repository method here). + */ + async listOrderedForReorder(trip_id: number): Promise { + return await this.qb('d') + .select(['d.id', 'd.day_number', 'd.date']) + .where({ trip: trip_id }) + .orderBy({ day_number: 'asc' }) + .execute('all', false); + } + + /** DY27/DY30 — `UPDATE days SET day_number = ? WHERE id = ?` (the two-phase renumber's per-row write). */ + async setDayNumber(id: number, day_number: number): Promise { + await this.nativeUpdate({ id }, { day_number }); + } + + /** DY28/DY33 — `UPDATE days SET day_number = ?, date = ? WHERE id = ?`. */ + async setDayNumberAndDate(id: number, day_number: number, date: string | null): Promise { + await this.nativeUpdate({ id }, { day_number, date }); + } + + /** + * DY31/DY34 — `INSERT INTO days (trip_id, day_number, date) VALUES (?, ?, + * ?)` (no `notes` column named, unlike `createDay`'s DY6 — the row's + * `notes` is left to the column's own NULL default). Returns the inserted + * id (`em.insert()`'s returned PK, R6's `lastInsertRowid` replacement); + * the caller re-selects separately (DY32/DY36), after its transaction + * resolves, so no read-back happens here. + */ + async insertDay(input: { trip_id: number; day_number: number; date: string | null }): Promise { + return await this.insert({ + trip: input.trip_id, + day_number: input.day_number, + date: input.date, + }); + } + + /** DY21 — `SELECT id, day_number FROM days WHERE trip_id = ? AND date = ? LIMIT 1`. */ + async findByTripAndDate(trip_id: number, date: string): Promise { + const row = await this.qb('d') + .select(['d.id', 'd.day_number']) + .where({ trip: trip_id, date }) + .limit(1) + .execute('get', false); + return row ?? undefined; + } } diff --git a/server/src/db/repositories/Trips.repository.ts b/server/src/db/repositories/Trips.repository.ts index 441ec45a6..dce84872e 100644 --- a/server/src/db/repositories/Trips.repository.ts +++ b/server/src/db/repositories/Trips.repository.ts @@ -50,6 +50,16 @@ export class TripsRepository extends TrekRepository { return { id: row.id, user_id: row.user_id, currency: row.currency ?? null }; } + /** + * DY35 (`days.service.ts::insert`'s dated path) — `UPDATE trips SET + * end_date = ? WHERE id = ?`: a dated insert extends the trip by one day. + * Plan 3c Task 7 (`TripsService`) reuses this for its own `end_date` + * writes rather than duplicating the statement. + */ + async setEndDate(id: number, end_date: string | null): Promise { + await this.nativeUpdate({ id }, { end_date }); + } + /** `SELECT id FROM trips WHERE id = ? AND user_id = ?` */ async isOwner(trip_id: number | string, user_id: number): Promise { const row = await this.qb('t') diff --git a/server/src/nest/days/days.module.ts b/server/src/nest/days/days.module.ts index 6eb6b9c1b..3c101a6b4 100644 --- a/server/src/nest/days/days.module.ts +++ b/server/src/nest/days/days.module.ts @@ -1,4 +1,5 @@ import { Module } from '@nestjs/common'; +import { MikroOrmModule } from '@mikro-orm/nestjs'; import { DaysController } from './days.controller'; import { DaysService } from './days.service'; import { DaysMcp } from './days.mcp'; @@ -10,6 +11,10 @@ import { QueryHelpersModule } from '../query-helpers/query-helpers.module'; import { PlacesModule } from '../places/places.module'; import { AuthModule } from '../auth/auth.module'; import { McpSharedModule } from '../mcp-shared/mcp-shared.module'; +import { Days } from '../../db/entities/Days.entity'; +import { DayAssignments } from '../../db/entities/DayAssignments.entity'; +import { DayNotes } from '../../db/entities/DayNotes.entity'; +import { Trips } from '../../db/entities/Trips.entity'; /** * Days (S6 — Phase 2 trip sub-domain), mounted at /api/trips/:tripId/days. @@ -19,9 +24,25 @@ import { McpSharedModule } from '../mcp-shared/mcp-shared.module'; * * Day notes used to live here too, with their own full file set; they are their * own domain now (day-notes/). + * + * `MikroOrmModule.forFeature([Days, DayAssignments, DayNotes, Trips])` + * registers `DaysRepository`/`DayAssignmentsRepository`/`DayNotesRepository`/ + * `TripsRepository` for `DaysService`'s `@InjectRepository` constructor + * params (Plan 3c Task 2) — the same forFeature + `@InjectRepository` + * wiring every converted domain copies (`trip-membership.module.ts`'s + * precedent for pulling in a repository this module doesn't otherwise own). */ @Module({ - imports: [McpSharedModule, PermissionsModule, QueryHelpersModule, PlacesModule, AuthModule, RealtimeModule, PluginGuardsModule], + imports: [ + McpSharedModule, + PermissionsModule, + QueryHelpersModule, + PlacesModule, + AuthModule, + RealtimeModule, + PluginGuardsModule, + MikroOrmModule.forFeature([Days, DayAssignments, DayNotes, Trips]), + ], controllers: [DaysController], providers: [DaysService, DaysMcp, DaysRpc], exports: [DaysService], diff --git a/server/src/nest/days/days.service.ts b/server/src/nest/days/days.service.ts index bd8fdea4a..ebc442354 100644 --- a/server/src/nest/days/days.service.ts +++ b/server/src/nest/days/days.service.ts @@ -1,12 +1,22 @@ import { Injectable } from '@nestjs/common'; +import { InjectRepository } from '@mikro-orm/nestjs'; import type { TrekWsPayload, TrekWsTripEventName } from '@trek/shared'; import { RealtimeService } from '../realtime/realtime.service'; import { DatabaseService, type TripAccess } from '../database/database.service'; import { PermissionsService } from '../permissions/permissions.service'; import { QueryHelpersService } from '../query-helpers/query-helpers.service'; import { formatAssignmentWithPlace } from '../common/rowShape'; -import type { AssignmentRow, Day, DayNote, User } from '../../types'; +import type { AssignmentRow, User } from '../../types'; import { UnitOfWork } from '../database/unit-of-work'; +import { toRowId } from '../common/row-id'; +import { Days } from '../../db/entities/Days.entity'; +import type { DaysRepository, DayOrderRow } from '../../db/repositories/Days.repository'; +import { DayAssignments } from '../../db/entities/DayAssignments.entity'; +import type { DayAssignmentsRepository } from '../../db/repositories/DayAssignments.repository'; +import { DayNotes } from '../../db/entities/DayNotes.entity'; +import type { DayNotesRepository } from '../../db/repositories/DayNotes.repository'; +import { Trips } from '../../db/entities/Trips.entity'; +import type { TripsRepository } from '../../db/repositories/Trips.repository'; type Trip = TripAccess; @@ -44,21 +54,30 @@ function withDatePart(timestamp: string, date: string): string { export class DayReorderError extends Error {} /** - * Day domain service — owns the day + accommodation SQL (moved 1:1 from the - * legacy services/dayService.ts: identical statements, the `||` falsy-coercion - * defaults, the post-write re-selects, the two-phase negative-day_number - * renumber and the reservation re-stamping). The legacy hand-rolled - * BEGIN/COMMIT blocks in reorder/insert became uow.transactional() (same - * rollback-on-throw semantics, savepoint-safe when nested). Verified defects - * were fixed after the port (2026-07): update() now presence-sentinels BOTH - * columns (the legacy always-write wiped notes when only a title was sent), - * createAccommodation/deleteAccommodation run their multi-statement writes in - * a transaction, and getAssignmentsForDay batch-loads tags instead of one - * query per assignment. Trip access - * rides DatabaseService.canAccessTrip; mutations use the - * 'day_edit' permission; the WebSocket broadcast keeps its legacy call path. - * There are no non-Nest consumers left. days.bridge.ts served them and was - * deleted once the last one folded into the container. + * Day domain service — the day + day-assignment-projection SQL now lives in + * `DaysRepository`/`DayAssignmentsRepository`/`DayNotesRepository` (Plan 3c + * Task 2); `restampReservationDates`/`assertNoInvertedAccommodation` and the + * `day_accommodations`/`reservations`/`reservation_endpoints` halves of + * `resyncAccommodationDays` stay on the raw `DatabaseService` connection — + * those tables belong to Plan 3d (`// DYn — Plan 3d` marks each site). Trip + * access still rides `DatabaseService.canAccessTrip` (Task 0b's 110 + * unconverted callers, this among them); mutations use the 'day_edit' + * permission; the WebSocket broadcast keeps its legacy call path. + * + * Day ids arriving from a route (`:id`) are an **affinity seam**: the legacy + * statement bound the route string straight into `WHERE id = ?` with no + * `Number()`/validation of its own, relying on SQLite's own text/integer + * affinity. `getDay` is the one gate that converts and answers the legacy + * not-found (`toRowId` → `undefined`, program rule 15); every mutation below + * is only ever reached downstream of a successful `getDay` call in the same + * request (REST/MCP/RPC all check-then-act), so `toRowId(id)!` there is not + * a live 404 path, just the same non-null-assert style this file already + * used before conversion (`this.db.get(...)!`). `tripId` is the DY0 + * seam: `TripAccessGuard` already validated it with the SAME raw-bind + * escape hatch `TripsRepository.findAccessible` uses, so every id-shape a + * plain `Number(tripId)` would mis-parse relative to SQLite's affinity was + * already refused before this service's handler ever runs — safe to convert + * with a bare `Number()` downstream of that guard. */ @Injectable() export class DaysService { @@ -68,6 +87,10 @@ export class DaysService { private readonly realtime: RealtimeService, private readonly queryHelpers: QueryHelpersService, private readonly uow: UnitOfWork, + @InjectRepository(Days) private readonly daysRepo: DaysRepository, + @InjectRepository(DayAssignments) private readonly dayAssignmentsRepo: DayAssignmentsRepository, + @InjectRepository(DayNotes) private readonly dayNotesRepo: DayNotesRepository, + @InjectRepository(Trips) private readonly tripsRepo: TripsRepository, ) {} async verifyTripAccess(tripId: string | number, userId: number) { @@ -86,21 +109,9 @@ export class DaysService { // Day assignment helpers // ------------------------------------------------------------------------- + /** DY1 — `DayAssignmentsRepository.listForDay`, the single-day ordered projection. */ async getAssignmentsForDay(dayId: number | string) { - const assignments = this.db.all(` - SELECT da.*, p.id as place_id, p.name as place_name, p.description as place_description, - p.lat, p.lng, p.address, p.category_id, p.price, p.currency as place_currency, - COALESCE(da.assignment_time, p.place_time) as place_time, - COALESCE(da.assignment_end_time, p.end_time) as end_time, - p.duration_minutes, p.notes as place_notes, - p.image_url, p.transport_mode, p.google_place_id, p.google_ftid, p.osm_id, p.amap_poi_id, p.website, p.phone, p.stop_type, p.fill_percent, - c.name as category_name, c.color as category_color, c.icon as category_icon - FROM day_assignments da - JOIN places p ON da.place_id = p.id - LEFT JOIN categories c ON p.category_id = c.id - WHERE da.day_id = ? - ORDER BY da.order_index ASC, da.created_at ASC - `, dayId); + const assignments = await this.dayAssignmentsRepo.listForDay(Number(dayId)); // One batched tag load instead of the legacy per-assignment query; the // non-compact loader returns the same full tag rows (t.* minus the join @@ -163,29 +174,15 @@ export class DaysService { // ------------------------------------------------------------------------- async list(tripId: string | number) { - const days = this.db.all('SELECT * FROM days WHERE trip_id = ? ORDER BY day_number ASC', tripId); + const days = await this.daysRepo.listByTrip(Number(tripId)); if (days.length === 0) { return { days: [] }; } const dayIds = days.map(d => d.id); - const dayPlaceholders = dayIds.map(() => '?').join(','); - const allAssignments = this.db.all(` - SELECT da.*, p.id as place_id, p.name as place_name, p.description as place_description, - p.lat, p.lng, p.address, p.category_id, p.price, p.currency as place_currency, - COALESCE(da.assignment_time, p.place_time) as place_time, - COALESCE(da.assignment_end_time, p.end_time) as end_time, - p.duration_minutes, p.notes as place_notes, - p.image_url, p.transport_mode, p.google_place_id, p.google_ftid, p.osm_id, p.amap_poi_id, p.website, p.phone, p.stop_type, p.fill_percent, - c.name as category_name, c.color as category_color, c.icon as category_icon - FROM day_assignments da - JOIN places p ON da.place_id = p.id - LEFT JOIN categories c ON p.category_id = c.id - WHERE da.day_id IN (${dayPlaceholders}) - ORDER BY da.order_index ASC, da.created_at ASC - `, ...dayIds); + const allAssignments = await this.dayAssignmentsRepo.listWithPlaceAndCategory(dayIds); const placeIds = [...new Set(allAssignments.map(a => a.place_id))]; const tagsByPlaceId = await this.queryHelpers.loadTagsByPlaceIds(placeIds, { compact: true }); @@ -196,14 +193,15 @@ export class DaysService { const assignmentsByDayId: Record[]> = {}; for (const a of allAssignments) { if (!assignmentsByDayId[a.day_id]) assignmentsByDayId[a.day_id] = []; - assignmentsByDayId[a.day_id].push(formatAssignmentWithPlace(a, tagsByPlaceId[a.place_id] || [], participantsByAssignment[a.id] || [])); + // The repository row is honestly nullable per column (rule 16); `AssignmentRow` + // (types.ts) narrows some of those to non-null the same way the legacy + // `this.db.all(...)` cast already trusted unchecked — same trust + // boundary, moved from the raw-SQL generic to this call site. + assignmentsByDayId[a.day_id].push(formatAssignmentWithPlace(a as unknown as AssignmentRow, tagsByPlaceId[a.place_id] || [], participantsByAssignment[a.id] || [])); } - const allNotes = this.db.all( - `SELECT * FROM day_notes WHERE day_id IN (${dayPlaceholders}) ORDER BY sort_order ASC, created_at ASC`, - ...dayIds - ); - const notesByDayId: Record = {}; + const allNotes = await this.dayNotesRepo.listByDayIds(dayIds); + const notesByDayId: Record = {}; for (const note of allNotes) { if (!notesByDayId[note.day_id]) notesByDayId[note.day_id] = []; notesByDayId[note.day_id].push(note); @@ -219,34 +217,42 @@ export class DaysService { } async create(tripId: string | number, date?: string, notes?: string) { - const maxDay = this.db.get<{ max: number | null }>('SELECT MAX(day_number) as max FROM days WHERE trip_id = ?', tripId)!; - const dayNumber = (maxDay.max || 0) + 1; + const tripIdNum = Number(tripId); + // DY5: `DaysRepository.maxDayNumber` already folds the no-rows case to 0 + // (`?? 0`); the `|| 0` here is the legacy expression kept verbatim so a + // stored 0 still becomes 1 — the same outcome either fold produces, but + // the ruling is to keep the literal formula in the service. + const maxDayNumber = await this.daysRepo.maxDayNumber(tripIdNum); + const dayNumber = (maxDayNumber || 0) + 1; - const result = this.db.run( - 'INSERT INTO days (trip_id, day_number, date, notes) VALUES (?, ?, ?, ?)', - tripId, dayNumber, date || null, notes || null - ); - - const day = this.db.get('SELECT * FROM days WHERE id = ?', result.lastInsertRowid)!; + const day = await this.daysRepo.createDay({ + trip_id: tripIdNum, + day_number: dayNumber, + date: date || null, + notes: notes || null, + }); return { ...day, assignments: [] }; } async getDay(id: string | number, tripId: string | number) { - return this.db.get('SELECT * FROM days WHERE id = ? AND trip_id = ?', id, tripId); + const dayId = toRowId(id); + if (dayId === null) return undefined; + return await this.daysRepo.findInTrip(dayId, Number(tripId)); } - async update(id: string | number, current: Day, fields: { notes?: string; title?: string | null }) { + async update(id: string | number, current: { notes?: string | null; title?: string | null }, fields: { notes?: string; title?: string | null }) { + const dayId = toRowId(id)!; // Both columns use the presence sentinel: an absent key preserves the // current value (the legacy version always wrote notes, so setting a title // silently wiped the day's notes — the client sends the two fields in - // separate requests). - this.db.run('UPDATE days SET notes = ?, title = ? WHERE id = ?', - 'notes' in fields ? (fields.notes || null) : (current.notes ?? null), - 'title' in fields ? (fields.title ?? null) : (current.title ?? null), - id - ); - const updatedDay = this.db.get('SELECT * FROM days WHERE id = ?', id)!; - return { ...updatedDay, assignments: await this.getAssignmentsForDay(id) }; + // separate requests). Note the asymmetry: `notes` falls back through `||` + // (an empty string clears too), `title` through `??` (only null/undefined + // clear) — both byte-identical to the legacy statement's own coercions. + const notes = 'notes' in fields ? (fields.notes || null) : (current.notes ?? null); + const title = 'title' in fields ? (fields.title ?? null) : (current.title ?? null); + await this.daysRepo.updateNotesAndTitle(dayId, notes, title); + const updatedDay = (await this.daysRepo.findById(dayId))!; + return { ...updatedDay, assignments: await this.getAssignmentsForDay(dayId) }; } /** @@ -255,13 +261,16 @@ export class DaysService { * per-leg assignment transport setter. Per-segment leg modes still override it. */ async setDefaultTransportMode(id: string | number, mode: string | null) { - this.db.run('UPDATE days SET default_transport_mode = ? WHERE id = ?', mode ?? null, id); - const updatedDay = this.db.get('SELECT * FROM days WHERE id = ?', id)!; - return { ...updatedDay, assignments: await this.getAssignmentsForDay(id) }; + const dayId = toRowId(id)!; + await this.daysRepo.setDefaultTransportMode(dayId, mode ?? null); + const updatedDay = (await this.daysRepo.findById(dayId))!; + return { ...updatedDay, assignments: await this.getAssignmentsForDay(dayId) }; } + /** DY13 — unscoped by trip: the caller (every route) already proved trip access via `getDay`. */ async remove(id: string | number): Promise { - this.db.run('DELETE FROM days WHERE id = ?', id); + const dayId = toRowId(id)!; + await this.daysRepo.deleteById(dayId); } // ------------------------------------------------------------------------- @@ -282,12 +291,17 @@ export class DaysService { * moved day so reservation_time/reservation_end_time follow their day's new * date (time-of-day preserved). Transport endpoints (flight legs) shift by the * same per-booking day delta so multi-leg timing stays internally consistent. + * + * DY14–DY18 — Plan 3d (`reservations`/`reservation_endpoints`): stays raw on + * `DatabaseService` inside the caller's `uow.transactional` block, per the + * Plan 3b `UserCleanupService` precedent for cross-domain writes. */ async restampReservationDates( tripId: string | number, oldDateById: Map, newDateById: Map, ): Promise { + // DY14 — Plan 3d const reservations = this.db.all<{ id: number; day_id: number | null; end_day_id: number | null; reservation_time: string | null; reservation_end_time: string | null; @@ -296,9 +310,13 @@ export class DaysService { tripId ); + // DY15 — Plan 3d const setTime = this.db.prepare('UPDATE reservations SET reservation_time = ? WHERE id = ?'); + // DY16 — Plan 3d const setEndTime = this.db.prepare('UPDATE reservations SET reservation_end_time = ? WHERE id = ?'); + // DY17 — Plan 3d const endpoints = this.db.prepare('SELECT id, local_date FROM reservation_endpoints WHERE reservation_id = ?'); + // DY18 — Plan 3d const setEndpointDate = this.db.prepare('UPDATE reservation_endpoints SET local_date = ? WHERE id = ?'); for (const r of reservations) { @@ -326,7 +344,14 @@ export class DaysService { } } - /** A stay must not end before it begins after a reorder/insert. */ + /** + * A stay must not end before it begins after a reorder/insert. + * + * DY19 — Plan 3d: the statement's root table is `day_accommodations` + * (joined to `days` only to read `day_number`), so it stays raw on + * `DatabaseService` per the inventory's ruling, even though `days` itself + * is owned here. + */ private async assertNoInvertedAccommodation(tripId: string | number): Promise { const spans = this.db.all<{ id: number; start_no: number; end_no: number }>(` SELECT a.id, s.day_number AS start_no, e.day_number AS end_no @@ -351,54 +376,61 @@ export class DaysService { * its day rows, mirroring resyncReservationDays' out-of-range semantics, so moving a * whole trip still shifts everything together. The linked hotel reservation follows * its accommodation's start day in both branches. + * + * DY20/DY22/DY23 — Plan 3d (`day_accommodations`/`reservations`) stay raw; + * DY21 (`DaysRepository.findByTripAndDate`) and DY25 + * (`DaysRepository.findById`) convert — both root on `days`, which this + * plan owns. DY24 (the `day_assignments` stop that follows its booking) + * converts via `DayAssignmentsRepository.reanchorToDay`, a Kysely + * statement proven to join this method's ambient transaction by rollback + * (see the task report). */ async resyncAccommodationDays( tripId: string | number, prevDateByDayId: Map, ): Promise { + const tripIdNum = Number(tripId); + // DY20 — Plan 3d const stays = this.db.all<{ id: number; start_day_id: number; end_day_id: number }>( 'SELECT id, start_day_id, end_day_id FROM day_accommodations WHERE trip_id = ?', tripId ); if (stays.length === 0) return; - const dayByDate = this.db.prepare('SELECT id, day_number FROM days WHERE trip_id = ? AND date = ? LIMIT 1'); + // DY22 — Plan 3d const updateStay = this.db.prepare('UPDATE day_accommodations SET start_day_id = ?, end_day_id = ? WHERE id = ?'); + // DY23 — Plan 3d const restampLinkedRes = this.db.prepare(` UPDATE reservations SET day_id = :dayId, reservation_time = CASE WHEN reservation_time IS NULL THEN :date ELSE :date || SUBSTR(reservation_time, 11) END WHERE accommodation_id = :accId AND type = 'hotel' `); - // The day stop a booking wrote moves with it, the way its linked booking does. - // Left behind it would sit on a day the traveller no longer sleeps there, with - // nothing on screen to say why. Re-indexed to the end of the target day, because - // its old position belonged to a day it is leaving. - const moveStayStop = this.db.prepare(` - UPDATE day_assignments - SET day_id = :dayId, - order_index = COALESCE((SELECT MAX(order_index) FROM day_assignments WHERE day_id = :dayId), -1) + 1 - WHERE accommodation_id = :accId - `); for (const stay of stays) { const oldStartDate = prevDateByDayId.get(stay.start_day_id); const oldEndDate = prevDateByDayId.get(stay.end_day_id); if (oldStartDate && oldEndDate) { - const newStart = dayByDate.get(tripId, oldStartDate) as { id: number; day_number: number } | undefined; - const newEnd = dayByDate.get(tripId, oldEndDate) as { id: number; day_number: number } | undefined; + // DY21 + const newStart = await this.daysRepo.findByTripAndDate(tripIdNum, oldStartDate); + const newEnd = await this.daysRepo.findByTripAndDate(tripIdNum, oldEndDate); if (newStart && newEnd && newStart.day_number <= newEnd.day_number && (newStart.id !== stay.start_day_id || newEnd.id !== stay.end_day_id)) { updateStay.run(newStart.id, newEnd.id, stay.id); - moveStayStop.run({ dayId: newStart.id, accId: stay.id }); + // The day stop a booking wrote moves with it, the way its linked booking does. + // Left behind it would sit on a day the traveller no longer sleeps there, with + // nothing on screen to say why. Re-indexed to the end of the target day, because + // its old position belonged to a day it is leaving. DY24, via Kysely. + await this.dayAssignmentsRepo.reanchorToDay(stay.id, newStart.id); stay.start_day_id = newStart.id; } } // Keep the linked reservation on the stay's (possibly re-dated) start day — its // reservation_time is a snapshot of that day's date, stale after any range change. - const startDayDate = this.db.get<{ date: string | null }>('SELECT date FROM days WHERE id = ?', stay.start_day_id)?.date; - if (startDayDate) { - restampLinkedRes.run({ dayId: stay.start_day_id, date: startDayDate, accId: stay.id }); + // DY25 + const startDay = await this.daysRepo.findById(stay.start_day_id); + if (startDay?.date) { + restampLinkedRes.run({ dayId: stay.start_day_id, date: startDay.date, accId: stay.id }); } } } @@ -408,10 +440,8 @@ export class DaysService { * day ids (a permutation of the current ids). */ async reorder(tripId: string | number, orderedIds: number[]) { - const rows = this.db.all<{ id: number; day_number: number; date: string | null }>( - 'SELECT id, day_number, date FROM days WHERE trip_id = ? ORDER BY day_number', - tripId - ); + const tripIdNum = Number(tripId); + const rows: DayOrderRow[] = await this.daysRepo.listOrderedForReorder(tripIdNum); const existingIds = new Set(rows.map(r => r.id)); if (orderedIds.length !== rows.length || !orderedIds.every(id => existingIds.has(id))) { @@ -423,18 +453,19 @@ export class DaysService { const sortedDates = rows.map(r => r.date).filter((d): d is string => !!d).sort((a, b) => (a < b ? -1 : a > b ? 1 : 0)); const isDated = sortedDates.length > 0; - const setDayNumber = this.db.prepare('UPDATE days SET day_number = ? WHERE id = ?'); - const setDayNumberAndDate = this.db.prepare('UPDATE days SET day_number = ?, date = ? WHERE id = ?'); - await this.uow.transactional(async () => { - // Two-phase renumber to dodge UNIQUE(trip_id, day_number) collisions. - orderedIds.forEach((id, i) => setDayNumber.run(-(i + 1), id)); + // Two-phase renumber to dodge UNIQUE(trip_id, day_number) collisions — + // one `nativeUpdate` per row, in the legacy order, both phases inside + // this same transaction. + for (const [i, id] of orderedIds.entries()) { + await this.daysRepo.setDayNumber(id, -(i + 1)); + } const newDateById = new Map(); - orderedIds.forEach((id, i) => { + for (const [i, id] of orderedIds.entries()) { const date = isDated ? (sortedDates[i] ?? null) : null; - setDayNumberAndDate.run(i + 1, date, id); + await this.daysRepo.setDayNumberAndDate(id, i + 1, date); newDateById.set(id, date); - }); + } if (isDated) await this.restampReservationDates(tripId, oldDateById, newDateById); await this.assertNoInvertedAccommodation(tripId); @@ -450,26 +481,22 @@ export class DaysService { * shifted days have their dates re-stamped (same rules as reorder). */ async insert(tripId: string | number, position?: number) { - const rows = this.db.all<{ id: number; day_number: number; date: string | null }>( - 'SELECT id, day_number, date FROM days WHERE trip_id = ? ORDER BY day_number', - tripId - ); + const tripIdNum = Number(tripId); + const rows: DayOrderRow[] = await this.daysRepo.listOrderedForReorder(tripIdNum); const n = rows.length; const pos = Math.min(Math.max(position ?? n + 1, 1), n + 1); const datedRows = rows.filter(r => r.date) as { id: number; day_number: number; date: string }[]; const isDated = datedRows.length > 0; - const setDayNumber = this.db.prepare('UPDATE days SET day_number = ? WHERE id = ?'); - if (!isDated) { - const newRowid = await this.uow.transactional(async () => { + const newId = await this.uow.transactional(async () => { const toShift = rows.filter(r => r.day_number >= pos); - toShift.forEach(r => setDayNumber.run(-r.day_number, r.id)); - const result = this.db.run('INSERT INTO days (trip_id, day_number, date) VALUES (?, ?, NULL)', tripId, pos); - toShift.forEach(r => setDayNumber.run(r.day_number + 1, r.id)); - return result.lastInsertRowid; + for (const r of toShift) await this.daysRepo.setDayNumber(r.id, -r.day_number); + const insertedId = await this.daysRepo.insertDay({ trip_id: tripIdNum, day_number: pos, date: null }); + for (const r of toShift) await this.daysRepo.setDayNumber(r.id, r.day_number + 1); + return insertedId; }); - const day = this.db.get('SELECT * FROM days WHERE id = ?', newRowid)!; + const day = (await this.daysRepo.findById(newId))!; return { ...day, assignments: [], notes_items: [] }; } @@ -477,28 +504,27 @@ export class DaysService { const start = datedRows.map(r => r.date).sort((a, b) => (a < b ? -1 : a > b ? 1 : 0))[0]; const dates = Array.from({ length: n + 1 }, (_, i) => addDays(start, i)); const oldDateById = new Map(rows.map(r => [r.id, r.date])); - const setDayNumberAndDate = this.db.prepare('UPDATE days SET day_number = ?, date = ? WHERE id = ?'); const newId = await this.uow.transactional(async () => { - rows.forEach((r, i) => setDayNumber.run(-(i + 1), r.id)); - const result = this.db.run('INSERT INTO days (trip_id, day_number, date) VALUES (?, ?, ?)', tripId, pos, dates[pos - 1]); - const insertedId = Number(result.lastInsertRowid); + for (const [i, r] of rows.entries()) await this.daysRepo.setDayNumber(r.id, -(i + 1)); + const insertedId = await this.daysRepo.insertDay({ trip_id: tripIdNum, day_number: pos, date: dates[pos - 1] }); const orderedIds = rows.map(r => r.id); orderedIds.splice(pos - 1, 0, insertedId); const newDateById = new Map(); - orderedIds.forEach((id, i) => { - setDayNumberAndDate.run(i + 1, dates[i], id); + for (const [i, id] of orderedIds.entries()) { + await this.daysRepo.setDayNumberAndDate(id, i + 1, dates[i]); newDateById.set(id, dates[i]); - }); + } await this.restampReservationDates(tripId, oldDateById, newDateById); await this.assertNoInvertedAccommodation(tripId); - this.db.run('UPDATE trips SET end_date = ? WHERE id = ?', dates[dates.length - 1], tripId); + // DY35 — `TripsRepository.setEndDate`, added this task for Task 7 to reuse. + await this.tripsRepo.setEndDate(tripIdNum, dates[dates.length - 1]); return insertedId; }); - const day = this.db.get('SELECT * FROM days WHERE id = ?', newId)!; + const day = (await this.daysRepo.findById(newId))!; return { ...day, assignments: [], notes_items: [] }; } diff --git a/server/tests/e2e/days.e2e.test.ts b/server/tests/e2e/days.e2e.test.ts index 01b941141..4cc4dc024 100644 --- a/server/tests/e2e/days.e2e.test.ts +++ b/server/tests/e2e/days.e2e.test.ts @@ -1,10 +1,21 @@ /** * Days + day-notes module e2e — exercises both migrated mounts through the real - * JwtAuthGuard against a temp SQLite db. DaysService and DayNotesService run - * their real SQL via DatabaseModule (the DATABASE_CONNECTION factory picks up - * the mocked db singleton); trip access resolves through a real-SQL - * canAccessTrip over the temp db. Only the permission check and the WebSocket - * broadcast stay mocked. + * JwtAuthGuard against a real migrated-and-seeded temp SQLite db + * (createSnapshotTestDb(), Plan 3c Task 2 — this used to hand-roll a dozen + * CREATE TABLEs, a second hand-maintained schema copy that omitted + * `roadtrip_day_tracks` and the rest of the entity graph `DaysRepository + * .listByTrip`'s ORM `find()` now needs to resolve at query time, the exact + * "e2e suites build their schema from hand-written partial DDL" risk the + * plan's inventory §15c flagged — `no such table: roadtrip_day_tracks` on + * this suite's very first `GET /api/trips/:id/days` was the failure that + * proved it). DaysService and DayNotesService now run through real + * repositories over the same migrated connection; trip access resolves + * through `TripsRepository.findAccessible` via the real request-scoped + * `EntityManager` `createTestMikroOrmModule` wires in, not a hand-rolled + * `canAccessTrip` mock (the legacy override this file used to export from + * `db/database.ts` — deleted there since Plan 3c Task 0b; a stale mock here + * would silently do nothing, not fail loudly, which is worse than removing + * it). Only the permission check and the WebSocket broadcast stay mocked. */ import { describe, it, expect, beforeAll, afterAll, beforeEach, vi, type MockInstance } from 'vitest'; import request from 'supertest'; @@ -13,70 +24,15 @@ import type { Server } from 'http'; import { DatabaseModule } from '../../src/nest/database/database.module'; import { RealtimeModule } from '../../src/nest/realtime/realtime.module'; import { Test } from '@nestjs/testing'; -import { seedUser, sessionCookie } from './harness'; +import { sessionCookie } from './harness'; -const { db } = vi.hoisted(() => { - // eslint-disable-next-line @typescript-eslint/no-require-imports - const Database = require('better-sqlite3'); - const tmp = new Database(':memory:'); - tmp.exec('PRAGMA journal_mode = WAL'); - tmp.exec(`CREATE TABLE users (id INTEGER PRIMARY KEY AUTOINCREMENT, username TEXT NOT NULL, - email TEXT NOT NULL UNIQUE, role TEXT NOT NULL DEFAULT 'user', password_version INTEGER NOT NULL DEFAULT 0, - avatar TEXT);`); - tmp.exec('CREATE TABLE trips (id INTEGER PRIMARY KEY AUTOINCREMENT, user_id INTEGER, title TEXT, end_date TEXT, currency TEXT);'); - tmp.exec('CREATE TABLE trip_members (trip_id INTEGER NOT NULL, user_id INTEGER NOT NULL);'); - // The tables DaysService really queries (real SQL, no service mock). - tmp.exec(`CREATE TABLE days (id INTEGER PRIMARY KEY AUTOINCREMENT, trip_id INTEGER NOT NULL, - day_number INTEGER, date TEXT, title TEXT, notes TEXT, default_transport_mode TEXT, - created_at DATETIME DEFAULT CURRENT_TIMESTAMP, UNIQUE(trip_id, day_number));`); - tmp.exec('CREATE TABLE categories (id INTEGER PRIMARY KEY AUTOINCREMENT, name TEXT, color TEXT, icon TEXT);'); - tmp.exec(`CREATE TABLE places (id INTEGER PRIMARY KEY AUTOINCREMENT, trip_id INTEGER NOT NULL, name TEXT, - description TEXT, lat REAL, lng REAL, address TEXT, category_id INTEGER, price REAL, currency TEXT, - place_time TEXT, end_time TEXT, duration_minutes INTEGER, notes TEXT, image_url TEXT, transport_mode TEXT, - google_place_id TEXT, google_ftid TEXT, osm_id TEXT, amap_poi_id TEXT, website TEXT, phone TEXT, stop_type TEXT, fill_percent INTEGER);`); - tmp.exec(`CREATE TABLE day_assignments (id INTEGER PRIMARY KEY AUTOINCREMENT, day_id INTEGER NOT NULL, - place_id INTEGER NOT NULL, order_index INTEGER DEFAULT 0, notes TEXT, - assignment_time TEXT, assignment_end_time TEXT, created_at DATETIME DEFAULT CURRENT_TIMESTAMP);`); - tmp.exec('CREATE TABLE tags (id INTEGER PRIMARY KEY AUTOINCREMENT, trip_id INTEGER, name TEXT, color TEXT);'); - tmp.exec('CREATE TABLE place_tags (place_id INTEGER NOT NULL, tag_id INTEGER NOT NULL);'); - tmp.exec(`CREATE TABLE assignment_participants (assignment_id INTEGER NOT NULL, user_id INTEGER NOT NULL, - PRIMARY KEY (assignment_id, user_id));`); - tmp.exec(`CREATE TABLE day_notes (id INTEGER PRIMARY KEY AUTOINCREMENT, day_id INTEGER NOT NULL, - trip_id INTEGER NOT NULL, text TEXT NOT NULL, time TEXT, icon TEXT DEFAULT '📝', - color TEXT, sort_order REAL DEFAULT 0, created_at DATETIME DEFAULT CURRENT_TIMESTAMP);`); - // Reorder/insert touch accommodations + reservation restamping. - tmp.exec(`CREATE TABLE day_accommodations (id INTEGER PRIMARY KEY AUTOINCREMENT, trip_id INTEGER NOT NULL, - place_id INTEGER, start_day_id INTEGER, end_day_id INTEGER, check_in TEXT, check_in_end TEXT, - check_out TEXT, confirmation TEXT, notes TEXT, created_at DATETIME DEFAULT CURRENT_TIMESTAMP);`); - tmp.exec(`CREATE TABLE reservations (id INTEGER PRIMARY KEY AUTOINCREMENT, trip_id INTEGER, title TEXT, - day_id INTEGER, end_day_id INTEGER, type TEXT, status TEXT DEFAULT 'pending', reservation_time TEXT, - reservation_end_time TEXT, location TEXT, confirmation_number TEXT, notes TEXT, accommodation_id TEXT, - metadata TEXT, created_at DATETIME DEFAULT CURRENT_TIMESTAMP);`); - tmp.exec(`CREATE TABLE reservation_endpoints (id INTEGER PRIMARY KEY AUTOINCREMENT, reservation_id INTEGER NOT NULL, - local_date TEXT);`); - // StorageRegistryService (behind StorageModule, now in this module chain) reads - // this at onModuleInit. - tmp.exec('CREATE TABLE app_settings (key TEXT PRIMARY KEY, value TEXT);'); - return { db: tmp }; +vi.mock('../../src/db/database', async () => { + const { createSnapshotTestDb, buildDbMock } = await import('../helpers/db-mock'); + return buildDbMock(createSnapshotTestDb()); }); - -vi.mock('../../src/db/database', () => ({ - db, - // Real-SQL trip access over the temp db — DaysService.verifyTripAccess and - // DatabaseModule both read the mocked singleton. - canAccessTrip: (tripId: number | string, userId: number) => - db.prepare(` - SELECT t.id, t.user_id FROM trips t - LEFT JOIN trip_members m ON m.trip_id = t.id AND m.user_id = ? - WHERE t.id = ? AND (t.user_id = ? OR m.user_id IS NOT NULL) - `).get(userId, tripId, userId), - isOwner: () => false, - getPlaceWithTags: () => null, - closeDb: () => {}, - reinitialize: () => {}, -})); vi.mock('../../src/websocket', () => ({ broadcast: vi.fn() })); +import { db } from '../../src/db/database'; import { PermissionsService } from '../../src/nest/permissions/permissions.service'; // Since the permissions DI migration, the check is a spy on the container's @@ -108,7 +64,13 @@ describe('Days + day-notes e2e (real auth guard + temp SQLite, real day SQL)', ( } beforeAll(async () => { - seedUser(db as never, { id: 1 }); + // harness.ts's seedUser() omits password_hash, which the real migrated + // schema requires NOT NULL (addons.e2e.test.ts/share.e2e.test.ts hit the + // same thing) — a raw insert here instead, matching the SeededUser shape + // id/role/password_version=0 that sessionCookie(1) needs. + db.prepare( + "INSERT INTO users (id, username, email, password_hash, role, password_version) VALUES (1, 'e2e-user', 'e2e@example.test', 'x', 'user', 0)", + ).run(); db.prepare('INSERT INTO trips (id, user_id, title) VALUES (5, 1, ?)').run('Trip'); db.prepare('INSERT INTO days (id, trip_id, day_number) VALUES (3, 5, 1)').run(); app = await build(); diff --git a/server/tests/helpers/mcp-test-controllers.ts b/server/tests/helpers/mcp-test-controllers.ts index ee6ef6a3e..aa21e4704 100644 --- a/server/tests/helpers/mcp-test-controllers.ts +++ b/server/tests/helpers/mcp-test-controllers.ts @@ -114,7 +114,10 @@ import { AirtrailImportService } from '../../src/nest/integrations/airtrail-impo import { ReservationImportMcp } from '../../src/nest/reservation-import/reservation-import.mcp'; import { HelpMcp } from '../../src/nest/help/help.mcp'; import { AddonsMcp } from '../../src/nest/addons/addons.mcp'; -import { createTestUnitOfWork, createTestAppSettingsRepo, createTestCategoriesRepo, createTestTagsRepo, createTestSettingsRepo } from './test-uow'; +import { + createTestUnitOfWork, createTestAppSettingsRepo, createTestCategoriesRepo, createTestTagsRepo, createTestSettingsRepo, + createTestDaysRepo, createTestDayAssignmentsRepo, createTestDayNotesRepo, createTestTripsRepo, +} from './test-uow'; import { createTestOrm } from './test-orm'; import { AppSettings } from '../../src/db/entities/AppSettings.entity'; import { AuditLog } from '../../src/db/entities/AuditLog.entity'; @@ -170,7 +173,17 @@ export async function createMcpTestRegistry(): Promise { appSettings, usersRepo, inviteTokensRepo, mcpTokensRepoForAuth, oauthTokensRepo, webauthnCredentialsRepoForAuth, passwordResetTokensRepo, ); const queryHelpersService = new QueryHelpersService(dbService); - const daysService = new DaysService(dbService, permissionsService, realtimeService, queryHelpersService, await createTestUnitOfWork(dbService.connection)); + const daysService = new DaysService( + dbService, + permissionsService, + realtimeService, + queryHelpersService, + await createTestUnitOfWork(dbService.connection), + await createTestDaysRepo(dbService.connection), + await createTestDayAssignmentsRepo(dbService.connection), + await createTestDayNotesRepo(dbService.connection), + await createTestTripsRepo(dbService.connection), + ); const todoService = new TodoService(dbService, permissionsService, realtimeService, await createTestUnitOfWork(dbService.connection)); const packingService = new PackingService(dbService, permissionsService, realtimeService, notificationsStub(), await createTestUnitOfWork(dbService.connection)); const collabService = new CollabService(dbService, permissionsService, realtimeService, notificationsStub(), generalStorage, new RateLimitService(), await createTestUnitOfWork(dbService.connection)); diff --git a/server/tests/helpers/plugin-host.ts b/server/tests/helpers/plugin-host.ts index e29ea2a98..4aa59a6c7 100644 --- a/server/tests/helpers/plugin-host.ts +++ b/server/tests/helpers/plugin-host.ts @@ -72,7 +72,10 @@ import { PlacePhotoCacheService } from '../../src/nest/place-photos/place-photo- import { TrekPhotosRepository } from '../../src/nest/photos/trek-photos.repository'; import { RuntimeEnvService } from '../../src/nest/app-config/runtime-env.service'; import { makeStorageFixture } from './storage-fixture'; -import { createTestUnitOfWork, createTestAppSettingsRepo, createTestCategoriesRepo, createTestTagsRepo, createTestSettingsRepo, sharedTestOrm } from './test-uow'; +import { + createTestUnitOfWork, createTestAppSettingsRepo, createTestCategoriesRepo, createTestTagsRepo, createTestSettingsRepo, sharedTestOrm, + createTestDaysRepo, createTestDayAssignmentsRepo, createTestDayNotesRepo, createTestTripsRepo, +} from './test-uow'; import { AppSettings } from '../../src/db/entities/AppSettings.entity'; import { AuditLog } from '../../src/db/entities/AuditLog.entity'; import { Users } from '../../src/db/entities/Users.entity'; @@ -102,7 +105,17 @@ export async function createPluginRpcHostFactory(dbs: DatabaseService): Promise< const files = new FilesService(dbs, permissions, realtime, new EphemeralTokenService(), generalStorage, (await sharedTestOrm(dbs.connection)).em); const collab = new CollabService(dbs, permissions, realtime, notificationsStub(), generalStorage, new RateLimitService(), await createTestUnitOfWork(dbs.connection)); const vacay = new VacayService(dbs, realtime, notificationsStub(), await createTestUnitOfWork(dbs.connection)); - const days = new DaysService(dbs, permissions, realtime, queryHelpers, await createTestUnitOfWork(dbs.connection)); + const days = new DaysService( + dbs, + permissions, + realtime, + queryHelpers, + await createTestUnitOfWork(dbs.connection), + await createTestDaysRepo(dbs.connection), + await createTestDayAssignmentsRepo(dbs.connection), + await createTestDayNotesRepo(dbs.connection), + await createTestTripsRepo(dbs.connection), + ); const photoCache = new PlacePhotoCacheService(dbs, makeStorageFixture('photos/google/').storage); const unsplash = new UnsplashService(appSettings, usersRepo, new RuntimeEnvService(), generalStorage); const journey = new JourneyDomainService(dbs, realtime, new TrekPhotosRepository(dbs), await createTestUnitOfWork(dbs.connection)); diff --git a/server/tests/helpers/test-uow.ts b/server/tests/helpers/test-uow.ts index a4e984ec1..40197f985 100644 --- a/server/tests/helpers/test-uow.ts +++ b/server/tests/helpers/test-uow.ts @@ -31,6 +31,12 @@ import type { TripMembersRepository } from '../../src/db/repositories/TripMember import { Places } from '../../src/db/entities/Places.entity'; import type { PlacesRepository } from '../../src/db/repositories/Places.repository'; import { DatabaseService } from '../../src/nest/database/database.service'; +import { Days } from '../../src/db/entities/Days.entity'; +import type { DaysRepository } from '../../src/db/repositories/Days.repository'; +import { DayAssignments } from '../../src/db/entities/DayAssignments.entity'; +import type { DayAssignmentsRepository } from '../../src/db/repositories/DayAssignments.repository'; +import { DayNotes } from '../../src/db/entities/DayNotes.entity'; +import type { DayNotesRepository } from '../../src/db/repositories/DayNotes.repository'; const perHandle = new WeakMap>(); const appSettingsPerHandle = new WeakMap>(); @@ -342,3 +348,41 @@ export async function createTestDatabaseService(db: Database.Database): Promise< const t = await sharedTestOrm(db); return new DatabaseService(db, t.em); } + +// --------------------------------------------------------------------------- +// Plan 3c Task 2 (`DaysService` + day notes onto `DaysRepository`/ +// `DayAssignmentsRepository`/`DayNotesRepository`/`TripsRepository.setEndDate`) +// — appended at the end per the task's own file-ownership rule (append only). +// Same memoisation-per-handle pattern as every helper above. +// --------------------------------------------------------------------------- + +const daysRepoPerHandle = new WeakMap>(); +const dayAssignmentsRepoPerHandle = new WeakMap>(); +const dayNotesRepoPerHandle = new WeakMap>(); + +/** The `DaysRepository` a hand-constructed `DaysService` needs. */ +export function createTestDaysRepo(db: Database.Database): Promise { + const existing = daysRepoPerHandle.get(db); + if (existing !== undefined) return existing; + const pending = sharedTestOrm(db).then((t) => t.repo(Days)); + daysRepoPerHandle.set(db, pending); + return pending; +} + +/** The `DayAssignmentsRepository` a hand-constructed `DaysService` needs. */ +export function createTestDayAssignmentsRepo(db: Database.Database): Promise { + const existing = dayAssignmentsRepoPerHandle.get(db); + if (existing !== undefined) return existing; + const pending = sharedTestOrm(db).then((t) => t.repo(DayAssignments)); + dayAssignmentsRepoPerHandle.set(db, pending); + return pending; +} + +/** The `DayNotesRepository` a hand-constructed `DaysService` needs (DY4). */ +export function createTestDayNotesRepo(db: Database.Database): Promise { + const existing = dayNotesRepoPerHandle.get(db); + if (existing !== undefined) return existing; + const pending = sharedTestOrm(db).then((t) => t.repo(DayNotes)); + dayNotesRepoPerHandle.set(db, pending); + return pending; +} diff --git a/server/tests/integration/dayReorder.test.ts b/server/tests/integration/dayReorder.test.ts index 6e92f4649..fbaa3adb0 100644 --- a/server/tests/integration/dayReorder.test.ts +++ b/server/tests/integration/dayReorder.test.ts @@ -25,11 +25,24 @@ import { PermissionsService } from '../../src/nest/permissions/permissions.servi import { DaysService, DayReorderError } from '../../src/nest/days/days.service'; import { RealtimeService } from '../../src/nest/realtime/realtime.service'; import { QueryHelpersService } from '../../src/nest/query-helpers/query-helpers.service'; -import { createTestUnitOfWork, createTestAppSettingsRepo } from '../helpers/test-uow'; +import { + createTestUnitOfWork, createTestAppSettingsRepo, + createTestDaysRepo, createTestDayAssignmentsRepo, createTestDayNotesRepo, createTestTripsRepo, +} from '../helpers/test-uow'; let svc: DaysService; beforeAll(async () => { - svc = new DaysService(new DatabaseService(testDb), new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new RealtimeService(), new QueryHelpersService(new DatabaseService(testDb)), await createTestUnitOfWork(testDb)); + svc = new DaysService( + new DatabaseService(testDb), + new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), + new RealtimeService(), + new QueryHelpersService(new DatabaseService(testDb)), + await createTestUnitOfWork(testDb), + await createTestDaysRepo(testDb), + await createTestDayAssignmentsRepo(testDb), + await createTestDayNotesRepo(testDb), + await createTestTripsRepo(testDb), + ); }); const reorderDays = async (tripId: number, orderedIds: number[]) => await svc.reorder(tripId, orderedIds); const insertDay = async (tripId: number, position?: number) => await svc.insert(tripId, position); diff --git a/server/tests/unit/db/repositories/DayAssignments.repository.test.ts b/server/tests/unit/db/repositories/DayAssignments.repository.test.ts new file mode 100644 index 000000000..db029c65d --- /dev/null +++ b/server/tests/unit/db/repositories/DayAssignments.repository.test.ts @@ -0,0 +1,266 @@ +/** + * `DayAssignmentsRepository` (Plan 3c Task 2) — the DY1/DY3/AS1/AS3 + * assignment-with-place projection contract (built here, Task 3 consumes it + * unchanged for AS1/AS3) plus DY24's `reanchorToDay` (a correlated scalar + * subquery in an UPDATE SET clause, via Kysely). + */ +import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import { createSnapshotTestDb } from '../../../helpers/db-mock'; +import { resetTestDb } from '../../../helpers/test-db'; +import { createTestOrm, type TestOrm } from '../../../helpers/test-orm'; +import { createCategory, createDay, createDayAssignment, createPlace, createTrip, createUser } from '../../../helpers/factories'; +import { DayAssignments } from '../../../../src/db/entities/DayAssignments.entity'; +import type { DayAssignmentsRepository } from '../../../../src/db/repositories/DayAssignments.repository'; +import { UnitOfWork } from '../../../../src/nest/database/unit-of-work'; +import { withRequestContext } from '../../../../src/nest/database/request-context'; + +const testDb = createSnapshotTestDb(); +let t: TestOrm; +let assignments: DayAssignmentsRepository; +let uow: UnitOfWork; + +beforeAll(async () => { + t = await createTestOrm(testDb); + assignments = t.repo(DayAssignments); + uow = new UnitOfWork(t.em); +}); +beforeEach(() => { resetTestDb(testDb); t.clear(); }); +afterAll(async () => { await t.close(); testDb.close(); }); + +/** The DY1/DY3/AS1/AS3 statement, run raw — the parity oracle every assertion below is checked against. */ +function legacyProjectionRow(id: number): unknown { + return testDb.prepare(` + SELECT da.*, p.id as place_id, p.name as place_name, p.description as place_description, + p.lat, p.lng, p.address, p.category_id, p.price, p.currency as place_currency, + COALESCE(da.assignment_time, p.place_time) as place_time, + COALESCE(da.assignment_end_time, p.end_time) as end_time, + p.duration_minutes, p.notes as place_notes, + p.image_url, p.transport_mode, p.google_place_id, p.google_ftid, p.osm_id, p.amap_poi_id, p.website, p.phone, p.stop_type, p.fill_percent, + c.name as category_name, c.color as category_color, c.icon as category_icon + FROM day_assignments da + JOIN places p ON da.place_id = p.id + LEFT JOIN categories c ON p.category_id = c.id + WHERE da.id = ? + `).get(id); +} + +async function withQueryCount(fn: () => Promise): Promise<{ value: T; queries: number }> { + const connection = t.orm.em.getConnection(); + const spy = vi.spyOn(connection, 'execute'); + try { + const value = await fn(); + return { value, queries: spy.mock.calls.length }; + } finally { + spy.mockRestore(); + } +} + +describe('DayAssignmentsRepository — the DY1/DY3/AS1/AS3 projection', () => { + it('ASSIGNPLACEREPO-001: listForDay returns [] for a day with no assignments', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + expect(await assignments.listForDay(day.id)).toEqual([]); + }); + + it('ASSIGNPLACEREPO-002: listForDay is byte-identical to the legacy statement, column for column', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const category = createCategory(testDb, { name: 'Museum', color: '#111111', icon: '🏛️' }); + const place = createPlace(testDb, trip.id, { name: 'Louvre', category_id: category.id }); + const assignment = createDayAssignment(testDb, day.id, place.id); + + const rows = await assignments.listForDay(day.id); + expect(rows).toHaveLength(1); + expect(rows[0]).toStrictEqual(legacyProjectionRow(assignment.id)); + // Named spot-checks so a future field rename fails loudly here, not just via toStrictEqual. + expect(rows[0]).toMatchObject({ + id: assignment.id, day_id: day.id, place_id: place.id, + place_name: 'Louvre', category_id: category.id, + category_name: 'Museum', category_color: '#111111', category_icon: '🏛️', + }); + }); + + it('ASSIGNPLACEREPO-003: findWithPlaceAndCategory(id) is the same row, and null for a missing id', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const place = createPlace(testDb, trip.id, { name: 'Eiffel Tower' }); + const assignment = createDayAssignment(testDb, day.id, place.id); + + expect(await assignments.findWithPlaceAndCategory(assignment.id)).toStrictEqual(legacyProjectionRow(assignment.id)); + expect(await assignments.findWithPlaceAndCategory(999999)).toBeNull(); + }); + + it('ASSIGNPLACEREPO-004: place_time/end_time COALESCE — the assignment\'s own time wins, the place\'s time is the fallback', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const place = createPlace(testDb, trip.id); + testDb.prepare('UPDATE places SET place_time = ?, end_time = ? WHERE id = ?').run('09:00', '10:00', place.id); + const withOwnTime = createDayAssignment(testDb, day.id, place.id); + testDb.prepare('UPDATE day_assignments SET assignment_time = ?, assignment_end_time = ? WHERE id = ?').run('14:00', '15:00', withOwnTime.id); + const withoutOwnTime = createDayAssignment(testDb, day.id, place.id); + + const rows = await assignments.listForDay(day.id); + const own = rows.find((r) => r.id === withOwnTime.id)!; + const fallback = rows.find((r) => r.id === withoutOwnTime.id)!; + expect(own.place_time).toBe('14:00'); + expect(own.end_time).toBe('15:00'); + expect(fallback.place_time).toBe('09:00'); + expect(fallback.end_time).toBe('10:00'); + }); + + it('ASSIGNPLACEREPO-005: category is null for an uncategorised place (LEFT JOIN, not INNER)', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const place = createPlace(testDb, trip.id); + testDb.prepare('UPDATE places SET category_id = NULL WHERE id = ?').run(place.id); + const assignment = createDayAssignment(testDb, day.id, place.id); + + const [row] = await assignments.listForDay(day.id); + expect(row.category_id).toBeNull(); + expect(row.category_name).toBeNull(); + expect(row.category_color).toBeNull(); + expect(row.category_icon).toBeNull(); + expect(row.id).toBe(assignment.id); + }); + + it('ASSIGNPLACEREPO-006: ORDER BY order_index ASC, created_at ASC', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const placeA = createPlace(testDb, trip.id, { name: 'Second' }); + const placeB = createPlace(testDb, trip.id, { name: 'First' }); + createDayAssignment(testDb, day.id, placeA.id, { order_index: 2 }); + createDayAssignment(testDb, day.id, placeB.id, { order_index: 1 }); + + const rows = await assignments.listForDay(day.id); + expect(rows.map((r) => r.place_name)).toEqual(['First', 'Second']); + }); + + it('ASSIGNPLACEREPO-007: listWithPlaceAndCategory spans several days, in the same order; an empty array short-circuits with 0 queries', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const dayA = createDay(testDb, trip.id, { day_number: 1 }); + const dayB = createDay(testDb, trip.id, { day_number: 2 }); + const placeA = createPlace(testDb, trip.id, { name: 'A' }); + const placeB = createPlace(testDb, trip.id, { name: 'B' }); + const a = createDayAssignment(testDb, dayA.id, placeA.id); + const b = createDayAssignment(testDb, dayB.id, placeB.id); + + const rows = await assignments.listWithPlaceAndCategory([dayA.id, dayB.id]); + expect(rows.map((r) => r.id).sort()).toEqual([a.id, b.id].sort()); + + const { value, queries } = await withQueryCount(() => assignments.listWithPlaceAndCategory([])); + expect(value).toEqual([]); + expect(queries).toBe(0); + }); + + // D-shape: every method here is a `qb().execute(..., false)` projection, which + // never hydrates an entity into the identity map by construction — proven + // anyway, the "not required, proven regardless" shape (AssignmentParticipants + // precedent). + it('ASSIGNPLACEREPO-008 (D-shape): a place rename after an unrelated identity-map read is visible in the FIRST wider projection', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const place = createPlace(testDb, trip.id, { name: 'Old Name' }); + const assignment = createDayAssignment(testDb, day.id, place.id); + await t.repo(DayAssignments).find({}); // populate the identity map with an unrelated read + testDb.prepare('UPDATE places SET name = ? WHERE id = ?').run('Renamed', place.id); + + const [row] = await assignments.listForDay(day.id); + expect(row.place_name).toBe('Renamed'); + expect(row.id).toBe(assignment.id); + }); +}); + +describe('DayAssignmentsRepository.reanchorToDay (DY24, Kysely)', () => { + function setAccommodation(assignmentId: number, accId: number): void { + testDb.prepare('UPDATE day_assignments SET accommodation_id = ? WHERE id = ?').run(accId, assignmentId); + } + + it('REANCHORDAY-001: moves the stop to the target day, re-indexed past the target day\'s existing max order_index', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const fromDay = createDay(testDb, trip.id, { day_number: 1 }); + const toDay = createDay(testDb, trip.id, { day_number: 2 }); + const stopPlace = createPlace(testDb, trip.id, { name: 'Hotel stop' }); + const otherPlace = createPlace(testDb, trip.id, { name: 'Existing stop' }); + createDayAssignment(testDb, toDay.id, otherPlace.id, { order_index: 5 }); + const stop = createDayAssignment(testDb, fromDay.id, stopPlace.id, { order_index: 0 }); + setAccommodation(stop.id, 4242); + + await withRequestContext(t.orm, async () => { + await assignments.reanchorToDay(4242, toDay.id); + }); + + const row = testDb.prepare('SELECT day_id, order_index FROM day_assignments WHERE id = ?').get(stop.id) as { day_id: number; order_index: number }; + expect(row.day_id).toBe(toDay.id); + expect(row.order_index).toBe(6); + }); + + it('REANCHORDAY-002: the target day starting empty yields order_index 0 (COALESCE(NULL, -1) + 1)', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const fromDay = createDay(testDb, trip.id, { day_number: 1 }); + const toDay = createDay(testDb, trip.id, { day_number: 2 }); + const place = createPlace(testDb, trip.id); + const stop = createDayAssignment(testDb, fromDay.id, place.id, { order_index: 0 }); + setAccommodation(stop.id, 7); + + await withRequestContext(t.orm, async () => { + await assignments.reanchorToDay(7, toDay.id); + }); + + const row = testDb.prepare('SELECT day_id, order_index FROM day_assignments WHERE id = ?').get(stop.id) as { day_id: number; order_index: number }; + expect(row.day_id).toBe(toDay.id); + expect(row.order_index).toBe(0); + }); + + it('REANCHORDAY-003: inside a uow.transactional that then ROLLS BACK, the row is left un-moved (joins the ambient transaction, proven by rollback)', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const fromDay = createDay(testDb, trip.id, { day_number: 1 }); + const toDay = createDay(testDb, trip.id, { day_number: 2 }); + const place = createPlace(testDb, trip.id); + const stop = createDayAssignment(testDb, fromDay.id, place.id, { order_index: 0 }); + setAccommodation(stop.id, 99); + + let caught: unknown; + try { + await withRequestContext(t.orm, async () => { + await uow.transactional(async () => { + await assignments.reanchorToDay(99, toDay.id); + throw new Error('force rollback'); + }); + }); + } catch (e) { caught = e; } + expect((caught as Error).message).toBe('force rollback'); + + const row = testDb.prepare('SELECT day_id FROM day_assignments WHERE id = ?').get(stop.id) as { day_id: number }; + expect(row.day_id).toBe(fromDay.id); + }); + + it('REANCHORDAY-004: the same sequence, committed, actually moves the row', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const fromDay = createDay(testDb, trip.id, { day_number: 1 }); + const toDay = createDay(testDb, trip.id, { day_number: 2 }); + const place = createPlace(testDb, trip.id); + const stop = createDayAssignment(testDb, fromDay.id, place.id, { order_index: 0 }); + setAccommodation(stop.id, 100); + + await withRequestContext(t.orm, async () => { + await uow.transactional(async () => { + await assignments.reanchorToDay(100, toDay.id); + }); + }); + + const row = testDb.prepare('SELECT day_id FROM day_assignments WHERE id = ?').get(stop.id) as { day_id: number }; + expect(row.day_id).toBe(toDay.id); + }); +}); diff --git a/server/tests/unit/db/repositories/DayNotes.repository.test.ts b/server/tests/unit/db/repositories/DayNotes.repository.test.ts index bf7b168e8..9858b68c4 100644 --- a/server/tests/unit/db/repositories/DayNotes.repository.test.ts +++ b/server/tests/unit/db/repositories/DayNotes.repository.test.ts @@ -2,7 +2,7 @@ import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vites import { createSnapshotTestDb } from '../../../helpers/db-mock'; import { resetTestDb } from '../../../helpers/test-db'; import { createTestOrm, type TestOrm } from '../../../helpers/test-orm'; -import { createDay, createTrip, createUser } from '../../../helpers/factories'; +import { createDay, createDayNote, createTrip, createUser } from '../../../helpers/factories'; import { DayNotes } from '../../../../src/db/entities/DayNotes.entity'; import type { DayNotesRepository } from '../../../../src/db/repositories/DayNotes.repository'; import { DB_TIMESTAMP_RE } from '../../../../src/db/types'; @@ -59,3 +59,38 @@ describe('DayNotesRepository timestamps', () => { spy.mockRestore(); }); }); + +// --------------------------------------------------------------------------- +// Plan 3c Task 2 (DY4) — DaysService.list's day_notes batch load. +// --------------------------------------------------------------------------- + +describe('DayNotesRepository.listByDayIds (DY4)', () => { + it('NOTEREPO-005: an empty day_ids array short-circuits to [] without querying', async () => { + const connection = t.orm.em.getConnection(); + const spy = vi.spyOn(connection, 'execute'); + expect(await notes.listByDayIds([])).toEqual([]); + expect(spy).not.toHaveBeenCalled(); + spy.mockRestore(); + }); + + it('NOTEREPO-006: spans several days, ordered by sort_order ASC then created_at ASC', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const dayA = createDay(testDb, trip.id, { day_number: 1 }); + const dayB = createDay(testDb, trip.id, { day_number: 2 }); + createDayNote(testDb, dayA.id, trip.id, { text: 'Second', sort_order: 2 }); + createDayNote(testDb, dayA.id, trip.id, { text: 'First', sort_order: 1 }); + createDayNote(testDb, dayB.id, trip.id, { text: 'Only' }); + + const rows = await notes.listByDayIds([dayA.id, dayB.id]); + expect(rows.map((r) => r.text)).toEqual(['First', 'Second', 'Only']); + expect(rows.map((r) => r.day_id)).toEqual([dayA.id, dayA.id, dayB.id]); + }); + + it('NOTEREPO-007: a day with no notes contributes no rows', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + expect(await notes.listByDayIds([day.id])).toEqual([]); + }); +}); diff --git a/server/tests/unit/db/repositories/Days.repository.test.ts b/server/tests/unit/db/repositories/Days.repository.test.ts index 16f081dc2..06c343273 100644 --- a/server/tests/unit/db/repositories/Days.repository.test.ts +++ b/server/tests/unit/db/repositories/Days.repository.test.ts @@ -86,3 +86,106 @@ describe('DaysRepository', () => { spy.mockRestore(); }); }); + +// --------------------------------------------------------------------------- +// Plan 3c Task 2 — the DaysService conversion's remaining methods (DY7/DY9/ +// DY11/DY13/DY21/DY26–DY36). +// --------------------------------------------------------------------------- + +describe('DaysRepository — Task 2 additions', () => { + it('DAYREPO-010: findById is the SELECT * row, undefined for a missing id', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id, { title: 'Arrival' }); + expect(await days.findById(day.id)).toStrictEqual(rawDay(day.id)); + expect(await days.findById(999999)).toBeUndefined(); + }); + + it('DAYREPO-011: updateNotesAndTitle writes both columns verbatim (the presence-sentinel decision itself lives in the service)', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + await days.updateNotesAndTitle(day.id, 'Walking day', 'Arrival'); + expect(await days.findById(day.id)).toMatchObject({ notes: 'Walking day', title: 'Arrival' }); + await days.updateNotesAndTitle(day.id, null, null); + expect(await days.findById(day.id)).toMatchObject({ notes: null, title: null }); + }); + + it('DAYREPO-012: setDefaultTransportMode sets and clears the column', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + await days.setDefaultTransportMode(day.id, 'walk'); + expect(await days.findById(day.id)).toMatchObject({ default_transport_mode: 'walk' }); + await days.setDefaultTransportMode(day.id, null); + expect(await days.findById(day.id)).toMatchObject({ default_transport_mode: null }); + }); + + it('DAYREPO-013: deleteById is unscoped by trip', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + await days.deleteById(day.id); + expect(await days.findById(day.id)).toBeUndefined(); + }); + + it('DAYREPO-014: listOrderedForReorder returns {id, day_number, date}, ordered by day_number', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const d2 = createDay(testDb, trip.id, { day_number: 2, date: '2026-01-02' }); + const d1 = createDay(testDb, trip.id, { day_number: 1, date: '2026-01-01' }); + const rows = await days.listOrderedForReorder(trip.id); + expect(rows).toEqual([ + { id: d1.id, day_number: 1, date: '2026-01-01' }, + { id: d2.id, day_number: 2, date: '2026-01-02' }, + ]); + }); + + it('DAYREPO-015: setDayNumber and setDayNumberAndDate write their columns; the two-phase (negative then positive) sequence a caller drives here never collides', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const a = createDay(testDb, trip.id, { day_number: 1 }); + const b = createDay(testDb, trip.id, { day_number: 2 }); + // Phase 1: negative, to dodge UNIQUE(trip_id, day_number) while swapping. + await days.setDayNumber(a.id, -2); + await days.setDayNumber(b.id, -1); + // Phase 2: positive (the swap). + await days.setDayNumberAndDate(a.id, 2, '2026-02-02'); + await days.setDayNumberAndDate(b.id, 1, '2026-02-01'); + expect(await days.findById(a.id)).toMatchObject({ day_number: 2, date: '2026-02-02' }); + expect(await days.findById(b.id)).toMatchObject({ day_number: 1, date: '2026-02-01' }); + }); + + it('DAYREPO-016: insertDay writes trip_id/day_number/date only (no notes column named) and returns the inserted id', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const id = await days.insertDay({ trip_id: trip.id, day_number: 1, date: '2026-03-01' }); + expect(typeof id).toBe('number'); + const row = rawDay(id) as { notes: string | null; date: string | null; day_number: number; trip_id: number }; + expect(row).toMatchObject({ trip_id: trip.id, day_number: 1, date: '2026-03-01', notes: null }); + }); + + it('DAYREPO-017: findByTripAndDate finds the day at that date, undefined otherwise, scoped to the trip', async () => { + const { user } = createUser(testDb); + const tripA = createTrip(testDb, user.id); + const tripB = createTrip(testDb, user.id); + const day = createDay(testDb, tripA.id, { date: '2026-04-01' }); + createDay(testDb, tripB.id, { date: '2026-04-01' }); + expect(await days.findByTripAndDate(tripA.id, '2026-04-01')).toEqual({ id: day.id, day_number: day.day_number }); + expect(await days.findByTripAndDate(tripA.id, '2026-04-02')).toBeUndefined(); + }); + + // D-shape: `findById` (findOne-based, `disableIdentityMap: true` by the base + // class default) must see a column written after an unrelated identity-map + // read populated the same entity type — written column (`notes`) is outside + // the narrower read's own projection scope, so a stale write-back would + // revert it if the base class's guarantee ever regressed. + it('DAYREPO-018 (D-shape): a notes write after an unrelated identity-map read is visible in findById', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + await t.repo(Days).find({}); // populate the identity map with an unrelated read + await days.updateNotesAndTitle(day.id, 'Fresh notes', null); + expect(await days.findById(day.id)).toMatchObject({ notes: 'Fresh notes' }); + }); +}); diff --git a/server/tests/unit/db/repositories/Trips.repository.test.ts b/server/tests/unit/db/repositories/Trips.repository.test.ts index 3ca6431f2..fee635258 100644 --- a/server/tests/unit/db/repositories/Trips.repository.test.ts +++ b/server/tests/unit/db/repositories/Trips.repository.test.ts @@ -80,3 +80,18 @@ describe('TripsRepository.findAccessible — parity with canAccessTrip', () => { expect(wrap(entity).toObject()).toStrictEqual(raw); }); }); + +// Plan 3c Task 2 (DY35): `days.service.ts::insert`'s dated path extends the +// trip by one day. Owned exclusively by Task 2 per the plan's file +// ownership (`Trips.repository.ts` ONLY for `setEndDate`) — appended here +// rather than interleaved with Task 1's blocks above. +describe('TripsRepository.setEndDate (Plan 3c Task 2, DY35)', () => { + it('TRIPREPO-013: writes end_date verbatim, including clearing it to null', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + await trips.setEndDate(trip.id, '2026-09-30'); + expect((testDb.prepare('SELECT end_date FROM trips WHERE id = ?').get(trip.id) as { end_date: string | null }).end_date).toBe('2026-09-30'); + await trips.setEndDate(trip.id, null); + expect((testDb.prepare('SELECT end_date FROM trips WHERE id = ?').get(trip.id) as { end_date: string | null }).end_date).toBeNull(); + }); +}); diff --git a/server/tests/unit/nest/days.service.test.ts b/server/tests/unit/nest/days.service.test.ts index 749178b5b..6d152f2e9 100644 --- a/server/tests/unit/nest/days.service.test.ts +++ b/server/tests/unit/nest/days.service.test.ts @@ -64,12 +64,25 @@ import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; import { QueryHelpersService } from '../../../src/nest/query-helpers/query-helpers.service'; import { makeAccommodationsService } from '../../helpers/accommodations-service'; import type { Day } from '../../../src/types'; -import { createTestUnitOfWork, createTestAppSettingsRepo, createTestDatabaseService } from '../../helpers/test-uow'; +import { + createTestUnitOfWork, createTestAppSettingsRepo, createTestDatabaseService, + createTestDaysRepo, createTestDayAssignmentsRepo, createTestDayNotesRepo, createTestTripsRepo, +} from '../../helpers/test-uow'; let svc: DaysService; beforeAll(async () => { const dbs = await createTestDatabaseService(testDb); - svc = new DaysService(dbs, new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new RealtimeService(), new QueryHelpersService(dbs), await createTestUnitOfWork(testDb)); + svc = new DaysService( + dbs, + new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), + new RealtimeService(), + new QueryHelpersService(dbs), + await createTestUnitOfWork(testDb), + await createTestDaysRepo(testDb), + await createTestDayAssignmentsRepo(testDb), + await createTestDayNotesRepo(testDb), + await createTestTripsRepo(testDb), + ); }); let accommodations: Awaited>; beforeAll(async () => { @@ -799,7 +812,17 @@ describe('DaysService.canEdit', () => { it('DAY-SVC-090 asks for day_edit and flags a non-owner as shared', async () => { const checkPermission = vi.fn(() => true); const permissions = { checkPermission } as unknown as PermissionsService; - const withStub = new DaysService(new DatabaseService(testDb), permissions, new RealtimeService(), new QueryHelpersService(new DatabaseService(testDb)), await createTestUnitOfWork(testDb)); + const withStub = new DaysService( + new DatabaseService(testDb), + permissions, + new RealtimeService(), + new QueryHelpersService(new DatabaseService(testDb)), + await createTestUnitOfWork(testDb), + await createTestDaysRepo(testDb), + await createTestDayAssignmentsRepo(testDb), + await createTestDayNotesRepo(testDb), + await createTestTripsRepo(testDb), + ); const trip = { id: 1, user_id: 1 } as never; expect(await withStub.canEdit(trip, { id: 1, role: 'user' } as never)).toBe(true); diff --git a/server/tests/unit/nest/trip-read-model.service.test.ts b/server/tests/unit/nest/trip-read-model.service.test.ts index 486e43fa1..21c92faef 100644 --- a/server/tests/unit/nest/trip-read-model.service.test.ts +++ b/server/tests/unit/nest/trip-read-model.service.test.ts @@ -78,7 +78,10 @@ import { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env.serv import { makeStorageFixture } from '../../helpers/storage-fixture'; import { JourneyDomainService } from '../../../src/nest/journey/journey-domain.service'; import { TrekPhotosRepository } from '../../../src/nest/photos/trek-photos.repository'; -import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, sharedTestOrm } from '../../helpers/test-uow'; +import { + createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, sharedTestOrm, + createTestDaysRepo, createTestDayAssignmentsRepo, createTestDayNotesRepo, createTestTripsRepo, +} from '../../helpers/test-uow'; // Real sibling services over the same in-memory DB — the aggregation runs the // actual SQL of every domain it fans out to, so a shape change downstream shows @@ -100,7 +103,17 @@ let placesSvc: PlacesService; let membersSvc: TripMembersService; beforeAll(async () => { budgetSvc = new BudgetService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(dbs().connection)); - daysSvc = new DaysService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), new QueryHelpersService(dbs()), await createTestUnitOfWork(dbs().connection)); + daysSvc = new DaysService( + dbs(), + new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), + new RealtimeService(), + new QueryHelpersService(dbs()), + await createTestUnitOfWork(dbs().connection), + await createTestDaysRepo(dbs().connection), + await createTestDayAssignmentsRepo(dbs().connection), + await createTestDayNotesRepo(dbs().connection), + await createTestTripsRepo(dbs().connection), + ); placesSvc = new PlacesService( dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), new MapsService(dbs(), photoCache, await createTestAppSettingsRepo(dbs().connection), await createTestUsersRepo(dbs().connection)), new QueryHelpersService(dbs()), diff --git a/server/tests/unit/nest/trips.service.test.ts b/server/tests/unit/nest/trips.service.test.ts index 8497da0f5..e9f9c1efd 100644 --- a/server/tests/unit/nest/trips.service.test.ts +++ b/server/tests/unit/nest/trips.service.test.ts @@ -88,7 +88,10 @@ import fs from 'fs'; import path from 'path'; import { notificationsStub } from '../../helpers/notifications'; import { EphemeralTokenService } from '../../../src/nest/auth/ephemeral-token.service'; -import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, sharedTestOrm } from '../../helpers/test-uow'; +import { + createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, sharedTestOrm, + createTestDaysRepo, createTestDayAssignmentsRepo, createTestDayNotesRepo, createTestTripsRepo, +} from '../../helpers/test-uow'; // Real sibling services over the same in-memory DB — updateTrip's date-shift // resyncs and the summary/bundle aggregation run their actual SQL. @@ -133,7 +136,17 @@ let readModelSvc: TripReadModelService; beforeAll(async () => { dbsEm = (await sharedTestOrm(testDb)).em; budgetSvc = new BudgetService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(dbs().connection)); - daysSvc = new DaysService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), new QueryHelpersService(dbs()), await createTestUnitOfWork(dbs().connection)); + daysSvc = new DaysService( + dbs(), + new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), + new RealtimeService(), + new QueryHelpersService(dbs()), + await createTestUnitOfWork(dbs().connection), + await createTestDaysRepo(dbs().connection), + await createTestDayAssignmentsRepo(dbs().connection), + await createTestDayNotesRepo(dbs().connection), + await createTestTripsRepo(dbs().connection), + ); placesSvc = new PlacesService( dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)),