mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-24 07:53:09 +00:00
refactor(trips): trip access through TripsRepository; demo seed in a request context; dialect helpers for the trips cluster
This commit is contained in:
+8
-101
@@ -1,5 +1,4 @@
|
||||
import { readEnv } from '../app-config';
|
||||
import { Place, Tag } from '../types';
|
||||
import { resolveDbPath } from './db-path';
|
||||
import { applyDurabilityPragmas } from './durability';
|
||||
import { readSchemaSnapshot } from './schema-snapshot';
|
||||
@@ -129,104 +128,12 @@ async function reinitialize(): Promise<void> {
|
||||
console.log('[DB] Database reinitialized successfully');
|
||||
}
|
||||
|
||||
interface PlaceWithCategory extends Place {
|
||||
category_name: string | null;
|
||||
category_color: string | null;
|
||||
category_icon: string | null;
|
||||
}
|
||||
// getPlaceWithTags/canAccessTrip/isOwner and the TripAccess/PlaceWithTags
|
||||
// interfaces lived here through Plan 3c Task 0a's async sweep. Task 0b moved
|
||||
// their bodies onto TripsRepository.findAccessible/isOwner and
|
||||
// PlacesRepository.findWithTagsAndRatings — db/repositories/Trips.repository.ts's
|
||||
// TripAccess is now the single source (db/database.ts's copy is deleted, not
|
||||
// re-exported); DatabaseService re-exports both type names from the
|
||||
// repositories that own them now, so no importer's path changed.
|
||||
|
||||
interface PlaceWithTags extends Place {
|
||||
category: { id: number; name: string; color: string; icon: string } | null;
|
||||
tags: Tag[];
|
||||
ratings: { user_id: number; username: string; avatar: string | null; rating: number }[];
|
||||
rating_avg: number | null;
|
||||
rating_count: number;
|
||||
}
|
||||
|
||||
async function getPlaceWithTags(placeId: number | string): Promise<PlaceWithTags | null> {
|
||||
const place = db
|
||||
.prepare(
|
||||
`
|
||||
SELECT p.*, c.name as category_name, c.color as category_color, c.icon as category_icon
|
||||
FROM places p
|
||||
LEFT JOIN categories c ON p.category_id = c.id
|
||||
WHERE p.id = ?
|
||||
`,
|
||||
)
|
||||
.get(placeId) as PlaceWithCategory | undefined;
|
||||
|
||||
if (!place) return null;
|
||||
|
||||
const tags = db
|
||||
.prepare(
|
||||
`
|
||||
SELECT t.* FROM tags t
|
||||
JOIN place_tags pt ON t.id = pt.tag_id
|
||||
WHERE pt.place_id = ?
|
||||
`,
|
||||
)
|
||||
.all(placeId) as Tag[];
|
||||
|
||||
// Collaborative ratings (#1435): every voter with username/avatar for the
|
||||
// who-voted tooltip; the displayed value is the average.
|
||||
const ratings = db
|
||||
.prepare(
|
||||
`
|
||||
SELECT pr.user_id, u.username, u.avatar, pr.rating FROM place_ratings pr
|
||||
JOIN users u ON pr.user_id = u.id
|
||||
WHERE pr.place_id = ? ORDER BY pr.created_at
|
||||
`,
|
||||
)
|
||||
.all(placeId) as { user_id: number; username: string; avatar: string | null; rating: number }[];
|
||||
|
||||
return {
|
||||
...place,
|
||||
category: place.category_id
|
||||
? {
|
||||
id: place.category_id,
|
||||
name: place.category_name!,
|
||||
color: place.category_color!,
|
||||
icon: place.category_icon!,
|
||||
}
|
||||
: null,
|
||||
tags,
|
||||
ratings,
|
||||
rating_avg: ratings.length > 0 ? ratings.reduce((s, r) => s + r.rating, 0) / ratings.length : null,
|
||||
rating_count: ratings.length,
|
||||
};
|
||||
}
|
||||
|
||||
interface TripAccess {
|
||||
id: number;
|
||||
user_id: number;
|
||||
currency: string | null;
|
||||
}
|
||||
|
||||
async function canAccessTrip(tripId: number | string, userId: number): Promise<TripAccess | undefined> {
|
||||
return db
|
||||
.prepare(
|
||||
`
|
||||
SELECT t.id, t.user_id, t.currency FROM trips t
|
||||
LEFT JOIN trip_members m ON m.trip_id = t.id AND m.user_id = ?
|
||||
WHERE t.id = ? AND (t.user_id = ? OR m.user_id IS NOT NULL)
|
||||
`,
|
||||
)
|
||||
.get(userId, tripId, userId) as TripAccess | undefined;
|
||||
}
|
||||
|
||||
async function isOwner(tripId: number | string, userId: number): Promise<boolean> {
|
||||
return !!db.prepare('SELECT id FROM trips WHERE id = ? AND user_id = ?').get(tripId, userId);
|
||||
}
|
||||
|
||||
export {
|
||||
db,
|
||||
closeDb,
|
||||
reinitialize,
|
||||
getPlaceWithTags,
|
||||
canAccessTrip,
|
||||
isOwner,
|
||||
getRawConnection,
|
||||
registerReinitializeHook,
|
||||
runDemoSeed,
|
||||
};
|
||||
export type { TripAccess, PlaceWithTags };
|
||||
export { db, closeDb, reinitialize, getRawConnection, registerReinitializeHook, runDemoSeed };
|
||||
|
||||
@@ -127,3 +127,85 @@ export function lowerParam(platform: Platform, value: string): RawQueryFragment
|
||||
return unsupported(platform);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Plan 3c Task 0b (R6) — no consumer yet: Tasks 1–8 wire these in as each
|
||||
// converts the statement that needs them (`place-shadow.service.ts:192`,
|
||||
// `places.service.ts:642,651-652`, the ~16 `COALESCE` sites across
|
||||
// `days`/`assignments`/`trip-members`/`places`). Added here, with their own
|
||||
// SQLF-0xx tests pinning the SQLite text against a raw statement, so a later
|
||||
// task imports a tested helper instead of writing one inline.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* `datetime('now', '-N days')` — the SQLite clock shifted backwards by whole
|
||||
* days, as TEXT (not a stored column's value — `dateAdd` above is the wrong
|
||||
* shape for this: it shifts a *column*, not "now"). `days` is validated to be
|
||||
* a non-negative integer and spelled directly into the fragment rather than
|
||||
* bound: the legacy statement (`place-shadow.service.ts:192`) binds the
|
||||
* WHOLE modifier string (`'-30 days'`) as one parameter, and MikroORM's raw
|
||||
* fragments do not accept a placeholder inside the modifier argument of a
|
||||
* SQLite date/time function — `datetime('now', ?)` with `'-30 days'` bound
|
||||
* does work in raw SQLite, but composing that shape through a `raw()`
|
||||
* fragment used as a QueryBuilder value needs the modifier already in the
|
||||
* SQL text, the same way `dateAdd` above spells its own day count.
|
||||
*/
|
||||
export function nowMinusDays(platform: Platform, days: number): RawQueryFragment {
|
||||
if (!Number.isInteger(days) || days < 0) {
|
||||
throw new Error(`sql-functions: nowMinusDays needs a non-negative integer day count, got ${days}`);
|
||||
}
|
||||
if (platform instanceof SqlitePlatform) return raw(`datetime('now', '-${days} days')`);
|
||||
return unsupported(platform);
|
||||
}
|
||||
|
||||
/** `TRIM(<col>)`. Pairs with `lower()` for the `LOWER(TRIM(name))` shape (`places.service.ts:642`). */
|
||||
export function trim(platform: Platform, ref: string): RawQueryFragment {
|
||||
if (platform instanceof SqlitePlatform) return raw(`TRIM(${column(ref)})`);
|
||||
return unsupported(platform);
|
||||
}
|
||||
|
||||
/**
|
||||
* `COALESCE(<col>, <fallbackCol>)` — both sides column references, for the
|
||||
* `COALESCE(a, b)` SELECT-projection shape (`days.service.ts:93-94`,
|
||||
* `assignments.service.ts:103-104`, `trip-members.service.ts:116-143`, …).
|
||||
*
|
||||
* Not composed by nesting a `RawQueryFragment` inside this one: MikroORM's
|
||||
* `RawQueryFragment[Symbol.toPrimitive]` only returns a usable value for the
|
||||
* `'string'` coercion hint (object-key position) — a template-literal
|
||||
* `${fragment}` embed uses the `'default'` hint and throws
|
||||
* (`node_modules/@mikro-orm/core/utils/RawQueryFragment.js`, verified
|
||||
* directly, not assumed). So `coalesce`'s second argument is a second
|
||||
* `column()`-validated identifier, spelled into the same fragment as the
|
||||
* first — two columns, one statement, matching `lower`/`lowerParam`'s
|
||||
* two-function split for the same reason (a filter KEY vs a bound VALUE
|
||||
* are different shapes).
|
||||
*/
|
||||
export function coalesce(platform: Platform, ref: string, fallbackRef: string): RawQueryFragment {
|
||||
if (platform instanceof SqlitePlatform) return raw(`COALESCE(${column(ref)}, ${column(fallbackRef)})`);
|
||||
return unsupported(platform);
|
||||
}
|
||||
|
||||
/**
|
||||
* `COALESCE(<col>, ?)` — the value-side twin of `coalesce()` above, for the
|
||||
* `COALESCE(col, ?)` UPDATE-SET shape (`places.service.ts:344,351,363,…`).
|
||||
* `value` is bound as a genuine parameter, never interpolated.
|
||||
*/
|
||||
export function coalesceParam(platform: Platform, ref: string, value: string | number | null): RawQueryFragment {
|
||||
if (platform instanceof SqlitePlatform) return raw(`COALESCE(${column(ref)}, ?)`, [value]);
|
||||
return unsupported(platform);
|
||||
}
|
||||
|
||||
/**
|
||||
* `ABS(<col> - ?)`, for use as a filter KEY (the `'string'`-hint coercion
|
||||
* `lower()` above relies on) so it composes with `$lte`/`$gte`:
|
||||
* `{ [absDifference(platform, 'lat', targetLat)]: { $lte: tolerance } }`
|
||||
* renders `WHERE ABS(lat - ?) <= ?`, matching `places.service.ts:651-652`'s
|
||||
* `abs(lat - ?) <= ?` (and the sibling `lng` comparison at `:652`) exactly.
|
||||
* Typed `RawQueryFragment & symbol`, same reason `lower()` is: only a
|
||||
* `string | number | symbol` is a valid computed property name, and the
|
||||
* brand only lives on that typing for values meant to be used as a key.
|
||||
*/
|
||||
export function absDifference(platform: Platform, ref: string, value: number): RawQueryFragment & symbol {
|
||||
if (platform instanceof SqlitePlatform) return raw(`ABS(${column(ref)} - ?)`, [value]);
|
||||
return unsupported(platform);
|
||||
}
|
||||
|
||||
|
||||
+16
-1
@@ -1,5 +1,6 @@
|
||||
import mikroOrmConfig from '../mikro-orm.config';
|
||||
import { registerReinitializeHook, runDemoSeed } from './database';
|
||||
import { withRequestContext } from '../nest/database/request-context';
|
||||
import type { AnyEntity, EntityClass, EntityManager, EntitySchema, IDatabaseDriver, MikroORM } from '@mikro-orm/core';
|
||||
import type { Migrator } from '@mikro-orm/migrations';
|
||||
import { MikroORM as SqliteMikroORM } from '@mikro-orm/sqlite';
|
||||
@@ -41,7 +42,21 @@ export async function runSchemaBootstrap(orm: AnyOrm): Promise<void> {
|
||||
|
||||
// Demo data depends on the seeded categories (hard-coded ids under an FK), so
|
||||
// it can only run once the seeders above have.
|
||||
runDemoSeed();
|
||||
//
|
||||
// Plan 3c Task 0b (R9): `runDemoSeed` itself stays raw better-sqlite3 today
|
||||
// (`demo-seed.ts`'s own docstring: "the domain phase that gives demo
|
||||
// seeding a repository read must wrap it in withRequestContext then") — but
|
||||
// wrapping the call HERE, the one place `runSchemaBootstrap` already has
|
||||
// `orm` in scope, covers BOTH of its real call sites at once
|
||||
// (`bootstrap.ts`'s initial boot and `attachOrm`'s restore hook below, both
|
||||
// of which only ever reach `runDemoSeed` through this function) with no
|
||||
// second wrapper to keep in sync. A synchronous `fn` stays synchronous
|
||||
// through `withRequestContext`/`RequestContext.create`, so this changes
|
||||
// nothing about what `runDemoSeed` does today — only that a repository read
|
||||
// added to it later already has a valid context, so Plan 3i's demo
|
||||
// conversion never meets a second C1 (task-6-review-parity.md's boot-sweep
|
||||
// finding).
|
||||
withRequestContext(orm, () => runDemoSeed());
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -1,4 +1,144 @@
|
||||
import type { Places } from '../entities/Places.entity';
|
||||
import type { Tags } from '../entities/Tags.entity';
|
||||
import { type AssertRowKeys } from './_shared/rows';
|
||||
import { TrekRepository } from './_shared/trek-repository';
|
||||
|
||||
export class PlacesRepository extends TrekRepository<Places> {}
|
||||
/** A `places` row as the API emits it — every scalar column of the entity. */
|
||||
export interface PlaceRow {
|
||||
id: number;
|
||||
trip_id: number;
|
||||
name: string;
|
||||
description: string | null;
|
||||
lat: number | null;
|
||||
lng: number | null;
|
||||
address: string | null;
|
||||
category_id: number | null;
|
||||
price: number | null;
|
||||
currency: string | null;
|
||||
reservation_status: string | null;
|
||||
reservation_notes: string | null;
|
||||
reservation_datetime: string | null;
|
||||
place_time: string | null;
|
||||
end_time: string | null;
|
||||
duration_minutes: number | null;
|
||||
notes: string | null;
|
||||
image_url: string | null;
|
||||
google_place_id: string | null;
|
||||
google_ftid: string | null;
|
||||
website: string | null;
|
||||
phone: string | null;
|
||||
transport_mode: string | null;
|
||||
created_at: string | null;
|
||||
updated_at: string | null;
|
||||
osm_id: string | null;
|
||||
route_geometry: string | null;
|
||||
route_color: string | null;
|
||||
stop_type: string | null;
|
||||
fill_percent: number | null;
|
||||
amap_poi_id: string | null;
|
||||
source: string | null;
|
||||
}
|
||||
|
||||
const _placeRowKeys: AssertRowKeys<PlaceRow, Places> = true;
|
||||
|
||||
/** A `tags` row, byte-identical to the legacy `SELECT t.* FROM tags t ...` shape. */
|
||||
export interface TagRow {
|
||||
id: number;
|
||||
user_id: number;
|
||||
name: string;
|
||||
color: string | null;
|
||||
created_at: string | null;
|
||||
}
|
||||
|
||||
const _tagRowKeys: AssertRowKeys<TagRow, Tags> = true;
|
||||
|
||||
interface PlaceWithCategoryRow extends PlaceRow {
|
||||
category_name: string | null;
|
||||
category_color: string | null;
|
||||
category_icon: string | null;
|
||||
}
|
||||
|
||||
interface PlaceRatingRow {
|
||||
user_id: number;
|
||||
username: string;
|
||||
avatar: string | null;
|
||||
rating: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* `getPlaceWithTags` (`db/database.ts:146-197`, 0b's Concern for Task 4):
|
||||
* the 11 `PlacesService` call sites move in Task 4, so `DatabaseService
|
||||
* .getPlaceWithTags` keeps its own name/shape and delegates to this method
|
||||
* NOW — no caller beyond `DatabaseService` changes in this task.
|
||||
*/
|
||||
export interface PlaceWithTagsRow extends PlaceRow {
|
||||
category: { id: number; name: string; color: string; icon: string } | null;
|
||||
tags: TagRow[];
|
||||
ratings: PlaceRatingRow[];
|
||||
rating_avg: number | null;
|
||||
rating_count: number;
|
||||
}
|
||||
|
||||
export class PlacesRepository extends TrekRepository<Places> {
|
||||
/**
|
||||
* The legacy `getPlaceWithTags`'s three statements, byte-for-byte in intent
|
||||
* (`mapResults: false` on every read, matching `TripsRepository
|
||||
* .findAccessible`/`InviteTokensRepository.listWithCreatorAndTrip`):
|
||||
*
|
||||
* ```sql
|
||||
* SELECT p.*, c.name as category_name, c.color as category_color, c.icon as category_icon
|
||||
* FROM places p LEFT JOIN categories c ON p.category_id = c.id WHERE p.id = ?
|
||||
*
|
||||
* SELECT t.* FROM tags t JOIN place_tags pt ON t.id = pt.tag_id WHERE pt.place_id = ?
|
||||
*
|
||||
* SELECT pr.user_id, u.username, u.avatar, pr.rating FROM place_ratings pr
|
||||
* JOIN users u ON pr.user_id = u.id WHERE pr.place_id = ? ORDER BY pr.created_at
|
||||
* ```
|
||||
*
|
||||
* `p.category`/`p.place_tags`/`p.place_ratings_collection` are the entity's
|
||||
* own relation properties for these three joins (`Places.entity.ts`), so no
|
||||
* hand-spelled join condition is needed; `pr.user` resolves to the
|
||||
* `place_ratings.user_id` FK column the same way `TripsRepository
|
||||
* .findAccessible`'s `t.user` resolves to `trips.user_id`.
|
||||
*/
|
||||
async findWithTagsAndRatings(place_id: number | string): Promise<PlaceWithTagsRow | null> {
|
||||
// Raw conditions (D4's T5 escape hatch), not `.where({ 'p.id': place_id })`:
|
||||
// MikroORM's typed filter rejects a `string` against `p.id`'s branded
|
||||
// `number` type — matching `TripsRepository.findAccessible`'s own
|
||||
// raw-bind seam (`legacy getPlaceWithTags(placeId: number | string)`
|
||||
// bound the raw value with no `Number()` conversion). `p.id` is the
|
||||
// physical column name.
|
||||
const place = await this.qb('p')
|
||||
.leftJoin('p.category', 'c')
|
||||
.select(['p.*', 'c.name as category_name', 'c.color as category_color', 'c.icon as category_icon'])
|
||||
.where('p.id = ?', [place_id])
|
||||
.execute<PlaceWithCategoryRow | undefined>('get', false);
|
||||
if (!place) return null;
|
||||
|
||||
const tags = await this.qb('p')
|
||||
.join('p.place_tags', 't')
|
||||
.select(['t.*'])
|
||||
.where('p.id = ?', [place_id])
|
||||
.execute<TagRow[]>('all', false);
|
||||
|
||||
const ratings = await this.qb('p')
|
||||
.join('p.place_ratings_collection', 'pr')
|
||||
.join('pr.user', 'u')
|
||||
.select(['pr.user', 'u.username', 'u.avatar', 'pr.rating'])
|
||||
.where('p.id = ?', [place_id])
|
||||
.orderBy({ 'pr.created_at': 'asc' })
|
||||
.execute<PlaceRatingRow[]>('all', false);
|
||||
|
||||
const { category_name, category_color, category_icon, ...placeRest } = place;
|
||||
return {
|
||||
...placeRest,
|
||||
category: place.category_id
|
||||
? { id: place.category_id, name: category_name!, color: category_color!, icon: category_icon! }
|
||||
: null,
|
||||
tags,
|
||||
ratings,
|
||||
rating_avg: ratings.length > 0 ? ratings.reduce((s, r) => s + r.rating, 0) / ratings.length : null,
|
||||
rating_count: ratings.length,
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,4 +1,45 @@
|
||||
import type { TripMembers } from '../entities/TripMembers.entity';
|
||||
import { Trips } from '../entities/Trips.entity';
|
||||
import { TrekRepository } from './_shared/trek-repository';
|
||||
|
||||
export class TripMembersRepository extends TrekRepository<TripMembers> {}
|
||||
export class TripMembersRepository extends TrekRepository<TripMembers> {
|
||||
/**
|
||||
* The user ids a trip may refer to: its members plus the owner. Byte-for-byte
|
||||
* the legacy `DatabaseService.rosterUserIds`'s statement in intent —
|
||||
* `SELECT user_id FROM trip_members WHERE trip_id = ? UNION SELECT user_id
|
||||
* FROM trips WHERE id = ?` — as two reads merged through a `Set` (which
|
||||
* already dedupes, the same guarantee `UNION` gives) rather than one `UNION`
|
||||
* statement: the two `SELECT`s read different tables with no FK between
|
||||
* them, so a single-entity `QueryBuilder` (rooted on `TripMembers`) cannot
|
||||
* express the second half without losing rows on a trip that has no
|
||||
* members yet — an inner join from `trip_members` would produce zero rows
|
||||
* for exactly the case (an owner-only trip) the legacy statement's second
|
||||
* `SELECT` exists to cover. `trips` has no `TripMembersRepository` field to
|
||||
* reach it through, so this repository asks the `EntityManager` for
|
||||
* `TripsRepository` the same way `DatabaseService`/the guards resolve a
|
||||
* repository outside their own entity — one `EntityManager`, two
|
||||
* repositories, two independent reads.
|
||||
*/
|
||||
async rosterUserIds(trip_id: number | string): Promise<Set<number>> {
|
||||
// Raw conditions (D4's T5 escape hatch), not `.where({ 'm.trip': trip_id })`:
|
||||
// MikroORM's typed filter rejects a `string` against the relation's
|
||||
// branded id type, and coercing to `Number(trip_id)` first would change
|
||||
// the raw-bind id-shape parity `TripsRepository`'s docstring describes.
|
||||
// `m.trip_id`/`t.id` are the physical column names.
|
||||
const members = await this.qb('m')
|
||||
.select(['m.user'])
|
||||
.where('m.trip_id = ?', [trip_id])
|
||||
.execute<{ user_id: number }[]>('all', false);
|
||||
|
||||
const trips = this.getEntityManager().getRepository(Trips);
|
||||
const owner = await trips
|
||||
.qb('t')
|
||||
.select(['t.user'])
|
||||
.where('t.id = ?', [trip_id])
|
||||
.execute<{ user_id: number } | undefined>('get', false);
|
||||
|
||||
const ids = new Set(members.map((m) => m.user_id));
|
||||
if (owner) ids.add(owner.user_id);
|
||||
return ids;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -16,12 +16,31 @@ export class TripsRepository extends TrekRepository<Trips> {
|
||||
* SELECT t.id, t.user_id, t.currency FROM trips t
|
||||
* LEFT JOIN trip_members m ON m.trip_id = t.id AND m.user_id = ?
|
||||
* WHERE t.id = ? AND (t.user_id = ? OR m.user_id IS NOT NULL)
|
||||
*
|
||||
* `trip_id: number | string` (Plan 3c Task 0b): the legacy free function
|
||||
* bound whatever `req.params.tripId` resolved to straight into the
|
||||
* statement with no `Number()`/`toRowId` conversion first — the raw-bind
|
||||
* seam. `TripAccessGuard` still does `Number(tripId)` before calling this
|
||||
* (unchanged), but `DatabaseService.canAccessTrip`/`isOwner` and every
|
||||
* other of the 41 in-cluster callers pass a `number | string` through
|
||||
* unconverted, so this method (and `isOwner` below) must accept and bind
|
||||
* that raw value rather than coerce it — a coercion here would silently
|
||||
* change the `'0x10'`/`'007'`-shaped id parity the boot matrix pins
|
||||
* (`Number('0x10') === 16`, a real trip id, where SQLite's own text/integer
|
||||
* affinity comparison of the unconverted string never matches one).
|
||||
*/
|
||||
async findAccessible(trip_id: number, user_id: number): Promise<TripAccess | undefined> {
|
||||
async findAccessible(trip_id: number | string, user_id: number): Promise<TripAccess | undefined> {
|
||||
const row = await this.qb('t')
|
||||
.select(['t.id', 't.user', 't.currency'])
|
||||
.leftJoin('t.trip_members_collection', 'm', { 'm.user': user_id })
|
||||
.where({ 't.id': trip_id })
|
||||
// Raw condition (D4's T5 escape hatch), not `.where({ 't.id': trip_id })`:
|
||||
// MikroORM's typed filter rejects a `string` against `t.id`'s branded
|
||||
// `number` type, and coercing to `Number(trip_id)` first would be
|
||||
// exactly the parity break the class docstring above describes.
|
||||
// `t.id`/`t.user_id` are the physical column names (not translated
|
||||
// through the entity's `trip`/`user` property aliasing the way a typed
|
||||
// filter object is), so the raw text names them directly.
|
||||
.where('t.id = ?', [trip_id])
|
||||
.andWhere({ $or: [{ 't.user': user_id }, { 'm.user': { $ne: null } }] })
|
||||
// `mapResults: false` leaves the driver's row alone: the keys are the
|
||||
// column names, which is why the result type is spelled out here rather
|
||||
@@ -32,7 +51,12 @@ export class TripsRepository extends TrekRepository<Trips> {
|
||||
}
|
||||
|
||||
/** `SELECT id FROM trips WHERE id = ? AND user_id = ?` */
|
||||
async isOwner(trip_id: number, user_id: number): Promise<boolean> {
|
||||
return (await this.count({ id: trip_id, user: user_id })) > 0;
|
||||
async isOwner(trip_id: number | string, user_id: number): Promise<boolean> {
|
||||
const row = await this.qb('t')
|
||||
.select(['t.id'])
|
||||
.where('t.id = ?', [trip_id])
|
||||
.andWhere({ user: user_id })
|
||||
.execute<{ id: number } | undefined>('get', false);
|
||||
return !!row;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,10 +1,21 @@
|
||||
import { Inject, Injectable } from '@nestjs/common';
|
||||
import { Inject, Injectable, Optional } from '@nestjs/common';
|
||||
import { EntityManager } from '@mikro-orm/core';
|
||||
import type Database from 'better-sqlite3';
|
||||
import { canAccessTrip, getPlaceWithTags, isOwner } from '../../db/database';
|
||||
import type { PlaceWithTags, TripAccess } from '../../db/database';
|
||||
import { Trips } from '../../db/entities/Trips.entity';
|
||||
import { TripMembers } from '../../db/entities/TripMembers.entity';
|
||||
import { Places } from '../../db/entities/Places.entity';
|
||||
import type { TripAccess } from '../../db/repositories/Trips.repository';
|
||||
import type { PlaceWithTagsRow as PlaceWithTags } from '../../db/repositories/Places.repository';
|
||||
import { DATABASE_CONNECTION } from './database.tokens';
|
||||
|
||||
export type { PlaceWithTags, TripAccess };
|
||||
// Plan 3c Task 0b: `TripAccess`/`PlaceWithTags` used to be db/database.ts's
|
||||
// own interfaces, re-exported from here (§18.1's duplicate-declaration
|
||||
// surprise). Both are deleted there now — `Trips.repository.ts`'s
|
||||
// `TripAccess` and `Places.repository.ts`'s `PlaceWithTagsRow` are the single
|
||||
// sources, re-exported under their original names so every one of the ~40
|
||||
// importers of `{ TripAccess, PlaceWithTags }` from this module needs no
|
||||
// change.
|
||||
export type { TripAccess, PlaceWithTags };
|
||||
|
||||
/**
|
||||
* Injectable wrapper around TREK's existing better-sqlite3 connection.
|
||||
@@ -15,7 +26,26 @@ export type { PlaceWithTags, TripAccess };
|
||||
*/
|
||||
@Injectable()
|
||||
export class DatabaseService {
|
||||
constructor(@Inject(DATABASE_CONNECTION) private readonly conn: Database.Database) {}
|
||||
constructor(
|
||||
@Inject(DATABASE_CONNECTION) private readonly conn: Database.Database,
|
||||
// Plan 3c Task 0b: `canAccessTrip`/`isOwner`/`rosterUserIds`/
|
||||
// `getPlaceWithTags` moved off the raw better-sqlite3 singleton onto
|
||||
// TripsRepository/TripMembersRepository/PlacesRepository, which need an
|
||||
// EntityManager to resolve — `@Optional()` (not just `?`, which is
|
||||
// TS-only and does not tell Nest's DI the provider may be absent),
|
||||
// matching CronRegistrarService's own `@Optional() orm` ruling: this
|
||||
// service is provided by the `@Global()` DatabaseModule, which some e2e
|
||||
// harnesses compose standalone without MikroOrmModule.forRoot in the
|
||||
// container (see `orm.module.ts`'s docstring) — a hard EntityManager
|
||||
// dependency here would fail every one of those at `compile()`, the same
|
||||
// class of break `@InjectRepository` would cause (D5). Production
|
||||
// (buildApp) always has it, since `MikroOrmModule.forRoot`'s core module
|
||||
// is itself `@Global()` — the same resolution `JwtAuthGuard` relies on.
|
||||
// A hand-built `DatabaseService` in a unit test that calls one of the
|
||||
// four methods below must pass one explicitly (`sharedTestOrm(db)).em`),
|
||||
// same as `createTestUsersRepo`'s callers do for `UsersRepository`.
|
||||
@Optional() private readonly em?: EntityManager,
|
||||
) {}
|
||||
|
||||
/** The shared better-sqlite3 connection (same singleton the legacy app uses). */
|
||||
get connection(): Database.Database {
|
||||
@@ -38,17 +68,29 @@ export class DatabaseService {
|
||||
return this.conn.prepare(sql).run(...params);
|
||||
}
|
||||
|
||||
// Trip-access helpers delegate to the db/database exports (not this.conn):
|
||||
// tests vi.mock that module and stub the helpers themselves, so the stubs
|
||||
// must keep flowing through here.
|
||||
/**
|
||||
* The EntityManager the four repository-backed methods below resolve their
|
||||
* repository through. Throws its OWN clear error when absent — never
|
||||
* MikroORM's generic "global EntityManager" wording — so a hand-built
|
||||
* instance that forgot to pass one fails loudly and distinctly, the same
|
||||
* fail-closed shape `CronRegistrarService.runOnBoot` uses.
|
||||
*/
|
||||
private entityManager(): EntityManager {
|
||||
if (!this.em) {
|
||||
throw new Error(
|
||||
'DatabaseService: no EntityManager available — construct with one (Nest DI supplies it in production) to call canAccessTrip/isOwner/rosterUserIds/getPlaceWithTags',
|
||||
);
|
||||
}
|
||||
return this.em;
|
||||
}
|
||||
|
||||
/** Trip visible to the user (owner or member); undefined when no access. */
|
||||
async canAccessTrip(tripId: number | string, userId: number): Promise<TripAccess | undefined> {
|
||||
return await canAccessTrip(tripId, userId);
|
||||
return await this.entityManager().getRepository(Trips).findAccessible(tripId, userId);
|
||||
}
|
||||
|
||||
async isOwner(tripId: number | string, userId: number): Promise<boolean> {
|
||||
return await isOwner(tripId, userId);
|
||||
return await this.entityManager().getRepository(Trips).isOwner(tripId, userId);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -59,18 +101,19 @@ export class DatabaseService {
|
||||
* This is canAccessTrip's question asked as a set rather than a predicate, so
|
||||
* it lives beside it. Domains that accept user ids in a write body intersect
|
||||
* against it: holding the permission says you may edit the trip, not that any
|
||||
* id you name belongs to it. Runs on the connection rather than delegating to
|
||||
* db/database, because it has no legacy export to stay parity with.
|
||||
* id you name belongs to it.
|
||||
*/
|
||||
async rosterUserIds(tripId: number | string): Promise<Set<number>> {
|
||||
const rows = this.all<{ user_id: number }>(
|
||||
'SELECT user_id FROM trip_members WHERE trip_id = ? UNION SELECT user_id FROM trips WHERE id = ?',
|
||||
tripId, tripId,
|
||||
);
|
||||
return new Set(rows.map(r => r.user_id));
|
||||
return await this.entityManager().getRepository(TripMembers).rosterUserIds(tripId);
|
||||
}
|
||||
|
||||
/**
|
||||
* Delegates to `PlacesRepository.findWithTagsAndRatings` (Plan 3c Task 0b).
|
||||
* `PlacesService`'s 11 call sites are Task 4's — this method keeps its own
|
||||
* name/shape on `DatabaseService` for now, so Task 4 is the only future
|
||||
* change those 11 sites need.
|
||||
*/
|
||||
async getPlaceWithTags(placeId: number | string): Promise<PlaceWithTags | null> {
|
||||
return await getPlaceWithTags(placeId);
|
||||
return await this.entityManager().getRepository(Places).findWithTagsAndRatings(placeId);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,7 +1,9 @@
|
||||
import { CanActivate, ExecutionContext, HttpException, Injectable, SetMetadata } from '@nestjs/common';
|
||||
import { Reflector } from '@nestjs/core';
|
||||
import { EntityManager } from '@mikro-orm/core';
|
||||
import type { Request } from 'express';
|
||||
import { DatabaseService, type TripAccess } from '../database/database.service';
|
||||
import { Trips } from '../../db/entities/Trips.entity';
|
||||
import type { TripAccess } from '../database/database.service';
|
||||
import { PermissionsService } from './permissions.service';
|
||||
import type { User } from '../../types';
|
||||
|
||||
@@ -38,11 +40,23 @@ type TripRequest = Request & { user?: User; [TRIP_REQUEST_KEY]?: TripAccess };
|
||||
* It deliberately does NOT replace the `verifyTripAccess`/`canEdit` methods on the
|
||||
* domain services. Of their callers, the large majority are `*.mcp.ts` tools, which
|
||||
* never pass through an HTTP guard; those methods stay exactly where they are.
|
||||
*
|
||||
* Injects `EntityManager`, not `DatabaseService`/`@InjectRepository(Trips)`
|
||||
* (Plan 3c Task 0b, the `JwtAuthGuard` precedent): this guard is applied via
|
||||
* `@UseGuards(…, TripAccessGuard)` on 146 handlers across 22 controller
|
||||
* classes, and Nest resolves a class-referenced guard's constructor
|
||||
* dependencies from the HOST CONTROLLER'S OWN module graph —
|
||||
* `@InjectRepository(Trips)` would need `MikroOrmModule.forFeature([Trips])`
|
||||
* added to every one of those modules (D5's blast radius). `EntityManager`
|
||||
* comes from `MikroOrmModule.forRoot`'s core module, which IS `@Global()`, so
|
||||
* no module needs new wiring. `this.em.getRepository(Trips)` inside
|
||||
* `canActivate` is the same per-request resolution `JwtAuthGuard` uses for
|
||||
* `Users`.
|
||||
*/
|
||||
@Injectable()
|
||||
export class TripAccessGuard implements CanActivate {
|
||||
constructor(
|
||||
private readonly db: DatabaseService,
|
||||
private readonly em: EntityManager,
|
||||
private readonly permissions: PermissionsService,
|
||||
private readonly reflector: Reflector,
|
||||
) {}
|
||||
@@ -56,7 +70,7 @@ export class TripAccessGuard implements CanActivate {
|
||||
if (!user) throw new HttpException({ error: 'Unauthorized' }, 401);
|
||||
|
||||
const tripId = Number((request.params as Record<string, string>)?.tripId);
|
||||
const trip = Number.isFinite(tripId) ? await this.db.canAccessTrip(tripId, user.id) : undefined;
|
||||
const trip = Number.isFinite(tripId) ? await this.em.getRepository(Trips).findAccessible(tripId, user.id) : undefined;
|
||||
// A trip the user may not see is reported as absent, never as forbidden: a 403
|
||||
// would confirm the id exists to someone who has no business knowing.
|
||||
if (!trip) throw new HttpException({ error: 'Trip not found' }, 404);
|
||||
|
||||
@@ -1,7 +1,9 @@
|
||||
import { CanActivate, ExecutionContext, HttpException, Injectable, SetMetadata } from '@nestjs/common';
|
||||
import { Reflector } from '@nestjs/core';
|
||||
import { EntityManager } from '@mikro-orm/core';
|
||||
import type { Request } from 'express';
|
||||
import { DatabaseService, type TripAccess } from '../database/database.service';
|
||||
import { Trips } from '../../db/entities/Trips.entity';
|
||||
import type { TripAccess } from '../database/database.service';
|
||||
import { TRIP_REQUEST_KEY } from './trip-access.guard';
|
||||
import type { User } from '../../types';
|
||||
|
||||
@@ -43,11 +45,21 @@ type TripRequest = Request & { user?: User; [TRIP_REQUEST_KEY]?: TripAccess };
|
||||
* Absent access is a 404, never a 403, for the same reason TripAccessGuard does
|
||||
* it that way: a 403 would confirm the id exists to someone who has no business
|
||||
* knowing. A 403 only ever goes to somebody who can already see the trip.
|
||||
*
|
||||
* Injects `EntityManager`, not `DatabaseService` (Plan 3c Task 0b, the
|
||||
* `task-0a-review-security.md` F-A1 correction): `trip-members.controller.ts`
|
||||
* has no class-level `TripAccessGuard` (its own docstring explains why —
|
||||
* guards run before pipes and the e2e suite pins the error ordering), so on
|
||||
* its four owner-only routes `request[TRIP_REQUEST_KEY]` is never parked and
|
||||
* THIS guard's own lookup at `:73` below is the sole access check standing
|
||||
* between a stranger and those routes — the same `TripAccessGuard` precedent
|
||||
* applies here for the same reason (`EntityManager` is `@Global()`, no
|
||||
* module needs new wiring; `@InjectRepository(Trips)` would).
|
||||
*/
|
||||
@Injectable()
|
||||
export class TripOwnerGuard implements CanActivate {
|
||||
constructor(
|
||||
private readonly db: DatabaseService,
|
||||
private readonly em: EntityManager,
|
||||
private readonly reflector: Reflector,
|
||||
) {}
|
||||
|
||||
@@ -70,7 +82,7 @@ export class TripOwnerGuard implements CanActivate {
|
||||
let trip = request[TRIP_REQUEST_KEY];
|
||||
if (!trip) {
|
||||
const tripId = Number((request.params as Record<string, string>)?.[meta?.param ?? 'tripId']);
|
||||
trip = Number.isFinite(tripId) ? (await this.db.canAccessTrip(tripId, user.id)) ?? undefined : undefined;
|
||||
trip = Number.isFinite(tripId) ? (await this.em.getRepository(Trips).findAccessible(tripId, user.id)) ?? undefined : undefined;
|
||||
if (trip) request[TRIP_REQUEST_KEY] = trip;
|
||||
}
|
||||
if (!trip) throw new HttpException({ error: 'Trip not found' }, 404);
|
||||
|
||||
@@ -104,8 +104,18 @@ export class TripCardContributionsController {
|
||||
.filter((n) => Number.isInteger(n) && n > 0)
|
||||
.slice(0, MAX_TRIP_IDS);
|
||||
const uniqueRequested = [...new Set(requested)];
|
||||
const accessFlags = await Promise.all(uniqueRequested.map((id) => this.dbs.canAccessTrip(id, userId)));
|
||||
const accessible = uniqueRequested.filter((_, i) => accessFlags[i]);
|
||||
// Sequential, not `Promise.all` (Plan 3c Task 0b, task-0a-review-security.md
|
||||
// F-A5): the 0a async sweep's `Promise.all` was a runtime no-op then (every
|
||||
// read settled on the SAME synchronous free function), but `canAccessTrip`
|
||||
// is a genuine repository read now — dispatching `uniqueRequested.length`
|
||||
// of them concurrently would run up to `MAX_TRIP_IDS` queries against one
|
||||
// forked EntityManager at once, which MikroORM does not support. `accessible`
|
||||
// is passed positionally to `hooks.tripCards` below, so the order this
|
||||
// produces (the filtered `uniqueRequested` order) is load-bearing.
|
||||
const accessible: number[] = [];
|
||||
for (const id of uniqueRequested) {
|
||||
if (await this.dbs.canAccessTrip(id, userId)) accessible.push(id);
|
||||
}
|
||||
if (accessible.length === 0) return { contributions: [] };
|
||||
const allowed = new Set(accessible);
|
||||
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
import { Injectable } from '@nestjs/common';
|
||||
import path from 'path';
|
||||
import { EntityManager } from '@mikro-orm/core';
|
||||
import { DatabaseService } from '../database/database.service';
|
||||
import { Trips } from '../../db/entities/Trips.entity';
|
||||
import { MAX_TRIP_DAYS, tripSpanDays, type ActiveTrip, type TrekWsPayload, type TrekWsTripEventName } from '@trek/shared';
|
||||
import { RealtimeService } from '../realtime/realtime.service';
|
||||
import { PermissionsService } from '../permissions/permissions.service';
|
||||
@@ -161,6 +163,13 @@ export class TripsService {
|
||||
private readonly unsplash: UnsplashService,
|
||||
private readonly storage: StorageService,
|
||||
private readonly uow: UnitOfWork,
|
||||
// Plan 3c Task 0b (task-0a-review-security.md F-A1): `canAccessTrip`/
|
||||
// `isOwner` below resolve `TripsRepository` directly through this,
|
||||
// rather than through `this.dbs.canAccessTrip`/`isOwner` — the third of
|
||||
// the security review's three sites, alongside `TripAccessGuard` and
|
||||
// `TripOwnerGuard`. `EntityManager` is `@Global()` (`MikroOrmModule
|
||||
// .forRoot`'s core module), so no module needs new wiring.
|
||||
private readonly em: EntityManager,
|
||||
) {}
|
||||
|
||||
private get db() {
|
||||
@@ -168,12 +177,12 @@ export class TripsService {
|
||||
}
|
||||
|
||||
async canAccessTrip(tripId: string | number, userId: number) {
|
||||
const access = await this.dbs.canAccessTrip(tripId, userId);
|
||||
const access = await this.em.getRepository(Trips).findAccessible(tripId, userId);
|
||||
return access as { user_id: number } | null | undefined;
|
||||
}
|
||||
|
||||
async isOwner(tripId: string | number, userId: number): Promise<boolean> {
|
||||
return await this.dbs.isOwner(tripId, userId);
|
||||
return await this.em.getRepository(Trips).isOwner(tripId, userId);
|
||||
}
|
||||
|
||||
async can(action: string, role: string, ownerId: number | null, userId: number, isMember: boolean): Promise<boolean> {
|
||||
|
||||
@@ -50,6 +50,10 @@ const { db } = vi.hoisted(() => {
|
||||
created_at TEXT DEFAULT CURRENT_TIMESTAMP);`);
|
||||
tmp.exec(`CREATE TABLE tags (id INTEGER PRIMARY KEY AUTOINCREMENT, name TEXT, color TEXT, created_at TEXT);`);
|
||||
tmp.exec(`CREATE TABLE place_tags (place_id INTEGER NOT NULL, tag_id INTEGER NOT NULL);`);
|
||||
// TripAccessGuard now reads TripsRepository.findAccessible directly
|
||||
// (Plan 3c Task 0b), a real join against trip_members.
|
||||
tmp.exec('CREATE TABLE trips (id INTEGER PRIMARY KEY AUTOINCREMENT, user_id INTEGER, title TEXT, currency TEXT);');
|
||||
tmp.exec('CREATE TABLE trip_members (trip_id INTEGER NOT NULL, user_id INTEGER NOT NULL);');
|
||||
// StorageRegistryService (behind StorageModule, now in this module chain) reads
|
||||
// this at onModuleInit.
|
||||
tmp.exec('CREATE TABLE app_settings (key TEXT PRIMARY KEY, value TEXT);');
|
||||
@@ -98,6 +102,11 @@ describe('Assignments e2e (real auth guard + temp SQLite)', () => {
|
||||
beforeAll(async () => {
|
||||
seedUser(db as never, { id: 1 });
|
||||
seedUser(db as never, { id: 2, username: 'peer', email: 'peer@example.test' });
|
||||
// Plan 3c Task 0b: TripAccessGuard reads TripsRepository.findAccessible
|
||||
// directly now, a real query — `canAccessTrip.mockReturnValue(...)` no
|
||||
// longer intercepts it, so trip 5's real row (owned by user 1) is
|
||||
// seeded once here rather than faked per test.
|
||||
db.prepare("INSERT INTO trips (id, user_id, title) VALUES (5, 1, 'Trip')").run();
|
||||
db.prepare('INSERT INTO days (id, trip_id) VALUES (3, 5), (4, 5)').run();
|
||||
db.prepare('INSERT INTO places (id, trip_id, name) VALUES (2, 5, ?)').run('Louvre');
|
||||
app = await build();
|
||||
@@ -106,7 +115,6 @@ describe('Assignments e2e (real auth guard + temp SQLite)', () => {
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
canAccessTrip.mockReturnValue({ id: 5, user_id: 1 });
|
||||
checkPermission.mockReturnValue(true);
|
||||
db.prepare('DELETE FROM day_assignments').run();
|
||||
db.prepare('DELETE FROM assignment_participants').run();
|
||||
@@ -331,7 +339,11 @@ describe('Assignments e2e (real auth guard + temp SQLite)', () => {
|
||||
const FOREIGN_TRIP = 9;
|
||||
|
||||
beforeEach(() => {
|
||||
canAccessTrip.mockImplementation((tripId: unknown) => (Number(tripId) === 5 ? { id: 5, user_id: 1 } : undefined));
|
||||
// Plan 3c Task 0b: real access now — trip 9 exists but is owned by
|
||||
// user 2 (not the caller, user 1, and not a member), so
|
||||
// TripsRepository.findAccessible genuinely refuses it, the same
|
||||
// outcome `canAccessTrip.mockImplementation` used to fake.
|
||||
db.prepare('INSERT OR IGNORE INTO trips (id, user_id, title) VALUES (?, 2, ?)').run(FOREIGN_TRIP, 'Their trip');
|
||||
db.prepare('INSERT OR IGNORE INTO days (id, trip_id) VALUES (30, ?), (31, ?)').run(FOREIGN_TRIP, FOREIGN_TRIP);
|
||||
db.prepare('INSERT OR IGNORE INTO places (id, trip_id, name) VALUES (20, ?, ?)').run(FOREIGN_TRIP, 'Their hotel');
|
||||
});
|
||||
|
||||
@@ -104,10 +104,20 @@ describe('Budget e2e (real auth guard + temp SQLite, real budget SQL)', () => {
|
||||
});
|
||||
|
||||
it('404 when the trip is not accessible', async () => {
|
||||
canAccessTrip.mockReturnValue(undefined);
|
||||
const res = await request(server).get(`/api/trips/${tripId}/budget`).set('Cookie', sessionCookie(1));
|
||||
expect(res.status).toBe(404);
|
||||
expect(res.body).toEqual({ error: 'Trip not found' });
|
||||
// Plan 3c Task 0b: TripAccessGuard reads TripsRepository.findAccessible
|
||||
// directly now, a real query — `canAccessTrip.mockReturnValue(...)` no
|
||||
// longer intercepts it. The trip row is seeded once in `beforeAll` (not
|
||||
// re-seeded per test), so it is removed and restored around this one
|
||||
// assertion instead.
|
||||
db.prepare('DELETE FROM trips WHERE id = ?').run(tripId);
|
||||
try {
|
||||
const res = await request(server).get(`/api/trips/${tripId}/budget`).set('Cookie', sessionCookie(1));
|
||||
expect(res.status).toBe(404);
|
||||
expect(res.body).toEqual({ error: 'Trip not found' });
|
||||
} finally {
|
||||
db.prepare("INSERT INTO trips (id, user_id, title, currency) VALUES (?, 1, 'E2E Trip', 'EUR')").run(tripId);
|
||||
db.prepare('INSERT INTO trip_members (trip_id, user_id) VALUES (?, 2)').run(tripId);
|
||||
}
|
||||
});
|
||||
|
||||
it('201 on create with permission, then 200 list returns the stored row', async () => {
|
||||
|
||||
@@ -26,8 +26,10 @@ const { db } = vi.hoisted(() => {
|
||||
avatar TEXT);`);
|
||||
// The note/message notifications read the trip title fire-and-forget; the table
|
||||
// must exist so that query doesn't throw after the test has torn down.
|
||||
tmp.exec('CREATE TABLE trips (id INTEGER PRIMARY KEY AUTOINCREMENT, title TEXT);');
|
||||
tmp.prepare("INSERT INTO trips (id, title) VALUES (5, 'Trip')").run();
|
||||
tmp.exec('CREATE TABLE trips (id INTEGER PRIMARY KEY AUTOINCREMENT, title TEXT, user_id INTEGER, currency TEXT);');
|
||||
// TripAccessGuard now reads TripsRepository.findAccessible directly
|
||||
// (Plan 3c Task 0b), a real join against trip_members.
|
||||
tmp.exec('CREATE TABLE trip_members (trip_id INTEGER NOT NULL, user_id INTEGER NOT NULL);');
|
||||
// CollabService's real SQL (DI-injected, no mock) — the collab tables as the
|
||||
// real schema + migrations shape them (website on notes, deleted on messages,
|
||||
// note_id on trip_files).
|
||||
@@ -104,7 +106,12 @@ describe('Collab e2e (real auth guard + temp SQLite)', () => {
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
canAccessTrip.mockReturnValue({ id: 5, user_id: 1 });
|
||||
// Plan 3c Task 0b: TripAccessGuard reads TripsRepository.findAccessible
|
||||
// directly now, a real query — `canAccessTrip.mockReturnValue(...)` no
|
||||
// longer intercepts it, so trip 5's real row is (re-)seeded every test
|
||||
// instead, owned by user 1, matching what the mock used to fake.
|
||||
db.prepare('DELETE FROM trips WHERE id = 5').run();
|
||||
db.prepare("INSERT INTO trips (id, title, user_id) VALUES (5, 'Trip', 1)").run();
|
||||
checkPermission.mockReturnValue(true);
|
||||
db.prepare('DELETE FROM collab_message_reactions').run();
|
||||
db.prepare('DELETE FROM collab_poll_votes').run();
|
||||
@@ -132,7 +139,7 @@ describe('Collab e2e (real auth guard + temp SQLite)', () => {
|
||||
});
|
||||
|
||||
it('404 when the trip is not accessible', async () => {
|
||||
canAccessTrip.mockReturnValue(undefined);
|
||||
db.prepare('DELETE FROM trips WHERE id = 5').run();
|
||||
const res = await request(server).get('/api/trips/5/collab/notes').set('Cookie', sessionCookie(1));
|
||||
expect(res.status).toBe(404);
|
||||
expect(res.body).toEqual({ error: 'Trip not found' });
|
||||
@@ -184,7 +191,7 @@ describe('Collab e2e (real auth guard + temp SQLite)', () => {
|
||||
// The advisory this route was reported under: it answered anyone with a session,
|
||||
// for any trip id, and drove an outbound fetch from that.
|
||||
it('404 on link-preview for a trip the caller cannot reach', async () => {
|
||||
canAccessTrip.mockReturnValue(undefined);
|
||||
db.prepare('DELETE FROM trips WHERE id = 5').run();
|
||||
const res = await request(server)
|
||||
.get('/api/trips/5/collab/link-preview?url=https://example.com/')
|
||||
.set('Cookie', sessionCookie(1));
|
||||
@@ -242,7 +249,7 @@ describe('Collab e2e (real auth guard + temp SQLite)', () => {
|
||||
});
|
||||
|
||||
it('404 when the trip is not accessible', async () => {
|
||||
canAccessTrip.mockReturnValue(undefined);
|
||||
db.prepare('DELETE FROM trips WHERE id = 5').run();
|
||||
const res = await request(server).get('/api/trips/5/collab/links').set('Cookie', sessionCookie(1));
|
||||
expect(res.status).toBe(404);
|
||||
});
|
||||
|
||||
@@ -23,7 +23,7 @@ const { db } = vi.hoisted(() => {
|
||||
tmp.exec(`CREATE TABLE users (id INTEGER PRIMARY KEY AUTOINCREMENT, username TEXT NOT NULL,
|
||||
email TEXT NOT NULL UNIQUE, role TEXT NOT NULL DEFAULT 'user', password_version INTEGER NOT NULL DEFAULT 0,
|
||||
avatar TEXT);`);
|
||||
tmp.exec('CREATE TABLE trips (id INTEGER PRIMARY KEY AUTOINCREMENT, user_id INTEGER, title TEXT, end_date TEXT);');
|
||||
tmp.exec('CREATE TABLE trips (id INTEGER PRIMARY KEY AUTOINCREMENT, user_id INTEGER, title TEXT, end_date TEXT, currency TEXT);');
|
||||
tmp.exec('CREATE TABLE trip_members (trip_id INTEGER NOT NULL, user_id INTEGER NOT NULL);');
|
||||
// The tables DaysService really queries (real SQL, no service mock).
|
||||
tmp.exec(`CREATE TABLE days (id INTEGER PRIMARY KEY AUTOINCREMENT, trip_id INTEGER NOT NULL,
|
||||
|
||||
@@ -75,6 +75,8 @@ import { DocSyncModule } from '../../src/nest/doc-sync/doc-sync.module';
|
||||
import { DocSyncMcp } from '../../src/nest/doc-sync/doc-sync.mcp';
|
||||
import type { McpContext } from '../../src/nest-mcp';
|
||||
import { DatabaseModule } from '../../src/nest/database/database.module';
|
||||
import { MikroORM } from '@mikro-orm/core';
|
||||
import { withRequestContext } from '../../src/nest/database/request-context';
|
||||
import { AddonsService } from '../../src/nest/addons/addons.service';
|
||||
import { DOCUMENT_PROVIDERS } from '../../src/nest/doc-sync/document-provider';
|
||||
import { PaperlessDocumentProvider } from '../../src/nest/doc-sync/providers/paperless.provider';
|
||||
@@ -401,9 +403,14 @@ describe('Document sync e2e (real guards + real services + temp SQLite)', () =>
|
||||
.expect(200);
|
||||
expect(links.body[0]).toMatchObject({ providerId: 'paperless', providerName: 'Paperless-ngx' });
|
||||
|
||||
// The assistant reads the same bindings through its own tool.
|
||||
// The assistant reads the same bindings through its own tool. Called
|
||||
// directly on the injected controller (not through the real /mcp
|
||||
// transport), which is what forks a request context for a genuine MCP
|
||||
// call (nest-mcp/registry.ts) — wrapped here for the same reason
|
||||
// (Plan 3c Task 0b: `verifyTripAccess` now reaches `TripsRepository`,
|
||||
// which validates one).
|
||||
const ctx = { userId: memberId, scopes: null, isStaticToken: false } as McpContext;
|
||||
const result = await app.get(DocSyncMcp).getTripDocumentSync({ tripId }, ctx);
|
||||
const result = await withRequestContext(app.get(MikroORM), () => app.get(DocSyncMcp).getTripDocumentSync({ tripId }, ctx));
|
||||
const status = JSON.parse(result.content[0].text) as { links: Array<Record<string, unknown>> };
|
||||
expect(status.links[0]).toMatchObject({ providerId: 'paperless', providerName: 'Paperless-ngx' });
|
||||
} finally {
|
||||
|
||||
@@ -30,7 +30,7 @@ const { db } = vi.hoisted(() => {
|
||||
email TEXT NOT NULL UNIQUE, role TEXT NOT NULL DEFAULT 'user',
|
||||
password_version INTEGER NOT NULL DEFAULT 0, feed_token TEXT);`);
|
||||
tmp.exec(`CREATE TABLE trips (id INTEGER PRIMARY KEY AUTOINCREMENT, user_id INTEGER NOT NULL,
|
||||
title TEXT, is_archived INTEGER NOT NULL DEFAULT 0, start_date TEXT, end_date TEXT, feed_token TEXT);`);
|
||||
title TEXT, is_archived INTEGER NOT NULL DEFAULT 0, start_date TEXT, end_date TEXT, feed_token TEXT, currency TEXT);`);
|
||||
tmp.exec(`CREATE TABLE trip_members (trip_id INTEGER NOT NULL, user_id INTEGER NOT NULL);`);
|
||||
// StorageRegistryService (behind StorageModule, now in this module chain) reads
|
||||
// this at onModuleInit.
|
||||
|
||||
@@ -25,7 +25,10 @@ const { db } = vi.hoisted(() => {
|
||||
// FilesService runs its real SQL against these (FILE_SELECT joins reservations
|
||||
// and users; the link batch reads file_links; findForeignLinkTarget probes
|
||||
// reservations/places/day_assignments).
|
||||
tmp.exec('CREATE TABLE trips (id INTEGER PRIMARY KEY AUTOINCREMENT, user_id INTEGER, title TEXT);');
|
||||
tmp.exec('CREATE TABLE trips (id INTEGER PRIMARY KEY AUTOINCREMENT, user_id INTEGER, title TEXT, currency TEXT);');
|
||||
// TripAccessGuard now reads TripsRepository.findAccessible directly
|
||||
// (Plan 3c Task 0b), a real join against trip_members.
|
||||
tmp.exec('CREATE TABLE trip_members (trip_id INTEGER NOT NULL, user_id INTEGER NOT NULL);');
|
||||
tmp.exec(`CREATE TABLE trip_files (id INTEGER PRIMARY KEY AUTOINCREMENT, trip_id INTEGER NOT NULL,
|
||||
place_id INTEGER, reservation_id INTEGER, message_id INTEGER, filename TEXT NOT NULL,
|
||||
original_name TEXT NOT NULL, file_size INTEGER, mime_type TEXT, description TEXT,
|
||||
@@ -132,10 +135,19 @@ describe('Files + photos e2e (real auth guard + temp SQLite)', () => {
|
||||
});
|
||||
|
||||
it('404 when the trip is not accessible', async () => {
|
||||
canAccessTrip.mockReturnValue(undefined);
|
||||
const res = await request(server).get('/api/trips/5/files').set('Cookie', sessionCookie(1));
|
||||
expect(res.status).toBe(404);
|
||||
expect(res.body).toEqual({ error: 'Trip not found' });
|
||||
// Plan 3c Task 0b: TripAccessGuard reads TripsRepository.findAccessible
|
||||
// directly now, a real query — `canAccessTrip.mockReturnValue(...)` no
|
||||
// longer intercepts it. Trip 5 is a persistent row seeded once in
|
||||
// `beforeAll` (not re-seeded per test), so it is removed and restored
|
||||
// around this one assertion instead.
|
||||
db.prepare('DELETE FROM trips WHERE id = 5').run();
|
||||
try {
|
||||
const res = await request(server).get('/api/trips/5/files').set('Cookie', sessionCookie(1));
|
||||
expect(res.status).toBe(404);
|
||||
expect(res.body).toEqual({ error: 'Trip not found' });
|
||||
} finally {
|
||||
db.prepare("INSERT INTO trips (id, user_id, title) VALUES (5, 1, 'Trip')").run();
|
||||
}
|
||||
});
|
||||
|
||||
it('200 toggling a star with permission (real UPDATE + re-select)', async () => {
|
||||
|
||||
@@ -21,7 +21,7 @@ const { db } = vi.hoisted(() => {
|
||||
tmp.exec(`CREATE TABLE users (id INTEGER PRIMARY KEY AUTOINCREMENT, username TEXT NOT NULL,
|
||||
email TEXT NOT NULL UNIQUE, role TEXT NOT NULL DEFAULT 'user', password_version INTEGER NOT NULL DEFAULT 0,
|
||||
display_name TEXT, avatar TEXT);`);
|
||||
tmp.exec('CREATE TABLE trips (id INTEGER PRIMARY KEY AUTOINCREMENT, user_id INTEGER, title TEXT);');
|
||||
tmp.exec('CREATE TABLE trips (id INTEGER PRIMARY KEY AUTOINCREMENT, user_id INTEGER, title TEXT, currency TEXT);');
|
||||
tmp.exec('CREATE TABLE trip_members (trip_id INTEGER NOT NULL, user_id INTEGER NOT NULL);');
|
||||
// The post-migration shape of the packing tables (schema.ts + migrations.ts).
|
||||
tmp.exec(`CREATE TABLE packing_items (
|
||||
|
||||
@@ -40,7 +40,10 @@ const { db } = vi.hoisted(() => {
|
||||
// The assignment=unassigned/assigned filters join these.
|
||||
tmp.exec('CREATE TABLE days (id INTEGER PRIMARY KEY AUTOINCREMENT, trip_id INTEGER NOT NULL, day_number INTEGER, date TEXT, title TEXT);');
|
||||
// The GPX export reads the trip title for <metadata> and the filename.
|
||||
tmp.exec('CREATE TABLE trips (id INTEGER PRIMARY KEY AUTOINCREMENT, title TEXT);');
|
||||
tmp.exec('CREATE TABLE trips (id INTEGER PRIMARY KEY AUTOINCREMENT, title TEXT, user_id INTEGER, currency TEXT);');
|
||||
// TripAccessGuard now reads TripsRepository.findAccessible directly
|
||||
// (Plan 3c Task 0b), a real join against trip_members.
|
||||
tmp.exec('CREATE TABLE trip_members (trip_id INTEGER NOT NULL, user_id INTEGER NOT NULL);');
|
||||
tmp.exec(`CREATE TABLE day_assignments (id INTEGER PRIMARY KEY AUTOINCREMENT, day_id INTEGER NOT NULL,
|
||||
place_id INTEGER NOT NULL, order_index INTEGER DEFAULT 0);`);
|
||||
// reclaimPlaceImage ref-counts an uploaded thumbnail across both tables.
|
||||
@@ -127,6 +130,13 @@ describe('Places e2e (real auth guard + temp SQLite)', () => {
|
||||
|
||||
beforeEach(() => {
|
||||
db.exec('DELETE FROM trips; DELETE FROM places; DELETE FROM place_tags; DELETE FROM place_ratings; DELETE FROM day_assignments; DELETE FROM days;');
|
||||
// Plan 3c Task 0b: TripAccessGuard reads TripsRepository.findAccessible
|
||||
// directly now, a real query — `canAccessTrip.mockReturnValue(...)` no
|
||||
// longer intercepts it, so trip 5's real row is (re-)seeded every test
|
||||
// instead, owned by user 1, matching what the mock used to fake. GPX
|
||||
// tests below re-seed their own data-bearing row with `INSERT OR
|
||||
// REPLACE` over this default.
|
||||
db.prepare("INSERT INTO trips (id, title, user_id) VALUES (5, 'Trip', 1)").run();
|
||||
canAccessTrip.mockReturnValue({ id: 5, user_id: 1 });
|
||||
checkPermission.mockReturnValue(true);
|
||||
});
|
||||
@@ -196,7 +206,7 @@ describe('Places e2e (real auth guard + temp SQLite)', () => {
|
||||
|
||||
// And it fires ahead of the trip-access 404 it used to follow (documented
|
||||
// parity shift of the ratchet — the todo/trips precedent).
|
||||
canAccessTrip.mockReturnValue(undefined);
|
||||
db.prepare('DELETE FROM trips WHERE id = 5').run();
|
||||
const noTrip = await request(server).post('/api/trips/5/places').set('Cookie', sessionCookie(1)).send({});
|
||||
expect(noTrip.status).toBe(400);
|
||||
});
|
||||
@@ -306,7 +316,7 @@ describe('Places e2e (real auth guard + temp SQLite)', () => {
|
||||
});
|
||||
|
||||
it('404 trip when not accessible', async () => {
|
||||
canAccessTrip.mockReturnValue(undefined);
|
||||
db.prepare('DELETE FROM trips WHERE id = 5').run();
|
||||
const res = await request(server).get('/api/trips/5/places').set('Cookie', sessionCookie(1));
|
||||
expect(res.status).toBe(404);
|
||||
expect(res.body).toEqual({ error: 'Trip not found' });
|
||||
@@ -328,7 +338,7 @@ describe('Places e2e (real auth guard + temp SQLite)', () => {
|
||||
});
|
||||
|
||||
it('the guarded read routes 404 an inaccessible trip without touching the place', async () => {
|
||||
canAccessTrip.mockReturnValue(undefined);
|
||||
db.prepare('DELETE FROM trips WHERE id = 5').run();
|
||||
for (const path of ['/api/trips/5/places/9', '/api/trips/5/places/9/image']) {
|
||||
const res = await request(server).get(path).set('Cookie', sessionCookie(1));
|
||||
expect(res.status).toBe(404);
|
||||
@@ -342,7 +352,7 @@ describe('Places e2e (real auth guard + temp SQLite)', () => {
|
||||
// ── GPX export ────────────────────────────────────────────────────────────
|
||||
describe('GET export.gpx (#1442)', () => {
|
||||
const seedTrip = () => {
|
||||
db.prepare("INSERT INTO trips (id, title) VALUES (5, 'Alpine week')").run();
|
||||
db.prepare("INSERT OR REPLACE INTO trips (id, title, user_id) VALUES (5, 'Alpine week', 1)").run();
|
||||
db.prepare("INSERT INTO places (id, trip_id, name, lat, lng) VALUES (1, 5, 'Trailhead', 47.1, 11.2)").run();
|
||||
db.prepare("INSERT INTO places (id, trip_id, name, lat, lng, route_geometry) VALUES (2, 5, 'Ridge', 47.2, 11.3, '[[47.2,11.3],[47.25,11.35]]')").run();
|
||||
db.prepare("INSERT INTO days (id, trip_id, day_number, date, title) VALUES (1, 5, 1, '2026-05-01', 'Warm up')").run();
|
||||
@@ -366,7 +376,7 @@ describe('Places e2e (real auth guard + temp SQLite)', () => {
|
||||
// ERR_INVALID_CHAR before the first body byte. Now the header folds to
|
||||
// ASCII and carries the real name RFC 5987-encoded.
|
||||
it('a non-ASCII trip title exports 200 with a filename* header instead of a 500 (#2165)', async () => {
|
||||
db.prepare('INSERT INTO trips (id, title) VALUES (5, ?)').run('沖縄 4泊5日');
|
||||
db.prepare('INSERT OR REPLACE INTO trips (id, title, user_id) VALUES (5, ?, 1)').run('沖縄 4泊5日');
|
||||
db.prepare('INSERT INTO places (id, trip_id, name, lat, lng) VALUES (1, 5, ?, 26.217, 127.719)').run('首里城');
|
||||
|
||||
const res = await request(server).get('/api/trips/5/places/export.gpx').set('Cookie', sessionCookie(1));
|
||||
@@ -400,7 +410,7 @@ describe('Places e2e (real auth guard + temp SQLite)', () => {
|
||||
});
|
||||
|
||||
it('404s an empty trip rather than handing over a file that imports as nothing', async () => {
|
||||
db.prepare("INSERT INTO trips (id, title) VALUES (5, 'Nothing here')").run();
|
||||
db.prepare("INSERT OR REPLACE INTO trips (id, title, user_id) VALUES (5, 'Nothing here', 1)").run();
|
||||
const res = await request(server).get('/api/trips/5/places/export.gpx').set('Cookie', sessionCookie(1));
|
||||
expect(res.status).toBe(404);
|
||||
expect(res.body).toEqual({ error: 'Nothing to export' });
|
||||
@@ -408,7 +418,7 @@ describe('Places e2e (real auth guard + temp SQLite)', () => {
|
||||
|
||||
it('404s a trip the caller cannot reach, and 401s without a cookie', async () => {
|
||||
seedTrip();
|
||||
canAccessTrip.mockReturnValue(undefined);
|
||||
db.prepare('DELETE FROM trips WHERE id = 5').run();
|
||||
const res = await request(server).get('/api/trips/5/places/export.gpx').set('Cookie', sessionCookie(1));
|
||||
expect(res.status).toBe(404);
|
||||
expect(res.body).toEqual({ error: 'Trip not found' });
|
||||
@@ -428,7 +438,7 @@ describe('Places e2e (real auth guard + temp SQLite)', () => {
|
||||
// before the pipe, so guarding create would answer 404 where the suite above
|
||||
// pins a 400. This is the non-regression pin for that decision.
|
||||
it('a bad create body still 400s ahead of the trip 404', async () => {
|
||||
canAccessTrip.mockReturnValue(undefined);
|
||||
db.prepare('DELETE FROM trips WHERE id = 5').run();
|
||||
const res = await request(server).post('/api/trips/5/places').set('Cookie', sessionCookie(1)).send({});
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
|
||||
@@ -129,10 +129,19 @@ describe('Reservations + accommodations e2e (real auth guard + temp SQLite, real
|
||||
});
|
||||
|
||||
it('404 when trip not accessible (reservations)', async () => {
|
||||
canAccessTrip.mockReturnValue(undefined);
|
||||
const res = await request(server).get(`/api/trips/${tripId}/reservations`).set('Cookie', sessionCookie(1));
|
||||
expect(res.status).toBe(404);
|
||||
expect(res.body).toEqual({ error: 'Trip not found' });
|
||||
// Plan 3c Task 0b: TripAccessGuard reads TripsRepository.findAccessible
|
||||
// directly now, a real query — `canAccessTrip.mockReturnValue(...)` no
|
||||
// longer intercepts it. The trip row is seeded once in `beforeAll` (not
|
||||
// re-seeded per test), so it is removed and restored around this one
|
||||
// assertion instead.
|
||||
db.prepare('DELETE FROM trips WHERE id = ?').run(tripId);
|
||||
try {
|
||||
const res = await request(server).get(`/api/trips/${tripId}/reservations`).set('Cookie', sessionCookie(1));
|
||||
expect(res.status).toBe(404);
|
||||
expect(res.body).toEqual({ error: 'Trip not found' });
|
||||
} finally {
|
||||
db.prepare("INSERT INTO trips (id, user_id, title) VALUES (?, 1, 'E2E Trip')").run(tripId);
|
||||
}
|
||||
});
|
||||
|
||||
it('201 create reservation (real insert + booking notification), 400 without title', async () => {
|
||||
@@ -249,10 +258,16 @@ describe('Reservations + accommodations e2e (real auth guard + temp SQLite, real
|
||||
});
|
||||
|
||||
it('404 when trip not accessible (accommodations)', async () => {
|
||||
canAccessTrip.mockReturnValue(undefined);
|
||||
const res = await request(server).get(`/api/trips/${tripId}/accommodations`).set('Cookie', sessionCookie(1));
|
||||
expect(res.status).toBe(404);
|
||||
expect(res.body).toEqual({ error: 'Trip not found' });
|
||||
// See the reservations 404 case above for why this deletes/restores the
|
||||
// real row instead of mocking canAccessTrip.
|
||||
db.prepare('DELETE FROM trips WHERE id = ?').run(tripId);
|
||||
try {
|
||||
const res = await request(server).get(`/api/trips/${tripId}/accommodations`).set('Cookie', sessionCookie(1));
|
||||
expect(res.status).toBe(404);
|
||||
expect(res.body).toEqual({ error: 'Trip not found' });
|
||||
} finally {
|
||||
db.prepare("INSERT INTO trips (id, user_id, title) VALUES (?, 1, 'E2E Trip')").run(tripId);
|
||||
}
|
||||
});
|
||||
|
||||
it('400 accommodation create without refs', async () => {
|
||||
|
||||
@@ -40,7 +40,7 @@ const { db } = vi.hoisted(() => {
|
||||
tmp.exec(`CREATE TABLE users (id INTEGER PRIMARY KEY AUTOINCREMENT, username TEXT NOT NULL,
|
||||
email TEXT NOT NULL UNIQUE, role TEXT NOT NULL DEFAULT 'user', password_version INTEGER NOT NULL DEFAULT 0,
|
||||
avatar TEXT);`);
|
||||
tmp.exec('CREATE TABLE trips (id INTEGER PRIMARY KEY AUTOINCREMENT, user_id INTEGER, title TEXT, end_date TEXT);');
|
||||
tmp.exec('CREATE TABLE trips (id INTEGER PRIMARY KEY AUTOINCREMENT, user_id INTEGER, title TEXT, end_date TEXT, currency TEXT);');
|
||||
tmp.exec(
|
||||
'CREATE TABLE roadtrip_preferences (trip_id INTEGER NOT NULL REFERENCES trips(id) ON DELETE CASCADE, key TEXT NOT NULL, value TEXT NOT NULL, PRIMARY KEY(trip_id, key));',
|
||||
);
|
||||
|
||||
@@ -21,7 +21,7 @@ const { db } = vi.hoisted(() => {
|
||||
tmp.exec(`CREATE TABLE users (id INTEGER PRIMARY KEY AUTOINCREMENT, username TEXT NOT NULL,
|
||||
email TEXT NOT NULL UNIQUE, role TEXT NOT NULL DEFAULT 'user', password_version INTEGER NOT NULL DEFAULT 0,
|
||||
avatar TEXT);`);
|
||||
tmp.exec('CREATE TABLE trips (id INTEGER PRIMARY KEY AUTOINCREMENT, user_id INTEGER, title TEXT);');
|
||||
tmp.exec('CREATE TABLE trips (id INTEGER PRIMARY KEY AUTOINCREMENT, user_id INTEGER, title TEXT, currency TEXT);');
|
||||
tmp.exec('CREATE TABLE trip_members (trip_id INTEGER NOT NULL, user_id INTEGER NOT NULL);');
|
||||
tmp.exec(`CREATE TABLE todo_items (
|
||||
id INTEGER PRIMARY KEY AUTOINCREMENT,
|
||||
|
||||
@@ -21,7 +21,10 @@ const { db } = vi.hoisted(() => {
|
||||
tmp.exec('PRAGMA journal_mode = WAL');
|
||||
tmp.exec(`CREATE TABLE users (id INTEGER PRIMARY KEY AUTOINCREMENT, username TEXT NOT NULL,
|
||||
email TEXT NOT NULL UNIQUE, role TEXT NOT NULL DEFAULT 'user', password_version INTEGER NOT NULL DEFAULT 0);`);
|
||||
tmp.exec('CREATE TABLE trips (id INTEGER PRIMARY KEY AUTOINCREMENT, title TEXT);');
|
||||
tmp.exec('CREATE TABLE trips (id INTEGER PRIMARY KEY AUTOINCREMENT, title TEXT, user_id INTEGER, currency TEXT);');
|
||||
// TripAccessGuard/TripOwnerGuard now read TripsRepository.findAccessible
|
||||
// directly (Plan 3c Task 0b), a real join against trip_members.
|
||||
tmp.exec('CREATE TABLE trip_members (trip_id INTEGER NOT NULL, user_id INTEGER NOT NULL);');
|
||||
// TripInviteService now runs its real SQL (DI-injected, no mock) — mirror of
|
||||
// the trip_invite_tokens DDL from migration 153.
|
||||
tmp.exec(`CREATE TABLE trip_invite_tokens (
|
||||
@@ -81,8 +84,8 @@ describe('Trip invite-link e2e (real auth guard + temp SQLite)', () => {
|
||||
return nest;
|
||||
}
|
||||
|
||||
function seedTrip(id: number, title: string) {
|
||||
db.prepare('INSERT INTO trips (id, title) VALUES (?, ?)').run(id, title);
|
||||
function seedTrip(id: number, title: string, ownerId = 1) {
|
||||
db.prepare('INSERT INTO trips (id, title, user_id) VALUES (?, ?, ?)').run(id, title, ownerId);
|
||||
}
|
||||
function seedToken(tripId: number, token: string, expiresAt: string | null = null) {
|
||||
db.prepare('INSERT INTO trip_invite_tokens (trip_id, token, created_by, expires_at) VALUES (?, ?, 1, ?)')
|
||||
@@ -159,19 +162,24 @@ describe('Trip invite-link e2e (real auth guard + temp SQLite)', () => {
|
||||
});
|
||||
|
||||
it('403 to create without share_manage', async () => {
|
||||
seedTrip(5, 'Lisbon');
|
||||
checkPermission.mockReturnValue(false);
|
||||
const res = await request(server).post('/api/trips/5/invite-link').set('Cookie', sessionCookie(1)).send({});
|
||||
expect(res.status).toBe(403);
|
||||
});
|
||||
|
||||
it('403 to READ the link without share_manage (token grants membership)', async () => {
|
||||
seedTrip(5, 'Lisbon');
|
||||
checkPermission.mockReturnValue(false);
|
||||
const res = await request(server).get('/api/trips/5/invite-link').set('Cookie', sessionCookie(1));
|
||||
expect(res.status).toBe(403);
|
||||
});
|
||||
|
||||
it('404 when the trip is not accessible', async () => {
|
||||
canAccessTrip.mockReturnValue(undefined);
|
||||
// Plan 3c Task 0b: TripAccessGuard reads TripsRepository.findAccessible
|
||||
// directly now, a real query — no trip 5 row exists (nothing in this
|
||||
// test seeded one, and `beforeEach` clears the table), so the guard's
|
||||
// real 404 fires without a `canAccessTrip` mock to fake it.
|
||||
const res = await request(server).get('/api/trips/5/invite-link').set('Cookie', sessionCookie(1));
|
||||
expect(res.status).toBe(404);
|
||||
});
|
||||
|
||||
@@ -7,7 +7,7 @@ import { PermissionsService } from '../../src/nest/permissions/permissions.servi
|
||||
import { QueryHelpersService } from '../../src/nest/query-helpers/query-helpers.service';
|
||||
import { RealtimeService } from '../../src/nest/realtime/realtime.service';
|
||||
import { TrekPhotosRepository } from '../../src/nest/photos/trek-photos.repository';
|
||||
import { createTestUnitOfWork, createTestAppSettingsRepo } from './test-uow';
|
||||
import { createTestUnitOfWork, createTestAppSettingsRepo, sharedTestOrm } from './test-uow';
|
||||
|
||||
/**
|
||||
* AccommodationsService over a test connection.
|
||||
@@ -16,9 +16,14 @@ import { createTestUnitOfWork, createTestAppSettingsRepo } from './test-uow';
|
||||
* real AssignmentsService rather than a stub: the stop is a row these cases read
|
||||
* back, and it has to come out of the same connection. Five collaborators deep is
|
||||
* why this is a helper and not five copies across the suites.
|
||||
*
|
||||
* Plan 3c Task 0b: the `DatabaseService` built here needs a real
|
||||
* `EntityManager` now — `AccommodationsService.stampLodging` reaches
|
||||
* `getPlaceWithTags`, which is `PlacesRepository.findWithTagsAndRatings`
|
||||
* (not `db/database.ts`'s deleted free function).
|
||||
*/
|
||||
export async function makeAccommodationsService(conn: Database): Promise<AccommodationsService> {
|
||||
return accommodationsOver(new DatabaseService(conn));
|
||||
return accommodationsOver(new DatabaseService(conn, (await sharedTestOrm(conn)).em));
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -133,10 +133,15 @@ import { PasswordResetTokens } from '../../src/db/entities/PasswordResetTokens.e
|
||||
* src/db/database flowing through (same pattern as todo.bridge.ts).
|
||||
*/
|
||||
export async function createMcpTestRegistry(): Promise<McpRegistry> {
|
||||
const dbService = new DatabaseService(db);
|
||||
// Plan 3c Task 0b: `mcpOrm` built first (was built right after `dbService`)
|
||||
// so its `EntityManager` can be threaded into `DatabaseService`'s
|
||||
// constructor — `canAccessTrip`/`isOwner`/`rosterUserIds`/`getPlaceWithTags`
|
||||
// resolve `TripsRepository`/`TripMembersRepository`/`PlacesRepository`
|
||||
// through it now, not through `db/database.ts`'s deleted free functions.
|
||||
const mcpOrm = await createTestOrm(db);
|
||||
const dbService = new DatabaseService(db, mcpOrm.em);
|
||||
const generalStorage = makeStorageFixture('').storage;
|
||||
const appSettings = (await createTestOrm(dbService.connection)).repo(AppSettings);
|
||||
const mcpOrm = await createTestOrm(dbService.connection);
|
||||
const auditLogRepo = mcpOrm.repo(AuditLog);
|
||||
const usersRepo = mcpOrm.repo(Users);
|
||||
const inviteTokensRepo = mcpOrm.repo(InviteTokens);
|
||||
@@ -206,6 +211,7 @@ export async function createMcpTestRegistry(): Promise<McpRegistry> {
|
||||
new UnsplashService(appSettings, usersRepo, new RuntimeEnvService(), generalStorage),
|
||||
generalStorage,
|
||||
await createTestUnitOfWork(dbService.connection),
|
||||
mcpOrm.em,
|
||||
);
|
||||
const readModelService = new TripReadModelService(
|
||||
dbService, membersService, daysService, accommodationsService, budgetService,
|
||||
|
||||
@@ -119,7 +119,7 @@ export async function createPluginRpcHostFactory(dbs: DatabaseService): Promise<
|
||||
const places = new PlacesService(dbs, permissions, realtime, new MapsService(dbs, photoCache, appSettings, usersRepo), queryHelpers, unsplash, photoCache, journey, generalStorage, accommodations, await createTestUnitOfWork(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));
|
||||
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 guards = new PluginGuards(dbs, permissions, addons);
|
||||
|
||||
|
||||
@@ -24,6 +24,13 @@ import { OauthTokens } from '../../src/db/entities/OauthTokens.entity';
|
||||
import type { OauthTokensRepository } from '../../src/db/repositories/OauthTokens.repository';
|
||||
import { PasswordResetTokens } from '../../src/db/entities/PasswordResetTokens.entity';
|
||||
import type { PasswordResetTokensRepository } from '../../src/db/repositories/PasswordResetTokens.repository';
|
||||
import { Trips } from '../../src/db/entities/Trips.entity';
|
||||
import type { TripsRepository } from '../../src/db/repositories/Trips.repository';
|
||||
import { TripMembers } from '../../src/db/entities/TripMembers.entity';
|
||||
import type { TripMembersRepository } from '../../src/db/repositories/TripMembers.repository';
|
||||
import { Places } from '../../src/db/entities/Places.entity';
|
||||
import type { PlacesRepository } from '../../src/db/repositories/Places.repository';
|
||||
import { DatabaseService } from '../../src/nest/database/database.service';
|
||||
|
||||
const perHandle = new WeakMap<Database.Database, Promise<UnitOfWork>>();
|
||||
const appSettingsPerHandle = new WeakMap<Database.Database, Promise<AppSettingsRepository>>();
|
||||
@@ -277,3 +284,61 @@ export function createTestPasswordResetTokensRepo(db: Database.Database): Promis
|
||||
passwordResetTokensPerHandle.set(db, pending);
|
||||
return pending;
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Plan 3c Task 0b (trip-access primitives onto TripsRepository/
|
||||
// TripMembersRepository/PlacesRepository) — appended at the end per the
|
||||
// task's own file-ownership rule. Same memoisation-per-handle pattern as
|
||||
// every helper above.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
const tripsRepoPerHandle = new WeakMap<Database.Database, Promise<TripsRepository>>();
|
||||
const tripMembersRepoPerHandle = new WeakMap<Database.Database, Promise<TripMembersRepository>>();
|
||||
const placesRepoPerHandle = new WeakMap<Database.Database, Promise<PlacesRepository>>();
|
||||
|
||||
/** The `TripsRepository` a hand-constructed `DatabaseService`/`TripAccessGuard` double needs. */
|
||||
export function createTestTripsRepo(db: Database.Database): Promise<TripsRepository> {
|
||||
const existing = tripsRepoPerHandle.get(db);
|
||||
if (existing !== undefined) return existing;
|
||||
const pending = sharedTestOrm(db).then((t) => t.repo(Trips));
|
||||
tripsRepoPerHandle.set(db, pending);
|
||||
return pending;
|
||||
}
|
||||
|
||||
/** The `TripMembersRepository` `DatabaseService.rosterUserIds` needs. */
|
||||
export function createTestTripMembersRepo(db: Database.Database): Promise<TripMembersRepository> {
|
||||
const existing = tripMembersRepoPerHandle.get(db);
|
||||
if (existing !== undefined) return existing;
|
||||
const pending = sharedTestOrm(db).then((t) => t.repo(TripMembers));
|
||||
tripMembersRepoPerHandle.set(db, pending);
|
||||
return pending;
|
||||
}
|
||||
|
||||
/** The `PlacesRepository` `DatabaseService.getPlaceWithTags` needs. */
|
||||
export function createTestPlacesRepo(db: Database.Database): Promise<PlacesRepository> {
|
||||
const existing = placesRepoPerHandle.get(db);
|
||||
if (existing !== undefined) return existing;
|
||||
const pending = sharedTestOrm(db).then((t) => t.repo(Places));
|
||||
placesRepoPerHandle.set(db, pending);
|
||||
return pending;
|
||||
}
|
||||
|
||||
/**
|
||||
* A `DatabaseService` bound to the suite's own better-sqlite3 handle, with
|
||||
* its `EntityManager` wired through `sharedTestOrm` — so `canAccessTrip`/
|
||||
* `isOwner`/`rosterUserIds`/`getPlaceWithTags` (Plan 3c Task 0b, now
|
||||
* repository-backed rather than delegating to `db/database.ts`'s deleted
|
||||
* free functions) work on a directly-constructed instance the same way they
|
||||
* do through Nest DI in production. `sharedTestOrm`'s ORM is built with
|
||||
* `allowGlobalContext: true` (`test-orm.ts`), so these calls need no
|
||||
* `withRequestContext` wrapper here, unlike production.
|
||||
*
|
||||
* A file that only needs `new DatabaseService(db)`'s other methods
|
||||
* (`prepare`/`get`/`all`/`run`) does not need this helper — the
|
||||
* `EntityManager` constructor argument is `@Optional()`, and only these four
|
||||
* methods use it.
|
||||
*/
|
||||
export async function createTestDatabaseService(db: Database.Database): Promise<DatabaseService> {
|
||||
const t = await sharedTestOrm(db);
|
||||
return new DatabaseService(db, t.em);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,76 @@
|
||||
/**
|
||||
* `runDemoSeed()` (`src/demo/demo-seed.ts`, called from `runSchemaBootstrap`
|
||||
* in `db/orm.ts:44`) used to run before the first `withRequestContext`
|
||||
* (`bootstrap.ts:108`) and on every restore (`db/orm.ts`'s reinitialize
|
||||
* hook) — entirely raw `better-sqlite3`, so nothing needed a context yet,
|
||||
* but the seed's own docstring pre-authorised the fix: "The domain phase
|
||||
* that gives demo seeding a repository read must wrap it in
|
||||
* `withRequestContext` then." `runSchemaBootstrap` (Plan 3c Task 0b) now
|
||||
* wraps the call in `withRequestContext(orm, …)`, the one place it already
|
||||
* has `orm` in scope — covering both of `runDemoSeed`'s real call sites
|
||||
* (`bootstrap.ts`'s initial boot and `attachOrm`'s restore hook) with no
|
||||
* second wrapper to keep in sync.
|
||||
*
|
||||
* This is a real `DEMO_MODE` boot proving that wrap doesn't change what the
|
||||
* seed does and logs no missing-context error, so Plan 3i's demo
|
||||
* conversion never meets a second C1 (task-6-review-parity.md's boot-sweep
|
||||
* finding).
|
||||
*
|
||||
* Deliberately does NOT `vi.mock('../../src/db/database', ...)` the way
|
||||
* most integration suites do: `buildDbMock`'s `runDemoSeed` is a
|
||||
* deliberate no-op stub (tests/helpers/db-mock.ts) so that suite's app
|
||||
* boots don't carry demo-seed side effects — which would silence the very
|
||||
* thing this file exists to exercise. `NODE_ENV=test` already gives
|
||||
* `db/database.ts`'s own singleton an isolated, per-worker `:memory:` copy
|
||||
* of the migrated schema snapshot (`readSchemaSnapshot()`), so no mock is
|
||||
* needed here at all.
|
||||
*
|
||||
* `seedDemoData`'s own `require('../demo/demo-seed')` (a deliberate dynamic
|
||||
* require — see that file's docstring) cannot resolve under vitest's SWC
|
||||
* transform (`Cannot find module '../demo/demo-seed'`), independent of this
|
||||
* task and of request-context wrapping — `runDemoSeed`'s own try/catch
|
||||
* already swallows that into a `[Demo] Seed error:` log line in every
|
||||
* environment, wrapped or not, which is exactly why `buildDbMock` stubs the
|
||||
* whole function out for every other suite rather than letting it run for
|
||||
* real. This test cannot assert the demo user was actually created for that
|
||||
* reason (a genuine environment gap, not a functional one — production runs
|
||||
* compiled Node, not a vitest/SWC transform); what it CAN and does assert is
|
||||
* the one thing Task 0b changed: no `cannotUseGlobalContext`/"global
|
||||
* EntityManager" line reaches the log, and the pre-existing module-resolution
|
||||
* failure is what actually gets logged instead, unchanged by the wrap.
|
||||
*/
|
||||
import { describe, it, expect, afterEach, vi } from 'vitest';
|
||||
import type { INestApplication } from '@nestjs/common';
|
||||
import { buildApp } from '../../src/bootstrap';
|
||||
|
||||
describe('runDemoSeed runs inside a request context (Plan 3c Task 0b)', () => {
|
||||
let app: INestApplication | undefined;
|
||||
const prevDemo = process.env.DEMO_MODE;
|
||||
|
||||
afterEach(async () => {
|
||||
await app?.close();
|
||||
app = undefined;
|
||||
if (prevDemo === undefined) delete process.env.DEMO_MODE;
|
||||
else process.env.DEMO_MODE = prevDemo;
|
||||
});
|
||||
|
||||
it('DEMO-SEED-001: a DEMO_MODE boot runs the seed inside a request context (no cannotUseGlobalContext)', async () => {
|
||||
process.env.DEMO_MODE = 'true';
|
||||
const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
|
||||
try {
|
||||
app = await buildApp();
|
||||
|
||||
// runDemoSeed ran at all (proven by SOME [Demo] line reaching the log —
|
||||
// in this environment always the pre-existing require() gap above, but
|
||||
// ANY [Demo] line proves the function was invoked, not skipped).
|
||||
const demoLines = errSpy.mock.calls.map((args) => args.map(String).join(' ')).filter((line) => line.includes('[Demo]'));
|
||||
expect(demoLines.length).toBeGreaterThan(0);
|
||||
|
||||
// The one thing this task could regress: a missing request context.
|
||||
const suspicious = demoLines.filter((line) => /cannotUseGlobalContext|global EntityManager/i.test(line));
|
||||
expect(suspicious).toEqual([]);
|
||||
} finally {
|
||||
errSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -49,11 +49,13 @@ vi.mock('../../src/websocket', () => ({ broadcast: vi.fn(), broadcastToUser: vi.
|
||||
|
||||
import { db as testDb } from '../../src/db/database';
|
||||
import { MikroORM } from '@mikro-orm/core';
|
||||
import { SchedulerRegistry } from '@nestjs/schedule';
|
||||
import request from 'supertest';
|
||||
import type { Application } from 'express';
|
||||
import { buildApp } from '../../src/bootstrap';
|
||||
import { CronRegistrarService } from '../../src/nest/scheduling/cron-registrar.service';
|
||||
import { PluginRuntimeService } from '../../src/nest/plugins/plugin-runtime.service';
|
||||
import { PlaceShadowRetentionJob } from '../../src/nest/place-shadow/place-shadow.job';
|
||||
import { StorageService } from '../../src/nest/storage/storage.service';
|
||||
import { Users } from '../../src/db/entities/Users.entity';
|
||||
import { createUser } from '../helpers/factories';
|
||||
@@ -130,4 +132,45 @@ describe('ORM request-context seams populated in production', () => {
|
||||
spy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
/**
|
||||
* Plan 3c Task 0b (R9, inventory §12b): `PlaceShadowRetentionJob` only
|
||||
* `register()`s a nightly tick — it has no `runOnBoot` sweep, so
|
||||
* `boot-sweeps-request-context.test.ts`'s BOOT-SWEEP-001 cannot see it at
|
||||
* all. `CronRegistrarService.register()` already wraps every registered
|
||||
* tick in `withRequestContext` unconditionally (fail-closed on a missing
|
||||
* `orm`, `cron-registrar.service.ts:87-92`) — but nothing before this task
|
||||
* drove THIS job's own `onApplicationBootstrap` against the real,
|
||||
* production-wired registrar to prove the whole path (register → a real
|
||||
* `cron` `CronJob` lands in `SchedulerRegistry` → firing its tick resolves
|
||||
* with no missing-context error) actually holds, the Plan 3a `*-003`
|
||||
* per-job pattern for a register-only job.
|
||||
*/
|
||||
it('SEAM-003: PlaceShadowRetentionJob registers against the real CronRegistrarService, and firing its tick runs inside the same request context with no missing-context error', async () => {
|
||||
const registrar = app.get(CronRegistrarService);
|
||||
const isEnabledSpy = vi.spyOn(registrar, 'isEnabled').mockReturnValue(true);
|
||||
const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
|
||||
try {
|
||||
testDb.prepare(`
|
||||
INSERT INTO place_shadow_picks (created_at, query, source, live_rank, live_count, picked_name, picked_lat, picked_lng)
|
||||
VALUES (datetime('now', '-999 days'), 'seam-003', 'nominatim', 1, 1, 'expired pick', 0, 0)
|
||||
`).run();
|
||||
|
||||
app.get(PlaceShadowRetentionJob).onApplicationBootstrap();
|
||||
const job = app.get(SchedulerRegistry).getCronJob('place-shadow-retention');
|
||||
await job.fireOnTick();
|
||||
|
||||
const suspicious = errSpy.mock.calls
|
||||
.map((args) => args.map(String).join(' '))
|
||||
.filter((line) => /cannotUseGlobalContext|global EntityManager/i.test(line));
|
||||
expect(suspicious).toEqual([]);
|
||||
|
||||
const remaining = testDb.prepare("SELECT COUNT(*) as n FROM place_shadow_picks WHERE query = 'seam-003'").get() as { n: number };
|
||||
expect(remaining.n).toBe(0);
|
||||
} finally {
|
||||
registrar.unregister('place-shadow-retention');
|
||||
isEnabledSpy.mockRestore();
|
||||
errSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,19 +1,31 @@
|
||||
/**
|
||||
* Parity guards for the Plan 3c Task 0a async sweep.
|
||||
* Parity guards for the Plan 3c Task 0a async sweep, extended by Task 0b.
|
||||
*
|
||||
* `canAccessTrip`, `isOwner`, `rosterUserIds` and `getPlaceWithTags`
|
||||
* (`src/db/database.ts` + their `DatabaseService` delegations) went from
|
||||
* synchronous to `async` in that task, with every caller updated to `await`
|
||||
* them. These four tests each drive a REAL guarded route through the real
|
||||
* `buildApp()` DI graph — one per primitive — as owner / member / non-member
|
||||
* / anonymous, and pin the legacy status + body so a later task (0b: moving
|
||||
* the bodies onto `TripsRepository`) cannot silently change behaviour.
|
||||
* `canAccessTrip`, `isOwner`, `rosterUserIds` and `getPlaceWithTags` went
|
||||
* from synchronous to `async` in Task 0a (with every caller updated to
|
||||
* `await` them), still delegating to `src/db/database.ts`'s free functions.
|
||||
* Task 0b deleted those free functions and moved the bodies onto
|
||||
* `TripsRepository.findAccessible`/`isOwner`, `TripMembersRepository
|
||||
* .rosterUserIds` and `PlacesRepository.findWithTagsAndRatings` — these four
|
||||
* tests still drive a REAL guarded route through the real `buildApp()` DI
|
||||
* graph, one per primitive, as owner / member / non-member / anonymous, and
|
||||
* still pin the legacy status + body, but now against the real repository
|
||||
* path rather than a stand-in.
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest';
|
||||
import request from 'supertest';
|
||||
import type { Application } from 'express';
|
||||
import type { INestApplication } from '@nestjs/common';
|
||||
|
||||
// task-0a-review F4: this mock no longer intercepts any of the four
|
||||
// primitives — `db/database.ts` holds none of their bodies any more, so
|
||||
// `buildDbMock`'s own `canAccessTrip`/`isOwner`/`getPlaceWithTags`
|
||||
// properties are dead code for this file's purposes. It stays only for what
|
||||
// every other integration/e2e/WS suite in this program uses it for: an
|
||||
// isolated snapshot db that `getRawConnection`/`db` swap the shared
|
||||
// better-sqlite3 handle to, which the REAL `MikroORM` binds to underneath
|
||||
// (`db/orm-driver.ts`) — so `TripsRepository`/`TripMembersRepository`
|
||||
// /`PlacesRepository` read the SAME rows this file seeds through `testDb`.
|
||||
vi.mock('../../src/db/database', async () => {
|
||||
const { createSnapshotTestDb, buildDbMock } = await import('../helpers/db-mock');
|
||||
return buildDbMock(createSnapshotTestDb());
|
||||
@@ -148,6 +160,43 @@ describe('isOwner (async) — the delete_trip MCP tool', () => {
|
||||
expect(testDb.prepare('SELECT id FROM trips WHERE id = ?').get(trip.id)).toBeUndefined();
|
||||
});
|
||||
|
||||
/**
|
||||
* Plan 3c Task 0b (R9): `isOwner`'s only real caller anywhere is this MCP
|
||||
* tool (inventory §0d), and `TripsService.isOwner` → `DatabaseService
|
||||
* .isOwner` is now `TripsRepository.isOwner` underneath — a repository
|
||||
* read reached through the MCP transport's request context (`nest-mcp
|
||||
* /registry.ts:286-294`: `@mikro-orm/nestjs`'s `registerRequestContext`
|
||||
* middleware forks a context for every `/mcp` route, same as any other
|
||||
* Nest route). PRIM-ISOWN-001 above already proves the happy path (the
|
||||
* trip is actually deleted, which requires the repository read to
|
||||
* succeed); this is the explicit MCP-CTX-style structural assertion
|
||||
* (`mcp.test.ts`'s MCP-CTX-001/002 pattern) that no missing-context error
|
||||
* was swallowed along the way.
|
||||
*/
|
||||
it('PRIM-ISOWN-005 — the repository read inside isOwner runs inside the /mcp request context (MCP-CTX style, no cannotUseGlobalContext)', async () => {
|
||||
const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
|
||||
try {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
const sessionId = await mcpSession(owner.id);
|
||||
|
||||
const res = await request(app)
|
||||
.post('/mcp')
|
||||
.set('Authorization', `Bearer ${generateToken(owner.id)}`)
|
||||
.set('mcp-session-id', sessionId)
|
||||
.set('Accept', 'application/json, text/event-stream')
|
||||
.send(deleteTripBody(trip.id));
|
||||
expect(res.status).toBe(200);
|
||||
|
||||
const suspicious = errSpy.mock.calls
|
||||
.map((args) => args.map(String).join(' '))
|
||||
.filter((line) => /cannotUseGlobalContext|global EntityManager/i.test(line));
|
||||
expect(suspicious).toEqual([]);
|
||||
} finally {
|
||||
errSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it('PRIM-ISOWN-002 — member (not owner): refused with the same "no access" message as a stranger, trip untouched', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: member } = createUser(testDb);
|
||||
@@ -279,19 +328,34 @@ describe('getPlaceWithTags (async) — GET /api/trips/:tripId/places/:id', () =>
|
||||
it('PRIM-GPWT-001 — owner: 200 with the hydrated place (category/tags/ratings shape)', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
// createPlace defaults category_id to the first seeded category when no
|
||||
// override is given (tests/helpers/factories.ts) — read back whatever it
|
||||
// actually assigned rather than assuming none, so this pins the real
|
||||
// production shape instead of a factory default that happens to be null.
|
||||
const place = createPlace(testDb, trip.id, { name: 'Eiffel Tower' });
|
||||
const expectedCategory = place.category_id
|
||||
? testDb.prepare('SELECT id, name, color, icon FROM categories WHERE id = ?').get(place.category_id)
|
||||
: null;
|
||||
|
||||
const res = await request(app).get(`/api/trips/${trip.id}/places/${place.id}`).set('Cookie', authCookie(owner.id));
|
||||
expect(res.status).toBe(200);
|
||||
// The mocked db/database module's getPlaceWithTags stand-in (tests/helpers/db-mock.ts)
|
||||
// only carries category + tags, not the ratings aggregate — matched here, not the
|
||||
// production shape (§4 of the test-db.ts header explains the mock/real split).
|
||||
// Plan 3c Task 0b: `getPlaceWithTags` is `PlacesRepository
|
||||
// .findWithTagsAndRatings` now, reached for real (this file's `vi.mock`
|
||||
// of `src/db/database` only swaps the underlying connection to an
|
||||
// isolated snapshot db — see the top-of-file note — it no longer stands
|
||||
// in for the primitive itself, which db/database.ts doesn't hold any
|
||||
// more). The full production shape, including the ratings aggregate the
|
||||
// old mocked stand-in omitted, is asserted directly rather than only a
|
||||
// subset.
|
||||
expect(res.body.place).toMatchObject({
|
||||
id: place.id,
|
||||
name: 'Eiffel Tower',
|
||||
category: expectedCategory,
|
||||
tags: [],
|
||||
ratings: [],
|
||||
rating_avg: null,
|
||||
rating_count: 0,
|
||||
});
|
||||
expect(res.body.place).toHaveProperty('category');
|
||||
});
|
||||
|
||||
it('PRIM-GPWT-002 — member: 200 with the same hydrated place', async () => {
|
||||
|
||||
@@ -33,7 +33,7 @@ function build(opts: { addonOn?: boolean; canEdit?: boolean; missing?: boolean }
|
||||
} as unknown as BudgetService & Record<string, ReturnType<typeof vi.fn>>;
|
||||
const realtime = { broadcast: vi.fn() } as unknown as RealtimeService & { broadcast: ReturnType<typeof vi.fn> };
|
||||
const db = {
|
||||
canAccessTrip: vi.fn((tripId: number, userId: number) => (tripId === 1 && userId === 42 ? { id: 1, user_id: 42 } : undefined)),
|
||||
canAccessTrip: vi.fn(async (tripId: number, userId: number) => (tripId === 1 && userId === 42 ? { id: 1, user_id: 42 } : undefined)),
|
||||
prepare: vi.fn(() => ({ get: () => ({ role: 'user' }) })),
|
||||
} as unknown as DatabaseService;
|
||||
const guards = new PluginGuards(
|
||||
|
||||
@@ -35,7 +35,7 @@ function build(canEdit = true) {
|
||||
} as unknown as DayNotesService & Record<string, ReturnType<typeof vi.fn>>;
|
||||
const realtime = { broadcast: vi.fn() } as unknown as RealtimeService & { broadcast: ReturnType<typeof vi.fn> };
|
||||
const db = {
|
||||
canAccessTrip: vi.fn((tripId: number, userId: number) => (tripId === 1 && userId === 42 ? { id: 1, user_id: 42 } : undefined)),
|
||||
canAccessTrip: vi.fn(async (tripId: number, userId: number) => (tripId === 1 && userId === 42 ? { id: 1, user_id: 42 } : undefined)),
|
||||
prepare: vi.fn(() => ({ get: () => ({ role: 'user' }) })),
|
||||
} as unknown as DatabaseService;
|
||||
const guards = new PluginGuards(
|
||||
|
||||
@@ -44,7 +44,7 @@ function build(opts: { canEdit?: boolean } = {}) {
|
||||
const realtime = { broadcast: vi.fn() } as unknown as RealtimeService & { broadcast: ReturnType<typeof vi.fn> };
|
||||
const guards = new PluginGuards(
|
||||
{
|
||||
canAccessTrip: vi.fn((tripId: number, userId: number) => (tripId === 1 && userId === 42 ? { id: 1, user_id: 42 } : undefined)),
|
||||
canAccessTrip: vi.fn(async (tripId: number, userId: number) => (tripId === 1 && userId === 42 ? { id: 1, user_id: 42 } : undefined)),
|
||||
prepare: vi.fn(() => ({ get: () => ({ role: 'user' }) })),
|
||||
} as unknown as DatabaseService,
|
||||
{ checkPermission: vi.fn(() => opts.canEdit ?? true) } as unknown as PermissionsService,
|
||||
|
||||
@@ -5,7 +5,20 @@ import { resetTestDb } from '../../../helpers/test-db';
|
||||
import { createTestOrm, type TestOrm } from '../../../helpers/test-orm';
|
||||
import { createUser } from '../../../helpers/factories';
|
||||
import { Users } from '../../../../src/db/entities/Users.entity';
|
||||
import { columnIncrementedBy, columnRef, currentTimestamp, dateAdd, dateOf, lower, lowerParam } from '../../../../src/db/dialect/sql-functions';
|
||||
import {
|
||||
absDifference,
|
||||
coalesce,
|
||||
coalesceParam,
|
||||
columnIncrementedBy,
|
||||
columnRef,
|
||||
currentTimestamp,
|
||||
dateAdd,
|
||||
dateOf,
|
||||
lower,
|
||||
lowerParam,
|
||||
nowMinusDays,
|
||||
trim,
|
||||
} from '../../../../src/db/dialect/sql-functions';
|
||||
|
||||
const testDb = createSnapshotTestDb();
|
||||
let t: TestOrm;
|
||||
@@ -219,4 +232,98 @@ describe('sql-functions (sqlite)', () => {
|
||||
const none = await t.em.findOne(Users, { [lower(platform, 'email')]: lowerParam(platform, 'nobody@example.com') });
|
||||
expect(none).toBeNull();
|
||||
});
|
||||
|
||||
// Plan 3c Task 0b (R6): no consumer yet — Tasks 1–8 wire these in as each
|
||||
// converts the statement that needs them. Pinned here so a later task
|
||||
// imports a tested helper instead of writing one inline.
|
||||
|
||||
it("SQLF-016: nowMinusDays renders datetime('now', '-N days'), matching a JS-computed date within a few seconds' tolerance", async () => {
|
||||
const row = testDb.prepare(`SELECT ${nowMinusDays(t.em.getPlatform(), 5).sql} as d`).get() as { d: string };
|
||||
const got = new Date(`${row.d.replace(' ', 'T')}Z`).getTime();
|
||||
const expected = Date.now() - 5 * 24 * 60 * 60 * 1000;
|
||||
expect(Math.abs(got - expected)).toBeLessThan(10_000);
|
||||
});
|
||||
|
||||
it('SQLF-016b: nowMinusDays(0) is "now", not one day back', async () => {
|
||||
const row = testDb.prepare(`SELECT ${nowMinusDays(t.em.getPlatform(), 0).sql} as d`).get() as { d: string };
|
||||
const got = new Date(`${row.d.replace(' ', 'T')}Z`).getTime();
|
||||
expect(Math.abs(got - Date.now())).toBeLessThan(10_000);
|
||||
});
|
||||
|
||||
it('SQLF-017: nowMinusDays rejects a non-integer or negative day count', () => {
|
||||
expect(() => nowMinusDays(t.em.getPlatform(), 1.5)).toThrow(/non-negative integer day count/);
|
||||
expect(() => nowMinusDays(t.em.getPlatform(), -1)).toThrow(/non-negative integer day count/);
|
||||
expect(() => nowMinusDays(t.em.getPlatform(), Number.NaN)).toThrow(/non-negative integer day count/);
|
||||
});
|
||||
|
||||
it('SQLF-018: trim strips leading/trailing whitespace', async () => {
|
||||
const { user } = createUser(testDb, { username: ' Padded Name ' });
|
||||
const platform = t.em.getPlatform();
|
||||
const row = await t.em.createQueryBuilder(Users, 'u')
|
||||
.select([trim(platform, 'u.username').as('n')])
|
||||
.where({ id: user.id })
|
||||
.execute('get', false);
|
||||
expect((row as { n: string }).n).toBe('Padded Name');
|
||||
});
|
||||
|
||||
it('SQLF-019: coalesce(ref, fallbackRef) picks the first non-null column, else the second', async () => {
|
||||
const { user: withName } = createUser(testDb, { username: 'has-display-name' });
|
||||
testDb.prepare('UPDATE users SET display_name = ? WHERE id = ?').run('Display Name', withName.id);
|
||||
const { user: withoutName } = createUser(testDb, { username: 'no-display-name' });
|
||||
const platform = t.em.getPlatform();
|
||||
|
||||
const a = await t.em.createQueryBuilder(Users, 'u')
|
||||
.select([coalesce(platform, 'u.display_name', 'u.username').as('name')])
|
||||
.where({ id: withName.id })
|
||||
.execute('get', false);
|
||||
expect((a as { name: string }).name).toBe('Display Name');
|
||||
|
||||
const b = await t.em.createQueryBuilder(Users, 'u')
|
||||
.select([coalesce(platform, 'u.display_name', 'u.username').as('name')])
|
||||
.where({ id: withoutName.id })
|
||||
.execute('get', false);
|
||||
expect((b as { name: string }).name).toBe('no-display-name');
|
||||
});
|
||||
|
||||
it('SQLF-020: coalesceParam(ref, value) writes COALESCE(col, ?) with the fallback bound, not another column', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const platform = t.em.getPlatform();
|
||||
await t.em.createQueryBuilder(Users, 'u')
|
||||
.update({ display_name: coalesceParam(platform, 'display_name', 'Fallback Name') })
|
||||
.where({ id: user.id })
|
||||
.execute('run');
|
||||
t.clear();
|
||||
expect((testDb.prepare('SELECT display_name FROM users WHERE id = ?').get(user.id) as { display_name: string }).display_name).toBe('Fallback Name');
|
||||
|
||||
await t.em.createQueryBuilder(Users, 'u')
|
||||
.update({ display_name: coalesceParam(platform, 'display_name', 'Never Used') })
|
||||
.where({ id: user.id })
|
||||
.execute('run');
|
||||
t.clear();
|
||||
expect((testDb.prepare('SELECT display_name FROM users WHERE id = ?').get(user.id) as { display_name: string }).display_name).toBe('Fallback Name');
|
||||
});
|
||||
|
||||
it('SQLF-021: absDifference is usable as a filter key, matching ABS(col - ?) <= tolerance', async () => {
|
||||
const { user: near } = createUser(testDb);
|
||||
testDb.prepare('UPDATE users SET login_count = 12 WHERE id = ?').run(near.id);
|
||||
const { user: far } = createUser(testDb);
|
||||
testDb.prepare('UPDATE users SET login_count = 100 WHERE id = ?').run(far.id);
|
||||
const platform = t.em.getPlatform();
|
||||
|
||||
const rows = await t.em.find(Users, { [absDifference(platform, 'login_count', 10)]: { $lte: 5 } });
|
||||
expect(rows.map((r) => r.id)).toEqual([near.id]);
|
||||
|
||||
const none = await t.em.find(Users, { [absDifference(platform, 'login_count', 10)]: { $lte: 0 } });
|
||||
expect(none).toEqual([]);
|
||||
});
|
||||
|
||||
it('SQLF-022: an unknown platform fails closed for every Plan 3c Task 0b helper', () => {
|
||||
class FakePlatform extends Platform {}
|
||||
const foreign = new FakePlatform();
|
||||
expect(() => nowMinusDays(foreign, 1)).toThrow(/no implementation for platform FakePlatform/);
|
||||
expect(() => trim(foreign, 'u.name')).toThrow(/no implementation for platform FakePlatform/);
|
||||
expect(() => coalesce(foreign, 'u.a', 'u.b')).toThrow(/no implementation for platform FakePlatform/);
|
||||
expect(() => coalesceParam(foreign, 'u.a', 'x')).toThrow(/no implementation for platform FakePlatform/);
|
||||
expect(() => absDifference(foreign, 'u.lat', 1)).toThrow(/no implementation for platform FakePlatform/);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,98 @@
|
||||
/**
|
||||
* PlacesRepository.findWithTagsAndRatings (Plan 3c Task 0b): the legacy
|
||||
* `getPlaceWithTags`'s three statements (place+category, tags, ratings),
|
||||
* merged the same way. Real rows on the real test DB, including the
|
||||
* non-empty tags/ratings shape neither `trip-access-primitives.test.ts`'s
|
||||
* PRIM-GPWT-00x (empty tags/ratings only) nor any other suite exercises.
|
||||
*/
|
||||
import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest';
|
||||
import { createSnapshotTestDb } from '../../../helpers/db-mock';
|
||||
import { resetTestDb } from '../../../helpers/test-db';
|
||||
import { createTestOrm, type TestOrm } from '../../../helpers/test-orm';
|
||||
import { createCategory, createPlace, createTag, createTrip, createUser } from '../../../helpers/factories';
|
||||
import { Places } from '../../../../src/db/entities/Places.entity';
|
||||
import type { PlacesRepository } from '../../../../src/db/repositories/Places.repository';
|
||||
|
||||
const testDb = createSnapshotTestDb();
|
||||
let t: TestOrm;
|
||||
let places: PlacesRepository;
|
||||
|
||||
beforeAll(async () => {
|
||||
t = await createTestOrm(testDb);
|
||||
places = t.repo(Places);
|
||||
});
|
||||
beforeEach(() => { resetTestDb(testDb); t.clear(); });
|
||||
afterAll(async () => { await t.close(); testDb.close(); });
|
||||
|
||||
describe('PlacesRepository.findWithTagsAndRatings — parity with the legacy getPlaceWithTags', () => {
|
||||
it('PLACEREPO-001: no category, no tags, no ratings — null category, empty arrays, null average', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
// category_id: null override defeats createPlace's default-first-category fallback.
|
||||
testDb.prepare('DELETE FROM categories').run();
|
||||
const place = createPlace(testDb, trip.id, { name: 'Uncategorized Spot' });
|
||||
|
||||
const result = await places.findWithTagsAndRatings(place.id);
|
||||
expect(result).toMatchObject({
|
||||
id: place.id,
|
||||
name: 'Uncategorized Spot',
|
||||
category: null,
|
||||
tags: [],
|
||||
ratings: [],
|
||||
rating_avg: null,
|
||||
rating_count: 0,
|
||||
});
|
||||
});
|
||||
|
||||
it('PLACEREPO-002: a category joins in by name/color/icon, not just its id', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
const category = createCategory(testDb, { name: 'Museums', color: '#123456', icon: '🏛️' });
|
||||
const place = createPlace(testDb, trip.id, { category_id: category.id });
|
||||
|
||||
const result = await places.findWithTagsAndRatings(place.id);
|
||||
expect(result?.category).toEqual({ id: category.id, name: 'Museums', color: '#123456', icon: '🏛️' });
|
||||
});
|
||||
|
||||
it('PLACEREPO-003: tags attach through place_tags, each one the full tags row', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
const place = createPlace(testDb, trip.id);
|
||||
const tagA = createTag(testDb, owner.id, { name: 'Must see' });
|
||||
const tagB = createTag(testDb, owner.id, { name: 'Rainy day' });
|
||||
testDb.prepare('INSERT INTO place_tags (place_id, tag_id) VALUES (?, ?)').run(place.id, tagA.id);
|
||||
testDb.prepare('INSERT INTO place_tags (place_id, tag_id) VALUES (?, ?)').run(place.id, tagB.id);
|
||||
|
||||
const result = await places.findWithTagsAndRatings(place.id);
|
||||
expect(result?.tags).toHaveLength(2);
|
||||
expect(result?.tags.map((tg) => tg.name).sort()).toEqual(['Must see', 'Rainy day']);
|
||||
});
|
||||
|
||||
it('PLACEREPO-004: ratings hydrate with the voter\'s username/avatar, in created_at order, and average correctly', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: voterA } = createUser(testDb, { username: 'voter_a' });
|
||||
const { user: voterB } = createUser(testDb, { username: 'voter_b' });
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
const place = createPlace(testDb, trip.id);
|
||||
testDb
|
||||
.prepare("INSERT INTO place_ratings (place_id, user_id, rating, created_at) VALUES (?, ?, ?, datetime('now', '-2 minutes'))")
|
||||
.run(place.id, voterA.id, 4);
|
||||
testDb
|
||||
.prepare("INSERT INTO place_ratings (place_id, user_id, rating, created_at) VALUES (?, ?, ?, datetime('now', '-1 minutes'))")
|
||||
.run(place.id, voterB.id, 2);
|
||||
|
||||
const result = await places.findWithTagsAndRatings(place.id);
|
||||
expect(result?.ratings).toHaveLength(2);
|
||||
expect(result?.ratings.map((r) => r.username)).toEqual(['voter_a', 'voter_b']);
|
||||
expect(result?.rating_avg).toBe(3);
|
||||
expect(result?.rating_count).toBe(2);
|
||||
});
|
||||
|
||||
it('PLACEREPO-005: a missing place id resolves to null, not a throw', async () => {
|
||||
expect(await places.findWithTagsAndRatings(999999)).toBeNull();
|
||||
});
|
||||
|
||||
it('PLACEREPO-006: a non-numeric-looking id resolves to null, the same raw-bind seam TripsRepository documents', async () => {
|
||||
expect(await places.findWithTagsAndRatings('abc')).toBeNull();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,84 @@
|
||||
/**
|
||||
* TripMembersRepository.rosterUserIds (Plan 3c Task 0b): the legacy
|
||||
* `DatabaseService.rosterUserIds` statement in intent — `SELECT user_id FROM
|
||||
* trip_members WHERE trip_id = ? UNION SELECT user_id FROM trips WHERE id =
|
||||
* ?` — as two reads merged through a `Set`. Real rows, on the real test DB,
|
||||
* through the same `TestOrm` harness `Trips.repository.test.ts` uses.
|
||||
*/
|
||||
import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest';
|
||||
import { createSnapshotTestDb } from '../../../helpers/db-mock';
|
||||
import { resetTestDb } from '../../../helpers/test-db';
|
||||
import { createTestOrm, type TestOrm } from '../../../helpers/test-orm';
|
||||
import { addTripMember, createTrip, createUser } from '../../../helpers/factories';
|
||||
import { TripMembers } from '../../../../src/db/entities/TripMembers.entity';
|
||||
import type { TripMembersRepository } from '../../../../src/db/repositories/TripMembers.repository';
|
||||
|
||||
const testDb = createSnapshotTestDb();
|
||||
let t: TestOrm;
|
||||
let tripMembers: TripMembersRepository;
|
||||
|
||||
beforeAll(async () => {
|
||||
t = await createTestOrm(testDb);
|
||||
tripMembers = t.repo(TripMembers);
|
||||
});
|
||||
beforeEach(() => { resetTestDb(testDb); t.clear(); });
|
||||
afterAll(async () => { await t.close(); testDb.close(); });
|
||||
|
||||
const legacyRoster = (tripId: number): Set<number> => {
|
||||
const rows = testDb
|
||||
.prepare('SELECT user_id FROM trip_members WHERE trip_id = ? UNION SELECT user_id FROM trips WHERE id = ?')
|
||||
.all(tripId, tripId) as { user_id: number }[];
|
||||
return new Set(rows.map((r) => r.user_id));
|
||||
};
|
||||
|
||||
describe('TripMembersRepository.rosterUserIds — parity with the legacy UNION statement', () => {
|
||||
it('TMEMREPO-001: an owner-only trip (no members row) still yields the owner', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
expect(await tripMembers.rosterUserIds(trip.id)).toEqual(new Set([owner.id]));
|
||||
expect(await tripMembers.rosterUserIds(trip.id)).toEqual(legacyRoster(trip.id));
|
||||
});
|
||||
|
||||
it('TMEMREPO-002: owner + members are deduped into one set, in either read order', 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);
|
||||
|
||||
const roster = await tripMembers.rosterUserIds(trip.id);
|
||||
expect(roster).toEqual(new Set([owner.id, memberA.id, memberB.id]));
|
||||
expect(roster).toEqual(legacyRoster(trip.id));
|
||||
});
|
||||
|
||||
it('TMEMREPO-003: a member row for the owner (edge case) still dedupes to one id, not a Set of size 2', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
addTripMember(testDb, trip.id, owner.id);
|
||||
const roster = await tripMembers.rosterUserIds(trip.id);
|
||||
expect(roster).toEqual(new Set([owner.id]));
|
||||
expect(roster.size).toBe(1);
|
||||
});
|
||||
|
||||
it('TMEMREPO-004: a trip belonging to someone else never leaks its roster into another trip\'s read', async () => {
|
||||
const { user: ownerA } = createUser(testDb);
|
||||
const { user: ownerB } = createUser(testDb);
|
||||
const { user: memberOfA } = createUser(testDb);
|
||||
const tripA = createTrip(testDb, ownerA.id);
|
||||
const tripB = createTrip(testDb, ownerB.id);
|
||||
addTripMember(testDb, tripA.id, memberOfA.id);
|
||||
|
||||
expect(await tripMembers.rosterUserIds(tripB.id)).toEqual(new Set([ownerB.id]));
|
||||
});
|
||||
|
||||
it('TMEMREPO-005: a missing trip id yields an empty set, not a throw', async () => {
|
||||
const missingId = 999999;
|
||||
expect(await tripMembers.rosterUserIds(missingId)).toEqual(new Set());
|
||||
expect(legacyRoster(missingId)).toEqual(new Set());
|
||||
});
|
||||
|
||||
it('TMEMREPO-006: a non-numeric-looking id finds nothing, the same raw-bind seam TripsRepository documents', async () => {
|
||||
expect(await tripMembers.rosterUserIds('abc')).toEqual(new Set());
|
||||
});
|
||||
});
|
||||
@@ -63,6 +63,14 @@ describe('TripsRepository.findAccessible — parity with canAccessTrip', () => {
|
||||
expect(await trips.isOwner(trip.id, member.id)).toBe(false);
|
||||
});
|
||||
|
||||
it('TRIPREPO-007: a missing trip id resolves to undefined, not a throw, for both findAccessible and isOwner', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const missingId = 999999;
|
||||
expect(await trips.findAccessible(missingId, user.id)).toBeUndefined();
|
||||
expect(legacy(missingId, user.id)).toBeUndefined();
|
||||
expect(await trips.isOwner(missingId, user.id)).toBe(false);
|
||||
});
|
||||
|
||||
it('TRIPREPO-006: toObject on a trip omits the hidden relation and every unloaded collection', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
|
||||
@@ -1,56 +1,77 @@
|
||||
/**
|
||||
* canAccessTrip is the shared trip-access check every domain reads its trip row
|
||||
* from. It must return the trip's `currency`: the budget settlement takes its base
|
||||
* currency straight off this row, and when the column was missing from the SELECT
|
||||
* it silently fell back to 'EUR', inflating balances on every non-EUR trip that had
|
||||
* a foreign-currency expense (#1543).
|
||||
* `TripsRepository.findAccessible` (formerly the free-function `canAccessTrip`
|
||||
* in `db/database.ts`, retired onto the repository in Plan 3c Task 0b) is the
|
||||
* shared trip-access check every domain reads its trip row from. It must
|
||||
* return the trip's `currency`: the budget settlement takes its base currency
|
||||
* straight off this row, and when the column was missing from the SELECT it
|
||||
* silently fell back to 'EUR', inflating balances on every non-EUR trip that
|
||||
* had a foreign-currency expense (#1543).
|
||||
*/
|
||||
import { describe, it, expect, beforeEach } from 'vitest';
|
||||
import { db, canAccessTrip } from '../../../src/db/database';
|
||||
import { CAN_ACCESS_TRIP_SQL, buildDbMock, createTestDb } from '../../helpers/test-db';
|
||||
import { describe, it, expect, beforeAll, beforeEach, afterAll } from 'vitest';
|
||||
import { createSnapshotTestDb } from '../../helpers/db-mock';
|
||||
import { CAN_ACCESS_TRIP_SQL, buildDbMock, createTestDb, resetTestDb } from '../../helpers/test-db';
|
||||
import { createTestOrm, type TestOrm } from '../../helpers/test-orm';
|
||||
import { Trips } from '../../../src/db/entities/Trips.entity';
|
||||
import type { TripsRepository } from '../../../src/db/repositories/Trips.repository';
|
||||
|
||||
const testDb = createSnapshotTestDb();
|
||||
let t: TestOrm;
|
||||
let trips: TripsRepository;
|
||||
|
||||
beforeAll(async () => {
|
||||
t = await createTestOrm(testDb);
|
||||
trips = t.repo(Trips);
|
||||
});
|
||||
beforeEach(() => { resetTestDb(testDb); t.clear(); });
|
||||
afterAll(async () => { await t.close(); testDb.close(); });
|
||||
|
||||
function seedUser(username: string): number {
|
||||
return Number(
|
||||
db.prepare("INSERT INTO users (username, email, password_hash, role) VALUES (?, ?, 'x', 'user')")
|
||||
testDb.prepare("INSERT INTO users (username, email, password_hash, role) VALUES (?, ?, 'x', 'user')")
|
||||
.run(username, `${username}@example.test`).lastInsertRowid,
|
||||
);
|
||||
}
|
||||
|
||||
describe('canAccessTrip', () => {
|
||||
beforeEach(() => {
|
||||
db.exec('DELETE FROM trip_members; DELETE FROM trips; DELETE FROM users;');
|
||||
});
|
||||
|
||||
describe('TripsRepository.findAccessible (formerly canAccessTrip)', () => {
|
||||
it('returns the trip currency for the owner (#1543)', async () => {
|
||||
const owner = seedUser('owner');
|
||||
const tripId = Number(
|
||||
db.prepare("INSERT INTO trips (user_id, title, currency) VALUES (?, 'Trip', 'RUB')")
|
||||
testDb.prepare("INSERT INTO trips (user_id, title, currency) VALUES (?, 'Trip', 'RUB')")
|
||||
.run(owner).lastInsertRowid,
|
||||
);
|
||||
|
||||
expect(await canAccessTrip(tripId, owner)).toMatchObject({ id: tripId, user_id: owner, currency: 'RUB' });
|
||||
expect(await trips.findAccessible(tripId, owner)).toMatchObject({ id: tripId, user_id: owner, currency: 'RUB' });
|
||||
});
|
||||
|
||||
it('returns the trip currency for a member too', async () => {
|
||||
const owner = seedUser('owner2');
|
||||
const member = seedUser('member2');
|
||||
const tripId = Number(
|
||||
db.prepare("INSERT INTO trips (user_id, title, currency) VALUES (?, 'Trip', 'JPY')")
|
||||
testDb.prepare("INSERT INTO trips (user_id, title, currency) VALUES (?, 'Trip', 'JPY')")
|
||||
.run(owner).lastInsertRowid,
|
||||
);
|
||||
db.prepare('INSERT INTO trip_members (trip_id, user_id) VALUES (?, ?)').run(tripId, member);
|
||||
testDb.prepare('INSERT INTO trip_members (trip_id, user_id) VALUES (?, ?)').run(tripId, member);
|
||||
|
||||
expect(await canAccessTrip(tripId, member)).toMatchObject({ currency: 'JPY' });
|
||||
expect(await trips.findAccessible(tripId, member)).toMatchObject({ currency: 'JPY' });
|
||||
});
|
||||
|
||||
it('returns undefined for a user with no access', async () => {
|
||||
const owner = seedUser('owner3');
|
||||
const stranger = seedUser('stranger3');
|
||||
const tripId = Number(
|
||||
db.prepare("INSERT INTO trips (user_id, title) VALUES (?, 'Trip')").run(owner).lastInsertRowid,
|
||||
testDb.prepare("INSERT INTO trips (user_id, title) VALUES (?, 'Trip')").run(owner).lastInsertRowid,
|
||||
);
|
||||
|
||||
expect(await canAccessTrip(tripId, stranger)).toBeUndefined();
|
||||
expect(await trips.findAccessible(tripId, stranger)).toBeUndefined();
|
||||
});
|
||||
|
||||
// Plan 3c Task 0b: the raw-bind id seam (no `Number()`/`toRowId` conversion
|
||||
// before the value reaches the statement) survives the repository move —
|
||||
// pinned directly here rather than only inferred from the boot matrix.
|
||||
it('a non-numeric-looking id finds nothing, the same as the legacy raw-bind statement', async () => {
|
||||
const owner = seedUser('owner4');
|
||||
testDb.prepare("INSERT INTO trips (user_id, title) VALUES (?, 'Trip')").run(owner);
|
||||
expect(await trips.findAccessible('not-a-number', owner)).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -60,20 +81,20 @@ describe('canAccessTrip', () => {
|
||||
*/
|
||||
describe('the buildDbMock stand-in for canAccessTrip', () => {
|
||||
it('hands back the trip currency, like the real one', async () => {
|
||||
const testDb = createTestDb();
|
||||
const dbmockDb = createTestDb();
|
||||
try {
|
||||
const owner = Number(
|
||||
testDb.prepare("INSERT INTO users (username, email, password_hash, role) VALUES ('m', 'm@example.test', 'x', 'user')")
|
||||
dbmockDb.prepare("INSERT INTO users (username, email, password_hash, role) VALUES ('m', 'm@example.test', 'x', 'user')")
|
||||
.run().lastInsertRowid,
|
||||
);
|
||||
const tripId = Number(
|
||||
testDb.prepare("INSERT INTO trips (user_id, title, currency) VALUES (?, 'Trip', 'ISK')")
|
||||
dbmockDb.prepare("INSERT INTO trips (user_id, title, currency) VALUES (?, 'Trip', 'ISK')")
|
||||
.run(owner).lastInsertRowid,
|
||||
);
|
||||
|
||||
expect(await buildDbMock(testDb).canAccessTrip(tripId, owner)).toMatchObject({ id: tripId, currency: 'ISK' });
|
||||
expect(await buildDbMock(dbmockDb).canAccessTrip(tripId, owner)).toMatchObject({ id: tripId, currency: 'ISK' });
|
||||
} finally {
|
||||
testDb.close();
|
||||
dbmockDb.close();
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
@@ -46,7 +46,7 @@ function build(opts: { file?: Record<string, unknown> | undefined; foreign?: str
|
||||
softDeleteFile: vi.fn(),
|
||||
} as unknown as FilesService & Record<string, ReturnType<typeof vi.fn>>;
|
||||
const db = {
|
||||
canAccessTrip: vi.fn((tripId: number, userId: number) => (tripId === 1 && userId === 42 ? { id: 1, user_id: 42 } : undefined)),
|
||||
canAccessTrip: vi.fn(async (tripId: number, userId: number) => (tripId === 1 && userId === 42 ? { id: 1, user_id: 42 } : undefined)),
|
||||
prepare: vi.fn(() => ({ get: () => ({ role: 'user', email: 'real@example.test' }) })),
|
||||
} as unknown as DatabaseService;
|
||||
const permissions = {
|
||||
@@ -257,7 +257,7 @@ describe('FilesRpc writes', () => {
|
||||
const f = build();
|
||||
// The demo guard resolves the uploader's email; user 9 is the demo account.
|
||||
const db = {
|
||||
canAccessTrip: vi.fn(() => ({ id: 1, user_id: 42 })),
|
||||
canAccessTrip: vi.fn(async () => ({ id: 1, user_id: 42 })),
|
||||
prepare: vi.fn(() => ({ get: (id: number) => (id === 9 ? { role: 'user', email: 'demo@trek.app' } : { role: 'user', email: 'real@example.test' }) })),
|
||||
} as unknown as DatabaseService;
|
||||
const guards = new PluginGuards(
|
||||
|
||||
@@ -87,7 +87,8 @@ import type { TodoService } from '../../../src/nest/todo/todo.service';
|
||||
import type { CollabService } from '../../../src/nest/collab/collab.service';
|
||||
import { AddonsService } from '../../../src/nest/addons/addons.service';
|
||||
import { notificationsStub } from '../../helpers/notifications';
|
||||
import { createTestUnitOfWork, createTestAppSettingsRepo } from '../../helpers/test-uow';
|
||||
import { createTestUnitOfWork, createTestAppSettingsRepo, sharedTestOrm } from '../../helpers/test-uow';
|
||||
import type { EntityManager } from '@mikro-orm/core';
|
||||
|
||||
// The trip-summary prompt moved to the DI-discovered TripsMcp — its cases below
|
||||
// exercise it through a hand-built registry over a stub TripsService whose
|
||||
@@ -106,7 +107,14 @@ const readModelStub = {
|
||||
// The three remaining prompts moved to their domains' @McpController classes:
|
||||
// packing-list, budget-overview and the static-token notice. Built over the same
|
||||
// in-memory DB so the cases below keep asserting real rows.
|
||||
const promptDbs = () => new DatabaseService(testDb);
|
||||
//
|
||||
// Plan 3c Task 0b: `promptEm` is resolved once in the first `beforeAll` below
|
||||
// (before any `promptDbs()` call) — `canAccessTrip`/`isOwner`/`rosterUserIds`/
|
||||
// `getPlaceWithTags` resolve `TripsRepository`/`TripMembersRepository`/
|
||||
// `PlacesRepository` through it now, not through `db/database.ts`'s deleted
|
||||
// free functions this file's `dbMock` used to stand in for.
|
||||
let promptEm: EntityManager | undefined;
|
||||
const promptDbs = () => new DatabaseService(testDb, promptEm);
|
||||
const authStub = { isDemoUser: () => false } as unknown as AuthService;
|
||||
|
||||
|
||||
@@ -120,7 +128,8 @@ let packingMcp: PackingMcp;
|
||||
let budgetMcp: BudgetMcp;
|
||||
let tripPromptsMcp: TripPromptsMcp;
|
||||
beforeAll(async () => {
|
||||
promptGuards = new McpToolGuardsService(new DatabaseService(testDb), new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new RealtimeService());
|
||||
promptEm = (await sharedTestOrm(testDb)).em;
|
||||
promptGuards = new McpToolGuardsService(promptDbs(), new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new RealtimeService());
|
||||
tripsMcp = new TripsMcp(
|
||||
tripsStub,
|
||||
{ listItems: () => [] } as unknown as TodoService,
|
||||
|
||||
@@ -55,12 +55,21 @@ import { RealtimeService } from '../../../src/nest/realtime/realtime.service';
|
||||
import { QueryHelpersService } from '../../../src/nest/query-helpers/query-helpers.service';
|
||||
import { JourneyDomainService } from '../../../src/nest/journey/journey-domain.service';
|
||||
import { TrekPhotosRepository } from '../../../src/nest/photos/trek-photos.repository';
|
||||
import { createTestUnitOfWork, createTestAppSettingsRepo } from '../../helpers/test-uow';
|
||||
import { createTestUnitOfWork, createTestAppSettingsRepo, sharedTestOrm } from '../../helpers/test-uow';
|
||||
|
||||
const dbs = new DatabaseService(testDb);
|
||||
const realtime = new RealtimeService();
|
||||
let svc: AssignmentsService;
|
||||
beforeAll(async () => {
|
||||
// Plan 3c Task 0b: `dbs` is constructed at module load, before any
|
||||
// `beforeAll` can resolve a real `EntityManager` — the four
|
||||
// repository-backed methods are spied directly on this instance instead,
|
||||
// routed to a real `DatabaseService` built with one.
|
||||
const real = new DatabaseService(testDb, (await sharedTestOrm(testDb)).em);
|
||||
vi.spyOn(dbs, 'canAccessTrip').mockImplementation((...a) => real.canAccessTrip(...a));
|
||||
vi.spyOn(dbs, 'isOwner').mockImplementation((...a) => real.isOwner(...a));
|
||||
vi.spyOn(dbs, 'rosterUserIds').mockImplementation((...a) => real.rosterUserIds(...a));
|
||||
vi.spyOn(dbs, 'getPlaceWithTags').mockImplementation((...a) => real.getPlaceWithTags(...a));
|
||||
svc = new AssignmentsService(
|
||||
dbs,
|
||||
new PermissionsService(await createTestAppSettingsRepo(dbs.connection), await createTestUnitOfWork(dbs.connection)),
|
||||
|
||||
@@ -30,7 +30,7 @@ const mockDb = vi.hoisted(() => {
|
||||
closeDb: () => {},
|
||||
reinitialize: () => {},
|
||||
getPlaceWithTags: () => null,
|
||||
canAccessTrip: vi.fn(() => true),
|
||||
canAccessTrip: vi.fn(async () => true),
|
||||
isOwner: () => false,
|
||||
};
|
||||
});
|
||||
|
||||
@@ -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 } from '../../helpers/test-uow';
|
||||
import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, sharedTestOrm } from '../../helpers/test-uow';
|
||||
|
||||
|
||||
|
||||
@@ -80,7 +80,14 @@ import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo }
|
||||
// TripsService before, and on the deleted services/tripService before that);
|
||||
// deleteGuest routes through the SAME BudgetService domain SQL
|
||||
// (removeUserFromBudgetItems) under test.
|
||||
const dbs = () => new DatabaseService(testDb);
|
||||
//
|
||||
// Plan 3c Task 0b: `dbsEm` is resolved once, at the top of the `beforeAll`
|
||||
// below, before any `dbs()`/`new DatabaseService(testDb)` call —
|
||||
// `canAccessTrip`/`isOwner`/`rosterUserIds`/`getPlaceWithTags` resolve
|
||||
// `TripsRepository`/`TripMembersRepository`/`PlacesRepository` through it
|
||||
// now, not through `db/database.ts`'s deleted free functions.
|
||||
let dbsEm: import('@mikro-orm/core').EntityManager | undefined;
|
||||
const dbs = () => new DatabaseService(testDb, dbsEm);
|
||||
|
||||
|
||||
let budget: BudgetService;
|
||||
@@ -88,8 +95,9 @@ let membersSvc: TripMembersService;
|
||||
let createGuest: typeof membersSvc.createGuest;
|
||||
let deleteGuest: typeof membersSvc.deleteGuest;
|
||||
beforeAll(async () => {
|
||||
dbsEm = (await sharedTestOrm(testDb)).em;
|
||||
budget = new BudgetService(
|
||||
new DatabaseService(testDb),
|
||||
dbs(),
|
||||
new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)),
|
||||
new ExchangeRatesService(),
|
||||
new RealtimeService(),
|
||||
|
||||
@@ -48,8 +48,8 @@ import { UnitOfWork } from '../../../src/nest/database/unit-of-work';
|
||||
*/
|
||||
const uowStub = { transactional: <T>(fn: () => Promise<T>) => fn() } as unknown as UnitOfWork;
|
||||
|
||||
function svc() {
|
||||
return new BudgetService(new DatabaseService(dbConn), permissionsStub, exchangeRatesStub, new RealtimeService(), uowStub);
|
||||
function svc(db: DatabaseService = new DatabaseService(dbConn)) {
|
||||
return new BudgetService(db, permissionsStub, exchangeRatesStub, new RealtimeService(), uowStub);
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
@@ -59,9 +59,16 @@ beforeEach(() => {
|
||||
|
||||
describe('BudgetService', () => {
|
||||
it('verifyTripAccess resolves through DatabaseService.canAccessTrip', async () => {
|
||||
canAccessTrip.mockReturnValue({ id: 5, user_id: 2 });
|
||||
expect(await svc().verifyTripAccess('5', 2)).toEqual({ id: 5, user_id: 2 });
|
||||
expect(canAccessTrip).toHaveBeenCalledWith('5', 2);
|
||||
// Plan 3c Task 0b: `DatabaseService.canAccessTrip` is repository-backed
|
||||
// now (not `db/database.ts`'s deleted free function, which is why this
|
||||
// file's `db/database` mock's `canAccessTrip` above no longer reaches
|
||||
// it) — a fake `DatabaseService` in place of the real, EntityManager-less
|
||||
// one `svc()` defaults to, consistent with this suite's "no database
|
||||
// behind it" design (everything else here is a `prepare` stub).
|
||||
const findAccessible = vi.fn(async () => ({ id: 5, user_id: 2 }));
|
||||
const fakeDb = { canAccessTrip: findAccessible } as unknown as DatabaseService;
|
||||
expect(await svc(fakeDb).verifyTripAccess('5', 2)).toEqual({ id: 5, user_id: 2 });
|
||||
expect(findAccessible).toHaveBeenCalledWith('5', 2);
|
||||
});
|
||||
|
||||
it('canEdit forwards the ownership flag when the user owns the trip', async () => {
|
||||
@@ -150,11 +157,21 @@ describe('BudgetService', () => {
|
||||
// Both wrappers refuse a party who is not on the trip before they freeze
|
||||
// anything, so the roster lookup has to answer for these to reach the write
|
||||
// at all. Users 1 and 2 are the two parties every case here settles between.
|
||||
const rosterHas = (...userIds: number[]) => dbMock._stmt.all.mockReturnValue(userIds.map(user_id => ({ user_id })));
|
||||
//
|
||||
// Plan 3c Task 0b: `DatabaseService.rosterUserIds` is `TripMembersRepository
|
||||
// .rosterUserIds` now (not `this.conn`/the `dbMock._stmt.all` prepare stub,
|
||||
// which no longer intercepts it) — a real `DatabaseService` (so its other
|
||||
// methods, still served by the `dbMock._stmt` prepare stub, keep working)
|
||||
// with `rosterUserIds` spied directly, same fix as `verifyTripAccess`'s
|
||||
// test above.
|
||||
const rosterHas = (...userIds: number[]) => {
|
||||
const db = new DatabaseService(dbConn);
|
||||
vi.spyOn(db, 'rosterUserIds').mockResolvedValue(new Set(userIds));
|
||||
return db;
|
||||
};
|
||||
|
||||
it('createSettlement freezes the FX rate (await) before the raw insert', async () => {
|
||||
const s = svc();
|
||||
rosterHas(1, 2);
|
||||
const s = svc(rosterHas(1, 2));
|
||||
const freezeSpy = vi.spyOn(s, 'freezeForeignRate').mockResolvedValue();
|
||||
const insertSpy = vi.spyOn(s, 'insertSettlement').mockReturnValue({ id: 7 } as never);
|
||||
await s.createSettlement('5', { from_user_id: 1, to_user_id: 2, amount: 10 }, 3);
|
||||
@@ -163,8 +180,7 @@ describe('BudgetService', () => {
|
||||
});
|
||||
|
||||
it('updateSettlement threads the stored currency through the freeze', async () => {
|
||||
const s = svc();
|
||||
rosterHas(1, 2);
|
||||
const s = svc(rosterHas(1, 2));
|
||||
const freezeSpy = vi.spyOn(s, 'freezeForeignRate').mockResolvedValue();
|
||||
// Quirk fix: the stored-currency lookup is a targeted getSettlement, not
|
||||
// a full listSettlements scan.
|
||||
@@ -178,8 +194,7 @@ describe('BudgetService', () => {
|
||||
});
|
||||
|
||||
it('refuses a party who is not on the trip, before freezing or writing', async () => {
|
||||
const s = svc();
|
||||
rosterHas(1); // user 2 is not on this trip
|
||||
const s = svc(rosterHas(1)); // user 2 is not on this trip
|
||||
const freezeSpy = vi.spyOn(s, 'freezeForeignRate').mockResolvedValue();
|
||||
const insertSpy = vi.spyOn(s, 'insertSettlement').mockReturnValue({ id: 7 } as never);
|
||||
|
||||
|
||||
@@ -134,7 +134,7 @@ describe('Open charging data before the stop exists', () => {
|
||||
|
||||
it('serves the route and the MCP tool from one contract and one access check', async () => {
|
||||
const charging = { read: vi.fn(), lookup: vi.fn(async () => empty('unknown')) };
|
||||
const db = { canAccessTrip: vi.fn(() => false) };
|
||||
const db = { canAccessTrip: vi.fn(async () => false) };
|
||||
const tool = new ChargingMcp(charging as never, db as never, {} as never);
|
||||
const input = { lat: 48.1, lng: 11.5, name: 'Ladepark Nord' };
|
||||
|
||||
@@ -142,7 +142,7 @@ describe('Open charging data before the stop exists', () => {
|
||||
// TripAccessGuard refuses it on the route.
|
||||
expect((await tool.lookup({ tripId: 3, ...input }, { userId: 5 } as never)).isError).toBe(true);
|
||||
expect(charging.lookup).not.toHaveBeenCalled();
|
||||
db.canAccessTrip.mockReturnValue(true);
|
||||
db.canAccessTrip.mockResolvedValue(true);
|
||||
await tool.lookup({ tripId: 3, ...input }, { userId: 5 } as never);
|
||||
await new ChargingLookupController(charging as never).lookup(input as unknown as ChargingLookupDto);
|
||||
|
||||
|
||||
@@ -55,12 +55,12 @@ import { CollectionsService } from '../../../src/nest/collections/collections.se
|
||||
import { PlacePhotoCacheService } from '../../../src/nest/place-photos/place-photo-cache.service';
|
||||
import { makeStorageFixture } from '../../helpers/storage-fixture';
|
||||
import { notificationsStub } from '../../helpers/notifications';
|
||||
import { createTestUnitOfWork, createTestAppSettingsRepo } from '../../helpers/test-uow';
|
||||
import { createTestUnitOfWork, createTestAppSettingsRepo, createTestDatabaseService } from '../../helpers/test-uow';
|
||||
|
||||
const storageFx = makeStorageFixture('');
|
||||
let svc: CollectionsService;
|
||||
beforeAll(async () => {
|
||||
svc = new CollectionsService(new DatabaseService(testDb), new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new RealtimeService(), notificationsStub(notifSend), storageFx.storage, await createTestUnitOfWork(testDb));
|
||||
svc = new CollectionsService(await createTestDatabaseService(testDb), new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new RealtimeService(), notificationsStub(notifSend), storageFx.storage, await createTestUnitOfWork(testDb));
|
||||
});
|
||||
// The real cache: these cases assert what removeIfUnreferenced actually does
|
||||
// about collection_places (#1081), so a stub would assert nothing.
|
||||
|
||||
@@ -1,39 +1,60 @@
|
||||
/**
|
||||
* Locks in the delegation direction: DatabaseService's trip-access helpers
|
||||
* must call the db/database exports (which e2e suites stub in their vi.mock
|
||||
* factories), never reimplement the SQL against the injected connection.
|
||||
* Locks in the delegation direction: `DatabaseService`'s trip-access helpers
|
||||
* must call the repository each one owns (`TripsRepository.findAccessible`/
|
||||
* `isOwner`, `TripMembersRepository.rosterUserIds`, `PlacesRepository
|
||||
* .findWithTagsAndRatings`) through the injected `EntityManager`, never
|
||||
* reimplement the SQL against the raw connection.
|
||||
*
|
||||
* Plan 3c Task 0b: this used to lock in the OPPOSITE direction — that these
|
||||
* four methods delegated to `db/database.ts`'s free functions, which e2e
|
||||
* suites stubbed in their `vi.mock` factories. Those free functions (and the
|
||||
* duplicate `TripAccess` interface) are deleted now; `Trips.repository.ts`'s
|
||||
* `TripAccess` is the single source, and the primitive bodies live on the
|
||||
* repositories.
|
||||
*/
|
||||
import { describe, it, expect, vi } from 'vitest';
|
||||
|
||||
const { canAccessTrip, isOwner, getPlaceWithTags } = vi.hoisted(() => ({
|
||||
canAccessTrip: vi.fn(() => ({ id: -1, user_id: -2, currency: 'XXX' })),
|
||||
isOwner: vi.fn(() => true),
|
||||
getPlaceWithTags: vi.fn(() => null),
|
||||
}));
|
||||
|
||||
vi.mock('../../../src/db/database', () => ({
|
||||
db: {},
|
||||
closeDb: () => {},
|
||||
reinitialize: () => {},
|
||||
canAccessTrip,
|
||||
isOwner,
|
||||
getPlaceWithTags,
|
||||
}));
|
||||
|
||||
import type { EntityManager } from '@mikro-orm/core';
|
||||
import { Trips } from '../../../src/db/entities/Trips.entity';
|
||||
import { TripMembers } from '../../../src/db/entities/TripMembers.entity';
|
||||
import { Places } from '../../../src/db/entities/Places.entity';
|
||||
import { DatabaseService } from '../../../src/nest/database/database.service';
|
||||
|
||||
describe('DatabaseService (helper delegation)', () => {
|
||||
it('routes trip-access helpers through the db/database exports', async () => {
|
||||
const { db } = await import('../../../src/db/database');
|
||||
const svc = new DatabaseService(db);
|
||||
it('routes trip-access helpers through the EntityManager-resolved repositories', async () => {
|
||||
const findAccessible = vi.fn(async () => ({ id: -1, user_id: -2, currency: 'XXX' }));
|
||||
const isOwner = vi.fn(async () => true);
|
||||
const rosterUserIds = vi.fn(async () => new Set([1, 2, 3]));
|
||||
const findWithTagsAndRatings = vi.fn(async () => null);
|
||||
|
||||
const em = {
|
||||
getRepository: vi.fn((entity: unknown) => {
|
||||
if (entity === Trips) return { findAccessible, isOwner };
|
||||
if (entity === TripMembers) return { rosterUserIds };
|
||||
if (entity === Places) return { findWithTagsAndRatings };
|
||||
throw new Error(`unexpected entity: ${String(entity)}`);
|
||||
}),
|
||||
} as unknown as EntityManager;
|
||||
|
||||
const svc = new DatabaseService({} as never, em);
|
||||
|
||||
expect(await svc.canAccessTrip(7, 8)).toEqual({ id: -1, user_id: -2, currency: 'XXX' });
|
||||
expect(canAccessTrip).toHaveBeenCalledWith(7, 8);
|
||||
expect(findAccessible).toHaveBeenCalledWith(7, 8);
|
||||
|
||||
expect(await svc.isOwner(7, 8)).toBe(true);
|
||||
expect(isOwner).toHaveBeenCalledWith(7, 8);
|
||||
|
||||
expect(await svc.rosterUserIds(7)).toEqual(new Set([1, 2, 3]));
|
||||
expect(rosterUserIds).toHaveBeenCalledWith(7);
|
||||
|
||||
expect(await svc.getPlaceWithTags(9)).toBeNull();
|
||||
expect(getPlaceWithTags).toHaveBeenCalledWith(9);
|
||||
expect(findWithTagsAndRatings).toHaveBeenCalledWith(9);
|
||||
});
|
||||
|
||||
it('throws its own clear error — never MikroORM\'s generic one — when constructed with no EntityManager', async () => {
|
||||
const svc = new DatabaseService({} as never);
|
||||
await expect(svc.canAccessTrip(1, 2)).rejects.toThrow(/no EntityManager available/);
|
||||
await expect(svc.isOwner(1, 2)).rejects.toThrow(/no EntityManager available/);
|
||||
await expect(svc.rosterUserIds(1)).rejects.toThrow(/no EntityManager available/);
|
||||
await expect(svc.getPlaceWithTags(1)).rejects.toThrow(/no EntityManager available/);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -2,9 +2,11 @@
|
||||
* DatabaseService — the shared better-sqlite3 provider (F3). Exercises every
|
||||
* helper against the real connection so the typed query surface is covered.
|
||||
*/
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { describe, it, expect, beforeAll } from 'vitest';
|
||||
import { db } from '../../../src/db/database';
|
||||
import { DatabaseService } from '../../../src/nest/database/database.service';
|
||||
import { sharedTestOrm } from '../../helpers/test-uow';
|
||||
import type { EntityManager } from '@mikro-orm/core';
|
||||
|
||||
describe('DatabaseService (typed query helpers)', () => {
|
||||
const svc = new DatabaseService(db);
|
||||
@@ -34,7 +36,12 @@ describe('DatabaseService (typed query helpers)', () => {
|
||||
});
|
||||
|
||||
describe('DatabaseService (trip-access helpers)', () => {
|
||||
const svc = new DatabaseService(db);
|
||||
let svc: DatabaseService;
|
||||
let em: EntityManager;
|
||||
beforeAll(async () => {
|
||||
em = (await sharedTestOrm(db)).em;
|
||||
svc = new DatabaseService(db, em);
|
||||
});
|
||||
|
||||
it('canAccessTrip / isOwner / getPlaceWithTags delegate to the shared helpers', async () => {
|
||||
svc.run(
|
||||
|
||||
@@ -67,7 +67,7 @@ import type { PlacesService } from '../../../src/nest/places/places.service';
|
||||
import type { AssignmentsService } from '../../../src/nest/assignments/assignments.service';
|
||||
import type { PermissionsService } from '../../../src/nest/permissions/permissions.service';
|
||||
import type { JourneyDomainService } from '../../../src/nest/journey/journey-domain.service';
|
||||
import { createTestUnitOfWork } from '../../helpers/test-uow';
|
||||
import { createTestUnitOfWork, createTestDatabaseService } from '../../helpers/test-uow';
|
||||
|
||||
// ── Collaborator stubs ───────────────────────────────────────────────────────
|
||||
//
|
||||
@@ -114,9 +114,10 @@ const assignmentsStub = { dayExists: vi.fn(), createAssignment: vi.fn(), broadca
|
||||
const permissionsStub = { checkPermission: vi.fn() };
|
||||
const journeyStub = { canEdit: vi.fn(), createEntry: vi.fn() };
|
||||
|
||||
const dbs = new DatabaseService(testDb);
|
||||
let dbs: DatabaseService;
|
||||
let svc: DawarichSuggestionsService;
|
||||
beforeAll(async () => {
|
||||
dbs = await createTestDatabaseService(testDb);
|
||||
svc = new DawarichSuggestionsService(
|
||||
dbs,
|
||||
dawarichStub as unknown as DawarichService,
|
||||
@@ -1292,9 +1293,14 @@ describe('DawarichSuggestionsService — the Atlas hand-off', () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const id = seedSuggestion({ userId: user.id, tripId: trip.id });
|
||||
dbMock.getPlaceWithTags.mockReturnValue(null);
|
||||
// Plan 3c Task 0b: `getPlaceWithTags` is `PlacesRepository
|
||||
// .findWithTagsAndRatings` now (not `db/database.ts`'s deleted free
|
||||
// function `dbMock.getPlaceWithTags` used to stand in for), so the
|
||||
// "re-read comes back empty" case is spied directly on `dbs`.
|
||||
const getPlaceWithTagsSpy = vi.spyOn(dbs, 'getPlaceWithTags').mockResolvedValueOnce(null);
|
||||
|
||||
const result = await svc.accept(user.id, id, { target: 'place', tripId: trip.id }, 'socket-3');
|
||||
getPlaceWithTagsSpy.mockRestore();
|
||||
|
||||
expect(placesStub.broadcast).toHaveBeenCalledWith(
|
||||
String(trip.id),
|
||||
|
||||
@@ -138,7 +138,7 @@ interface Collaborators {
|
||||
}
|
||||
|
||||
function harness(over: Collaborators = {}) {
|
||||
const canAccessTrip = vi.fn().mockReturnValue('access' in over ? over.access : TRIP_ACCESS);
|
||||
const canAccessTrip = vi.fn().mockResolvedValue('access' in over ? over.access : TRIP_ACCESS);
|
||||
const get = vi
|
||||
.fn()
|
||||
.mockReturnValue(
|
||||
|
||||
@@ -23,7 +23,7 @@ function setup() {
|
||||
get: (sql: string, ...params: unknown[]) => raw.prepare(sql).get(...params),
|
||||
all: (sql: string, ...params: unknown[]) => raw.prepare(sql).all(...params),
|
||||
run: (sql: string, ...params: unknown[]) => raw.prepare(sql).run(...params),
|
||||
canAccessTrip: vi.fn(() => true),
|
||||
canAccessTrip: vi.fn(async () => true),
|
||||
};
|
||||
const service = new DayBoundariesService(db as unknown as DatabaseService);
|
||||
const realtime = { broadcast: vi.fn() };
|
||||
@@ -68,9 +68,9 @@ it('MCP checks demo, trip access and edit permission before saving', async () =>
|
||||
s.auth.isDemoUser.mockReturnValue(true);
|
||||
await s.mcp.save(request, ctx);
|
||||
s.auth.isDemoUser.mockReturnValue(false);
|
||||
s.db.canAccessTrip.mockReturnValue(false);
|
||||
s.db.canAccessTrip.mockResolvedValue(false);
|
||||
await s.mcp.save(request, ctx);
|
||||
s.db.canAccessTrip.mockReturnValue(true);
|
||||
s.db.canAccessTrip.mockResolvedValue(true);
|
||||
s.guards.hasTripPermission.mockReturnValue(false);
|
||||
await s.mcp.save(request, ctx);
|
||||
expect(await s.service.list(10)).toEqual([]);
|
||||
|
||||
@@ -51,11 +51,11 @@ import { PermissionsService } from '../../../src/nest/permissions/permissions.se
|
||||
import { DayNotesService } from '../../../src/nest/day-notes/day-notes.service';
|
||||
import type { DayNote } from '../../../src/types';
|
||||
import { RealtimeService } from '../../../src/nest/realtime/realtime.service';
|
||||
import { createTestUnitOfWork, createTestAppSettingsRepo } from '../../helpers/test-uow';
|
||||
import { createTestUnitOfWork, createTestAppSettingsRepo, createTestDatabaseService } from '../../helpers/test-uow';
|
||||
|
||||
let svc: DayNotesService;
|
||||
beforeAll(async () => {
|
||||
svc = new DayNotesService(new DatabaseService(testDb), new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new RealtimeService());
|
||||
svc = new DayNotesService(await createTestDatabaseService(testDb), new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new RealtimeService());
|
||||
});
|
||||
|
||||
beforeAll(() => {
|
||||
|
||||
@@ -64,11 +64,12 @@ import { RealtimeService } from '../../../src/nest/realtime/realtime.service';
|
||||
import { QueryHelpersService } from '../../../src/nest/query-helpers/query-helpers.service';
|
||||
import { makeAccommodationsService } from '../../helpers/accommodations-service';
|
||||
import type { Day } from '../../../src/types';
|
||||
import { createTestUnitOfWork, createTestAppSettingsRepo } from '../../helpers/test-uow';
|
||||
import { createTestUnitOfWork, createTestAppSettingsRepo, createTestDatabaseService } from '../../helpers/test-uow';
|
||||
|
||||
let svc: DaysService;
|
||||
beforeAll(async () => {
|
||||
svc = new DaysService(new DatabaseService(testDb), new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new RealtimeService(), new QueryHelpersService(new DatabaseService(testDb)), await createTestUnitOfWork(testDb));
|
||||
const dbs = await createTestDatabaseService(testDb);
|
||||
svc = new DaysService(dbs, new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new RealtimeService(), new QueryHelpersService(dbs), await createTestUnitOfWork(testDb));
|
||||
});
|
||||
let accommodations: Awaited<ReturnType<typeof makeAccommodationsService>>;
|
||||
beforeAll(async () => {
|
||||
|
||||
@@ -59,6 +59,7 @@ import { runMigrations } from '../../../src/db/migrations';
|
||||
import { resetTestDb } from '../../helpers/test-db';
|
||||
import { createUser, createTrip, addTripMember, createPlace, createReservation, createDay, createDayAssignment, setAppSetting } from '../../helpers/factories';
|
||||
import { DatabaseService, type TripAccess } from '../../../src/nest/database/database.service';
|
||||
import { sharedTestOrm } from '../../helpers/test-uow';
|
||||
import type { PermissionsService } from '../../../src/nest/permissions/permissions.service';
|
||||
import { FilesService } from '../../../src/nest/files/files.service';
|
||||
import { AllowedFileTypesService } from '../../../src/nest/files/allowed-file-types.service';
|
||||
@@ -96,9 +97,19 @@ beforeAll(() => {
|
||||
runMigrations(testDb);
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
beforeEach(async () => {
|
||||
resetTestDb(testDb);
|
||||
vi.clearAllMocks();
|
||||
// Plan 3c Task 0b: `svc`'s `DatabaseService` is constructed at module load,
|
||||
// before any `beforeAll`/`beforeEach` can resolve a real `EntityManager` —
|
||||
// spied directly on that instance, routed to a real `DatabaseService` built
|
||||
// with one. Re-applied every test (not `beforeAll`) because `afterEach`
|
||||
// below `vi.restoreAllMocks()`s it away after each one.
|
||||
const real = new DatabaseService(testDb, (await sharedTestOrm(testDb)).em);
|
||||
vi.spyOn(svc['db'] as DatabaseService, 'canAccessTrip').mockImplementation((...a) => real.canAccessTrip(...a));
|
||||
vi.spyOn(svc['db'] as DatabaseService, 'isOwner').mockImplementation((...a) => real.isOwner(...a));
|
||||
vi.spyOn(svc['db'] as DatabaseService, 'rosterUserIds').mockImplementation((...a) => real.rosterUserIds(...a));
|
||||
vi.spyOn(svc['db'] as DatabaseService, 'getPlaceWithTags').mockImplementation((...a) => real.getPlaceWithTags(...a));
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
|
||||
@@ -12,7 +12,7 @@ const routeTestDb = new Database(':memory:');
|
||||
|
||||
async function setup() {
|
||||
const maps = { geocodeQuery: vi.fn(), reverseGeocode: vi.fn().mockResolvedValue({ name: null, address: null }) };
|
||||
const db = { canAccessTrip: vi.fn(() => ({ user_id: 7 })), get: () => ({ role: 'user' }) };
|
||||
const db = { canAccessTrip: vi.fn(async () => ({ user_id: 7 })), get: () => ({ role: 'user' }) };
|
||||
const places = { create: vi.fn((_trip: string, stop: { name: string }) => ({ id: stop.name })), broadcast: vi.fn() };
|
||||
const assignments = { dayExists: vi.fn(() => true), createAssignment: vi.fn((dayId: number, placeId: string) => ({ dayId, placeId })), broadcast: vi.fn(), reconcile: vi.fn() };
|
||||
const permissions = { checkPermission: vi.fn(() => true) };
|
||||
@@ -68,7 +68,7 @@ describe('Google route import', () => {
|
||||
|
||||
it('checks membership, both editing permissions and the target day before writing', async () => {
|
||||
const { service, db, permissions, assignments, places } = await setup();
|
||||
db.canAccessTrip.mockReturnValueOnce(null as never);
|
||||
db.canAccessTrip.mockResolvedValueOnce(null as never);
|
||||
await expect(service.import(1, 7, input)).rejects.toThrow();
|
||||
permissions.checkPermission.mockReturnValueOnce(false);
|
||||
await expect(service.import(1, 7, input)).rejects.toThrow();
|
||||
|
||||
@@ -58,14 +58,15 @@ import { RealtimeService } from '../../../src/nest/realtime/realtime.service';
|
||||
import { TrekPhotosRepository } from '../../../src/nest/photos/trek-photos.repository';
|
||||
import { JourneyDomainService } from '../../../src/nest/journey/journey-domain.service';
|
||||
import { db as dbConn } from '../../../src/db/database';
|
||||
import { createTestUnitOfWork } from '../../helpers/test-uow';
|
||||
import { createTestUnitOfWork, sharedTestOrm } from '../../helpers/test-uow';
|
||||
|
||||
const dbs = new DatabaseService(dbConn);
|
||||
let dbs: DatabaseService;
|
||||
let svc: JourneyDomainService;
|
||||
|
||||
beforeAll(async () => {
|
||||
createTables(testDb);
|
||||
runMigrations(testDb);
|
||||
dbs = new DatabaseService(dbConn, (await sharedTestOrm(testDb)).em);
|
||||
svc = new JourneyDomainService(dbs, new RealtimeService(), new TrekPhotosRepository(dbs), await createTestUnitOfWork(testDb));
|
||||
});
|
||||
|
||||
|
||||
@@ -60,6 +60,8 @@ import { createUser, createTrip } from '../../helpers/factories';
|
||||
import { mapDbError, pipeAsset, type ServiceResult } from '../../../src/nest/memories/memories.helpers';
|
||||
import { MemoriesAccessService } from '../../../src/nest/memories/memories-access.service';
|
||||
import { DatabaseService } from '../../../src/nest/database/database.service';
|
||||
import { Trips } from '../../../src/db/entities/Trips.entity';
|
||||
import { sharedTestOrm } from '../../helpers/test-uow';
|
||||
|
||||
const access = new MemoriesAccessService(new DatabaseService(testDb));
|
||||
// A typed forwarder, not a `.bind` alias: a bound alias is typed `any`, which
|
||||
@@ -67,9 +69,18 @@ const access = new MemoriesAccessService(new DatabaseService(testDb));
|
||||
const getAlbumIdFromLink = (...a: Parameters<MemoriesAccessService['getAlbumIdFromLink']>) => access.getAlbumIdFromLink(...a);
|
||||
import { SsrfBlockedError } from '../../../src/utils/ssrfGuard';
|
||||
|
||||
beforeAll(() => {
|
||||
beforeAll(async () => {
|
||||
createTables(testDb);
|
||||
runMigrations(testDb);
|
||||
// Plan 3c Task 0b: `access` (and `DatabaseService.prototype.canAccessTrip`
|
||||
// it holds a reference to) is constructed at module load, before any
|
||||
// `beforeAll` can resolve a real `EntityManager` — patched onto the
|
||||
// PROTOTYPE instead, which the already-constructed instance's method
|
||||
// lookup still resolves through (`canAccessTrip` is not an own property).
|
||||
const em = (await sharedTestOrm(testDb)).em;
|
||||
vi.spyOn(DatabaseService.prototype, 'canAccessTrip').mockImplementation(async (tripId, userId) =>
|
||||
em.getRepository(Trips).findAccessible(tripId, userId),
|
||||
);
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
|
||||
@@ -60,7 +60,6 @@ import { createTables } from '../../../src/db/schema';
|
||||
import { runMigrations } from '../../../src/db/migrations';
|
||||
import { resetTestDb } from '../../helpers/test-db';
|
||||
import { createUser, createAdmin, createTrip, addTripMember } from '../../helpers/factories';
|
||||
import { DatabaseService } from '../../../src/nest/database/database.service';
|
||||
import type { PermissionsService } from '../../../src/nest/permissions/permissions.service';
|
||||
import { PackingService, isInvalidBagRef } from '../../../src/nest/packing/packing.service';
|
||||
// Was packing.bridge, deleted with the other three that had no consumer outside the
|
||||
@@ -68,7 +67,7 @@ import { PackingService, isInvalidBagRef } from '../../../src/nest/packing/packi
|
||||
const bridgeListItems = (tripId: string | number, viewerId?: number) => svc.listItems(tripId, viewerId);
|
||||
import { RealtimeService } from '../../../src/nest/realtime/realtime.service';
|
||||
import { notificationsStub } from '../../helpers/notifications';
|
||||
import { createTestUnitOfWork } from '../../helpers/test-uow';
|
||||
import { createTestUnitOfWork, createTestDatabaseService } from '../../helpers/test-uow';
|
||||
|
||||
let svc: PackingService;
|
||||
|
||||
@@ -77,7 +76,7 @@ let svc: PackingService;
|
||||
beforeAll(async () => {
|
||||
createTables(testDb);
|
||||
runMigrations(testDb);
|
||||
svc = new PackingService(new DatabaseService(testDb), permissionsStub, new RealtimeService(), notificationsStub(send), await createTestUnitOfWork(testDb));
|
||||
svc = new PackingService(await createTestDatabaseService(testDb), permissionsStub, new RealtimeService(), notificationsStub(send), await createTestUnitOfWork(testDb));
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
|
||||
@@ -54,7 +54,7 @@ import type { PlacePhotoCacheService } from '../../../src/nest/place-photos/plac
|
||||
import { JourneyDomainService } from '../../../src/nest/journey/journey-domain.service';
|
||||
import { TrekPhotosRepository } from '../../../src/nest/photos/trek-photos.repository';
|
||||
import { makeStorageFixture } from '../../helpers/storage-fixture';
|
||||
import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo } from '../../helpers/test-uow';
|
||||
import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, sharedTestOrm } from '../../helpers/test-uow';
|
||||
|
||||
const dbs = new DatabaseService(testDb);
|
||||
const photoCacheStub = { removeIfUnreferenced: vi.fn() } as unknown as PlacePhotoCacheService;
|
||||
@@ -107,9 +107,18 @@ const geocoder = () => vi.fn(async (query: string) => {
|
||||
|
||||
let tripId: string;
|
||||
|
||||
beforeAll(() => {
|
||||
beforeAll(async () => {
|
||||
createTables(testDb);
|
||||
runMigrations(testDb);
|
||||
// Plan 3c Task 0b: `dbs` is constructed at module load, before any
|
||||
// `beforeAll` can resolve a real `EntityManager` — the four
|
||||
// repository-backed methods are spied directly on this instance instead,
|
||||
// routed to a real `DatabaseService` built with one.
|
||||
const real = new DatabaseService(testDb, (await sharedTestOrm(testDb)).em);
|
||||
vi.spyOn(dbs, 'canAccessTrip').mockImplementation((...a) => real.canAccessTrip(...a));
|
||||
vi.spyOn(dbs, 'isOwner').mockImplementation((...a) => real.isOwner(...a));
|
||||
vi.spyOn(dbs, 'rosterUserIds').mockImplementation((...a) => real.rosterUserIds(...a));
|
||||
vi.spyOn(dbs, 'getPlaceWithTags').mockImplementation((...a) => real.getPlaceWithTags(...a));
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
|
||||
@@ -78,7 +78,7 @@ import { QueryHelpersService } from '../../../src/nest/query-helpers/query-helpe
|
||||
import { JourneyDomainService } from '../../../src/nest/journey/journey-domain.service';
|
||||
import { TrekPhotosRepository } from '../../../src/nest/photos/trek-photos.repository';
|
||||
import { makeStorageFixture } from '../../helpers/storage-fixture';
|
||||
import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo } from '../../helpers/test-uow';
|
||||
import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, sharedTestOrm } from '../../helpers/test-uow';
|
||||
import type { AppSettingsRepository } from '../../../src/db/repositories/AppSettings.repository';
|
||||
import type { UsersRepository } from '../../../src/db/repositories/Users.repository';
|
||||
|
||||
@@ -126,6 +126,15 @@ async function makePlacesService(
|
||||
let accommodations: Awaited<ReturnType<typeof accommodationsOver>>;
|
||||
let svc: Awaited<ReturnType<typeof makePlacesService>>;
|
||||
beforeAll(async () => {
|
||||
// Plan 3c Task 0b: `dbs` is constructed at module load, before any
|
||||
// `beforeAll` can resolve a real `EntityManager` — the four
|
||||
// repository-backed methods are spied directly on this instance instead,
|
||||
// routed to a real `DatabaseService` built with one.
|
||||
const real = new DatabaseService(testDb, (await sharedTestOrm(testDb)).em);
|
||||
vi.spyOn(dbs, 'canAccessTrip').mockImplementation((...a) => real.canAccessTrip(...a));
|
||||
vi.spyOn(dbs, 'isOwner').mockImplementation((...a) => real.isOwner(...a));
|
||||
vi.spyOn(dbs, 'rosterUserIds').mockImplementation((...a) => real.rosterUserIds(...a));
|
||||
vi.spyOn(dbs, 'getPlaceWithTags').mockImplementation((...a) => real.getPlaceWithTags(...a));
|
||||
accommodations = await accommodationsOver(dbs);
|
||||
svc = await makePlacesService();
|
||||
});
|
||||
|
||||
@@ -48,7 +48,6 @@ import { createTables } from '../../../src/db/schema';
|
||||
import { runMigrations } from '../../../src/db/migrations';
|
||||
import { resetTestDb } from '../../helpers/test-db';
|
||||
import { createUser, createTrip, createReservation, createBudgetItem, createPlace, createDay, createDayAccommodation, createDayAssignment, addTripMember } from '../../helpers/factories';
|
||||
import { DatabaseService } from '../../../src/nest/database/database.service';
|
||||
import type { PermissionsService } from '../../../src/nest/permissions/permissions.service';
|
||||
import { ReservationsService } from '../../../src/nest/reservations/reservations.service';
|
||||
import { ReservationsReadRepository } from '../../../src/nest/reservations/reservations-read.repository';
|
||||
@@ -76,11 +75,12 @@ const bridge = {
|
||||
import { RealtimeService } from '../../../src/nest/realtime/realtime.service';
|
||||
import { notificationsStub } from '../../helpers/notifications';
|
||||
import { makeAccommodationsService } from '../../helpers/accommodations-service';
|
||||
import { createTestUnitOfWork } from '../../helpers/test-uow';
|
||||
import { createTestUnitOfWork, createTestDatabaseService } from '../../helpers/test-uow';
|
||||
|
||||
let svc: ReservationsService;
|
||||
beforeAll(async () => {
|
||||
svc = new ReservationsService(new DatabaseService(testDb), permissionsStub, budget as unknown as BudgetService, new RealtimeService(), notificationsStub(notif.send), new ReservationsReadRepository(new DatabaseService(testDb)), await makeAccommodationsService(testDb), await createTestUnitOfWork(testDb));
|
||||
const dbs = await createTestDatabaseService(testDb);
|
||||
svc = new ReservationsService(dbs, permissionsStub, budget as unknown as BudgetService, new RealtimeService(), notificationsStub(notif.send), new ReservationsReadRepository(dbs), await makeAccommodationsService(testDb), await createTestUnitOfWork(testDb));
|
||||
});
|
||||
|
||||
beforeAll(() => { createTables(testDb); runMigrations(testDb); });
|
||||
|
||||
@@ -10,6 +10,7 @@ import { DatabaseService } from '../../../src/nest/database/database.service';
|
||||
import { RoadtripPlanService } from '../../../src/nest/roadtrip/roadtrip-plan.service';
|
||||
import { createDay, createDayAssignment, createPlace, createTrip, createUser } from '../../helpers/factories';
|
||||
import { resetTestDb } from '../../helpers/test-db';
|
||||
import { sharedTestOrm } from '../../helpers/test-uow';
|
||||
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
@@ -39,7 +40,7 @@ function hourlyRouter() {
|
||||
};
|
||||
}
|
||||
|
||||
function setup() {
|
||||
async function setup() {
|
||||
const { user } = createUser(db);
|
||||
const trip = createTrip(db, user.id);
|
||||
const day = createDay(db, trip.id);
|
||||
@@ -50,7 +51,7 @@ function setup() {
|
||||
});
|
||||
db.prepare("UPDATE day_assignments SET assignment_time = '09:00' WHERE id = ?").run(visits[0].id);
|
||||
const plans = new RoadtripPlanService(
|
||||
new DatabaseService(db),
|
||||
new DatabaseService(db, (await sharedTestOrm(db)).em),
|
||||
{ getUserSettings: () => ({}) } as never,
|
||||
{ read: () => ({}) } as never,
|
||||
hourlyRouter() as never,
|
||||
@@ -66,7 +67,7 @@ beforeEach(() => {
|
||||
|
||||
describe('a visit end time on the road trip', () => {
|
||||
it('is read from the visit, and from the place when the visit has none', async () => {
|
||||
const { user, trip, visits, plans } = setup();
|
||||
const { user, trip, visits, plans } = await setup();
|
||||
db.prepare("UPDATE day_assignments SET assignment_end_time = '14:00' WHERE id = ?").run(visits[1].id);
|
||||
db.prepare(
|
||||
"UPDATE places SET end_time = '18:00' WHERE id = (SELECT place_id FROM day_assignments WHERE id = ?)",
|
||||
@@ -82,7 +83,7 @@ describe('a visit end time on the road trip', () => {
|
||||
});
|
||||
|
||||
it('is when the drive leaves the stop, in place of its stay', async () => {
|
||||
const { user, trip, visits, plans } = setup();
|
||||
const { user, trip, visits, plans } = await setup();
|
||||
db.prepare("UPDATE day_assignments SET assignment_end_time = '14:00' WHERE id = ?").run(visits[1].id);
|
||||
|
||||
const { calculated } = await plans.calculate(trip.id, user.id);
|
||||
@@ -98,7 +99,7 @@ describe('a visit end time on the road trip', () => {
|
||||
});
|
||||
|
||||
it('is reported when the drive gets there after it', async () => {
|
||||
const { user, trip, visits, plans } = setup();
|
||||
const { user, trip, visits, plans } = await setup();
|
||||
db.prepare("UPDATE day_assignments SET assignment_end_time = '09:30' WHERE id = ?").run(visits[1].id);
|
||||
|
||||
const { calculated } = await plans.calculate(trip.id, user.id);
|
||||
@@ -108,7 +109,7 @@ describe('a visit end time on the road trip', () => {
|
||||
});
|
||||
|
||||
it('leaves a stop without one to its stay', async () => {
|
||||
const { user, trip, plans } = setup();
|
||||
const { user, trip, plans } = await setup();
|
||||
|
||||
const { calculated } = await plans.calculate(trip.id, user.id);
|
||||
|
||||
|
||||
@@ -59,7 +59,7 @@ function setup() {
|
||||
fill_percent: null,
|
||||
}));
|
||||
const db = {
|
||||
canAccessTrip: vi.fn(() => true),
|
||||
canAccessTrip: vi.fn(async () => true),
|
||||
all: vi.fn((sql: string) => (sql.includes('FROM day_assignments') ? visits : days)),
|
||||
};
|
||||
const router = {
|
||||
@@ -177,7 +177,7 @@ describe('browser-independent roadtrip calculation', () => {
|
||||
});
|
||||
it('checks trip access before reading or routing', async () => {
|
||||
const s = setup();
|
||||
s.db.canAccessTrip.mockReturnValue(false);
|
||||
s.db.canAccessTrip.mockResolvedValue(false);
|
||||
await expect(s.plans.calculate(20, 5)).rejects.toThrow();
|
||||
expect(s.db.all).not.toHaveBeenCalled();
|
||||
expect(s.router.route).not.toHaveBeenCalled();
|
||||
@@ -189,11 +189,11 @@ describe('browser-independent roadtrip calculation', () => {
|
||||
const s = setup();
|
||||
const mcp = new RoadtripPlanningMcp(s.plans, {} as never, {} as never);
|
||||
const reason = (res: { content: { text: string }[]; isError?: boolean }) => [res.isError, res.content[0].text];
|
||||
s.db.canAccessTrip.mockReturnValue(false);
|
||||
s.db.canAccessTrip.mockResolvedValue(false);
|
||||
expect(reason(await mcp.context({ tripId: 20 }, ctx))).toEqual([true, 'Trip not found']);
|
||||
expect(reason(await mcp.calculate({ tripId: 20, includeGeometry: false }, ctx))).toEqual([true, 'Trip not found']);
|
||||
expect(reason(await mcp.corridor({ tripId: 20, dayNumber: 1, category: 'fuel', widthKm: 5, offset: 0 } as never, ctx))).toEqual([true, 'Trip not found']);
|
||||
s.db.canAccessTrip.mockReturnValue(true);
|
||||
s.db.canAccessTrip.mockResolvedValue(true);
|
||||
const window = await mcp.calculate({ tripId: 10, includeGeometry: false, settings: { roadtrip_day_start: '18:00', roadtrip_day_end: '08:00' } }, ctx);
|
||||
expect(reason(window)).toEqual([true, 'Day end must be later than day start.']);
|
||||
expect(s.router.route).not.toHaveBeenCalled();
|
||||
@@ -376,7 +376,7 @@ describe('MCP trip preferences authorization', () => {
|
||||
const s = setup();
|
||||
const tool = new RoadtripPreferencesMcp(s.preferences, { isDemoUser: () => false } as never, {} as never, s.db as never, { hasTripPermission: () => true } as never);
|
||||
expect(JSON.stringify(await tool.read({ tripId: 10 }, ctx))).toContain('100');
|
||||
s.db.canAccessTrip.mockReturnValue(false);
|
||||
s.db.canAccessTrip.mockResolvedValue(false);
|
||||
expect((await tool.read({ tripId: 10 }, ctx)).isError).toBe(true);
|
||||
expect((await tool.update({ tripId: 10, settings: {} }, ctx)).isError).toBe(true);
|
||||
});
|
||||
|
||||
@@ -9,14 +9,14 @@ import { DayBoundariesMcp } from '../../../src/nest/roadtrip/day-boundaries.mcp'
|
||||
const ctx = { userId: 5 } as McpContext;
|
||||
describe('roadtrip read tools', () => {
|
||||
it('checks trip access before querying charging and hazards', async () => {
|
||||
const db = { canAccessTrip: vi.fn(() => false) };
|
||||
const db = { canAccessTrip: vi.fn(async () => false) };
|
||||
const source = { read: vi.fn(async () => ({ sources: ['public'] })) };
|
||||
const charging = new ChargingMcp(source as never, db as never, {} as never);
|
||||
const hazards = new RoadtripHazardsMcp(source as never, db as never, {} as never);
|
||||
expect((await charging.read({ tripId: 10, placeId: 2 }, ctx)).isError).toBe(true);
|
||||
expect((await hazards.read({ tripId: 10 }, ctx)).isError).toBe(true);
|
||||
expect(source.read).not.toHaveBeenCalled();
|
||||
db.canAccessTrip.mockReturnValue(true);
|
||||
db.canAccessTrip.mockResolvedValue(true);
|
||||
expect(JSON.stringify(await charging.read({ tripId: 10, placeId: 2 }, ctx))).toContain('public');
|
||||
expect(source.read).toHaveBeenCalledWith(10, 2);
|
||||
expect(JSON.stringify(await hazards.read({ tripId: 10 }, ctx))).toContain('public');
|
||||
@@ -51,7 +51,7 @@ describe('roadtrip read tools', () => {
|
||||
const refuse = (error: string, status: number) => () => { throw new HttpException({ error }, status); };
|
||||
const text = (res: { content: { text: string }[]; isError?: boolean }) => [res.isError, res.content[0].text];
|
||||
|
||||
const db = { canAccessTrip: vi.fn(() => true) };
|
||||
const db = { canAccessTrip: vi.fn(async () => true) };
|
||||
const charging = new ChargingMcp({ read: refuse('Place not found', 404) } as never, db as never, {} as never);
|
||||
expect(text(await charging.read({ tripId: 10, placeId: 2 }, ctx))).toEqual([true, 'Place not found']);
|
||||
|
||||
@@ -71,11 +71,11 @@ describe('roadtrip read tools', () => {
|
||||
|
||||
it('does not expose manual boundaries to nonmembers', async () => {
|
||||
const boundaries = { list: vi.fn(() => [{ day_number: 1 }]) };
|
||||
const db = { canAccessTrip: vi.fn(() => false) };
|
||||
const db = { canAccessTrip: vi.fn(async () => false) };
|
||||
const tool = new DayBoundariesMcp(boundaries as never, db as never, {} as never, {} as never, {} as never, {} as never);
|
||||
expect((await tool.list({ tripId: 10 }, ctx)).isError).toBe(true);
|
||||
expect(boundaries.list).not.toHaveBeenCalled();
|
||||
db.canAccessTrip.mockReturnValue(true);
|
||||
db.canAccessTrip.mockResolvedValue(true);
|
||||
expect(JSON.stringify(await tool.list({ tripId: 10 }, ctx))).toContain('day_number');
|
||||
expect(boundaries.list).toHaveBeenCalledWith(10);
|
||||
});
|
||||
|
||||
@@ -73,11 +73,12 @@ beforeAll(async () => {
|
||||
createTables(testDb);
|
||||
runMigrations(testDb);
|
||||
t = await sharedTestOrm(testDb);
|
||||
const dbs = new DatabaseService(testDb, t.em);
|
||||
svc = new ShareService(
|
||||
new DatabaseService(testDb),
|
||||
dbs,
|
||||
new SettingsService(await createTestUnitOfWork(testDb), await createTestAppSettingsRepo(testDb), await createTestSettingsRepo(testDb)),
|
||||
permissionsStub,
|
||||
new QueryHelpersService(new DatabaseService(testDb)),
|
||||
new QueryHelpersService(dbs),
|
||||
photoCacheStub,
|
||||
await createTestUnitOfWork(testDb),
|
||||
);
|
||||
|
||||
@@ -48,12 +48,12 @@ import { DatabaseService } from '../../../src/nest/database/database.service';
|
||||
import { PermissionsService } from '../../../src/nest/permissions/permissions.service';
|
||||
import { TodoService } from '../../../src/nest/todo/todo.service';
|
||||
import { RealtimeService } from '../../../src/nest/realtime/realtime.service';
|
||||
import { createTestUnitOfWork, createTestAppSettingsRepo } from '../../helpers/test-uow';
|
||||
import { createTestUnitOfWork, createTestAppSettingsRepo, createTestDatabaseService } from '../../helpers/test-uow';
|
||||
|
||||
let svc: TodoService;
|
||||
beforeAll(async () => {
|
||||
const uow = await createTestUnitOfWork(testDb);
|
||||
svc = new TodoService(new DatabaseService(testDb), new PermissionsService(await createTestAppSettingsRepo(testDb), uow), new RealtimeService(), uow);
|
||||
svc = new TodoService(await createTestDatabaseService(testDb), new PermissionsService(await createTestAppSettingsRepo(testDb), uow), new RealtimeService(), uow);
|
||||
});
|
||||
|
||||
beforeAll(() => {
|
||||
|
||||
@@ -18,27 +18,36 @@ import {
|
||||
TripAccessGuard,
|
||||
} from '../../../src/nest/permissions/trip-access.guard';
|
||||
import { Trip } from '../../../src/nest/permissions/trip.decorator';
|
||||
import type { DatabaseService } from '../../../src/nest/database/database.service';
|
||||
import type { PermissionsService } from '../../../src/nest/permissions/permissions.service';
|
||||
import type { User } from '../../../src/types';
|
||||
import type { EntityManager } from '@mikro-orm/core';
|
||||
|
||||
const owner = { id: 42, role: 'user' } as User;
|
||||
const member = { id: 7, role: 'user' } as User;
|
||||
/** Trip 5 belongs to user 42; user 7 is a member of it. Nothing else is reachable. */
|
||||
const TRIP = { id: 5, user_id: 42 };
|
||||
|
||||
/**
|
||||
* Plan 3c Task 0b: the guard now injects `EntityManager` and resolves
|
||||
* `TripsRepository` inside `canActivate` (the `JwtAuthGuard` precedent),
|
||||
* instead of `DatabaseService.canAccessTrip` — so the double here is an
|
||||
* `EntityManager` whose `getRepository(Trips)` returns a `findAccessible`
|
||||
* stand-in, not a `DatabaseService` stand-in.
|
||||
*/
|
||||
function makeGuard(options: { checkPermission?: ReturnType<typeof vi.fn>; action?: string } = {}) {
|
||||
const canAccessTrip = vi.fn((tripId: number, userId: number) =>
|
||||
// async, not a bare sync return: `TripAccessGuard.canActivate` awaits this
|
||||
// call, and a synchronous double would not catch a dropped `await` there
|
||||
// (a missing `await` on a genuinely async repository call leaves `trip` as
|
||||
// an always-truthy Promise, never falling into the 404 branch — task-0a
|
||||
// mechanics review F1).
|
||||
const findAccessible = vi.fn(async (tripId: number, userId: number) =>
|
||||
tripId === 5 && (userId === 42 || userId === 7) ? TRIP : undefined,
|
||||
);
|
||||
const em = { getRepository: vi.fn(() => ({ findAccessible })) } as unknown as EntityManager;
|
||||
const checkPermission = options.checkPermission ?? vi.fn(() => true);
|
||||
const reflector = { getAllAndOverride: vi.fn(() => options.action) } as unknown as Reflector;
|
||||
const guard = new TripAccessGuard(
|
||||
{ canAccessTrip } as unknown as DatabaseService,
|
||||
{ checkPermission } as unknown as PermissionsService,
|
||||
reflector,
|
||||
);
|
||||
return { guard, canAccessTrip, checkPermission, reflector };
|
||||
const guard = new TripAccessGuard(em, { checkPermission } as unknown as PermissionsService, reflector);
|
||||
return { guard, canAccessTrip: findAccessible, checkPermission, reflector };
|
||||
}
|
||||
|
||||
/** The slice of ExecutionContext the guard reads. */
|
||||
|
||||
@@ -28,7 +28,7 @@ const tc = (s: TripMembersService) => new TripMembersController(s, audit);
|
||||
|
||||
function svc(o: Partial<TripMembersService> = {}): TripMembersService {
|
||||
return {
|
||||
canAccessTrip: vi.fn().mockReturnValue({ user_id: 1 }),
|
||||
canAccessTrip: vi.fn().mockResolvedValue({ user_id: 1 }),
|
||||
can: vi.fn().mockReturnValue(true),
|
||||
broadcast: vi.fn(),
|
||||
notifyInvite: vi.fn(),
|
||||
@@ -51,7 +51,7 @@ beforeEach(() => vi.clearAllMocks());
|
||||
describe('TripMembersController', () => {
|
||||
describe('members', () => {
|
||||
it('GET 404 without access, else owner+members+current_user_id', async () => {
|
||||
expect(await thrown(() => tc(svc({ canAccessTrip: vi.fn().mockReturnValue(undefined) })).members(user, '9'))).toEqual({ status: 404, body: { error: 'Trip not found' } });
|
||||
expect(await thrown(() => tc(svc({ canAccessTrip: vi.fn().mockResolvedValue(undefined) })).members(user, '9'))).toEqual({ status: 404, body: { error: 'Trip not found' } });
|
||||
const s = svc({ listMembers: vi.fn().mockReturnValue({ owner: { id: 1 }, members: [] }) } as Partial<TripMembersService>);
|
||||
expect(await tc(s).members(user, '9')).toEqual({ owner: { id: 1 }, members: [], current_user_id: 1 });
|
||||
});
|
||||
@@ -66,7 +66,7 @@ describe('members', () => {
|
||||
});
|
||||
|
||||
it('POST 404 without trip access', async () => {
|
||||
const s = svc({ canAccessTrip: vi.fn().mockReturnValue(undefined) });
|
||||
const s = svc({ canAccessTrip: vi.fn().mockResolvedValue(undefined) });
|
||||
expect(await thrown(() => tc(s).addMember(user, '9', { identifier: 'bob@x.y' }))).toEqual({ status: 404, body: { error: 'Trip not found' } });
|
||||
});
|
||||
|
||||
@@ -80,7 +80,7 @@ describe('members', () => {
|
||||
});
|
||||
|
||||
it('DELETE 404 without trip access', async () => {
|
||||
const s = svc({ canAccessTrip: vi.fn().mockReturnValue(undefined) });
|
||||
const s = svc({ canAccessTrip: vi.fn().mockResolvedValue(undefined) });
|
||||
expect(await thrown(() => tc(s).removeMember(user, '9', '2'))).toEqual({ status: 404, body: { error: 'Trip not found' } });
|
||||
});
|
||||
|
||||
|
||||
@@ -65,13 +65,22 @@ 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 } from '../../helpers/test-uow';
|
||||
import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, sharedTestOrm } from '../../helpers/test-uow';
|
||||
import type { EntityManager } from '@mikro-orm/core';
|
||||
|
||||
const dbs = () => new DatabaseService(testDb);
|
||||
// Plan 3c Task 0b: `dbsEm` is resolved once, at the top of the `beforeAll`
|
||||
// below, before any `dbs()` call — `canAccessTrip`/`isOwner`/`rosterUserIds`/
|
||||
// `getPlaceWithTags` resolve `TripsRepository`/`TripMembersRepository`/
|
||||
// `PlacesRepository` through it now, not through `db/database.ts`'s deleted
|
||||
// free functions. Kept as a module-level variable (not threaded through
|
||||
// `dbs()`'s signature) so the ~17 existing `dbs()` call sites stay unchanged.
|
||||
let dbsEm: EntityManager | undefined;
|
||||
const dbs = () => new DatabaseService(testDb, dbsEm);
|
||||
|
||||
let budgetSvc: BudgetService;
|
||||
let roster: TripMembersService;
|
||||
beforeAll(async () => {
|
||||
dbsEm = (await sharedTestOrm(testDb)).em;
|
||||
budgetSvc = new BudgetService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(dbs().connection));
|
||||
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));
|
||||
});
|
||||
|
||||
@@ -13,7 +13,7 @@ import {
|
||||
TRIP_OWNER_KEY,
|
||||
TripOwnerGuard,
|
||||
} from '../../../src/nest/permissions/trip-owner.guard';
|
||||
import type { DatabaseService } from '../../../src/nest/database/database.service';
|
||||
import type { EntityManager } from '@mikro-orm/core';
|
||||
import type { User } from '../../../src/types';
|
||||
|
||||
const owner = { id: 42, role: 'user' } as User;
|
||||
@@ -22,13 +22,25 @@ const admin = { id: 99, role: 'admin' } as User;
|
||||
/** Trip 5 belongs to user 42; user 7 is a member. Nobody else can reach it. */
|
||||
const TRIP = { id: 5, user_id: 42 };
|
||||
|
||||
/**
|
||||
* Plan 3c Task 0b (task-0a-review-security.md F-A1): the guard now injects
|
||||
* `EntityManager` and resolves `TripsRepository` inside `canActivate` (the
|
||||
* `TripAccessGuard`/`JwtAuthGuard` precedent), instead of `DatabaseService
|
||||
* .canAccessTrip` — this guard is the SOLE access check on
|
||||
* `trip-members.controller.ts`'s four owner-only routes (no class-level
|
||||
* `TripAccessGuard` there), so it gets the same direct-repository wiring.
|
||||
*/
|
||||
function makeGuard(meta: { message: string; param?: string } | undefined = { message: 'Only the owner can do this' }) {
|
||||
const canAccessTrip = vi.fn((tripId: number, userId: number) =>
|
||||
// async, not a bare sync return: `TripOwnerGuard.canActivate` awaits this
|
||||
// call, and a synchronous double would not catch a dropped `await` there
|
||||
// (task-0a mechanics review F1).
|
||||
const findAccessible = vi.fn(async (tripId: number, userId: number) =>
|
||||
tripId === 5 && (userId === 42 || userId === 7) ? TRIP : undefined,
|
||||
);
|
||||
const em = { getRepository: vi.fn(() => ({ findAccessible })) } as unknown as EntityManager;
|
||||
const reflector = { getAllAndOverride: vi.fn(() => meta) } as unknown as Reflector;
|
||||
const guard = new TripOwnerGuard({ canAccessTrip } as unknown as DatabaseService, reflector);
|
||||
return { guard, canAccessTrip };
|
||||
const guard = new TripOwnerGuard(em, reflector);
|
||||
return { guard, canAccessTrip: findAccessible };
|
||||
}
|
||||
|
||||
function ctx(request: Record<string, unknown>) {
|
||||
@@ -77,9 +89,10 @@ describe('TripOwnerGuard', () => {
|
||||
// returns true for every admin and the trip actions are admin-lowerable, so routing
|
||||
// ownership through it would hand admins other people's trips. The guard does not
|
||||
// inject it at all.
|
||||
const canAccessTrip = vi.fn(() => TRIP);
|
||||
const findAccessible = vi.fn(async () => TRIP);
|
||||
const em = { getRepository: vi.fn(() => ({ findAccessible })) } as unknown as EntityManager;
|
||||
const reflector = { getAllAndOverride: vi.fn(() => ({ message: 'Only the owner can transfer ownership' })) } as unknown as Reflector;
|
||||
const guard = new TripOwnerGuard({ canAccessTrip } as unknown as DatabaseService, reflector);
|
||||
const guard = new TripOwnerGuard(em, reflector);
|
||||
expect(await thrown(() => guard.canActivate(ctx({ user: admin, params: { tripId: '5' } })))).toEqual({
|
||||
status: 403,
|
||||
body: { error: 'Only the owner can transfer ownership' },
|
||||
@@ -129,9 +142,10 @@ describe('TripOwnerGuard', () => {
|
||||
|
||||
it('OWNER-010: without metadata it still refuses a non-owner, with a generic message', async () => {
|
||||
// Built inline: passing undefined to makeGuard would hit its own default.
|
||||
const canAccessTrip = vi.fn(() => TRIP);
|
||||
const findAccessible = vi.fn(async () => TRIP);
|
||||
const em = { getRepository: vi.fn(() => ({ findAccessible })) } as unknown as EntityManager;
|
||||
const reflector = { getAllAndOverride: vi.fn(() => undefined) } as unknown as Reflector;
|
||||
const guard = new TripOwnerGuard({ canAccessTrip } as unknown as DatabaseService, reflector);
|
||||
const guard = new TripOwnerGuard(em, reflector);
|
||||
expect(await thrown(() => guard.canActivate(ctx({ user: member, params: { tripId: '5' } })))).toEqual({
|
||||
status: 403,
|
||||
body: { error: 'Only the trip owner can do this' },
|
||||
|
||||
@@ -42,7 +42,7 @@ const tc = (s: TripsService, calendar?: Partial<CalendarService>, readModel?: Pa
|
||||
|
||||
function svc(o: Partial<TripsService> = {}): TripsService {
|
||||
return {
|
||||
canAccessTrip: vi.fn().mockReturnValue({ user_id: 1 }),
|
||||
canAccessTrip: vi.fn().mockResolvedValue({ user_id: 1 }),
|
||||
can: vi.fn().mockReturnValue(true),
|
||||
broadcast: vi.fn(),
|
||||
notifyInvite: vi.fn(),
|
||||
@@ -206,7 +206,7 @@ describe('TripsController (parity with the legacy /api/trips route)', async () =
|
||||
|
||||
describe('PUT /:id', () => {
|
||||
it('404 when no access; 403 on archive without trip_archive', async () => {
|
||||
expect(await thrownAsync(() => tc(svc({ canAccessTrip: vi.fn().mockReturnValue(undefined) })).update(user, '9', {}, req))).toEqual({ status: 404, body: { error: 'Trip not found' } });
|
||||
expect(await thrownAsync(() => tc(svc({ canAccessTrip: vi.fn().mockResolvedValue(undefined) })).update(user, '9', {}, req))).toEqual({ status: 404, body: { error: 'Trip not found' } });
|
||||
const s = svc({ can: vi.fn().mockImplementation((a: string) => a !== 'trip_archive') });
|
||||
expect(await thrownAsync(() => tc(s).update(user, '9', { is_archived: 1 }, req))).toEqual({ status: 403, body: { error: 'No permission to archive/unarchive this trip' } });
|
||||
});
|
||||
@@ -273,7 +273,7 @@ describe('TripsController (parity with the legacy /api/trips route)', async () =
|
||||
describe('POST /:id/copy', () => {
|
||||
it('403 without trip_create, 404 without access', async () => {
|
||||
expect(await thrownAsync(() => tc(svc({ can: vi.fn().mockReturnValue(false) })).copy(user, '9', {}, req))).toEqual({ status: 403, body: { error: 'No permission to create trips' } });
|
||||
expect(await thrownAsync(() => tc(svc({ canAccessTrip: vi.fn().mockReturnValue(undefined) })).copy(user, '9', {}, req))).toEqual({ status: 404, body: { error: 'Trip not found' } });
|
||||
expect(await thrownAsync(() => tc(svc({ canAccessTrip: vi.fn().mockResolvedValue(undefined) })).copy(user, '9', {}, req))).toEqual({ status: 404, body: { error: 'Trip not found' } });
|
||||
});
|
||||
|
||||
it('copies + returns the new trip', async () => {
|
||||
@@ -290,7 +290,7 @@ describe('TripsController (parity with the legacy /api/trips route)', async () =
|
||||
});
|
||||
|
||||
it('404s for someone with no access, so the 403 is not an existence oracle', async () => {
|
||||
const s = svc({ getOwner: vi.fn().mockReturnValue({ user_id: 2 }), canAccessTrip: vi.fn().mockReturnValue(null), remove: vi.fn() } as Partial<TripsService>);
|
||||
const s = svc({ getOwner: vi.fn().mockReturnValue({ user_id: 2 }), canAccessTrip: vi.fn().mockResolvedValue(null), remove: vi.fn() } as Partial<TripsService>);
|
||||
expect(await thrownAsync(() => tc(s).remove(user, '9', req))).toEqual({ status: 404, body: { error: 'Trip not found' } });
|
||||
expect(s.remove).not.toHaveBeenCalled();
|
||||
});
|
||||
@@ -298,7 +298,7 @@ describe('TripsController (parity with the legacy /api/trips route)', async () =
|
||||
it('still lets an admin delete a trip they are not a member of', async () => {
|
||||
const admin = { id: 1, role: 'admin', email: 'a@example.test' } as User;
|
||||
const remove = vi.fn().mockReturnValue({ tripId: 9, title: 'T', isAdminDelete: true, ownerEmail: 'owner@x.y' });
|
||||
const s = svc({ getOwner: vi.fn().mockReturnValue({ user_id: 2 }), canAccessTrip: vi.fn().mockReturnValue(null), remove, broadcast: vi.fn() } as Partial<TripsService>);
|
||||
const s = svc({ getOwner: vi.fn().mockReturnValue({ user_id: 2 }), canAccessTrip: vi.fn().mockResolvedValue(null), remove, broadcast: vi.fn() } as Partial<TripsService>);
|
||||
expect(await tc(s).remove(admin, '9', req)).toEqual({ success: true });
|
||||
expect(remove).toHaveBeenCalledWith('9', 1, 'admin');
|
||||
});
|
||||
@@ -329,7 +329,7 @@ describe('TripsController (parity with the legacy /api/trips route)', async () =
|
||||
describe('POST /:id/cover', () => {
|
||||
const file = { filename: 'abc.jpg' } as Express.Multer.File;
|
||||
it('404 without access, 403 without permission, 404 raw trip, 400 no file, else commits + returns url', async () => {
|
||||
expect(await thrownAsync(() => tc(svc({ canAccessTrip: vi.fn().mockReturnValue(undefined) })).cover(user, '9', file))).toEqual({ status: 404, body: { error: 'Trip not found' } });
|
||||
expect(await thrownAsync(() => tc(svc({ canAccessTrip: vi.fn().mockResolvedValue(undefined) })).cover(user, '9', file))).toEqual({ status: 404, body: { error: 'Trip not found' } });
|
||||
expect(await thrownAsync(() => tc(svc({ can: vi.fn().mockReturnValue(false) })).cover(user, '9', file))).toEqual({ status: 403, body: { error: 'No permission to change the cover image' } });
|
||||
expect(await thrownAsync(() => tc(svc({ getRaw: vi.fn().mockReturnValue(undefined) } as Partial<TripsService>)).cover(user, '9', file))).toEqual({ status: 404, body: { error: 'Trip not found' } });
|
||||
expect(await thrownAsync(() => tc(svc({ getRaw: vi.fn().mockReturnValue({ cover_image: null }) } as Partial<TripsService>)).cover(user, '9', undefined))).toEqual({ status: 400, body: { error: 'No image uploaded' } });
|
||||
@@ -386,7 +386,7 @@ describe('TripsController (parity with the legacy /api/trips route)', async () =
|
||||
describe('GET /:id/export.ics', () => {
|
||||
function makeRes() { return { setHeader: vi.fn(), send: vi.fn() } as never; }
|
||||
it('404 without access, else sends the calendar with headers', async () => {
|
||||
expect(await thrownAsync(() => tc(svc({ canAccessTrip: vi.fn().mockReturnValue(undefined) })).exportIcs(user, '9', makeRes()))).toEqual({ status: 404, body: { error: 'Trip not found' } });
|
||||
expect(await thrownAsync(() => tc(svc({ canAccessTrip: vi.fn().mockResolvedValue(undefined) })).exportIcs(user, '9', makeRes()))).toEqual({ status: 404, body: { error: 'Trip not found' } });
|
||||
const res = { setHeader: vi.fn(), send: vi.fn() };
|
||||
const cal = { exportICS: vi.fn().mockReturnValue({ ics: 'BEGIN:VCALENDAR', filename: 'trip.ics' }) };
|
||||
await tc(svc(), cal).exportIcs(user, '9', res as never);
|
||||
|
||||
@@ -92,7 +92,16 @@ import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, s
|
||||
|
||||
// Real sibling services over the same in-memory DB — updateTrip's date-shift
|
||||
// resyncs and the summary/bundle aggregation run their actual SQL.
|
||||
const dbs = () => new DatabaseService(testDb);
|
||||
//
|
||||
// Plan 3c Task 0b: `dbsEm` is resolved once, at the top of the first
|
||||
// `beforeAll` below, before any `dbs()` call — `canAccessTrip`/`isOwner`/
|
||||
// `rosterUserIds`/`getPlaceWithTags` resolve `TripsRepository`/
|
||||
// `TripMembersRepository`/`PlacesRepository` through it now, not through
|
||||
// `db/database.ts`'s deleted free functions. Kept as a module-level variable
|
||||
// (not threaded through `dbs()`'s signature) so the many existing `dbs()`
|
||||
// call sites in this file stay unchanged.
|
||||
let dbsEm: import('@mikro-orm/core').EntityManager | undefined;
|
||||
const dbs = () => new DatabaseService(testDb, dbsEm);
|
||||
|
||||
|
||||
// Same collaborator set the container hands PlacesService (see places.service.test.ts).
|
||||
@@ -122,6 +131,7 @@ let svc: TripsService;
|
||||
let membersSvc: TripMembersService;
|
||||
let readModelSvc: TripReadModelService;
|
||||
beforeAll(async () => {
|
||||
dbsEm = (await sharedTestOrm(testDb)).em;
|
||||
budgetSvc = new BudgetService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(dbs().connection));
|
||||
daysSvc = new DaysService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new RealtimeService(), new QueryHelpersService(dbs()), await createTestUnitOfWork(dbs().connection));
|
||||
placesSvc = new PlacesService(
|
||||
@@ -147,6 +157,7 @@ beforeAll(async () => {
|
||||
undefined as never, // unsplash — not exercised here
|
||||
coversFx.storage,
|
||||
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));
|
||||
readModelSvc = new TripReadModelService(
|
||||
@@ -1214,6 +1225,7 @@ describe('quirk fixes', () => {
|
||||
undefined as never,
|
||||
coversFx.storage,
|
||||
await createTestUnitOfWork(dbs().connection),
|
||||
(await sharedTestOrm(testDb)).em,
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -51,7 +51,7 @@ import { DatabaseService } from '../../../src/nest/database/database.service';
|
||||
import type { ImmichService } from '../../../src/nest/memories/immich.service';
|
||||
import type { SynologyService } from '../../../src/nest/memories/synology.service';
|
||||
import { notificationsStub } from '../../helpers/notifications';
|
||||
import { createTestUnitOfWork } from '../../helpers/test-uow';
|
||||
import { createTestUnitOfWork, sharedTestOrm } from '../../helpers/test-uow';
|
||||
|
||||
// The album-sync paths are the providers' half and have their own suites; these
|
||||
// cases never reach them, so stubs keep the graph small.
|
||||
@@ -74,6 +74,13 @@ const removeAlbumLink = (...a: Parameters<Svc['removeAlbumLink']>) => svc.remove
|
||||
beforeAll(async () => {
|
||||
createTables(testDb);
|
||||
runMigrations(testDb);
|
||||
// Plan 3c Task 0b: `dbs` is constructed at module load, before any
|
||||
// `beforeAll` can resolve a real `EntityManager` — `canAccessTrip` (the
|
||||
// only one of the four primitives this suite's code paths reach) is spied
|
||||
// directly on this instance, routed to a real `DatabaseService` built
|
||||
// with one.
|
||||
const real = new DatabaseService(testDb, (await sharedTestOrm(testDb)).em);
|
||||
vi.spyOn(dbs, 'canAccessTrip').mockImplementation((...a) => real.canAccessTrip(...a));
|
||||
svc = new UnifiedMemoriesService(
|
||||
dbs,
|
||||
new TrekPhotosRepository(dbs),
|
||||
|
||||
@@ -49,13 +49,22 @@ import { ExchangeRatesService } from '../../../src/nest/budget/exchange-rates.se
|
||||
import { RealtimeService } from '../../../src/nest/realtime/realtime.service';
|
||||
import { BudgetService } from '../../../src/nest/budget/budget.service';
|
||||
import { UserCleanupService } from '../../../src/nest/auth/user-cleanup.service';
|
||||
import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo } from '../../helpers/test-uow';
|
||||
import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, sharedTestOrm } from '../../helpers/test-uow';
|
||||
|
||||
const dbs = new DatabaseService(testDb);
|
||||
|
||||
let budget: BudgetService;
|
||||
let svc: UserCleanupService;
|
||||
beforeAll(async () => {
|
||||
// Plan 3c Task 0b: `dbs` is constructed at module load, before any
|
||||
// `beforeAll` can resolve a real `EntityManager` — the four
|
||||
// repository-backed methods are spied directly on this instance instead,
|
||||
// routed to a real `DatabaseService` built with one.
|
||||
const real = new DatabaseService(testDb, (await sharedTestOrm(testDb)).em);
|
||||
vi.spyOn(dbs, 'canAccessTrip').mockImplementation((...a) => real.canAccessTrip(...a));
|
||||
vi.spyOn(dbs, 'isOwner').mockImplementation((...a) => real.isOwner(...a));
|
||||
vi.spyOn(dbs, 'rosterUserIds').mockImplementation((...a) => real.rosterUserIds(...a));
|
||||
vi.spyOn(dbs, 'getPlaceWithTags').mockImplementation((...a) => real.getPlaceWithTags(...a));
|
||||
budget = new BudgetService(dbs, new PermissionsService(await createTestAppSettingsRepo(dbs.connection), await createTestUnitOfWork(dbs.connection)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(dbs.connection));
|
||||
svc = new UserCleanupService(dbs, budget, await createTestUnitOfWork(dbs.connection), await createTestUsersRepo(dbs.connection));
|
||||
});
|
||||
|
||||
@@ -51,7 +51,7 @@ function build(opts: { canEdit?: boolean; before?: Item | undefined; updated?: I
|
||||
Object.assign(packing, data);
|
||||
const guards = new PluginGuards(
|
||||
{
|
||||
canAccessTrip: vi.fn((tripId: number, userId: number) => (tripId === 1 && userId === 42 ? { id: 1, user_id: 42 } : undefined)),
|
||||
canAccessTrip: vi.fn(async (tripId: number, userId: number) => (tripId === 1 && userId === 42 ? { id: 1, user_id: 42 } : undefined)),
|
||||
prepare: vi.fn(() => ({ get: () => ({ role: 'user' }) })),
|
||||
} as unknown as DatabaseService,
|
||||
{ checkPermission: vi.fn(() => opts.canEdit ?? true) } as unknown as PermissionsService,
|
||||
|
||||
@@ -52,7 +52,7 @@ function build(opts: { canEdit?: boolean; journeyThrows?: boolean } = {}) {
|
||||
const realtime = { broadcast: vi.fn() };
|
||||
const guards = new PluginGuards(
|
||||
{
|
||||
canAccessTrip: vi.fn((tripId: number, userId: number) => (tripId === 1 && userId === 42 ? { id: 1, user_id: 42 } : undefined)),
|
||||
canAccessTrip: vi.fn(async (tripId: number, userId: number) => (tripId === 1 && userId === 42 ? { id: 1, user_id: 42 } : undefined)),
|
||||
prepare: vi.fn(() => ({ get: () => ({ role: 'user' }) })),
|
||||
} as unknown as DatabaseService,
|
||||
{ checkPermission: vi.fn(() => opts.canEdit ?? true) } as unknown as PermissionsService,
|
||||
|
||||
@@ -38,7 +38,7 @@ function build(overrides: {
|
||||
collections?: Record<string, unknown>;
|
||||
} = {}) {
|
||||
const db = {
|
||||
canAccessTrip: vi.fn(() => ({ id: 1, user_id: 42 })),
|
||||
canAccessTrip: vi.fn(async () => ({ id: 1, user_id: 42 })),
|
||||
prepare: vi.fn(() => ({ get: () => ({ role: 'user' }), all: () => [] })),
|
||||
} as unknown as DatabaseService;
|
||||
const guards = new PluginGuards(
|
||||
|
||||
@@ -56,7 +56,7 @@ function spyService(calls: string[], name: string) {
|
||||
function build(addonOn: boolean) {
|
||||
const calls: string[] = [];
|
||||
const db = {
|
||||
canAccessTrip: vi.fn(() => ({ id: 1, user_id: 42 })),
|
||||
canAccessTrip: vi.fn(async () => ({ id: 1, user_id: 42 })),
|
||||
prepare: vi.fn(() => ({ get: () => ({ role: 'user' }), all: () => [] })),
|
||||
} as unknown as DatabaseService;
|
||||
const guards = new PluginGuards(
|
||||
|
||||
@@ -8,7 +8,7 @@
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
|
||||
const { canAccessTrip, pluginsEnabled, tripDays } = vi.hoisted(() => ({
|
||||
canAccessTrip: vi.fn((tripId: number, userId: number) => (tripId === 1 && userId === 5 ? { id: 1 } : undefined)),
|
||||
canAccessTrip: vi.fn(async (tripId: number, userId: number) => (tripId === 1 && userId === 5 ? { id: 1 } : undefined)),
|
||||
pluginsEnabled: vi.fn(() => true),
|
||||
tripDays: { value: [{ id: 10 }, { id: 11 }] as Array<{ id: number }> },
|
||||
}));
|
||||
@@ -16,7 +16,6 @@ vi.mock('../../../src/db/database', () => ({
|
||||
db: { prepare: () => ({ all: () => tripDays.value }) },
|
||||
canAccessTrip,
|
||||
}));
|
||||
import { db as dbConn } from '../../../src/db/database';
|
||||
import { DatabaseService } from '../../../src/nest/database/database.service';
|
||||
vi.mock('../../../src/nest/plugins/kill-switch', () => ({ pluginsEnabled }));
|
||||
|
||||
@@ -30,12 +29,12 @@ function controller(invoke: (id: string) => unknown, providers = ['p1']) {
|
||||
providersOf: vi.fn(() => providers),
|
||||
daySchedule: vi.fn(async (id: string) => invoke(id)),
|
||||
} as unknown as PluginHooks;
|
||||
return { c: new DayScheduleController(runtime, new DatabaseService(dbConn)), runtime };
|
||||
return { c: new DayScheduleController(runtime, { canAccessTrip, connection: { prepare: () => ({ all: () => tripDays.value }) } } as unknown as DatabaseService), runtime };
|
||||
}
|
||||
const item = (over: Record<string, unknown> = {}) => ({ id: 's1', dayId: 10, label: 'Charging', ...over });
|
||||
|
||||
describe('DayScheduleController', () => {
|
||||
beforeEach(() => { pluginsEnabled.mockReturnValue(true); canAccessTrip.mockReturnValue({ id: 1 } as never); });
|
||||
beforeEach(() => { pluginsEnabled.mockReturnValue(true); canAccessTrip.mockResolvedValue({ id: 1 } as never); });
|
||||
|
||||
it('gates: disabled / no user / non-member all return [] (no plugin calls on the first)', async () => {
|
||||
pluginsEnabled.mockReturnValue(false);
|
||||
@@ -45,7 +44,7 @@ describe('DayScheduleController', () => {
|
||||
pluginsEnabled.mockReturnValue(true);
|
||||
|
||||
expect((await controller(() => [item()]).c.get('1', req(undefined))).items).toEqual([]);
|
||||
canAccessTrip.mockReturnValue(undefined as never);
|
||||
canAccessTrip.mockResolvedValue(undefined as never);
|
||||
expect((await controller(() => [item()]).c.get('1', req(5))).items).toEqual([]);
|
||||
});
|
||||
|
||||
|
||||
@@ -8,7 +8,7 @@
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
|
||||
const { canAccessTrip, pluginsEnabled, tripDays } = vi.hoisted(() => ({
|
||||
canAccessTrip: vi.fn((tripId: number, userId: number) => (tripId === 1 && userId === 5 ? { id: 1 } : undefined)),
|
||||
canAccessTrip: vi.fn(async (tripId: number, userId: number) => (tripId === 1 && userId === 5 ? { id: 1 } : undefined)),
|
||||
pluginsEnabled: vi.fn(() => true),
|
||||
tripDays: { value: [{ id: 10 }, { id: 11 }] as Array<{ id: number }> },
|
||||
}));
|
||||
@@ -16,7 +16,6 @@ vi.mock('../../../src/db/database', () => ({
|
||||
db: { prepare: () => ({ all: () => tripDays.value }) },
|
||||
canAccessTrip,
|
||||
}));
|
||||
import { db as dbConn } from '../../../src/db/database';
|
||||
import { DatabaseService } from '../../../src/nest/database/database.service';
|
||||
vi.mock('../../../src/nest/plugins/kill-switch', () => ({ pluginsEnabled }));
|
||||
|
||||
@@ -30,12 +29,12 @@ function controller(invoke: (id: string) => unknown, providers = ['p1']) {
|
||||
providersOf: vi.fn(() => providers),
|
||||
dayTints: vi.fn(async (id: string) => invoke(id)),
|
||||
} as unknown as PluginHooks;
|
||||
return { c: new DayTintsController(runtime, new DatabaseService(dbConn)), runtime };
|
||||
return { c: new DayTintsController(runtime, { canAccessTrip, connection: { prepare: () => ({ all: () => tripDays.value }) } } as unknown as DatabaseService), runtime };
|
||||
}
|
||||
const tint = (over: Record<string, unknown> = {}) => ({ dayId: 10, tone: 'success', ...over });
|
||||
|
||||
describe('DayTintsController', () => {
|
||||
beforeEach(() => { pluginsEnabled.mockReturnValue(true); canAccessTrip.mockReturnValue({ id: 1 } as never); });
|
||||
beforeEach(() => { pluginsEnabled.mockReturnValue(true); canAccessTrip.mockResolvedValue({ id: 1 } as never); });
|
||||
|
||||
it('gates: disabled / no user / non-member all return [] (no plugin calls on the first)', async () => {
|
||||
pluginsEnabled.mockReturnValue(false);
|
||||
@@ -45,7 +44,7 @@ describe('DayTintsController', () => {
|
||||
pluginsEnabled.mockReturnValue(true);
|
||||
|
||||
expect((await controller(() => [tint()]).c.get('1', req(undefined))).tints).toEqual([]);
|
||||
canAccessTrip.mockReturnValue(undefined as never);
|
||||
canAccessTrip.mockResolvedValue(undefined as never);
|
||||
expect((await controller(() => [tint()]).c.get('1', req(5))).tints).toEqual([]);
|
||||
});
|
||||
|
||||
|
||||
@@ -96,6 +96,16 @@ const userSettings = {
|
||||
} as unknown as PluginUserSettingsService;
|
||||
|
||||
const dbs = new DatabaseService(mockDb);
|
||||
// Plan 3c Task 0b: `canAccessTrip` is `TripsRepository.findAccessible` now,
|
||||
// which needs a real EntityManager over the full migrated schema — this
|
||||
// file's `mockDb` is a hand-trimmed `:memory:` table set (no `currency`
|
||||
// column on `trips`, among others), so a real repository read isn't
|
||||
// possible against it. Spied directly with the exact access rule the old
|
||||
// `db/database` mock factory above encoded (trip 1 belongs to user 5, user 6
|
||||
// is a member), which is now dead code for this purpose.
|
||||
vi.spyOn(dbs, 'canAccessTrip').mockImplementation(async (tripId, userId) =>
|
||||
Number(tripId) === 1 && (userId === 5 || userId === 6) ? { id: 1, user_id: 5, currency: null } : undefined,
|
||||
);
|
||||
const guards = new PluginGuards(dbs, permissions, addons);
|
||||
const registry = createTestPluginRegistry([
|
||||
new DbRpc(userSettings),
|
||||
|
||||
@@ -8,7 +8,7 @@
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
|
||||
const { canAccessTrip, pluginsEnabled } = vi.hoisted(() => ({
|
||||
canAccessTrip: vi.fn((tripId: number, userId: number) => (tripId === 1 && userId === 5 ? { id: 1 } : undefined)),
|
||||
canAccessTrip: vi.fn(async (tripId: number, userId: number) => (tripId === 1 && userId === 5 ? { id: 1 } : undefined)),
|
||||
pluginsEnabled: vi.fn(() => true),
|
||||
}));
|
||||
vi.mock('../../../src/db/database', () => ({ db: { prepare: () => ({ get: () => undefined }) }, canAccessTrip }));
|
||||
@@ -35,7 +35,7 @@ const line = (n = 3, over: Record<string, unknown> = {}) => ({
|
||||
const layer = (features: unknown[], over: Record<string, unknown> = {}) => ({ id: 'l1', features, ...over });
|
||||
|
||||
describe('MapLayersController', () => {
|
||||
beforeEach(() => { pluginsEnabled.mockReturnValue(true); canAccessTrip.mockReturnValue({ id: 1 } as never); });
|
||||
beforeEach(() => { pluginsEnabled.mockReturnValue(true); canAccessTrip.mockResolvedValue({ id: 1 } as never); });
|
||||
|
||||
it('gates: disabled / no user / non-member all return [] (no plugin calls on the first)', async () => {
|
||||
pluginsEnabled.mockReturnValue(false);
|
||||
@@ -45,7 +45,7 @@ describe('MapLayersController', () => {
|
||||
pluginsEnabled.mockReturnValue(true);
|
||||
|
||||
expect((await controller(() => [layer([line()])]).c.get('1', req(undefined))).layers).toEqual([]);
|
||||
canAccessTrip.mockReturnValue(undefined as never);
|
||||
canAccessTrip.mockResolvedValue(undefined as never);
|
||||
expect((await controller(() => [layer([line()])]).c.get('1', req(5))).layers).toEqual([]);
|
||||
});
|
||||
|
||||
|
||||
@@ -7,7 +7,7 @@
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
|
||||
const { canAccessTrip, pluginsEnabled } = vi.hoisted(() => ({
|
||||
canAccessTrip: vi.fn((tripId: number, userId: number) => (tripId === 1 && userId === 5 ? { id: 1 } : undefined)),
|
||||
canAccessTrip: vi.fn(async (tripId: number, userId: number) => (tripId === 1 && userId === 5 ? { id: 1 } : undefined)),
|
||||
pluginsEnabled: vi.fn(() => true),
|
||||
}));
|
||||
vi.mock('../../../src/db/database', () => ({ db: { prepare: () => ({ get: () => undefined }) }, canAccessTrip }));
|
||||
@@ -29,7 +29,7 @@ function controller(invoke: (id: string) => unknown, providers = ['p1']) {
|
||||
const mk = (over: Record<string, unknown> = {}) => ({ id: 'm1', lat: 48.85, lng: 2.35, ...over });
|
||||
|
||||
describe('MapMarkersController', () => {
|
||||
beforeEach(() => { pluginsEnabled.mockReturnValue(true); canAccessTrip.mockReturnValue({ id: 1 } as never); });
|
||||
beforeEach(() => { pluginsEnabled.mockReturnValue(true); canAccessTrip.mockResolvedValue({ id: 1 } as never); });
|
||||
|
||||
it('gates: disabled / no user / non-member all return [] (no plugin calls on the first)', async () => {
|
||||
pluginsEnabled.mockReturnValue(false);
|
||||
@@ -39,7 +39,7 @@ describe('MapMarkersController', () => {
|
||||
pluginsEnabled.mockReturnValue(true);
|
||||
|
||||
expect((await controller(() => [mk()]).c.get('1', req(undefined))).markers).toEqual([]);
|
||||
canAccessTrip.mockReturnValue(undefined as never);
|
||||
canAccessTrip.mockResolvedValue(undefined as never);
|
||||
expect((await controller(() => [mk()]).c.get('1', req(5))).markers).toEqual([]);
|
||||
});
|
||||
|
||||
|
||||
@@ -8,7 +8,7 @@
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
|
||||
const { canAccessTrip, pluginsEnabled } = vi.hoisted(() => ({
|
||||
canAccessTrip: vi.fn((tripId: number, userId: number) => (tripId === 1 && userId === 5 ? { id: 1 } : undefined)),
|
||||
canAccessTrip: vi.fn(async (tripId: number, userId: number) => (tripId === 1 && userId === 5 ? { id: 1 } : undefined)),
|
||||
pluginsEnabled: vi.fn(() => true),
|
||||
}));
|
||||
vi.mock('../../../src/db/database', () => ({ db: { prepare: () => ({ get: () => undefined }) }, canAccessTrip }));
|
||||
@@ -30,7 +30,7 @@ function controller(invoke: (id: string) => unknown, providers = ['p1']) {
|
||||
const sec = (over: Record<string, unknown> = {}) => ({ title: 'Weather', ...over });
|
||||
|
||||
describe('PdfSectionsController', () => {
|
||||
beforeEach(() => { pluginsEnabled.mockReturnValue(true); canAccessTrip.mockReturnValue({ id: 1 } as never); });
|
||||
beforeEach(() => { pluginsEnabled.mockReturnValue(true); canAccessTrip.mockResolvedValue({ id: 1 } as never); });
|
||||
|
||||
it('gates: disabled / bad tripId / no user / non-member all return [] (no plugin calls on the first)', async () => {
|
||||
pluginsEnabled.mockReturnValue(false);
|
||||
@@ -41,7 +41,7 @@ describe('PdfSectionsController', () => {
|
||||
|
||||
expect((await controller(() => [sec()]).c.get('abc', req(5))).sections).toEqual([]);
|
||||
expect((await controller(() => [sec()]).c.get('1', req(undefined))).sections).toEqual([]);
|
||||
canAccessTrip.mockReturnValue(undefined as never);
|
||||
canAccessTrip.mockResolvedValue(undefined as never);
|
||||
expect((await controller(() => [sec()]).c.get('1', req(5))).sections).toEqual([]);
|
||||
});
|
||||
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
|
||||
const { canAccessTrip, placeTrip, pluginsEnabled } = vi.hoisted(() => ({
|
||||
canAccessTrip: vi.fn((tripId: number, userId: number) => (tripId === 1 && userId === 5 ? { id: 1 } : undefined)),
|
||||
canAccessTrip: vi.fn(async (tripId: number, userId: number) => (tripId === 1 && userId === 5 ? { id: 1 } : undefined)),
|
||||
placeTrip: vi.fn((placeId: number) => (placeId === 7 ? { trip_id: 1 } : undefined)),
|
||||
pluginsEnabled: vi.fn(() => true),
|
||||
}));
|
||||
@@ -9,7 +9,6 @@ vi.mock('../../../src/db/database', () => ({
|
||||
db: { prepare: () => ({ get: (placeId: number) => placeTrip(placeId) }) },
|
||||
canAccessTrip,
|
||||
}));
|
||||
import { db as dbConn } from '../../../src/db/database';
|
||||
import { DatabaseService } from '../../../src/nest/database/database.service';
|
||||
vi.mock('../../../src/nest/plugins/kill-switch', () => ({ pluginsEnabled }));
|
||||
|
||||
@@ -24,11 +23,11 @@ function controller(over: Partial<PluginHooks> = {}) {
|
||||
placeDetails: vi.fn(async (id: string) => (id === 'p2' ? [{ label: 'Rating', value: '4.5' }] : [{ label: 'Reviews', value: '12', url: 'https://x' }])),
|
||||
...over,
|
||||
} as unknown as PluginHooks;
|
||||
return { c: new PlaceDetailsController(runtime, new DatabaseService(dbConn)), runtime };
|
||||
return { c: new PlaceDetailsController(runtime, { canAccessTrip, connection: { prepare: () => ({ get: (placeId: number) => placeTrip(placeId) }) } } as unknown as DatabaseService), runtime };
|
||||
}
|
||||
|
||||
describe('PlaceDetailsController', () => {
|
||||
beforeEach(() => { pluginsEnabled.mockReturnValue(true); canAccessTrip.mockReturnValue({ id: 1 } as never); });
|
||||
beforeEach(() => { pluginsEnabled.mockReturnValue(true); canAccessTrip.mockResolvedValue({ id: 1 } as never); });
|
||||
|
||||
it('returns [] when the runtime is disabled (no plugin calls)', async () => {
|
||||
pluginsEnabled.mockReturnValue(false);
|
||||
@@ -40,7 +39,7 @@ describe('PlaceDetailsController', () => {
|
||||
it('returns [] for an unknown place or a place the caller cannot access', async () => {
|
||||
const { c } = controller();
|
||||
expect(await c.get('999', req(5))).toEqual({ providers: [] }); // place not found
|
||||
canAccessTrip.mockReturnValue(undefined as never);
|
||||
canAccessTrip.mockResolvedValue(undefined as never);
|
||||
expect(await c.get('7', req(5))).toEqual({ providers: [] }); // no access
|
||||
});
|
||||
|
||||
|
||||
@@ -30,7 +30,7 @@ const ctx = (actingUserId: number | undefined): PluginRpcContext => ({
|
||||
/** Trip 1 belongs to user 42 and only user 42 may reach it, mirroring rpc-host.test.ts. */
|
||||
function build(overrides: { role?: string | undefined; allow?: boolean; addonOn?: boolean } = {}) {
|
||||
const db = {
|
||||
canAccessTrip: vi.fn((tripId: number, userId: number) =>
|
||||
canAccessTrip: vi.fn(async (tripId: number, userId: number) =>
|
||||
tripId === 1 && userId === 42 ? { id: 1, user_id: 42 } : undefined,
|
||||
),
|
||||
prepare: vi.fn(() => ({
|
||||
@@ -128,7 +128,7 @@ describe('PluginGuards — requireTripEdit and canEditAs', () => {
|
||||
|
||||
it('PGUARD-014 a non-owner member is flagged as shared', async () => {
|
||||
const db = {
|
||||
canAccessTrip: vi.fn(() => ({ id: 1, user_id: 7 })),
|
||||
canAccessTrip: vi.fn(async () => ({ id: 1, user_id: 7 })),
|
||||
prepare: vi.fn(() => ({ get: () => ({ role: 'user' }) })),
|
||||
} as unknown as DatabaseService;
|
||||
const permissions = { checkPermission: vi.fn(() => true) } as unknown as PermissionsService;
|
||||
|
||||
@@ -8,7 +8,7 @@
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
|
||||
const { canAccessTrip, pluginsEnabled, capabilitiesRow } = vi.hoisted(() => ({
|
||||
canAccessTrip: vi.fn((tripId: number, userId: number) => (tripId === 1 && userId === 5 ? { id: 1 } : undefined)),
|
||||
canAccessTrip: vi.fn(async (tripId: number, userId: number) => (tripId === 1 && userId === 5 ? { id: 1 } : undefined)),
|
||||
pluginsEnabled: vi.fn(() => true),
|
||||
capabilitiesRow: { value: JSON.stringify({ routeProfiles: [{ id: 'ev', label: 'EV' }] }) as string | undefined },
|
||||
}));
|
||||
@@ -16,7 +16,6 @@ vi.mock('../../../src/db/database', () => ({
|
||||
db: { prepare: () => ({ get: () => (capabilitiesRow.value === undefined ? undefined : { capabilities: capabilitiesRow.value }) }) },
|
||||
canAccessTrip,
|
||||
}));
|
||||
import { db as dbConn } from '../../../src/db/database';
|
||||
import { DatabaseService } from '../../../src/nest/database/database.service';
|
||||
vi.mock('../../../src/nest/plugins/kill-switch', () => ({ pluginsEnabled }));
|
||||
|
||||
@@ -30,7 +29,7 @@ function controller(invoke: () => unknown, providers = ['ev-plug']) {
|
||||
providersOf: vi.fn(() => providers),
|
||||
route: vi.fn(async () => invoke()),
|
||||
} as unknown as PluginHooks;
|
||||
return { c: new PluginRoutesController(runtime, new DatabaseService(dbConn)), runtime };
|
||||
return { c: new PluginRoutesController(runtime, { canAccessTrip, connection: { prepare: () => ({ get: () => (capabilitiesRow.value === undefined ? undefined : { capabilities: capabilitiesRow.value }) }) } } as unknown as DatabaseService), runtime };
|
||||
}
|
||||
const wp = (n = 3) => Array.from({ length: n }, (_, i) => ({ lat: 48 + i * 0.1, lng: 2 + i * 0.1 }));
|
||||
const goodRoute = (n = 3, over: Record<string, unknown> = {}) => ({
|
||||
@@ -45,7 +44,7 @@ const body = (over: Record<string, unknown> = {}) => ({ tripId: 1, dayId: 7, way
|
||||
describe('PluginRoutesController', () => {
|
||||
beforeEach(() => {
|
||||
pluginsEnabled.mockReturnValue(true);
|
||||
canAccessTrip.mockReturnValue({ id: 1 } as never);
|
||||
canAccessTrip.mockResolvedValue({ id: 1 } as never);
|
||||
capabilitiesRow.value = JSON.stringify({ routeProfiles: [{ id: 'ev', label: 'EV' }] });
|
||||
});
|
||||
|
||||
@@ -57,9 +56,9 @@ describe('PluginRoutesController', () => {
|
||||
pluginsEnabled.mockReturnValue(true);
|
||||
|
||||
expect((await controller(() => goodRoute()).c.route('ev-plug', 'ev', body(), req(undefined))).route).toBeNull();
|
||||
canAccessTrip.mockReturnValue(undefined as never);
|
||||
canAccessTrip.mockResolvedValue(undefined as never);
|
||||
expect((await controller(() => goodRoute()).c.route('ev-plug', 'ev', body(), req(5))).route).toBeNull();
|
||||
canAccessTrip.mockReturnValue({ id: 1 } as never);
|
||||
canAccessTrip.mockResolvedValue({ id: 1 } as never);
|
||||
expect((await controller(() => goodRoute()).c.route('ev-plug', '../etc', body(), req(5))).route).toBeNull();
|
||||
});
|
||||
|
||||
|
||||
@@ -40,7 +40,7 @@ const ALL = new Set([
|
||||
|
||||
function guardsFor() {
|
||||
const db = {
|
||||
canAccessTrip: vi.fn(() => ({ id: 1, user_id: 42 })),
|
||||
canAccessTrip: vi.fn(async () => ({ id: 1, user_id: 42 })),
|
||||
prepare: vi.fn(() => ({ get: () => ({ role: 'user' }), all: () => [] })),
|
||||
} as unknown as DatabaseService;
|
||||
return {
|
||||
|
||||
@@ -9,7 +9,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
|
||||
const { canAccessTrip, pluginsEnabled } = vi.hoisted(() => ({
|
||||
// trips 1 and 2 are accessible to user 5; everything else is not.
|
||||
canAccessTrip: vi.fn((tripId: number, userId: number) => (userId === 5 && (tripId === 1 || tripId === 2) ? { id: tripId } : undefined)),
|
||||
canAccessTrip: vi.fn(async (tripId: number, userId: number) => (userId === 5 && (tripId === 1 || tripId === 2) ? { id: tripId } : undefined)),
|
||||
pluginsEnabled: vi.fn(() => true),
|
||||
}));
|
||||
vi.mock('../../../src/db/database', () => ({ db: { prepare: () => ({ get: () => undefined }) }, canAccessTrip }));
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
|
||||
const { canAccessTrip, pluginsEnabled } = vi.hoisted(() => ({
|
||||
canAccessTrip: vi.fn((tripId: number, userId: number) => (tripId === 1 && userId === 5 ? { id: 1 } : undefined)),
|
||||
canAccessTrip: vi.fn(async (tripId: number, userId: number) => (tripId === 1 && userId === 5 ? { id: 1 } : undefined)),
|
||||
pluginsEnabled: vi.fn(() => true),
|
||||
}));
|
||||
vi.mock('../../../src/db/database', () => ({ db: {}, canAccessTrip }));
|
||||
@@ -24,14 +24,14 @@ function controller(over: Partial<PluginHooks> = {}) {
|
||||
}
|
||||
|
||||
describe('TripWarningsController', () => {
|
||||
beforeEach(() => { pluginsEnabled.mockReturnValue(true); canAccessTrip.mockReturnValue({ id: 1 } as never); });
|
||||
beforeEach(() => { pluginsEnabled.mockReturnValue(true); canAccessTrip.mockResolvedValue({ id: 1 } as never); });
|
||||
|
||||
it('returns [] when disabled / no user / no access', async () => {
|
||||
pluginsEnabled.mockReturnValue(false);
|
||||
expect(await controller().c.get('1', req(5))).toEqual({ warnings: [] });
|
||||
pluginsEnabled.mockReturnValue(true);
|
||||
expect(await controller().c.get('1', req(undefined))).toEqual({ warnings: [] });
|
||||
canAccessTrip.mockReturnValue(undefined as never);
|
||||
canAccessTrip.mockResolvedValue(undefined as never);
|
||||
expect(await controller().c.get('1', req(5))).toEqual({ warnings: [] });
|
||||
});
|
||||
|
||||
|
||||
@@ -8,7 +8,7 @@
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
|
||||
const { canAccessTrip, pluginsEnabled } = vi.hoisted(() => ({
|
||||
canAccessTrip: vi.fn((tripId: number, userId: number) => (tripId === 1 && userId === 5 ? { id: 1 } : undefined)),
|
||||
canAccessTrip: vi.fn(async (tripId: number, userId: number) => (tripId === 1 && userId === 5 ? { id: 1 } : undefined)),
|
||||
pluginsEnabled: vi.fn(() => true),
|
||||
}));
|
||||
vi.mock('../../../src/db/database', () => ({ db: { prepare: () => ({ get: () => undefined }) }, canAccessTrip }));
|
||||
@@ -31,7 +31,7 @@ const col = (over: Record<string, unknown> = {}) => ({ kind: 'column', entityId:
|
||||
const act = (over: Record<string, unknown> = {}) => ({ kind: 'action', entityId: 1, id: 'a1', label: 'Go', target: { kind: 'frame', sub: '/ui' }, ...over });
|
||||
|
||||
describe('ViewContributionsController', () => {
|
||||
beforeEach(() => { pluginsEnabled.mockReturnValue(true); canAccessTrip.mockReturnValue({ id: 1 } as never); });
|
||||
beforeEach(() => { pluginsEnabled.mockReturnValue(true); canAccessTrip.mockResolvedValue({ id: 1 } as never); });
|
||||
|
||||
it('gates: disabled / unknown view / no user / non-member all return [] (no plugin calls on the first two)', async () => {
|
||||
pluginsEnabled.mockReturnValue(false);
|
||||
@@ -52,7 +52,7 @@ describe('ViewContributionsController', () => {
|
||||
}
|
||||
|
||||
expect((await controller(() => [col()]).c.get('places', '1', req(undefined))).contributions).toEqual([]);
|
||||
canAccessTrip.mockReturnValue(undefined as never);
|
||||
canAccessTrip.mockResolvedValue(undefined as never);
|
||||
expect((await controller(() => [col()]).c.get('day', '1', req(5))).contributions).toEqual([]);
|
||||
});
|
||||
|
||||
|
||||
@@ -80,7 +80,7 @@ function build(opts: { canEdit?: boolean; cascade?: boolean; stop?: boolean; see
|
||||
} as unknown as AccommodationsService & Record<string, ReturnType<typeof vi.fn>>;
|
||||
const guards = new PluginGuards(
|
||||
{
|
||||
canAccessTrip: vi.fn((tripId: number, userId: number) => (tripId === 1 && userId === 42 ? { id: 1, user_id: 42 } : undefined)),
|
||||
canAccessTrip: vi.fn(async (tripId: number, userId: number) => (tripId === 1 && userId === 42 ? { id: 1, user_id: 42 } : undefined)),
|
||||
prepare: vi.fn(() => ({ get: () => ({ role: 'user' }) })),
|
||||
} as unknown as DatabaseService,
|
||||
{
|
||||
|
||||
@@ -54,7 +54,7 @@ import { TrekPhotosRepository } from '../../../src/nest/photos/trek-photos.repos
|
||||
import { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env.service';
|
||||
import { notificationsStub } from '../../helpers/notifications';
|
||||
import { makeStorageFixture } from '../../helpers/storage-fixture';
|
||||
import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo } from '../../helpers/test-uow';
|
||||
import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, sharedTestOrm } from '../../helpers/test-uow';
|
||||
|
||||
const dbs = new DatabaseService(testDb);
|
||||
const realtime = new RealtimeService();
|
||||
@@ -67,6 +67,15 @@ const photoCache = new PlacePhotoCacheService(dbs, makeStorageFixture('photos/go
|
||||
let packing: PackingService;
|
||||
let places: PlacesService;
|
||||
beforeAll(async () => {
|
||||
// Plan 3c Task 0b: `dbs` is constructed at module load, before any
|
||||
// `beforeAll` can resolve a real `EntityManager` — the four
|
||||
// repository-backed methods are spied directly on this instance instead,
|
||||
// routed to a real `DatabaseService` built with one.
|
||||
const real = new DatabaseService(testDb, (await sharedTestOrm(testDb)).em);
|
||||
vi.spyOn(dbs, 'canAccessTrip').mockImplementation((...a) => real.canAccessTrip(...a));
|
||||
vi.spyOn(dbs, 'isOwner').mockImplementation((...a) => real.isOwner(...a));
|
||||
vi.spyOn(dbs, 'rosterUserIds').mockImplementation((...a) => real.rosterUserIds(...a));
|
||||
vi.spyOn(dbs, 'getPlaceWithTags').mockImplementation((...a) => real.getPlaceWithTags(...a));
|
||||
packing = new PackingService(dbs, new PermissionsService(await createTestAppSettingsRepo(dbs.connection), await createTestUnitOfWork(dbs.connection)), realtime, notificationsStub(), await createTestUnitOfWork(dbs.connection));
|
||||
places = new PlacesService(
|
||||
dbs,
|
||||
|
||||
@@ -32,7 +32,7 @@ function build(canEdit = true) {
|
||||
const permissions = { checkPermission: vi.fn(() => canEdit) } as unknown as PermissionsService;
|
||||
const guards = new PluginGuards(
|
||||
{
|
||||
canAccessTrip: vi.fn((tripId: number, userId: number) => (tripId === 1 && userId === 42 ? { id: 1, user_id: 42 } : undefined)),
|
||||
canAccessTrip: vi.fn(async (tripId: number, userId: number) => (tripId === 1 && userId === 42 ? { id: 1, user_id: 42 } : undefined)),
|
||||
prepare: vi.fn(() => ({ get: () => ({ role: 'user' }) })),
|
||||
} as unknown as DatabaseService,
|
||||
permissions,
|
||||
|
||||
@@ -61,7 +61,7 @@ export function build(opts: { allow?: (action: string) => boolean; updateThrows?
|
||||
} as unknown as TripMembersService & Record<string, ReturnType<typeof vi.fn>>;
|
||||
const membership = { joinTripAsMember: vi.fn(() => ({ joined: true })) } as unknown as TripMembershipService & Record<string, ReturnType<typeof vi.fn>>;
|
||||
const db = {
|
||||
canAccessTrip: vi.fn((tripId: number, userId: number) => (tripId === 1 && userId === 42 ? { id: 1, user_id: 42 } : undefined)),
|
||||
canAccessTrip: vi.fn(async (tripId: number, userId: number) => (tripId === 1 && userId === 42 ? { id: 1, user_id: 42 } : undefined)),
|
||||
prepare: vi.fn((sql: string) => ({
|
||||
get: (arg: number) => {
|
||||
if (sql.includes('FROM users WHERE')) return arg === 404 ? undefined : { id: arg, role: 'user' };
|
||||
|
||||
@@ -238,6 +238,47 @@ describe('WS rooms', () => {
|
||||
}
|
||||
});
|
||||
|
||||
/**
|
||||
* Plan 3c Task 0b (R9): `handleJoin`'s `this.db.canAccessTrip(...)` call
|
||||
* (`realtime.gateway.ts:173`) is now `TripsRepository.findAccessible`
|
||||
* underneath `DatabaseService`, reached through the request context
|
||||
* `TrekWsAdapter`'s `@SubscribeMessage` wrapper forks
|
||||
* (`nest/realtime/trek-ws.adapter.ts:273`, pinned generically by
|
||||
* SEAM-001/BOOT-SWEEP-001) — no ratchet existed for this SPECIFIC seam
|
||||
* before this task (inventory §12b). WS-004 above already proves the happy
|
||||
* path returns `{type:'joined'}`, which requires the repository read to
|
||||
* have succeeded; this test makes that explicit and structural, the same
|
||||
* `cannotUseGlobalContext`/"global EntityManager" console-error filter
|
||||
* `boot-sweeps-request-context.test.ts` uses, so a regression that routes
|
||||
* `handleJoin` around the adapter's request context fails HERE by message,
|
||||
* not only by an indirect "joined never arrived" timeout.
|
||||
*/
|
||||
it('WS-SEAM-001 — handleJoin\'s repository read runs inside the WS request context (no cannotUseGlobalContext)', async () => {
|
||||
const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
|
||||
try {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const token = createEphemeralToken(user.id, 'ws')!;
|
||||
|
||||
const client = await connectWs(token);
|
||||
try {
|
||||
await client.next(); // welcome
|
||||
client.send({ type: 'join', tripId: trip.id });
|
||||
const msg = await client.next();
|
||||
expect(msg.type).toBe('joined');
|
||||
} finally {
|
||||
client.close();
|
||||
}
|
||||
|
||||
const suspicious = errSpy.mock.calls
|
||||
.map((args) => args.map(String).join(' '))
|
||||
.filter((line) => /cannotUseGlobalContext|global EntityManager/i.test(line));
|
||||
expect(suspicious).toEqual([]);
|
||||
} finally {
|
||||
errSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it('WS-006 — leave room receives left confirmation', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
|
||||
Reference in New Issue
Block a user