mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-21 14:36:31 +00:00
refactor(trip-members): membership, guests and the trip read model through repositories
This commit is contained in:
@@ -266,3 +266,26 @@ export function maxOf(platform: Platform, ref: string, aliasName: string): RawQu
|
||||
return unsupported(platform);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Plan 3c Task 6 — `TripMembersRepository.listWithUserAndInviter`
|
||||
// (`trip-members.service.ts:117`, TM2): the `role` column, a `CASE WHEN`
|
||||
// comparing the joined user's id against the trip's owner id.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* `CASE WHEN <col> = ? THEN ? ELSE ? END`, for use as a `.select()` value
|
||||
* (chain `.as(aliasName)` at the call site, the same way `coalesce()`'s
|
||||
* callers do — this file does not bake the alias into the SQL text itself
|
||||
* for this one, unlike `countAll`/`minOf`/`maxOf`, because `.as()` already
|
||||
* covers it and a `value`-bearing fragment cannot reuse `alias()`'s bare
|
||||
* identifier validation the same way an aggregate's literal alias does).
|
||||
* `value`/`whenTrue`/`whenFalse` are all bound as parameters, never
|
||||
* interpolated — even though today's only caller (TM2's owner/member role
|
||||
* label) passes fixed literal strings, parity is by VALUE, not by whether
|
||||
* the string happens to be a source-code literal at the call site.
|
||||
*/
|
||||
export function caseWhenEquals(platform: Platform, ref: string, value: number, whenTrue: string, whenFalse: string): RawQueryFragment {
|
||||
if (platform instanceof SqlitePlatform) return raw(`CASE WHEN ${column(ref)} = ? THEN ? ELSE ? END`, [value, whenTrue, whenFalse]);
|
||||
return unsupported(platform);
|
||||
}
|
||||
|
||||
|
||||
@@ -1,7 +1,25 @@
|
||||
import type { TripMembers } from '../entities/TripMembers.entity';
|
||||
import { Trips } from '../entities/Trips.entity';
|
||||
import { caseWhenEquals, coalesce } from '../dialect/sql-functions';
|
||||
import { TrekRepository } from './_shared/trek-repository';
|
||||
|
||||
/**
|
||||
* One row of `TripMembersService.listMembers` (`trip-members.service.ts:117-125`,
|
||||
* TM2). `is_guest` stays the raw SQLite integer here — the `!!m.is_guest`
|
||||
* boolean coercion is a service-layer default (D4), same split every other
|
||||
* repository in this program keeps.
|
||||
*/
|
||||
export interface MemberWithUserAndInviterRow {
|
||||
id: number;
|
||||
username: string;
|
||||
email: string;
|
||||
avatar: string | null;
|
||||
is_guest: number;
|
||||
role: string;
|
||||
added_at: string | null;
|
||||
invited_by_username: string | null;
|
||||
}
|
||||
|
||||
export class TripMembersRepository extends TrekRepository<TripMembers> {
|
||||
/**
|
||||
* The user ids a trip may refer to: its members plus the owner. Byte-for-byte
|
||||
@@ -61,26 +79,182 @@ export class TripMembersRepository extends TrekRepository<TripMembers> {
|
||||
|
||||
/**
|
||||
* `SELECT id FROM trip_members WHERE trip_id = ? AND user_id = ?`
|
||||
* (`trip-membership.service.ts:70`, TB5) — `joinTripAsMember`'s
|
||||
* already-a-member check, the second leg of the non-transactional
|
||||
* check-then-act sequence (§18.4: unchanged by this conversion). Both
|
||||
* callers (`TripMembershipService.joinTripAsMember`) always pass real
|
||||
* `number`s, so no raw-bind seam here.
|
||||
* (`trip-membership.service.ts:70`, TB5 — `joinTripAsMember`'s
|
||||
* already-a-member check, one leg of a non-transactional check-then-act
|
||||
* sequence, §18.4: unchanged by this conversion; also `trip-members
|
||||
* .service.ts:151`, TM5 — `addMember`'s own "already has access" check,
|
||||
* the SAME non-transactional shape against TM6's insert). `TripMembershipService
|
||||
* .joinTripAsMember`'s `tripId` is always a real `number`; `TripMembersService
|
||||
* .addMember`'s is still the route's unconverted `tripId`
|
||||
* (`string | number`) — raw-bind (D4's T5 escape hatch, the same seam
|
||||
* `TripsRepository.findAccessible` documents) covers both without forking
|
||||
* the statement into two methods.
|
||||
*/
|
||||
async exists(trip_id: number, user_id: number): Promise<boolean> {
|
||||
async exists(trip_id: number | string, user_id: number): Promise<boolean> {
|
||||
const row = await this.qb('m')
|
||||
.select(['m.id'])
|
||||
.where({ trip: trip_id, user: user_id })
|
||||
.where('m.trip_id = ?', [trip_id])
|
||||
.andWhere({ user: user_id })
|
||||
.execute<{ id: number } | undefined>('get', false);
|
||||
return !!row;
|
||||
}
|
||||
|
||||
/**
|
||||
* `INSERT INTO trip_members (trip_id, user_id, invited_by) VALUES (?, ?, ?)`
|
||||
* (`trip-membership.service.ts:72`, TB6) — the third leg of the same
|
||||
* non-transactional sequence as `exists` above; `invitedBy` may be `null`.
|
||||
* (`trip-membership.service.ts:72`, TB6 — the third leg of the same
|
||||
* non-transactional sequence as `exists` above; also `trip-members
|
||||
* .service.ts:155`, TM6 — `addMember`'s own insert, and `:227`, TM17 —
|
||||
* `createGuest`'s membership row for the new guest, inside its own
|
||||
* transaction). `invitedBy` may be `null`. `trip_id: number`, not
|
||||
* `number | string`: unlike `exists`'s WHERE-comparison shape, `insert`'s
|
||||
* typed `EntityData` requires the relation's real `Primary<Trips>` type.
|
||||
* This is still exact parity for `TripMembersService`'s two still-string
|
||||
* `tripId` callers (TM6/TM17): by the time either call happens, the
|
||||
* route's `tripId` has already matched a real trip through one of the
|
||||
* four raw-bind access primitives earlier in the SAME request (the
|
||||
* controller's inline `canAccessTrip` check for `addMember`;
|
||||
* `TripOwnerGuard`'s `isOwner`/`findAccessible` for `createGuest`'s
|
||||
* owner-only route) — which only succeeds for a string SQLite's own
|
||||
* affinity rules already recognise as numeric, and `Number(...)` of
|
||||
* exactly that class of string always agrees with SQLite's own
|
||||
* numeric-affinity conversion. The two call sites document this and pass
|
||||
* `Number(tripId)`, not a widened raw bind.
|
||||
*/
|
||||
async addMember(trip_id: number, user_id: number, invitedBy: number | null): Promise<void> {
|
||||
await this.insert({ trip: trip_id, user: user_id, invited_by: invitedBy });
|
||||
}
|
||||
|
||||
/**
|
||||
* `SELECT u.id, COALESCE(u.display_name, u.username) AS username, u.email,
|
||||
* u.avatar, u.is_guest, CASE WHEN u.id = ? THEN 'owner' ELSE 'member' END
|
||||
* as role, m.added_at, COALESCE(ib.display_name, ib.username) as
|
||||
* invited_by_username FROM trip_members m JOIN users u ON u.id = m.user_id
|
||||
* LEFT JOIN users ib ON ib.id = m.invited_by WHERE m.trip_id = ?
|
||||
* ORDER BY m.added_at ASC` (`trip-members.service.ts:117-125`, TM2) —
|
||||
* the self-join on `users` (once as `u` for the member, once as `ib` for
|
||||
* the inviter) via `TripMembers.entity.ts`'s `invitedByRef` relation,
|
||||
* which exists precisely for this join. Every joined-alias column is
|
||||
* selected with an explicit inline `as <name>` (the `PlaceRatingsRepository
|
||||
* .listForPlaces`/`AssignmentParticipantsRepository.insertIgnore`
|
||||
* precedent — rule 19's trap: selecting a joined alias's bare `.id`
|
||||
* without an explicit alias risks MikroORM's own `<alias>__<col>` prefix
|
||||
* for a relation-property select, verified NOT to reproduce the legacy's
|
||||
* bare `id`/`email`/`avatar`/`is_guest`/`added_at` keys otherwise), never
|
||||
* left to MikroORM's own column-collision aliasing.
|
||||
*/
|
||||
async listWithUserAndInviter(trip_id: number | string, owner_id: number): Promise<MemberWithUserAndInviterRow[]> {
|
||||
const platform = this.getEntityManager().getPlatform();
|
||||
return this.qb('m')
|
||||
.join('m.user', 'u')
|
||||
.leftJoin('m.invitedByRef', 'ib')
|
||||
.select([
|
||||
'u.id as id',
|
||||
coalesce(platform, 'u.display_name', 'u.username').as('username'),
|
||||
'u.email as email',
|
||||
'u.avatar as avatar',
|
||||
'u.is_guest as is_guest',
|
||||
caseWhenEquals(platform, 'u.id', owner_id, 'owner', 'member').as('role'),
|
||||
'm.added_at as added_at',
|
||||
coalesce(platform, 'ib.display_name', 'ib.username').as('invited_by_username'),
|
||||
])
|
||||
.where('m.trip_id = ?', [trip_id])
|
||||
.orderBy({ 'm.added_at': 'asc' })
|
||||
.execute<MemberWithUserAndInviterRow[]>('all', false);
|
||||
}
|
||||
|
||||
/**
|
||||
* `SELECT u.id FROM users u JOIN trip_members m ON m.user_id = u.id
|
||||
* WHERE u.id = ? AND m.trip_id = ? AND u.is_guest = 1`
|
||||
* (`trip-members.service.ts:235-237`, TM18) — security-sensitive: the
|
||||
* scoping guard that keeps guest mutations trip-local (#1362). Rooted on
|
||||
* `TripMembers` (this repository's entity) rather than `Users`: the
|
||||
* legacy statement's `users u JOIN trip_members m` becomes `trip_members m
|
||||
* JOIN users u` here, the same join and the same result set, only rooted
|
||||
* the other way because that is this repository's entity.
|
||||
*
|
||||
* Raw-bind on `user_id` (not `.andWhere({ user: user_id })`) plus a
|
||||
* `Number.isFinite` short-circuit guard — the same reason `remove()` below
|
||||
* has both: `renameGuest`/`deleteGuest`'s callers still do a bare
|
||||
* `Number.parseInt(userId)` on the route's `:userId`, unvalidated — a
|
||||
* non-numeric id parses to `NaN`, and MikroORM/Kysely inline a non-finite
|
||||
* `number` as the literal SQL token `NaN` even through a raw `?`
|
||||
* placeholder (`no such column: NaN`, a 500 at PREPARE time — verified
|
||||
* directly against a compiled boot, not assumed; a typed filter has the
|
||||
* identical failure). The legacy statement bound it as a genuine
|
||||
* better-sqlite3 parameter, matching zero rows (404 `'Guest not found'`,
|
||||
* not a crash) — only skipping the query for a non-finite `user_id`
|
||||
* reproduces that byte for byte.
|
||||
*/
|
||||
async isGuestOfTrip(trip_id: number | string, user_id: number): Promise<boolean> {
|
||||
if (!Number.isFinite(user_id)) return false;
|
||||
const row = await this.qb('m')
|
||||
.join('m.user', 'u')
|
||||
.select(['u.id as id'])
|
||||
.where('m.trip_id = ?', [trip_id])
|
||||
.andWhere('m.user_id = ?', [user_id])
|
||||
.andWhere({ 'u.is_guest': 1 })
|
||||
.execute<{ id: number } | undefined>('get', false);
|
||||
return !!row;
|
||||
}
|
||||
|
||||
/**
|
||||
* `INSERT OR IGNORE INTO trip_members (trip_id, user_id, invited_by)
|
||||
* VALUES (?, ?, ?)` (`trip-members.service.ts:200`, TM15) —
|
||||
* security-sensitive: `transferOwnership`'s third statement, giving the
|
||||
* former owner membership back. `upsert` with `onConflictAction: 'ignore'`
|
||||
* on the table's real unique key, named by entity property (`trip`/`user`,
|
||||
* `AssignmentParticipantsRepository.insertIgnore`'s precedent for a
|
||||
* relation-backed `onConflictFields` list) — `UNIQUE(trip_id, user_id)`
|
||||
* per `TripMembersSchema`'s own `uniques` declaration, not assumed.
|
||||
* `trip_id: number`, the same reasoning as `addMember`'s docstring above
|
||||
* (an `upsert` is INSERT-shaped; the caller passes `Number(tripId)`,
|
||||
* justified there — `transferOwnership`'s route is `TripOwnerGuard`-gated,
|
||||
* so `tripId` has already matched a real trip through the raw-bind
|
||||
* `isOwner`/`findAccessible` primitive earlier in the same request).
|
||||
*/
|
||||
async addIgnoringConflict(trip_id: number, user_id: number, invitedBy: number | null): Promise<void> {
|
||||
// `invitedByRef`, not `invited_by`: the latter is a `persist(false)` mirror
|
||||
// column (like `trip_id`/`user_id`) that `upsert` — unlike `insert` above,
|
||||
// per `addMember`'s test — silently drops when building its INSERT.
|
||||
// `invitedByRef` is the real, joined-column-backed relation property.
|
||||
await this.upsert(
|
||||
{ trip: trip_id, user: user_id, invitedByRef: invitedBy },
|
||||
{ onConflictFields: ['trip', 'user'], onConflictAction: 'ignore' },
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* `DELETE FROM trip_members WHERE trip_id = ? AND user_id = ?`
|
||||
* (`trip-members.service.ts:166`, TM8 `removeMember`; `:198`, TM14
|
||||
* `transferOwnership`'s "no longer a plain member" step —
|
||||
* security-sensitive). Raw-bind on BOTH `trip_id` and `user_id` — not just
|
||||
* `trip_id` (D4's T5 escape hatch, the same `number | string` seam
|
||||
* `rosterUserIds`/`listUserIdsByTrip` above preserve): `TripMembersController
|
||||
* .removeMember` still does a bare `Number.parseInt(userId)` on the route's
|
||||
* `:userId` before calling this, unvalidated (rule 15's exact trap — a
|
||||
* non-numeric id parses to `NaN`).
|
||||
*
|
||||
* **`Number.isFinite` guard, checked directly against a compiled boot, not
|
||||
* assumed:** a typed `.andWhere({ user: user_id })` filter renders `NaN` as
|
||||
* a literal, unquoted SQL token (`WHERE … AND user_id = NaN`), which
|
||||
* SQLite's parser reads as a column reference and rejects at PREPARE time
|
||||
* (`no such column: NaN`, a 500). Switching to a raw `m.user_id = ?`
|
||||
* placeholder does NOT fix this — MikroORM/Kysely inline a non-finite
|
||||
* `number` as that same literal token even through a `?` placeholder's
|
||||
* bound-params array (reproduced directly: still `no such column: NaN` at
|
||||
* prepare time, before any bind happens). The legacy statement bound
|
||||
* `targetUserId` (`NaN`) straight into a real better-sqlite3 parameter,
|
||||
* which silently matches zero rows and returns 200 `{ success: true }` —
|
||||
* only skipping the query entirely for a non-finite `user_id` reproduces
|
||||
* that, byte for byte, since neither the typed filter nor the raw
|
||||
* placeholder route can pass `NaN` through the ORM layer intact.
|
||||
*/
|
||||
async remove(trip_id: number | string, user_id: number): Promise<void> {
|
||||
if (!Number.isFinite(user_id)) return;
|
||||
await this.qb('m')
|
||||
.delete()
|
||||
.where('m.trip_id = ?', [trip_id])
|
||||
.andWhere('m.user_id = ?', [user_id])
|
||||
.execute('run');
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import type { Trips } from '../entities/Trips.entity';
|
||||
import type { AssertRowKeys } from './_shared/rows';
|
||||
import { TrekRepository } from './_shared/trek-repository';
|
||||
|
||||
/** What a trip-scoped request learns about the trip once access is verified. */
|
||||
@@ -8,6 +9,32 @@ export interface TripAccess {
|
||||
currency: string | null;
|
||||
}
|
||||
|
||||
/**
|
||||
* `SELECT * FROM trips WHERE id = ?`, every scalar column — the row
|
||||
* `TripReadModelService.getTripSummary` (TR-B) and, once Task 7 lands,
|
||||
* `TripsService.getRaw` (TP22) both need, `feed_token` included: the
|
||||
* `withoutFeedToken()` JS strip that guards the credential stays in the
|
||||
* SERVICE (TR-B's ruling), so this repository method must hand the column
|
||||
* back intact for that strip to have something to delete.
|
||||
*/
|
||||
export interface TripRawRow {
|
||||
id: number;
|
||||
user_id: number;
|
||||
title: string;
|
||||
description: string | null;
|
||||
start_date: string | null;
|
||||
end_date: string | null;
|
||||
currency: string | null;
|
||||
cover_image: string | null;
|
||||
is_archived: number | null;
|
||||
reminder_days: number | null;
|
||||
feed_token: string | null;
|
||||
created_at: string | null;
|
||||
updated_at: string | null;
|
||||
}
|
||||
|
||||
const _tripRawRowKeys: AssertRowKeys<TripRawRow, Trips> = true;
|
||||
|
||||
export class TripsRepository extends TrekRepository<Trips> {
|
||||
/**
|
||||
* The `LEFT JOIN trip_members m ON m.trip_id = t.id AND m.user_id = ?
|
||||
@@ -132,4 +159,69 @@ export class TripsRepository extends TrekRepository<Trips> {
|
||||
.execute<{ id: number }[]>('all', false);
|
||||
return rows.map((r) => r.id);
|
||||
}
|
||||
|
||||
/**
|
||||
* `SELECT title FROM trips WHERE id = ?` (`trip-members.service.ts:156`,
|
||||
* TM7) — `addMember`'s notification-body read; the caller applies its own
|
||||
* `?? 'Untitled'` fallback (D4). Raw-bind (D4's T5 escape hatch), the same
|
||||
* `number | string` seam `findAccessible` documents.
|
||||
*/
|
||||
async getTitle(trip_id: number | string): Promise<string | null> {
|
||||
const row = await this.qb('t')
|
||||
.select(['t.title'])
|
||||
.where('t.id = ?', [trip_id])
|
||||
.execute<{ title: string } | undefined>('get', false);
|
||||
return row?.title ?? null;
|
||||
}
|
||||
|
||||
/**
|
||||
* `SELECT id, title, user_id FROM trips WHERE id = ?`
|
||||
* (`trip-members.service.ts:180`, TM9) — `transferOwnership`'s first
|
||||
* check-then-act read (miss → 404 `'Trip not found'`; wrong owner → 400
|
||||
* `'Only the owner can transfer ownership'`, both decided by the caller).
|
||||
* Raw-bind, the same seam as `getTitle` above.
|
||||
*/
|
||||
async findIdTitleOwner(trip_id: number | string): Promise<{ id: number; title: string; user_id: number } | undefined> {
|
||||
return this.qb('t')
|
||||
.select(['t.id', 't.title', 't.user'])
|
||||
.where('t.id = ?', [trip_id])
|
||||
.execute<{ id: number; title: string; user_id: number } | undefined>('get', false);
|
||||
}
|
||||
|
||||
/**
|
||||
* `UPDATE trips SET user_id = ? WHERE id = ?` (`trip-members.service.ts:196`,
|
||||
* TM13) — security-sensitive: the ownership handover itself, the first of
|
||||
* `transferOwnership`'s three transactional statements. `qb().update()`
|
||||
* rather than `nativeUpdate`: `nativeUpdate`'s typed `FilterQuery` rejects
|
||||
* a `string` against `id`'s branded `number` type the same way `find`'s
|
||||
* does, and `trip_id` here is still the route's unconverted string.
|
||||
*/
|
||||
async setOwner(trip_id: number | string, user_id: number): Promise<void> {
|
||||
await this.qb('t')
|
||||
.update({ user: user_id })
|
||||
.where('t.id = ?', [trip_id])
|
||||
.execute('run');
|
||||
}
|
||||
|
||||
/**
|
||||
* `SELECT * FROM trips WHERE id = ?` — `TripReadModelService.getTripSummary`'s
|
||||
* trip read (`trip-read-model.service.ts:52`, TR-B) and, once Task 7 lands,
|
||||
* `TripsService.getRaw` (TP22, `trips.service.ts:389`) — one method, two
|
||||
* sites, per the inventory's own note that they are the identical
|
||||
* statement. `feed_token` comes back intact; `withoutFeedToken()` stays a
|
||||
* JS strip in each CALLER (TR-B's ruling — this repository never blanks
|
||||
* it itself, unlike `TRIP_SELECT`'s `NULL AS feed_token` SQL-side trick).
|
||||
* `.select(['t.*'])` — MikroORM's QueryBuilder recognises the bare
|
||||
* `<alias>.*` marker (verified against `QueryBuilder.js`'s own
|
||||
* `prepareFields`, not assumed) and emits every column unprefixed, the
|
||||
* same `SELECT t.*` shape the legacy statement used. Raw-bind, the same
|
||||
* `number | string` seam `getTitle`/`findIdTitleOwner` above preserve.
|
||||
*/
|
||||
async findRaw(trip_id: number | string): Promise<TripRawRow | null> {
|
||||
const row = await this.qb('t')
|
||||
.select(['t.*'])
|
||||
.where('t.id = ?', [trip_id])
|
||||
.execute<TripRawRow | undefined>('get', false);
|
||||
return row ?? null;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { Users } from '../entities/Users.entity';
|
||||
import { toRow, type AssertRowKeys } from './_shared/rows';
|
||||
import { columnIncrementedBy, currentTimestamp, lower, lowerParam } from '../dialect/sql-functions';
|
||||
import { coalesce, columnIncrementedBy, currentTimestamp, lower, lowerParam } from '../dialect/sql-functions';
|
||||
import { TrekRepository } from './_shared/trek-repository';
|
||||
|
||||
/**
|
||||
@@ -78,6 +78,22 @@ export interface NewUserRow {
|
||||
avatar?: string | null;
|
||||
}
|
||||
|
||||
/**
|
||||
* `TripMembersService.createGuest` (TM16, `trip-members.service.ts:222-224`) —
|
||||
* a credential-less guest account (#1362). Additive, NOT a widening of
|
||||
* `NewUserRow` above (Plan 3b Task 1's shape, covering only the AU10/O14
|
||||
* real-account column sets): this method's `password_hash`/`role`/`is_guest`
|
||||
* are fixed literals the caller can never override, which is the whole
|
||||
* point — a second, narrower method keeps the "credential-less account"
|
||||
* invariant IN the method rather than in caller discipline (Task 6 brief's
|
||||
* explicit ruling).
|
||||
*/
|
||||
export interface NewGuestUserRow {
|
||||
username: string;
|
||||
email: string;
|
||||
display_name: string;
|
||||
}
|
||||
|
||||
/** `UserProfileService.updateApiKeys` (UP3) — the four encrypted key columns, already resolved by the service. */
|
||||
export interface ApiKeysPatch {
|
||||
maps_api_key: string | null;
|
||||
@@ -921,6 +937,120 @@ export class UsersRepository extends TrekRepository<Users> {
|
||||
return row ? { role: row.role, openweather_api_key: row.openweather_api_key ?? null } : null;
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------
|
||||
// TM3 — TripMembersService.listMembers's owner row
|
||||
// ---------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* `SELECT id, COALESCE(display_name, username) AS username, email, avatar
|
||||
* FROM users WHERE id = ?` (`trip-members.service.ts:129`, TM3). The
|
||||
* legacy statement's own docstring records this COALESCE as a quirk fix on
|
||||
* top of the 1:1 move (the ORIGINAL raw statement read the bare
|
||||
* `username`) — kept exactly as the code already stands, not re-litigated.
|
||||
*/
|
||||
async findOwnerSummary(id: number): Promise<{ id: number; username: string; email: string; avatar: string | null } | null> {
|
||||
const platform = this.getEntityManager().getPlatform();
|
||||
const row = await this.qb('u')
|
||||
.select(['u.id', coalesce(platform, 'u.display_name', 'u.username').as('username'), 'u.email', 'u.avatar'])
|
||||
.where({ id })
|
||||
.execute<{ id: number; username: string; email: string; avatar: string | null } | undefined>('get', false);
|
||||
return row ? { id: row.id, username: row.username, email: row.email, avatar: row.avatar ?? null } : null;
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------
|
||||
// TM4 — TripMembersService.addMember's invite-target lookup
|
||||
// ---------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* `SELECT id, username, email, avatar FROM users WHERE (email = ? OR
|
||||
* username = ?) AND COALESCE(is_guest, 0) = 0` (`trip-members.service.ts:142-144`,
|
||||
* TM4) — case-SENSITIVE (unlike `findByEmailCI`/`findIdByEmailCI` above;
|
||||
* kept exactly as written, program rule 18 only applies where the legacy
|
||||
* statement itself folds case), guests excluded so a trip-scoped guest can
|
||||
* never be resolved (and re-attached to another trip) through the invite
|
||||
* box. `identifier` is bound to both sides — the caller passes the SAME
|
||||
* already-trimmed string for both the email and username branch, matching
|
||||
* the legacy statement's `identifier.trim()` bound twice. `is_guest: 0` is
|
||||
* exact parity for `COALESCE(is_guest, 0) = 0` (`countNonGuest`'s
|
||||
* docstring above: the column is `NOT NULL DEFAULT 0`, so no row can store
|
||||
* `NULL` there).
|
||||
*/
|
||||
async findInvitableByEmailOrUsername(identifier: string): Promise<{ id: number; username: string; email: string; avatar: string | null } | null> {
|
||||
const row = await this.findOne(
|
||||
{ $or: [{ email: identifier }, { username: identifier }], is_guest: 0 },
|
||||
{ fields: ['id', 'username', 'email', 'avatar'] },
|
||||
);
|
||||
return row ? { id: row.id, username: row.username, email: row.email, avatar: row.avatar ?? null } : null;
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------
|
||||
// TM10 — TripMembersService.transferOwnership's new-owner guest check
|
||||
// ---------------------------------------------------------------------
|
||||
|
||||
/** `SELECT id, email, is_guest FROM users WHERE id = ?` (`trip-members.service.ts:185`, TM10) — guest → 400 `'Cannot transfer ownership to a guest'` (caller's decision). */
|
||||
async findIdEmailGuest(id: number): Promise<{ id: number; email: string; is_guest: number } | null> {
|
||||
const row = await this.findOne({ id }, { fields: ['id', 'email', 'is_guest'] });
|
||||
return row ? { id: row.id, email: row.email, is_guest: row.is_guest } : null;
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------
|
||||
// TM16 — TripMembersService.createGuest (Plan 3c Task 6, security-sensitive)
|
||||
// ---------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* `INSERT INTO users (username, email, password_hash, role, is_guest,
|
||||
* display_name) VALUES (?, ?, '', 'user', 1, ?)` (`trip-members.service.ts:222-224`,
|
||||
* TM16) — security-sensitive: creates a credential-less account (#1362).
|
||||
* `password_hash`/`role`/`is_guest` are fixed literals, never a caller
|
||||
* input (see `NewGuestUserRow`'s docstring above for why this is its own
|
||||
* method rather than a widened `insertUser`). Returns the generated id
|
||||
* only — the legacy caller reads `res.lastInsertRowid` and never re-selects
|
||||
* the row.
|
||||
*/
|
||||
async insertGuest(row: NewGuestUserRow): Promise<number> {
|
||||
return this.insert({
|
||||
username: row.username,
|
||||
email: row.email,
|
||||
password_hash: '',
|
||||
role: 'user',
|
||||
is_guest: 1,
|
||||
display_name: row.display_name,
|
||||
});
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------
|
||||
// TM19 — TripMembersService.renameGuest (security-sensitive)
|
||||
// ---------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* `UPDATE users SET display_name = ?, updated_at = CURRENT_TIMESTAMP WHERE
|
||||
* id = ? AND is_guest = 1` (`trip-members.service.ts:248`, TM19) —
|
||||
* security-sensitive: the `AND is_guest = 1` is a SECOND guard (belt and
|
||||
* braces) on top of `TripMembersRepository.isGuestOfTrip`'s trip-scoping
|
||||
* check that runs before this is ever called (#1362) — pinned by a test
|
||||
* that this method, called directly on a real non-guest id, updates the
|
||||
* row count of exactly zero rows.
|
||||
*/
|
||||
async renameGuest(id: number, displayName: string): Promise<void> {
|
||||
const platform = this.getEntityManager().getPlatform();
|
||||
await this.nativeUpdate({ id, is_guest: 1 }, { display_name: displayName, updated_at: currentTimestamp(platform) });
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------
|
||||
// TM20 — TripMembersService.deleteGuest (security-sensitive: account erasure)
|
||||
// ---------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* `DELETE FROM users WHERE id = ? AND is_guest = 1` (`trip-members.service.ts:268`,
|
||||
* TM20) — security-sensitive: account erasure, the same belt-and-braces
|
||||
* `is_guest = 1` predicate as `renameGuest` above. Cascades `trip_members`
|
||||
* and every assignment/budget/packing join via the entity's FK delete
|
||||
* rules, unchanged by this conversion.
|
||||
*/
|
||||
async deleteGuest(id: number): Promise<void> {
|
||||
await this.nativeDelete({ id, is_guest: 1 });
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------
|
||||
// UC11 — account erasure (Plan 3b Task 1 review, "contract gaps" item 2:
|
||||
// named in the plan's Task 1 bullet, deliberately deferred to Task 5 —
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { Module } from '@nestjs/common';
|
||||
import { MikroOrmModule } from '@mikro-orm/nestjs';
|
||||
import { TripMembersController } from './trip-members.controller';
|
||||
import { TripMembersService } from './trip-members.service';
|
||||
import { DatabaseModule } from '../database/database.module';
|
||||
@@ -8,6 +9,9 @@ import { AuditModule } from '../audit/audit.module';
|
||||
import { AuthModule } from '../auth/auth.module';
|
||||
import { BudgetModule } from '../budget/budget.module';
|
||||
import { NotificationsModule } from '../notifications/notifications.module';
|
||||
import { Trips } from '../../db/entities/Trips.entity';
|
||||
import { TripMembers } from '../../db/entities/TripMembers.entity';
|
||||
import { Users } from '../../db/entities/Users.entity';
|
||||
|
||||
/**
|
||||
* The member/guest roster of a trip.
|
||||
@@ -17,9 +21,18 @@ import { NotificationsModule } from '../notifications/notifications.module';
|
||||
* imports it back except trips. That is what keeps it legal — trip-membership/
|
||||
* next door cannot do the same, because AuthModule imports *that* one, so giving
|
||||
* it these dependencies would close the cycle.
|
||||
*
|
||||
* `MikroOrmModule.forFeature([Trips, TripMembers, Users])` (Plan 3c Task 6):
|
||||
* `DatabaseModule` stays too — `TripMembersService` still holds one documented
|
||||
* raw-SQL survivor (`getTripForViewer`, pending Task 7) and the still-`DatabaseService`
|
||||
* -routed `canAccessTrip` delegation, the same carve-out `DaysService`/every other
|
||||
* converted domain in this program keeps.
|
||||
*/
|
||||
@Module({
|
||||
imports: [NotificationsModule, DatabaseModule, PermissionsModule, RealtimeModule, AuditModule, AuthModule, BudgetModule],
|
||||
imports: [
|
||||
NotificationsModule, DatabaseModule, PermissionsModule, RealtimeModule, AuditModule, AuthModule, BudgetModule,
|
||||
MikroOrmModule.forFeature([Trips, TripMembers, Users]),
|
||||
],
|
||||
controllers: [TripMembersController],
|
||||
providers: [TripMembersService],
|
||||
exports: [TripMembersService],
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { Injectable } from '@nestjs/common';
|
||||
import { randomUUID } from 'crypto';
|
||||
import { InjectRepository } from '@mikro-orm/nestjs';
|
||||
import { DatabaseService } from '../database/database.service';
|
||||
import type { User } from '../../types';
|
||||
import { avatarUrl } from '../common/avatarUrl';
|
||||
@@ -13,6 +14,12 @@ import type { TrekWsPayload, TrekWsTripEventName } from '@trek/shared';
|
||||
import { emitUserDeleted } from '../../plugin-user-lifecycle';
|
||||
import { NotFoundError, ValidationError } from '../common/domain-errors';
|
||||
import { NotificationsService } from '../notifications/notifications.service';
|
||||
import { Trips } from '../../db/entities/Trips.entity';
|
||||
import type { TripsRepository } from '../../db/repositories/Trips.repository';
|
||||
import { TripMembers } from '../../db/entities/TripMembers.entity';
|
||||
import type { TripMembersRepository } from '../../db/repositories/TripMembers.repository';
|
||||
import { Users } from '../../db/entities/Users.entity';
|
||||
import type { UsersRepository } from '../../db/repositories/Users.repository';
|
||||
|
||||
export interface AddMemberResult {
|
||||
member: { id: number; username: string; email: string; avatar?: string | null; role: string; avatar_url: string | null };
|
||||
@@ -68,8 +75,19 @@ export class TripMembersService {
|
||||
private readonly realtime: RealtimeService,
|
||||
private readonly notifications: NotificationsService,
|
||||
private readonly uow: UnitOfWork,
|
||||
@InjectRepository(Trips) private readonly tripsRepo: TripsRepository,
|
||||
@InjectRepository(TripMembers) private readonly tripMembersRepo: TripMembersRepository,
|
||||
@InjectRepository(Users) private readonly usersRepo: UsersRepository,
|
||||
) {}
|
||||
|
||||
/**
|
||||
* Task 7: `TripsService` still owns `TRIP_SELECT` and its own raw
|
||||
* `this.dbs.connection` handle (`trips.service.ts:166-168`) — `getTripForViewer`
|
||||
* below is this service's one documented raw-SQL survivor, importing the
|
||||
* projection rather than copying it, unchanged until Task 7 converts
|
||||
* `TripsService` itself and this call points at a repository method
|
||||
* instead (`TripsRepository.findForViewer`, per the inventory's proposal).
|
||||
*/
|
||||
private get db() {
|
||||
return this.dbs.connection;
|
||||
}
|
||||
@@ -88,7 +106,8 @@ export class TripMembersService {
|
||||
}
|
||||
|
||||
/** The trip in list shape, for the re-read a handover broadcasts. Same query the
|
||||
* trip routes use, imported rather than copied so the two cannot drift. */
|
||||
* trip routes use, imported rather than copied so the two cannot drift.
|
||||
* // Task 7: converts once TripsService itself does — see the class docstring above. */
|
||||
async getTripForViewer(tripId: string | number, userId: number) {
|
||||
return this.db.prepare(`${TRIP_SELECT} WHERE t.id = :tripId`).get({ userId, tripId });
|
||||
}
|
||||
@@ -113,24 +132,14 @@ export class TripMembersService {
|
||||
async listMembers(tripId: string | number, tripOwnerId: number) {
|
||||
// u.is_guest rides along (#1362) so guests stay assignable everywhere a member is,
|
||||
// while the UI can badge them and suppress owner-only actions. The owner is never a guest.
|
||||
const members = this.db.prepare(`
|
||||
SELECT u.id, COALESCE(u.display_name, u.username) AS username, u.email, u.avatar, u.is_guest,
|
||||
CASE WHEN u.id = ? THEN 'owner' ELSE 'member' END as role,
|
||||
m.added_at,
|
||||
COALESCE(ib.display_name, ib.username) as invited_by_username
|
||||
FROM trip_members m
|
||||
JOIN users u ON u.id = m.user_id
|
||||
LEFT JOIN users ib ON ib.id = m.invited_by
|
||||
WHERE m.trip_id = ?
|
||||
ORDER BY m.added_at ASC
|
||||
`).all(tripOwnerId, tripId) as { id: number; username: string; email: string; avatar: string | null; is_guest: number; role: string; added_at: string; invited_by_username: string | null }[];
|
||||
const members = await this.tripMembersRepo.listWithUserAndInviter(tripId, tripOwnerId);
|
||||
|
||||
// Quirk fix on top of the 1:1 move: the owner row prefers display_name like
|
||||
// every member row does (the legacy query read the raw username only).
|
||||
const owner = this.db.prepare('SELECT id, COALESCE(display_name, username) AS username, email, avatar FROM users WHERE id = ?').get(tripOwnerId) as Pick<User, 'id' | 'username' | 'email' | 'avatar'>;
|
||||
const owner = await this.usersRepo.findOwnerSummary(tripOwnerId);
|
||||
|
||||
return {
|
||||
owner: { ...owner, role: 'owner', is_guest: false, avatar_url: avatarUrl(owner) },
|
||||
owner: { ...owner, role: 'owner', is_guest: false, avatar_url: avatarUrl(owner!) },
|
||||
members: members.map(m => ({ ...m, is_guest: !!m.is_guest, avatar_url: avatarUrl(m) })),
|
||||
};
|
||||
}
|
||||
@@ -140,31 +149,34 @@ export class TripMembersService {
|
||||
|
||||
// Guests (#1362) are not invitable accounts — exclude them so a trip-scoped guest
|
||||
// can never be resolved (and re-attached to another trip) through the invite box.
|
||||
const target = this.db.prepare(
|
||||
'SELECT id, username, email, avatar FROM users WHERE (email = ? OR username = ?) AND COALESCE(is_guest, 0) = 0'
|
||||
).get(identifier.trim(), identifier.trim()) as Pick<User, 'id' | 'username' | 'email' | 'avatar'> | undefined;
|
||||
const target = await this.usersRepo.findInvitableByEmailOrUsername(identifier.trim());
|
||||
|
||||
if (!target) throw new NotFoundError('User not found');
|
||||
|
||||
if (target.id === tripOwnerId)
|
||||
throw new ValidationError('Trip owner is already a member');
|
||||
|
||||
const existing = this.db.prepare('SELECT id FROM trip_members WHERE trip_id = ? AND user_id = ?').get(tripId, target.id);
|
||||
const existing = await this.tripMembersRepo.exists(tripId, target.id);
|
||||
if (existing) throw new ValidationError('User already has access');
|
||||
|
||||
this.db.prepare('INSERT INTO trip_members (trip_id, user_id, invited_by) VALUES (?, ?, ?)').run(tripId, target.id, invitedByUserId);
|
||||
// `Number(tripId)`, not the raw string: `addMember`'s INSERT needs the
|
||||
// real `Primary<Trips>` type (its docstring). Safe here — `tripId` has
|
||||
// already matched a real trip through the inline `canAccessTrip` check
|
||||
// above this method's controller call, which only succeeds for a string
|
||||
// SQLite's own affinity rules already recognise as numeric.
|
||||
await this.tripMembersRepo.addMember(Number(tripId), target.id, invitedByUserId);
|
||||
|
||||
const tripInfo = this.db.prepare('SELECT title FROM trips WHERE id = ?').get(tripId) as { title: string } | undefined;
|
||||
const tripTitle = await this.tripsRepo.getTitle(tripId);
|
||||
|
||||
return {
|
||||
member: { ...target, role: 'member', avatar_url: avatarUrl(target) },
|
||||
targetUserId: target.id,
|
||||
tripTitle: tripInfo?.title || 'Untitled',
|
||||
tripTitle: tripTitle || 'Untitled',
|
||||
};
|
||||
}
|
||||
|
||||
async removeMember(tripId: string | number, targetUserId: number): Promise<void> {
|
||||
this.db.prepare('DELETE FROM trip_members WHERE trip_id = ? AND user_id = ?').run(tripId, targetUserId);
|
||||
await this.tripMembersRepo.remove(tripId, targetUserId);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -178,27 +190,31 @@ export class TripMembersService {
|
||||
newOwnerId: number,
|
||||
currentOwnerId: number,
|
||||
): Promise<TransferOwnershipResult> {
|
||||
const trip = this.db.prepare('SELECT id, title, user_id FROM trips WHERE id = ?').get(tripId) as { id: number; title: string; user_id: number } | undefined;
|
||||
const trip = await this.tripsRepo.findIdTitleOwner(tripId);
|
||||
if (!trip) throw new NotFoundError('Trip not found');
|
||||
if (trip.user_id !== currentOwnerId) throw new ValidationError('Only the owner can transfer ownership');
|
||||
if (newOwnerId === currentOwnerId) throw new ValidationError('You already own this trip');
|
||||
|
||||
const newOwner = this.db.prepare('SELECT id, email, is_guest FROM users WHERE id = ?').get(newOwnerId) as { id: number; email: string; is_guest?: number } | undefined;
|
||||
const newOwner = await this.usersRepo.findIdEmailGuest(newOwnerId);
|
||||
if (!newOwner) throw new NotFoundError('User not found');
|
||||
// A guest (#1362) can never log in, so it must never become the owner of a trip.
|
||||
if (newOwner.is_guest) throw new ValidationError('Cannot transfer ownership to a guest');
|
||||
|
||||
const isMember = this.db.prepare('SELECT id FROM trip_members WHERE trip_id = ? AND user_id = ?').get(tripId, newOwnerId);
|
||||
const isMember = await this.tripMembersRepo.exists(tripId, newOwnerId);
|
||||
if (!isMember) throw new ValidationError('New owner must be a trip member');
|
||||
|
||||
const fromEmail = (this.db.prepare('SELECT email FROM users WHERE id = ?').get(currentOwnerId) as { email: string } | undefined)?.email || '';
|
||||
const fromEmail = (await this.usersRepo.getEmail(currentOwnerId)) || '';
|
||||
|
||||
// `Number(tripId)` at the two writes below that need the real
|
||||
// `Primary<Trips>` type (`setOwner`'s docstring / `addIgnoringConflict`'s
|
||||
// docstring) — safe: this route runs behind `TripOwnerGuard`, so `tripId`
|
||||
// has already matched a real trip through its raw-bind `isOwner` check.
|
||||
await this.uow.transactional(async () => {
|
||||
this.db.prepare('UPDATE trips SET user_id = ? WHERE id = ?').run(newOwnerId, tripId);
|
||||
await this.tripsRepo.setOwner(tripId, newOwnerId);
|
||||
// The new owner is no longer a plain member…
|
||||
this.db.prepare('DELETE FROM trip_members WHERE trip_id = ? AND user_id = ?').run(tripId, newOwnerId);
|
||||
await this.tripMembersRepo.remove(tripId, newOwnerId);
|
||||
// …and the former owner keeps access as a member.
|
||||
this.db.prepare('INSERT OR IGNORE INTO trip_members (trip_id, user_id, invited_by) VALUES (?, ?, ?)').run(tripId, currentOwnerId, newOwnerId);
|
||||
await this.tripMembersRepo.addIgnoringConflict(Number(tripId), currentOwnerId, newOwnerId);
|
||||
});
|
||||
|
||||
return { tripTitle: trip.title, fromEmail, toEmail: newOwner.email };
|
||||
@@ -219,12 +235,12 @@ export class TripMembersService {
|
||||
const email = `guest-${randomUUID()}@guests.invalid`;
|
||||
const username = `guest-${randomUUID()}`;
|
||||
|
||||
// `Number(tripId)` for the same reason `addMember`'s docstring gives:
|
||||
// `createGuest`'s route runs behind `TripOwnerGuard`, so `tripId` has
|
||||
// already matched a real trip through its raw-bind `isOwner` check.
|
||||
const guestId = await this.uow.transactional(async () => {
|
||||
const res = this.db.prepare(
|
||||
"INSERT INTO users (username, email, password_hash, role, is_guest, display_name) VALUES (?, ?, '', 'user', 1, ?)"
|
||||
).run(username, email, display);
|
||||
const newGuestId = Number(res.lastInsertRowid);
|
||||
this.db.prepare('INSERT INTO trip_members (trip_id, user_id, invited_by) VALUES (?, ?, ?)').run(tripId, newGuestId, invitedByUserId);
|
||||
const newGuestId = await this.usersRepo.insertGuest({ username, email, display_name: display });
|
||||
await this.tripMembersRepo.addMember(Number(tripId), newGuestId, invitedByUserId);
|
||||
return newGuestId;
|
||||
});
|
||||
|
||||
@@ -233,9 +249,7 @@ export class TripMembersService {
|
||||
|
||||
/** Confirms a user id is a guest of THIS trip, so guest mutations stay trip-scoped. */
|
||||
private async guestOfTrip(tripId: string | number, guestUserId: number): Promise<boolean> {
|
||||
return !!this.db.prepare(
|
||||
'SELECT u.id FROM users u JOIN trip_members m ON m.user_id = u.id WHERE u.id = ? AND m.trip_id = ? AND u.is_guest = 1'
|
||||
).get(guestUserId, tripId);
|
||||
return this.tripMembersRepo.isGuestOfTrip(tripId, guestUserId);
|
||||
}
|
||||
|
||||
async renameGuest(tripId: string | number, guestUserId: number, name: string): Promise<boolean> {
|
||||
@@ -246,7 +260,7 @@ export class TripMembersService {
|
||||
|
||||
// Rename only the display name — no global-uniqueness dedup, so a rename to a name
|
||||
// another trip's guest already uses no longer produces "Name 2" (#1446).
|
||||
this.db.prepare('UPDATE users SET display_name = ?, updated_at = CURRENT_TIMESTAMP WHERE id = ? AND is_guest = 1').run(display, guestUserId);
|
||||
await this.usersRepo.renameGuest(guestUserId, display);
|
||||
return true;
|
||||
}
|
||||
|
||||
@@ -266,7 +280,7 @@ export class TripMembersService {
|
||||
await this.budget.removeUserFromBudgetItems(guestUserId);
|
||||
// Deleting the guest's users row cascades its membership and every assignment join
|
||||
// (trip_members, budget/packing/assignment links) via the ON DELETE foreign keys.
|
||||
this.db.prepare('DELETE FROM users WHERE id = ? AND is_guest = 1').run(guestUserId);
|
||||
await this.usersRepo.deleteGuest(guestUserId);
|
||||
});
|
||||
await emitUserDeleted(guestUserId); // deliver the erasure to any active plugin now
|
||||
return true;
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { Module } from '@nestjs/common';
|
||||
import { MikroOrmModule } from '@mikro-orm/nestjs';
|
||||
import { TripReadModelService } from './trip-read-model.service';
|
||||
import { DatabaseModule } from '../database/database.module';
|
||||
import { Trips } from '../../db/entities/Trips.entity';
|
||||
import { DaysModule } from '../days/days.module';
|
||||
import { AccommodationsModule } from '../accommodations/accommodations.module';
|
||||
import { BudgetModule } from '../budget/budget.module';
|
||||
@@ -13,10 +14,15 @@ import { FilesModule } from '../files/files.module';
|
||||
import { TripMembersModule } from '../trip-members/trip-members.module';
|
||||
|
||||
/** Where the trip read aggregates keep their fan-out, so the write path does not
|
||||
* have to carry it. Nothing imports this except trips. */
|
||||
* have to carry it. Nothing imports this except trips.
|
||||
*
|
||||
* `MikroOrmModule.forFeature([Trips])` replaces `DatabaseModule` (Plan 3c
|
||||
* Task 6): `TripReadModelService` is now SQL-free — its two sites
|
||||
* (`getOwner`/`getTripSummary`'s trip read) both convert to `TripsRepository`
|
||||
* methods, unlike `TripMembersModule`'s carve-out next door. */
|
||||
@Module({
|
||||
imports: [
|
||||
DatabaseModule, TripMembersModule, DaysModule, AccommodationsModule, BudgetModule,
|
||||
MikroOrmModule.forFeature([Trips]), TripMembersModule, DaysModule, AccommodationsModule, BudgetModule,
|
||||
PackingModule, ReservationsModule, CollabModule, PlacesModule, TodoModule, FilesModule,
|
||||
],
|
||||
providers: [TripReadModelService],
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
import { Injectable } from '@nestjs/common';
|
||||
import { DatabaseService } from '../database/database.service';
|
||||
import { InjectRepository } from '@mikro-orm/nestjs';
|
||||
import { Trips } from '../../db/entities/Trips.entity';
|
||||
import type { TripsRepository } from '../../db/repositories/Trips.repository';
|
||||
import { DaysService } from '../days/days.service';
|
||||
import { AccommodationsService } from '../accommodations/accommodations.service';
|
||||
import { BudgetService } from '../budget/budget.service';
|
||||
@@ -24,7 +26,7 @@ import { withoutFeedToken } from '../trips/trips.service';
|
||||
@Injectable()
|
||||
export class TripReadModelService {
|
||||
constructor(
|
||||
private readonly dbs: DatabaseService,
|
||||
@InjectRepository(Trips) private readonly tripsRepo: TripsRepository,
|
||||
private readonly members: TripMembersService,
|
||||
private readonly days: DaysService,
|
||||
private readonly accommodations: AccommodationsService,
|
||||
@@ -37,25 +39,15 @@ export class TripReadModelService {
|
||||
private readonly files: FilesService,
|
||||
) {}
|
||||
|
||||
private get db() {
|
||||
return this.dbs.connection;
|
||||
}
|
||||
|
||||
private async getOwner(tripId: string | number): Promise<{ user_id: number } | undefined> {
|
||||
return this.db.prepare('SELECT user_id FROM trips WHERE id = ?').get(tripId) as { user_id: number } | undefined;
|
||||
}
|
||||
|
||||
// ── Trip summary (used by MCP get_trip_summary tool) ──────────────────────
|
||||
|
||||
async getTripSummary(tripId: number, viewerUserId?: number) {
|
||||
const trip = withoutFeedToken(
|
||||
this.db.prepare('SELECT * FROM trips WHERE id = ?').get(tripId) as Record<string, unknown> | undefined,
|
||||
);
|
||||
const trip = withoutFeedToken(await this.tripsRepo.findRaw(tripId));
|
||||
if (!trip) return null;
|
||||
|
||||
const ownerRow = await this.getOwner(tripId);
|
||||
if (!ownerRow) return null;
|
||||
const { owner, members } = await this.members.listMembers(tripId, ownerRow.user_id);
|
||||
const ownerId = await this.tripsRepo.getOwnerId(tripId);
|
||||
if (ownerId === null) return null;
|
||||
const { owner, members } = await this.members.listMembers(tripId, ownerId);
|
||||
|
||||
const { days: rawDays } = await this.days.list(tripId);
|
||||
const days = rawDays.map(({ notes_items, ...day }) => ({ ...day, notes: notes_items }));
|
||||
|
||||
@@ -218,10 +218,15 @@ export async function createMcpTestRegistry(): Promise<McpRegistry> {
|
||||
journeyDomain,
|
||||
generalStorage,
|
||||
accommodationsService, await createTestUnitOfWork(dbService.connection),
|
||||
await createTestPlacesRepo(dbService.connection),
|
||||
await createTestTagsRepo(dbService.connection),
|
||||
await createTestPlaceRatingsRepo(dbService.connection),
|
||||
await createTestTripMembersRepo(dbService.connection),
|
||||
await createTestDayAssignmentsRepo(dbService.connection),
|
||||
);
|
||||
// Built after it: a hotel booking writes the stay's day stop through this one.
|
||||
const reservationsService = new ReservationsService(dbService, permissionsService, budgetService, realtimeService, notificationsStub(), new ReservationsReadRepository(dbService), accommodationsService, await createTestUnitOfWork(dbService.connection));
|
||||
const membersService = new TripMembersService(dbService, budgetService, new UserCleanupService(dbService, budgetService, await createTestUnitOfWork(dbService.connection), usersRepo), permissionsService, realtimeService, notificationsStub(), await createTestUnitOfWork(dbService.connection));
|
||||
const membersService = new TripMembersService(dbService, budgetService, new UserCleanupService(dbService, budgetService, await createTestUnitOfWork(dbService.connection), usersRepo), permissionsService, realtimeService, notificationsStub(), await createTestUnitOfWork(dbService.connection), await createTestTripsRepo(dbService.connection), await createTestTripMembersRepo(dbService.connection), usersRepo);
|
||||
const tripsService = new TripsService(
|
||||
dbService,
|
||||
reservationsService,
|
||||
@@ -236,7 +241,7 @@ export async function createMcpTestRegistry(): Promise<McpRegistry> {
|
||||
mcpOrm.em,
|
||||
);
|
||||
const readModelService = new TripReadModelService(
|
||||
dbService, membersService, daysService, accommodationsService, budgetService,
|
||||
await createTestTripsRepo(dbService.connection), membersService, daysService, accommodationsService, budgetService,
|
||||
packingService, reservationsService, collabService, placesService, todoService,
|
||||
new FilesService(dbService, permissionsService, realtimeService, new EphemeralTokenService(), generalStorage, mcpOrm.em),
|
||||
);
|
||||
@@ -282,7 +287,7 @@ export async function createMcpTestRegistry(): Promise<McpRegistry> {
|
||||
new FeedsMcp(new FeedsService(dbService, calendarService), dbService, new RuntimeEnvService(), guards),
|
||||
new TripInviteMcp(new TripInviteService(dbService, permissionsService, new TripMembershipService(await createTestTripsRepo(dbService.connection), await createTestTripMembersRepo(dbService.connection)), await createTestUnitOfWork(dbService.connection)), dbService, new RuntimeEnvService(), guards, new AuditService(auditLogRepo, usersRepo)),
|
||||
new MapsMcp(mapsService),
|
||||
new PlacesMcp(placesService, mapsService, dbService, authService, journeyDomain, assignmentsService, guards, await createTestUnitOfWork(dbService.connection)),
|
||||
new PlacesMcp(placesService, mapsService, await createTestTripsRepo(dbService.connection), authService, journeyDomain, assignmentsService, guards, await createTestUnitOfWork(dbService.connection)),
|
||||
new CollectionsMcp(new CollectionsService(dbService, permissionsService, realtimeService, notificationsStub(), generalStorage, await createTestUnitOfWork(dbService.connection)), dbService, authService, addonsService),
|
||||
new TransitMcp(new TransitService(new GoogleTransitProvider(dbService, appSettings, usersRepo)), daysService, reservationsService, dbService, authService, guards),
|
||||
new AtlasMcp(new AtlasService(dbService, await createTestUnitOfWork(dbService.connection)), addonsService, authService),
|
||||
|
||||
@@ -138,11 +138,18 @@ export async function createPluginRpcHostFactory(dbs: DatabaseService): Promise<
|
||||
const oauth = new PluginOAuthService(dbs);
|
||||
const accommodations = new AccommodationsService(dbs, permissions, realtime, assignments, await createTestUnitOfWork(dbs.connection));
|
||||
// After it: deleting a place cancels the nights booked at it through this one.
|
||||
const places = new PlacesService(dbs, permissions, realtime, new MapsService(dbs, photoCache, appSettings, usersRepo), queryHelpers, unsplash, photoCache, journey, generalStorage, accommodations, await createTestUnitOfWork(dbs.connection));
|
||||
const places = new PlacesService(
|
||||
dbs, permissions, realtime, new MapsService(dbs, photoCache, appSettings, usersRepo), queryHelpers, unsplash, photoCache, journey, generalStorage, accommodations, await createTestUnitOfWork(dbs.connection),
|
||||
await createTestPlacesRepo(dbs.connection),
|
||||
await createTestTagsRepo(dbs.connection),
|
||||
await createTestPlaceRatingsRepo(dbs.connection),
|
||||
await createTestTripMembersRepo(dbs.connection),
|
||||
await createTestDayAssignmentsRepo(dbs.connection),
|
||||
);
|
||||
// After accommodations: a hotel booking writes the stay's day stop through it.
|
||||
const reservations = new ReservationsService(dbs, permissions, budget, realtime, notificationsStub(), new ReservationsReadRepository(dbs), accommodations, await createTestUnitOfWork(dbs.connection));
|
||||
const trips = new TripsService(dbs, reservations, days, permissions, budget, vacay, realtime, unsplash, generalStorage, await createTestUnitOfWork(dbs.connection), (await sharedTestOrm(dbs.connection)).em);
|
||||
const members = new TripMembersService(dbs, budget, new UserCleanupService(dbs, budget, await createTestUnitOfWork(dbs.connection), usersRepo), permissions, realtime, notificationsStub(), await createTestUnitOfWork(dbs.connection));
|
||||
const members = new TripMembersService(dbs, budget, new UserCleanupService(dbs, budget, await createTestUnitOfWork(dbs.connection), usersRepo), permissions, realtime, notificationsStub(), await createTestUnitOfWork(dbs.connection), await createTestTripsRepo(dbs.connection), await createTestTripMembersRepo(dbs.connection), usersRepo);
|
||||
const guards = new PluginGuards(dbs, permissions, addons);
|
||||
|
||||
const registry = createTestPluginRegistry([
|
||||
|
||||
@@ -7,6 +7,7 @@ import { createUser } from '../../../helpers/factories';
|
||||
import { Users } from '../../../../src/db/entities/Users.entity';
|
||||
import {
|
||||
absDifference,
|
||||
caseWhenEquals,
|
||||
coalesce,
|
||||
coalesceParam,
|
||||
columnIncrementedBy,
|
||||
@@ -428,4 +429,30 @@ describe('sql-functions (sqlite)', () => {
|
||||
const foreign = new FakePlatform();
|
||||
expect(() => lowerTrim(foreign, 'u.name')).toThrow(/no implementation for platform FakePlatform/);
|
||||
});
|
||||
|
||||
// Plan 3c Task 6 — `TripMembersRepository.listWithUserAndInviter`'s `role`
|
||||
// column (TM2): `CASE WHEN u.id = ? THEN 'owner' ELSE 'member' END`.
|
||||
it("SQLF-030: caseWhenEquals renders CASE WHEN <col> = ? THEN ? ELSE ? END, matching a hand-written statement", async () => {
|
||||
const { user: a } = createUser(testDb);
|
||||
const { user: b } = createUser(testDb);
|
||||
const platform = t.em.getPlatform();
|
||||
|
||||
const rows = await t.em.createQueryBuilder(Users, 'u')
|
||||
.select(['u.id', caseWhenEquals(platform, 'u.id', a.id, 'owner', 'member').as('role')])
|
||||
.where({ id: { $in: [a.id, b.id] } })
|
||||
.orderBy({ id: 'asc' })
|
||||
.execute('all', false);
|
||||
expect(rows).toEqual([{ id: a.id, role: 'owner' }, { id: b.id, role: 'member' }]);
|
||||
|
||||
const expected = testDb
|
||||
.prepare('SELECT id, CASE WHEN id = ? THEN ? ELSE ? END as role FROM users WHERE id IN (?, ?) ORDER BY id ASC')
|
||||
.all(a.id, 'owner', 'member', a.id, b.id);
|
||||
expect(rows).toEqual(expected);
|
||||
});
|
||||
|
||||
it('SQLF-031: an unknown platform fails closed for caseWhenEquals', () => {
|
||||
class FakePlatform extends Platform {}
|
||||
const foreign = new FakePlatform();
|
||||
expect(() => caseWhenEquals(foreign, 'u.id', 1, 'owner', 'member')).toThrow(/no implementation for platform FakePlatform/);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -134,3 +134,209 @@ describe('TripMembersRepository — listUserIdsByTrip / exists / addMember (Plan
|
||||
expect(row2).toEqual({ invited_by: null });
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Plan 3c Task 6 — TripMembersService (TM2/TM8/TM14/TM15/TM18). Appended
|
||||
// after Task 1's own suite above, per this task's file-ownership rule.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/** TM2's statement, run raw on the same rows — the parity oracle every assertion below is checked against. */
|
||||
function legacyListWithUserAndInviter(tripId: number, ownerId: number): unknown {
|
||||
return testDb.prepare(`
|
||||
SELECT u.id, COALESCE(u.display_name, u.username) AS username, u.email, u.avatar, u.is_guest,
|
||||
CASE WHEN u.id = ? THEN 'owner' ELSE 'member' END as role,
|
||||
m.added_at,
|
||||
COALESCE(ib.display_name, ib.username) as invited_by_username
|
||||
FROM trip_members m
|
||||
JOIN users u ON u.id = m.user_id
|
||||
LEFT JOIN users ib ON ib.id = m.invited_by
|
||||
WHERE m.trip_id = ?
|
||||
ORDER BY m.added_at ASC
|
||||
`).all(ownerId, tripId);
|
||||
}
|
||||
|
||||
describe('TripMembersRepository.listWithUserAndInviter (TM2, Plan 3c Task 6)', () => {
|
||||
it('TMEMREPO-011: parity with the legacy statement on a fully seeded roster — a member with display_name, one without, a guest, an inviter with/without display_name, NULL avatars', async () => {
|
||||
const { user: owner } = createUser(testDb, { username: 'owner-handle' });
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
|
||||
const { user: inviterWithName } = createUser(testDb, { username: 'inviter-1' });
|
||||
testDb.prepare('UPDATE users SET display_name = ? WHERE id = ?').run('Inviter One', inviterWithName.id);
|
||||
const { user: inviterBare } = createUser(testDb, { username: 'inviter-2' });
|
||||
|
||||
const { user: memberWithDisplayName } = createUser(testDb, { username: 'member-1-handle' });
|
||||
testDb.prepare('UPDATE users SET display_name = ?, avatar = ? WHERE id = ?').run('Member One', 'm1.png', memberWithDisplayName.id);
|
||||
testDb.prepare('INSERT INTO trip_members (trip_id, user_id, invited_by) VALUES (?, ?, ?)').run(trip.id, memberWithDisplayName.id, inviterWithName.id);
|
||||
|
||||
const { user: memberBare } = createUser(testDb, { username: 'member-2-handle' });
|
||||
testDb.prepare('INSERT INTO trip_members (trip_id, user_id, invited_by) VALUES (?, ?, ?)').run(trip.id, memberBare.id, inviterBare.id);
|
||||
|
||||
const { user: memberNoInviter } = createUser(testDb, { username: 'member-3-handle' });
|
||||
testDb.prepare('INSERT INTO trip_members (trip_id, user_id, invited_by) VALUES (?, ?, NULL)').run(trip.id, memberNoInviter.id);
|
||||
|
||||
const { user: guest } = createUser(testDb, { username: 'guest-handle' });
|
||||
testDb.prepare('UPDATE users SET is_guest = 1 WHERE id = ?').run(guest.id);
|
||||
testDb.prepare('INSERT INTO trip_members (trip_id, user_id, invited_by) VALUES (?, ?, ?)').run(trip.id, guest.id, owner.id);
|
||||
|
||||
testDb.prepare('UPDATE trip_members SET added_at = ? WHERE trip_id = ? AND user_id = ?').run('2026-01-01 00:00:00', trip.id, memberWithDisplayName.id);
|
||||
testDb.prepare('UPDATE trip_members SET added_at = ? WHERE trip_id = ? AND user_id = ?').run('2026-01-02 00:00:00', trip.id, memberBare.id);
|
||||
testDb.prepare('UPDATE trip_members SET added_at = ? WHERE trip_id = ? AND user_id = ?').run('2026-01-03 00:00:00', trip.id, memberNoInviter.id);
|
||||
testDb.prepare('UPDATE trip_members SET added_at = ? WHERE trip_id = ? AND user_id = ?').run('2026-01-04 00:00:00', trip.id, guest.id);
|
||||
|
||||
const rows = await tripMembers.listWithUserAndInviter(trip.id, owner.id);
|
||||
expect(rows).toEqual(legacyListWithUserAndInviter(trip.id, owner.id));
|
||||
expect(rows).toEqual([
|
||||
{ id: memberWithDisplayName.id, username: 'Member One', email: memberWithDisplayName.email, avatar: 'm1.png', is_guest: 0, role: 'member', added_at: '2026-01-01 00:00:00', invited_by_username: 'Inviter One' },
|
||||
{ id: memberBare.id, username: 'member-2-handle', email: memberBare.email, avatar: null, is_guest: 0, role: 'member', added_at: '2026-01-02 00:00:00', invited_by_username: 'inviter-2' },
|
||||
{ id: memberNoInviter.id, username: 'member-3-handle', email: memberNoInviter.email, avatar: null, is_guest: 0, role: 'member', added_at: '2026-01-03 00:00:00', invited_by_username: null },
|
||||
{ id: guest.id, username: 'guest-handle', email: guest.email, avatar: null, is_guest: 1, role: 'member', added_at: '2026-01-04 00:00:00', invited_by_username: 'owner-handle' },
|
||||
]);
|
||||
});
|
||||
|
||||
it('TMEMREPO-012: the trip owner\'s own row (if ever a member) reports role "owner"', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
testDb.prepare('INSERT INTO trip_members (trip_id, user_id, invited_by) VALUES (?, ?, NULL)').run(trip.id, owner.id);
|
||||
const rows = await tripMembers.listWithUserAndInviter(trip.id, owner.id);
|
||||
expect(rows).toEqual([expect.objectContaining({ id: owner.id, role: 'owner' })]);
|
||||
});
|
||||
|
||||
it('TMEMREPO-013: an owner-only trip (no members row) returns an empty array', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
expect(await tripMembers.listWithUserAndInviter(trip.id, owner.id)).toEqual([]);
|
||||
});
|
||||
|
||||
// D-shape (rule 20): a `qb().execute('all', false)` projection, which
|
||||
// never hydrates an entity into the identity map by construction — proven
|
||||
// anyway, the `AssignmentParticipantsRepository`/`PlaceRatingsRepository`
|
||||
// precedent shape.
|
||||
it('TMEMREPO-014 (D-shape): a member added after an unrelated identity-map read is visible in the FIRST wider projection', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: fresh } = createUser(testDb, { username: 'fresh-member' });
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
await t.repo(TripMembers).find({}, { disableIdentityMap: false }); // populate the identity map with an unrelated read
|
||||
addTripMember(testDb, trip.id, fresh.id);
|
||||
const rows = await tripMembers.listWithUserAndInviter(trip.id, owner.id);
|
||||
expect(rows).toEqual([expect.objectContaining({ id: fresh.id, username: 'fresh-member' })]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('TripMembersRepository.isGuestOfTrip (TM18, security-sensitive, Plan 3c Task 6)', () => {
|
||||
it('TMEMREPO-015: true only for a guest who is actually a member of THIS trip', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: guest } = createUser(testDb, { username: 'guest-scoped' });
|
||||
testDb.prepare('UPDATE users SET is_guest = 1 WHERE id = ?').run(guest.id);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
addTripMember(testDb, trip.id, guest.id);
|
||||
|
||||
expect(await tripMembers.isGuestOfTrip(trip.id, guest.id)).toBe(true);
|
||||
});
|
||||
|
||||
it('TMEMREPO-016: false for a real member (not a guest)', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: member } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
addTripMember(testDb, trip.id, member.id);
|
||||
expect(await tripMembers.isGuestOfTrip(trip.id, member.id)).toBe(false);
|
||||
});
|
||||
|
||||
it('TMEMREPO-017: false for a guest of a DIFFERENT trip — the scoping guard that keeps guest mutations trip-local', async () => {
|
||||
const { user: ownerA } = createUser(testDb);
|
||||
const { user: ownerB } = createUser(testDb);
|
||||
const { user: guest } = createUser(testDb, { username: 'guest-of-a' });
|
||||
testDb.prepare('UPDATE users SET is_guest = 1 WHERE id = ?').run(guest.id);
|
||||
const tripA = createTrip(testDb, ownerA.id);
|
||||
const tripB = createTrip(testDb, ownerB.id);
|
||||
addTripMember(testDb, tripA.id, guest.id);
|
||||
|
||||
expect(await tripMembers.isGuestOfTrip(tripA.id, guest.id)).toBe(true);
|
||||
expect(await tripMembers.isGuestOfTrip(tripB.id, guest.id)).toBe(false);
|
||||
});
|
||||
|
||||
it('TMEMREPO-018: false for a nonexistent user id', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
expect(await tripMembers.isGuestOfTrip(trip.id, 999999)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('TripMembersRepository.addIgnoringConflict / remove (TM8/TM14/TM15, security-sensitive, Plan 3c Task 6)', () => {
|
||||
it('TMEMREPO-019: addIgnoringConflict inserts a fresh row', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: former } = createUser(testDb);
|
||||
const { user: newOwner } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
await tripMembers.addIgnoringConflict(trip.id, former.id, newOwner.id);
|
||||
const row = testDb.prepare('SELECT trip_id, user_id, invited_by FROM trip_members WHERE trip_id = ? AND user_id = ?').get(trip.id, former.id);
|
||||
expect(row).toEqual({ trip_id: trip.id, user_id: former.id, invited_by: newOwner.id });
|
||||
});
|
||||
|
||||
it('TMEMREPO-020: addIgnoringConflict is a true no-op (not a throw) when the (trip, user) row already exists', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: member } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
addTripMember(testDb, trip.id, member.id);
|
||||
// The existing row's invited_by (NULL, from addTripMember) must survive —
|
||||
// an upsert-merge would overwrite it, which is NOT what `INSERT OR IGNORE` does.
|
||||
await expect(tripMembers.addIgnoringConflict(trip.id, member.id, owner.id)).resolves.toBeUndefined();
|
||||
const row = testDb.prepare('SELECT invited_by FROM trip_members WHERE trip_id = ? AND user_id = ?').get(trip.id, member.id);
|
||||
expect(row).toEqual({ invited_by: null });
|
||||
expect((testDb.prepare('SELECT COUNT(*) AS n FROM trip_members WHERE trip_id = ? AND user_id = ?').get(trip.id, member.id) as { n: number }).n).toBe(1);
|
||||
});
|
||||
|
||||
it('TMEMREPO-021: remove deletes exactly the (trip, user) row, leaving other members untouched', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: memberA } = createUser(testDb);
|
||||
const { user: memberB } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
addTripMember(testDb, trip.id, memberA.id);
|
||||
addTripMember(testDb, trip.id, memberB.id);
|
||||
|
||||
await tripMembers.remove(trip.id, memberA.id);
|
||||
expect(testDb.prepare('SELECT id FROM trip_members WHERE trip_id = ? AND user_id = ?').get(trip.id, memberA.id)).toBeUndefined();
|
||||
expect(testDb.prepare('SELECT id FROM trip_members WHERE trip_id = ? AND user_id = ?').get(trip.id, memberB.id)).toBeDefined();
|
||||
});
|
||||
|
||||
it('TMEMREPO-022: remove on a missing row is a silent no-op', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
await expect(tripMembers.remove(trip.id, 999999)).resolves.toBeUndefined();
|
||||
});
|
||||
|
||||
it('TMEMREPO-023: remove binds a string trip id raw, the same seam rosterUserIds/listUserIdsByTrip document', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: member } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
addTripMember(testDb, trip.id, member.id);
|
||||
await tripMembers.remove(String(trip.id), member.id);
|
||||
expect(testDb.prepare('SELECT id FROM trip_members WHERE trip_id = ? AND user_id = ?').get(trip.id, member.id)).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
// Rule 15's exact trap, caught on a compiled boot (`DELETE /api/trips/:id/
|
||||
// members/abc` 500'd with `no such column: NaN` before this guard existed):
|
||||
// `TripMembersController.removeMember`/`renameGuest`/`deleteGuest` still do a
|
||||
// bare `Number.parseInt(userId)` on the route's `:userId`, unvalidated — a
|
||||
// non-numeric id parses to `NaN`, which MikroORM/Kysely inline as the
|
||||
// literal SQL token `NaN` even through a raw `?` placeholder.
|
||||
describe('NaN user_id — rule 15 (a non-numeric route id parses to NaN, must not 500)', () => {
|
||||
it('TMEMREPO-024: remove(tripId, NaN) is a silent no-op, matching the legacy better-sqlite3 bind', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: member } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
addTripMember(testDb, trip.id, member.id);
|
||||
await expect(tripMembers.remove(trip.id, NaN)).resolves.toBeUndefined();
|
||||
// The real member row survives — NaN must not accidentally match anything.
|
||||
expect(testDb.prepare('SELECT id FROM trip_members WHERE trip_id = ? AND user_id = ?').get(trip.id, member.id)).toBeDefined();
|
||||
});
|
||||
|
||||
it('TMEMREPO-025: isGuestOfTrip(tripId, NaN) resolves false, matching the legacy 404 Guest not found', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: guest } = createUser(testDb);
|
||||
testDb.prepare('UPDATE users SET is_guest = 1 WHERE id = ?').run(guest.id);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
addTripMember(testDb, trip.id, guest.id);
|
||||
await expect(tripMembers.isGuestOfTrip(trip.id, NaN)).resolves.toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -152,3 +152,86 @@ describe('TripsRepository.setEndDate (Plan 3c Task 2, DY35)', () => {
|
||||
expect((testDb.prepare('SELECT end_date FROM trips WHERE id = ?').get(trip.id) as { end_date: string | null }).end_date).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Plan 3c Task 6 — TripMembersService (TM7/TM9/TM13) and TripReadModelService
|
||||
// (TR-A/TR-B). Appended per this task's file-ownership rule (additive
|
||||
// methods only, this repository is shared across tasks).
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('TripsRepository — TripMembersService / TripReadModelService (Plan 3c Task 6)', () => {
|
||||
it('TRIPREPO-014: getTitle (TM7) reads the title, null for a missing trip', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id, { title: 'Readable' });
|
||||
expect(await trips.getTitle(trip.id)).toBe('Readable');
|
||||
expect(await trips.getTitle(999999)).toBeNull();
|
||||
});
|
||||
|
||||
it('TRIPREPO-015: getTitle binds a string id raw, the same `0x10`/`007`-shaped id parity seam TripsRepository documents', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id, { title: 'Stringy' });
|
||||
expect(await trips.getTitle(String(trip.id))).toBe('Stringy');
|
||||
expect(await trips.getTitle('not-a-number')).toBeNull();
|
||||
});
|
||||
|
||||
it('TRIPREPO-016: findIdTitleOwner (TM9) returns {id, title, user_id}, undefined for a missing trip', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id, { title: 'Handover' });
|
||||
expect(await trips.findIdTitleOwner(trip.id)).toEqual({ id: trip.id, title: 'Handover', user_id: user.id });
|
||||
expect(await trips.findIdTitleOwner(999999)).toBeUndefined();
|
||||
});
|
||||
|
||||
it('TRIPREPO-017: setOwner (TM13, security-sensitive) writes user_id verbatim', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: newOwner } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
await trips.setOwner(trip.id, newOwner.id);
|
||||
expect((testDb.prepare('SELECT user_id FROM trips WHERE id = ?').get(trip.id) as { user_id: number }).user_id).toBe(newOwner.id);
|
||||
});
|
||||
|
||||
it('TRIPREPO-018: setOwner binds a string trip id raw — the same seam its docstring documents', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: newOwner } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
await trips.setOwner(String(trip.id), newOwner.id);
|
||||
expect((testDb.prepare('SELECT user_id FROM trips WHERE id = ?').get(trip.id) as { user_id: number }).user_id).toBe(newOwner.id);
|
||||
});
|
||||
|
||||
describe('findRaw (TR-B, shared with TripsService.getRaw once Task 7 lands)', () => {
|
||||
it('TRIPREPO-019: every scalar column comes back, feed_token included (the JS strip is the caller\'s job)', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id, { title: 'Full Row', description: 'desc', start_date: '2026-01-01', end_date: '2026-01-05' });
|
||||
testDb.prepare('UPDATE trips SET feed_token = ?, cover_image = ? WHERE id = ?').run('secret-token', 'cover.png', trip.id);
|
||||
const legacyRow = testDb.prepare('SELECT * FROM trips WHERE id = ?').get(trip.id);
|
||||
const row = await trips.findRaw(trip.id);
|
||||
expect(row).toEqual(legacyRow);
|
||||
expect(row?.feed_token).toBe('secret-token');
|
||||
});
|
||||
|
||||
it('TRIPREPO-020: NULL-bearing nullable columns come back null, not undefined (rule 16); a missing trip is null', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const row = await trips.findRaw(trip.id);
|
||||
expect(row).toMatchObject({ description: null, start_date: null, end_date: null, cover_image: null, feed_token: null });
|
||||
expect(await trips.findRaw(999999)).toBeNull();
|
||||
});
|
||||
|
||||
it('TRIPREPO-021: binds a string trip id raw, the same seam every other method here preserves', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id, { title: 'Stringy Raw' });
|
||||
expect((await trips.findRaw(String(trip.id)))?.title).toBe('Stringy Raw');
|
||||
expect(await trips.findRaw('not-a-number')).toBeNull();
|
||||
});
|
||||
|
||||
// D-shape (rule 20): `findRaw` is a `qb().execute('get', false)`
|
||||
// projection with an explicit `t.*` select, never hydrating an entity
|
||||
// into the identity map by construction — proven anyway.
|
||||
it('TRIPREPO-022 (D-shape): a title written after an unrelated identity-map read is visible in the FIRST wider projection', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id, { title: 'Before' });
|
||||
await t.repo(Trips).find({}, { disableIdentityMap: false }); // populate the identity map with an unrelated read
|
||||
testDb.prepare('UPDATE trips SET title = ? WHERE id = ?').run('After', trip.id);
|
||||
expect((await trips.findRaw(trip.id))?.title).toBe('After');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -765,3 +765,125 @@ describe('UsersRepository', () => {
|
||||
expect(testDb.prepare('SELECT id FROM users WHERE id = ?').get(user.id)).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Plan 3c Task 6 — TripMembersService's guest lifecycle (TM3/TM4/TM10/TM16/
|
||||
// TM19/TM20). Appended after the existing `describe('UsersRepository', ...)`
|
||||
// block per this task's file-ownership rule (additive methods only).
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('UsersRepository — TripMembersService (Plan 3c Task 6)', () => {
|
||||
describe('findOwnerSummary (TM3)', () => {
|
||||
it('USERSREPO-059: prefers display_name over username, matching the legacy COALESCE', async () => {
|
||||
const { user } = createUser(testDb, { username: 'raw-handle' });
|
||||
testDb.prepare('UPDATE users SET display_name = ? WHERE id = ?').run('Displayed Name', user.id);
|
||||
expect(await users.findOwnerSummary(user.id)).toEqual({
|
||||
id: user.id, username: 'Displayed Name', email: user.email, avatar: null,
|
||||
});
|
||||
});
|
||||
|
||||
it('USERSREPO-060: falls back to username when display_name is NULL; a missing user id is null, not a throw', async () => {
|
||||
const { user } = createUser(testDb, { username: 'bare-handle' });
|
||||
expect(await users.findOwnerSummary(user.id)).toEqual({
|
||||
id: user.id, username: 'bare-handle', email: user.email, avatar: null,
|
||||
});
|
||||
expect(await users.findOwnerSummary(999999)).toBeNull();
|
||||
});
|
||||
|
||||
it('USERSREPO-061: a NULL avatar comes back null, not undefined (rule 16)', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
testDb.prepare('UPDATE users SET avatar = ? WHERE id = ?').run('me.png', user.id);
|
||||
expect((await users.findOwnerSummary(user.id))?.avatar).toBe('me.png');
|
||||
});
|
||||
|
||||
// D-shape (rule 20): `findOwnerSummary` is a `qb().execute('get', false)`
|
||||
// projection, which never hydrates an entity into the identity map by
|
||||
// construction — proven anyway, the same "not required, proven
|
||||
// regardless" shape `AssignmentParticipantsRepository`'s D-shape tests
|
||||
// document. The FIRST, wider setup read passes `{ disableIdentityMap:
|
||||
// false }` explicitly per the program rule.
|
||||
it('USERSREPO-062 (D-shape): a display_name written after an unrelated identity-map read is visible in the FIRST wider projection', async () => {
|
||||
const { user } = createUser(testDb, { username: 'fresh-owner' });
|
||||
await t.repo(Users).find({}, { disableIdentityMap: false }); // populate the identity map with an unrelated read
|
||||
testDb.prepare('UPDATE users SET display_name = ? WHERE id = ?').run('Fresh Name', user.id);
|
||||
expect(await users.findOwnerSummary(user.id)).toEqual({
|
||||
id: user.id, username: 'Fresh Name', email: user.email, avatar: null,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('findInvitableByEmailOrUsername (TM4)', () => {
|
||||
it('USERSREPO-063: matches on email OR username, case-SENSITIVE (unlike findByEmailCI/findIdByEmailCI above)', async () => {
|
||||
const { user: byEmail } = createUser(testDb, { email: 'invitee@example.test', username: 'invitee-handle' });
|
||||
expect(await users.findInvitableByEmailOrUsername('invitee@example.test')).toEqual({
|
||||
id: byEmail.id, username: 'invitee-handle', email: 'invitee@example.test', avatar: null,
|
||||
});
|
||||
expect(await users.findInvitableByEmailOrUsername('invitee-handle')).toEqual({
|
||||
id: byEmail.id, username: 'invitee-handle', email: 'invitee@example.test', avatar: null,
|
||||
});
|
||||
// Case-sensitive: an uppercased email must NOT match (program rule 18
|
||||
// only applies where the legacy statement itself folds case — TM4's does not).
|
||||
expect(await users.findInvitableByEmailOrUsername('INVITEE@EXAMPLE.TEST')).toBeNull();
|
||||
});
|
||||
|
||||
it('USERSREPO-064: excludes guests — a trip-scoped guest can never be re-invited through the box', async () => {
|
||||
const { user: guest } = createUser(testDb, { email: 'guest-look@example.test', username: 'guest-handle' });
|
||||
testDb.prepare('UPDATE users SET is_guest = 1 WHERE id = ?').run(guest.id);
|
||||
expect(await users.findInvitableByEmailOrUsername('guest-look@example.test')).toBeNull();
|
||||
expect(await users.findInvitableByEmailOrUsername('guest-handle')).toBeNull();
|
||||
});
|
||||
|
||||
it('USERSREPO-065: no match returns null, not a throw', async () => {
|
||||
expect(await users.findInvitableByEmailOrUsername('nobody@example.test')).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('findIdEmailGuest (TM10)', () => {
|
||||
it('USERSREPO-066: reads id/email/is_guest; a missing user is null', async () => {
|
||||
const { user } = createUser(testDb, { email: 'target@example.test' });
|
||||
expect(await users.findIdEmailGuest(user.id)).toEqual({ id: user.id, email: 'target@example.test', is_guest: 0 });
|
||||
testDb.prepare('UPDATE users SET is_guest = 1 WHERE id = ?').run(user.id);
|
||||
expect(await users.findIdEmailGuest(user.id)).toEqual({ id: user.id, email: 'target@example.test', is_guest: 1 });
|
||||
expect(await users.findIdEmailGuest(999999)).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('insertGuest (TM16, security-sensitive)', () => {
|
||||
it('USERSREPO-067: writes the fixed literal columns and the given display_name; returns the generated id', async () => {
|
||||
const id = await users.insertGuest({ username: 'guest-abc', email: 'guest-abc@guests.invalid', display_name: 'Ida' });
|
||||
const row = testDb.prepare('SELECT username, email, password_hash, role, is_guest, display_name FROM users WHERE id = ?').get(id);
|
||||
expect(row).toEqual({
|
||||
username: 'guest-abc', email: 'guest-abc@guests.invalid', password_hash: '', role: 'user', is_guest: 1, display_name: 'Ida',
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('renameGuest / deleteGuest (TM19/TM20, security-sensitive, belt-and-braces is_guest = 1)', () => {
|
||||
it('USERSREPO-068: renameGuest updates display_name + updated_at only for an is_guest = 1 row', async () => {
|
||||
const id = await users.insertGuest({ username: 'guest-rn', email: 'guest-rn@guests.invalid', display_name: 'Old' });
|
||||
await users.renameGuest(id, 'New Name');
|
||||
const row = testDb.prepare('SELECT display_name, updated_at FROM users WHERE id = ?').get(id) as { display_name: string; updated_at: string | null };
|
||||
expect(row.display_name).toBe('New Name');
|
||||
expect(row.updated_at).not.toBeNull();
|
||||
});
|
||||
|
||||
it('USERSREPO-069: renameGuest is a no-op on a real (non-guest) user id — the belt-and-braces predicate, pinned', async () => {
|
||||
const { user } = createUser(testDb, { username: 'real-user' });
|
||||
testDb.prepare('UPDATE users SET display_name = ? WHERE id = ?').run('Untouched', user.id);
|
||||
await users.renameGuest(user.id, 'Attempted Rename');
|
||||
expect((testDb.prepare('SELECT display_name FROM users WHERE id = ?').get(user.id) as { display_name: string }).display_name).toBe('Untouched');
|
||||
});
|
||||
|
||||
it('USERSREPO-070: deleteGuest removes only an is_guest = 1 row', async () => {
|
||||
const id = await users.insertGuest({ username: 'guest-del', email: 'guest-del@guests.invalid', display_name: 'Gone' });
|
||||
await users.deleteGuest(id);
|
||||
expect(testDb.prepare('SELECT id FROM users WHERE id = ?').get(id)).toBeUndefined();
|
||||
});
|
||||
|
||||
it('USERSREPO-071: deleteGuest is a no-op on a real (non-guest) user id — the belt-and-braces predicate, pinned', async () => {
|
||||
const { user } = createUser(testDb, { username: 'real-user-2' });
|
||||
await users.deleteGuest(user.id);
|
||||
expect(testDb.prepare('SELECT id FROM users WHERE id = ?').get(user.id)).toBeDefined();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -72,7 +72,7 @@ import { CollabService } from '../../../src/nest/collab/collab.service';
|
||||
import { VacayService } from '../../../src/nest/vacay/vacay.service';
|
||||
import { QueryHelpersService } from '../../../src/nest/query-helpers/query-helpers.service';
|
||||
import { notificationsStub } from '../../helpers/notifications';
|
||||
import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, sharedTestOrm } from '../../helpers/test-uow';
|
||||
import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, sharedTestOrm, createTestTripsRepo, createTestTripMembersRepo } from '../../helpers/test-uow';
|
||||
|
||||
|
||||
|
||||
@@ -111,6 +111,9 @@ beforeAll(async () => {
|
||||
new RealtimeService(),
|
||||
notificationsStub(),
|
||||
await createTestUnitOfWork(testDb),
|
||||
await createTestTripsRepo(testDb),
|
||||
await createTestTripMembersRepo(testDb),
|
||||
await createTestUsersRepo(testDb),
|
||||
);
|
||||
createGuest = (...args) => membersSvc.createGuest(...args);
|
||||
deleteGuest = (...args) => membersSvc.deleteGuest(...args);
|
||||
|
||||
@@ -65,8 +65,10 @@ import { TripMembersService } from '../../../src/nest/trip-members/trip-members.
|
||||
import { NotFoundError, ValidationError } from '../../../src/nest/common/domain-errors';
|
||||
import type { User } from '../../../src/types';
|
||||
import { notificationsStub } from '../../helpers/notifications';
|
||||
import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, sharedTestOrm } from '../../helpers/test-uow';
|
||||
import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, createTestTripsRepo, createTestTripMembersRepo, sharedTestOrm } from '../../helpers/test-uow';
|
||||
import type { EntityManager } from '@mikro-orm/core';
|
||||
import type { TripsRepository } from '../../../src/db/repositories/Trips.repository';
|
||||
import type { UsersRepository } from '../../../src/db/repositories/Users.repository';
|
||||
|
||||
// Plan 3c Task 0b: `dbsEm` is resolved once, at the top of the `beforeAll`
|
||||
// below, before any `dbs()` call — `canAccessTrip`/`isOwner`/`rosterUserIds`/
|
||||
@@ -79,43 +81,22 @@ const dbs = () => new DatabaseService(testDb, dbsEm);
|
||||
|
||||
let budgetSvc: BudgetService;
|
||||
let roster: TripMembersService;
|
||||
let tripsRepo: TripsRepository;
|
||||
let usersRepo: UsersRepository;
|
||||
beforeAll(async () => {
|
||||
dbsEm = (await sharedTestOrm(testDb)).em;
|
||||
tripsRepo = await createTestTripsRepo(testDb);
|
||||
usersRepo = await createTestUsersRepo(testDb);
|
||||
budgetSvc = new BudgetService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(dbs().connection));
|
||||
roster = new TripMembersService(dbs(), budgetSvc, new UserCleanupService(dbs(), budgetSvc, await createTestUnitOfWork(dbs().connection), await createTestUsersRepo(dbs().connection)), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), notificationsStub(notifySend), await createTestUnitOfWork(dbs().connection));
|
||||
roster = new TripMembersService(
|
||||
dbs(), budgetSvc,
|
||||
new UserCleanupService(dbs(), budgetSvc, await createTestUnitOfWork(dbs().connection), usersRepo),
|
||||
new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)),
|
||||
new RealtimeService(), notificationsStub(notifySend), await createTestUnitOfWork(dbs().connection),
|
||||
tripsRepo, await createTestTripMembersRepo(testDb), usersRepo,
|
||||
);
|
||||
});
|
||||
|
||||
/**
|
||||
* A roster whose connection reports "no such row" for the first result of every
|
||||
* statement containing `match`. Both fallbacks it serves sit behind a FOREIGN
|
||||
* KEY, so the missing row cannot be produced through the real schema — freezing
|
||||
* the read is the only way to keep those branches honest. Same Proxy shape as
|
||||
* the failingConnection helper in trips.service.test.ts.
|
||||
*/
|
||||
async function rosterWithMissingRow(match: string) {
|
||||
const conn = new Proxy(testDb, {
|
||||
get(target, prop) {
|
||||
if (prop === 'prepare') {
|
||||
return (sql: string) => {
|
||||
const stmt = target.prepare(sql);
|
||||
if (!sql.includes(match)) return stmt;
|
||||
return new Proxy(stmt, {
|
||||
get(s, p) {
|
||||
if (p === 'get') return () => undefined;
|
||||
const v = (s as any)[p];
|
||||
return typeof v === 'function' ? v.bind(s) : v;
|
||||
},
|
||||
});
|
||||
};
|
||||
}
|
||||
const v = (target as any)[prop];
|
||||
return typeof v === 'function' ? v.bind(target) : v;
|
||||
},
|
||||
});
|
||||
const frozen = { connection: conn, canAccessTrip: dbMock.canAccessTrip, isOwner: dbMock.isOwner } as unknown as DatabaseService;
|
||||
return new TripMembersService(frozen, budgetSvc, new UserCleanupService(dbs(), budgetSvc, await createTestUnitOfWork(dbs().connection), await createTestUsersRepo(dbs().connection)), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), notificationsStub(notifySend), await createTestUnitOfWork(dbs().connection));
|
||||
}
|
||||
|
||||
beforeAll(() => {
|
||||
createTables(testDb);
|
||||
runMigrations(testDb);
|
||||
@@ -216,6 +197,11 @@ describe('TripMembersService delegation', () => {
|
||||
// ── addMember fallbacks ──────────────────────────────────────────────────────
|
||||
|
||||
describe('addMember fallbacks', () => {
|
||||
// R8 (Plan 3c program brief item 8): the SQL-text-keyed Proxy fault
|
||||
// injection this test used to build (`rosterWithMissingRow`, keyed on the
|
||||
// literal `'SELECT title FROM trips WHERE id = ?'` fragment) is rewritten
|
||||
// onto a repository-level fault — `TripsRepository.getTitle` resolving
|
||||
// `null`, the shape it already returns on a genuine miss.
|
||||
it("MEMBERS-SVC-007: addMember still reports a title when the trip row cannot be read ('Untitled')", async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: invitee } = createUser(testDb);
|
||||
@@ -224,8 +210,13 @@ describe('addMember fallbacks', () => {
|
||||
// The title only feeds the invite notification, and it is read after the
|
||||
// membership is inserted — losing that row must not cost the invitee their
|
||||
// access or throw on an undefined title.
|
||||
const broken = await rosterWithMissingRow('SELECT title FROM trips WHERE id = ?');
|
||||
const result = await broken.addMember(trip.id, invitee.email, owner.id, owner.id);
|
||||
const spy = vi.spyOn(tripsRepo, 'getTitle').mockResolvedValueOnce(null);
|
||||
let result;
|
||||
try {
|
||||
result = await roster.addMember(trip.id, invitee.email, owner.id, owner.id);
|
||||
} finally {
|
||||
spy.mockRestore();
|
||||
}
|
||||
expect(result.tripTitle).toBe('Untitled');
|
||||
expect(testDb.prepare('SELECT id FROM trip_members WHERE trip_id = ? AND user_id = ?').get(trip.id, invitee.id)).toBeDefined();
|
||||
});
|
||||
@@ -266,6 +257,9 @@ describe('transferOwnership guard rails', () => {
|
||||
expect((testDb.prepare('SELECT user_id FROM trips WHERE id = ?').get(trip.id) as { user_id: number }).user_id).toBe(owner.id);
|
||||
});
|
||||
|
||||
// R8: the same rewrite as MEMBERS-SVC-007 above, on `UsersRepository.getEmail`
|
||||
// (TM12) instead of `TripsRepository.getTitle` — `findIdEmailGuest`'s own
|
||||
// read for the new owner (TM10) is untouched, so only the fromEmail leg fails.
|
||||
it('MEMBERS-SVC-011: completes with an empty fromEmail when the former owner cannot be read', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: member } = createUser(testDb);
|
||||
@@ -275,8 +269,13 @@ describe('transferOwnership guard rails', () => {
|
||||
// fromEmail is audit detail only; an unreadable row must not abort the
|
||||
// handover halfway, which would leave the owner pointer and the member rows
|
||||
// disagreeing about who owns the trip.
|
||||
const broken = await rosterWithMissingRow('SELECT email FROM users WHERE id = ?');
|
||||
const result = await broken.transferOwnership(trip.id, member.id, owner.id);
|
||||
const spy = vi.spyOn(usersRepo, 'getEmail').mockResolvedValueOnce(null);
|
||||
let result;
|
||||
try {
|
||||
result = await roster.transferOwnership(trip.id, member.id, owner.id);
|
||||
} finally {
|
||||
spy.mockRestore();
|
||||
}
|
||||
expect(result.fromEmail).toBe('');
|
||||
expect(result.toEmail).toBe(member.email);
|
||||
expect((testDb.prepare('SELECT user_id FROM trips WHERE id = ?').get(trip.id) as { user_id: number }).user_id).toBe(member.id);
|
||||
|
||||
@@ -78,10 +78,11 @@ 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 type { TripsRepository } from '../../../src/db/repositories/Trips.repository';
|
||||
import {
|
||||
createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, sharedTestOrm,
|
||||
createTestDaysRepo, createTestDayAssignmentsRepo, createTestDayNotesRepo, createTestTripsRepo,
|
||||
createTestTagsRepo, createTestPlaceRatingsRepo, createTestAssignmentParticipantsRepo,
|
||||
createTestTripMembersRepo, createTestTagsRepo, createTestPlaceRatingsRepo, createTestAssignmentParticipantsRepo,
|
||||
createTestGooglePlacePhotoMetaRepo, createTestPlacesRepo,
|
||||
} from '../../helpers/test-uow';
|
||||
|
||||
@@ -130,13 +131,18 @@ beforeAll(async () => {
|
||||
new JourneyDomainService(dbs(), new RealtimeService(), new TrekPhotosRepository(dbs()), await createTestUnitOfWork(dbs().connection)),
|
||||
makeStorageFixture('').storage,
|
||||
await accommodationsOver(dbs()), await createTestUnitOfWork(dbs().connection),
|
||||
await createTestPlacesRepo(dbs().connection),
|
||||
await createTestTagsRepo(dbs().connection),
|
||||
await createTestPlaceRatingsRepo(dbs().connection),
|
||||
await createTestTripMembersRepo(dbs().connection),
|
||||
await createTestDayAssignmentsRepo(dbs().connection),
|
||||
);
|
||||
membersSvc = new TripMembersService(dbs(), budgetSvc, new UserCleanupService(dbs(), budgetSvc, await createTestUnitOfWork(dbs().connection), await createTestUsersRepo(dbs().connection)), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), notificationsStub(), await createTestUnitOfWork(dbs().connection));
|
||||
membersSvc = new TripMembersService(dbs(), budgetSvc, new UserCleanupService(dbs(), budgetSvc, await createTestUnitOfWork(dbs().connection), await createTestUsersRepo(dbs().connection)), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), notificationsStub(), await createTestUnitOfWork(dbs().connection), await createTestTripsRepo(dbs().connection), await createTestTripMembersRepo(dbs().connection), await createTestUsersRepo(dbs().connection));
|
||||
});
|
||||
|
||||
const buildReadModel = async (database: DatabaseService, roster: TripMembersService = membersSvc) =>
|
||||
const buildReadModel = async (tripsRepo: TripsRepository, roster: TripMembersService = membersSvc) =>
|
||||
new TripReadModelService(
|
||||
database, roster, daysSvc, accommodationsSvc, budgetSvc,
|
||||
tripsRepo, roster, daysSvc, accommodationsSvc, budgetSvc,
|
||||
new PackingService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), notificationsStub(), await createTestUnitOfWork(dbs().connection)),
|
||||
new ReservationsService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), budgetSvc, new RealtimeService(), notificationsStub(), new ReservationsReadRepository(dbs()), await accommodationsOver(dbs()), await createTestUnitOfWork(dbs().connection)),
|
||||
new CollabService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), notificationsStub(), makeStorageFixture('').storage, new RateLimitService(), await createTestUnitOfWork(dbs().connection)),
|
||||
@@ -146,8 +152,10 @@ const buildReadModel = async (database: DatabaseService, roster: TripMembersServ
|
||||
);
|
||||
|
||||
let svc: Awaited<ReturnType<typeof buildReadModel>>;
|
||||
let tripsRepo: TripsRepository;
|
||||
beforeAll(async () => {
|
||||
svc = await buildReadModel(dbs());
|
||||
tripsRepo = await createTestTripsRepo(dbs().connection);
|
||||
svc = await buildReadModel(tripsRepo);
|
||||
});
|
||||
|
||||
beforeAll(() => {
|
||||
@@ -173,24 +181,13 @@ const addPackingItem = (tripId: number, name: string, checked: number) =>
|
||||
testDb.prepare('INSERT INTO packing_items (trip_id, name, checked) VALUES (?, ?, ?)')
|
||||
.run(tripId, name, checked);
|
||||
|
||||
/**
|
||||
* A DatabaseService whose connection cannot resolve the owner lookup, standing in
|
||||
* for the trip row disappearing between the two SELECTs getTripSummary issues.
|
||||
* Only the read model's own two statements go through this connection; the
|
||||
* sibling services keep the real one.
|
||||
*/
|
||||
function ownerlessDbs(): DatabaseService {
|
||||
const conn = new Proxy(testDb, {
|
||||
get(target, prop) {
|
||||
if (prop === 'prepare') {
|
||||
return (sql: string) =>
|
||||
(sql.includes('SELECT user_id FROM trips') ? { get: () => undefined } : target.prepare(sql));
|
||||
}
|
||||
const v = (target as any)[prop];
|
||||
return typeof v === 'function' ? v.bind(target) : v;
|
||||
},
|
||||
});
|
||||
return { connection: conn } as unknown as DatabaseService;
|
||||
// R8 (Plan 3c program brief item 8): the SQL-text-keyed Proxy fault injection
|
||||
// this file used to build (`ownerlessDbs`, keyed on the literal
|
||||
// `'SELECT user_id FROM trips'` fragment) is rewritten onto a repository-level
|
||||
// fault: TR-A's `getOwnerId` failing while TR-B's `findRaw` still succeeds —
|
||||
// the trip row disappearing between the two reads `getTripSummary` issues.
|
||||
function forceMissingOwner(): ReturnType<typeof vi.spyOn> {
|
||||
return vi.spyOn(tripsRepo, 'getOwnerId').mockResolvedValueOnce(null);
|
||||
}
|
||||
|
||||
// ── Tests ─────────────────────────────────────────────────────────────────────
|
||||
@@ -208,17 +205,42 @@ describe('getTripSummary guards', () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
|
||||
// Trip row and owner id are two separate SELECTs; if the second one comes back
|
||||
// Trip row and owner id are two separate reads; if the second one comes back
|
||||
// empty (trip deleted in between) the guard has to stop. Without it listMembers
|
||||
// runs with an undefined owner id and the summary reports an ownerless trip.
|
||||
expect(await (await buildReadModel(ownerlessDbs())).getTripSummary(trip.id, owner.id)).toBeNull();
|
||||
const spy = forceMissingOwner();
|
||||
try {
|
||||
expect(await svc.getTripSummary(trip.id, owner.id)).toBeNull();
|
||||
} finally {
|
||||
spy.mockRestore();
|
||||
}
|
||||
|
||||
// Same trip through the real connection still aggregates — the null above is
|
||||
// Same trip through the real repository still aggregates — the null above is
|
||||
// the missing owner row, not a broken fixture.
|
||||
expect(await svc.getTripSummary(trip.id, owner.id)).not.toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('getTripSummary — feed_token never reaches the wire (TR-B)', () => {
|
||||
it('TRIP-READ-002b: withoutFeedToken strips feed_token even though findRaw hands it back intact', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
testDb.prepare('UPDATE trips SET feed_token = ? WHERE id = ?').run('secret-anon-feed-credential', trip.id);
|
||||
|
||||
// TripsRepository.findRaw is a plain `SELECT *` — it hands feed_token back
|
||||
// intact (proven directly, bypassing the service, so this test would fail
|
||||
// if the repository ever started blanking the column itself).
|
||||
expect((await tripsRepo.findRaw(trip.id))?.feed_token).toBe('secret-anon-feed-credential');
|
||||
|
||||
// getTripSummary's own withoutFeedToken() strip is the only thing standing
|
||||
// between that credential and an MCP reader — assert it on the actual
|
||||
// result object, not just that the call succeeds.
|
||||
const summary = (await svc.getTripSummary(trip.id, owner.id))!;
|
||||
expect(summary.trip).not.toHaveProperty('feed_token');
|
||||
expect(JSON.stringify(summary)).not.toContain('secret-anon-feed-credential');
|
||||
});
|
||||
});
|
||||
|
||||
describe('getTripSummary shaping', () => {
|
||||
it('TRIP-READ-003: folds a falsy total_price into the budget total instead of poisoning it', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
|
||||
@@ -91,7 +91,7 @@ import { EphemeralTokenService } from '../../../src/nest/auth/ephemeral-token.se
|
||||
import {
|
||||
createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, sharedTestOrm,
|
||||
createTestDaysRepo, createTestDayAssignmentsRepo, createTestDayNotesRepo, createTestTripsRepo,
|
||||
createTestTagsRepo, createTestPlaceRatingsRepo, createTestAssignmentParticipantsRepo,
|
||||
createTestTripMembersRepo, createTestTagsRepo, createTestPlaceRatingsRepo, createTestAssignmentParticipantsRepo,
|
||||
createTestGooglePlacePhotoMetaRepo, createTestPlacesRepo,
|
||||
} from '../../helpers/test-uow';
|
||||
|
||||
@@ -168,6 +168,11 @@ beforeAll(async () => {
|
||||
new JourneyDomainService(dbs(), new RealtimeService(), new TrekPhotosRepository(dbs()), await createTestUnitOfWork(dbs().connection)),
|
||||
makeStorageFixture('').storage,
|
||||
await accommodationsOver(dbs()), await createTestUnitOfWork(dbs().connection),
|
||||
await createTestPlacesRepo(dbs().connection),
|
||||
await createTestTagsRepo(dbs().connection),
|
||||
await createTestPlaceRatingsRepo(dbs().connection),
|
||||
await createTestTripMembersRepo(dbs().connection),
|
||||
await createTestDayAssignmentsRepo(dbs().connection),
|
||||
);
|
||||
svc = new TripsService(
|
||||
dbs(),
|
||||
@@ -182,9 +187,9 @@ beforeAll(async () => {
|
||||
await createTestUnitOfWork(dbs().connection),
|
||||
(await sharedTestOrm(testDb)).em,
|
||||
);
|
||||
membersSvc = new TripMembersService(dbs(), budgetSvc, new UserCleanupService(dbs(), budgetSvc, await createTestUnitOfWork(dbs().connection), await createTestUsersRepo(dbs().connection)), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), notificationsStub(), await createTestUnitOfWork(dbs().connection));
|
||||
membersSvc = new TripMembersService(dbs(), budgetSvc, new UserCleanupService(dbs(), budgetSvc, await createTestUnitOfWork(dbs().connection), await createTestUsersRepo(dbs().connection)), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), notificationsStub(), await createTestUnitOfWork(dbs().connection), await createTestTripsRepo(dbs().connection), await createTestTripMembersRepo(dbs().connection), await createTestUsersRepo(dbs().connection));
|
||||
readModelSvc = new TripReadModelService(
|
||||
dbs(), membersSvc, daysSvc, accommodationsSvc, budgetSvc,
|
||||
await createTestTripsRepo(dbs().connection), membersSvc, daysSvc, accommodationsSvc, budgetSvc,
|
||||
new PackingService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), notificationsStub(), await createTestUnitOfWork(dbs().connection)),
|
||||
new ReservationsService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), budgetSvc, new RealtimeService(), notificationsStub(), new ReservationsReadRepository(dbs()), accommodationsSvc, await createTestUnitOfWork(dbs().connection)),
|
||||
new CollabService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), notificationsStub(), coversFx.storage, new RateLimitService(), await createTestUnitOfWork(dbs().connection)),
|
||||
@@ -1252,15 +1257,6 @@ describe('quirk fixes', () => {
|
||||
);
|
||||
}
|
||||
|
||||
/** Same frozen connection, for the guest deletion that now lives on the roster. */
|
||||
async function failingMembers(match: string) {
|
||||
return new TripMembersService(
|
||||
failingConnection(match), budgetSvc, new UserCleanupService(dbs(), budgetSvc, await createTestUnitOfWork(dbs().connection), await createTestUsersRepo(dbs().connection)),
|
||||
new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(),
|
||||
notificationsStub(), await createTestUnitOfWork(dbs().connection),
|
||||
);
|
||||
}
|
||||
|
||||
it('TRIP-SVC-051: remove is atomic — a failed trip DELETE keeps the journey entries intact', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
@@ -1279,14 +1275,25 @@ describe('quirk fixes', () => {
|
||||
expect(testDb.prepare('SELECT id FROM trips WHERE id = ?').get(trip.id)).toBeDefined();
|
||||
});
|
||||
|
||||
// R8 (Plan 3c program brief item 8): the SQL-text-keyed `failingConnection`
|
||||
// Proxy this test used to build a `failingMembers(match)` service around is
|
||||
// rewritten as a repository-level fault (Task 6 converted `deleteGuest` off
|
||||
// `this.db.prepare(...)` entirely, onto `UsersRepository.deleteGuest` — the
|
||||
// Proxy could never trigger any more, since no statement text runs through
|
||||
// `this.db` inside `deleteGuest`'s transaction).
|
||||
it('TRIP-SVC-052: deleteGuest is atomic — a failed user DELETE rolls the budget re-split back', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
const { member: guest } = await membersSvc.createGuest(trip.id, 'Gia', owner.id);
|
||||
const item = await budgetSvc.createBudgetItem(trip.id, { name: 'Dinner', total_price: 80, member_ids: [owner.id, guest.id] });
|
||||
|
||||
const broken = await failingMembers('DELETE FROM users WHERE id = ? AND is_guest = 1');
|
||||
await expect(broken.deleteGuest(trip.id, guest.id)).rejects.toThrow('boom');
|
||||
const usersRepo = await createTestUsersRepo(dbs().connection);
|
||||
const spy = vi.spyOn(usersRepo, 'deleteGuest').mockRejectedValueOnce(new Error('boom'));
|
||||
try {
|
||||
await expect(membersSvc.deleteGuest(trip.id, guest.id)).rejects.toThrow('boom');
|
||||
} finally {
|
||||
spy.mockRestore();
|
||||
}
|
||||
|
||||
// Neither the guest nor their split membership was touched.
|
||||
expect(testDb.prepare('SELECT id FROM users WHERE id = ?').get(guest.id)).toBeDefined();
|
||||
|
||||
Reference in New Issue
Block a user