From 955809f15fd6c56c04e22557139cb71703e72c20 Mon Sep 17 00:00:00 2001 From: jubnl Date: Wed, 23 Sep 2026 08:31:36 +0200 Subject: [PATCH] fix(server): place-shadow source counts ordered in SQL; identity-map ratchets load-bearing; photo-cache race pinned --- server/src/db/dialect/sql-functions.ts | 24 ++++++++++ .../PlaceShadowPicks.repository.ts | 26 ++++++---- .../place-photos/place-photo-cache.service.ts | 10 ++++ server/tests/e2e/collab.e2e.test.ts | 2 +- server/tests/e2e/files.e2e.test.ts | 2 +- server/tests/e2e/reservations.e2e.test.ts | 1 - server/tests/e2e/trip-invite.e2e.test.ts | 2 +- server/tests/e2e/trips.e2e.test.ts | 2 +- .../unit/db/dialect/sql-functions.test.ts | 40 ++++++++++++++-- .../GooglePlacePhotoMeta.repository.test.ts | 10 +++- .../PlaceShadowPicks.repository.test.ts | 39 +++++++++++++-- .../place-photo-cache.service.test.ts | 48 +++++++++++++++++++ .../tests/unit/nest/trips.controller.test.ts | 4 +- .../plugins/plugin-retrust.controller.test.ts | 2 +- 14 files changed, 186 insertions(+), 26 deletions(-) diff --git a/server/src/db/dialect/sql-functions.ts b/server/src/db/dialect/sql-functions.ts index 4af5cadb4..d7824f7c0 100644 --- a/server/src/db/dialect/sql-functions.ts +++ b/server/src/db/dialect/sql-functions.ts @@ -289,3 +289,27 @@ export function caseWhenEquals(platform: Platform, ref: string, value: number, w return unsupported(platform); } +// --------------------------------------------------------------------------- +// Plan 3c Task 1 fix round (review M1) — `PlaceShadowPicksRepository +// .countBySource` (PS5): `.orderBy()` on `countAll()`'s aliased column throws +// ("Trying to query by not existing property" — `raw()`'s alias carries no +// literal type for `ExtractRawAliases` to register), and sorting the fetched +// rows in JS is NOT equivalent to the legacy `ORDER BY count DESC`: SQLite's +// own sorter is not stable, so on a tie it orders descending by GROUP BY key, +// while `Array.prototype.sort` (stable) leaves ties in the rows' incoming +// (ascending) order — verified directly (five sources, two tied pairs). +// --------------------------------------------------------------------------- + +/** + * `COUNT(*)` as an ORDER BY EXPRESSION (not an alias — `.orderBy()` cannot + * order by `countAll()`'s aliased column; ordering by the expression itself + * works and reproduces the legacy `ORDER BY count DESC` tie order exactly). + * Typed `RawQueryFragment & symbol`, the same brand `lower()`/`lowerTrim()` + * carry, so it is usable as an `.orderBy()` object key: + * `.orderBy({ [countAllRef(platform)]: 'desc' })`. + */ +export function countAllRef(platform: Platform): RawQueryFragment & symbol { + if (platform instanceof SqlitePlatform) return raw('COUNT(*)'); + return unsupported(platform); +} + diff --git a/server/src/db/repositories/PlaceShadowPicks.repository.ts b/server/src/db/repositories/PlaceShadowPicks.repository.ts index 013d5f816..5831071c1 100644 --- a/server/src/db/repositories/PlaceShadowPicks.repository.ts +++ b/server/src/db/repositories/PlaceShadowPicks.repository.ts @@ -1,5 +1,5 @@ import type { PlaceShadowPicks } from '../entities/PlaceShadowPicks.entity'; -import { countAll, maxOf, minOf, nowMinusDays } from '../dialect/sql-functions'; +import { countAll, countAllRef, maxOf, minOf, nowMinusDays } from '../dialect/sql-functions'; import { toRow, type AssertRowKeys } from './_shared/rows'; import { TrekRepository } from './_shared/trek-repository'; @@ -81,21 +81,27 @@ export class PlaceShadowPicksRepository extends TrekRepository /** * `SELECT source, COUNT(*) AS count FROM place_shadow_picks GROUP BY source - * ORDER BY count DESC` (PS5). The `ORDER BY` is applied in JS, not SQL: - * MikroORM's QueryBuilder cannot order by an ad-hoc raw-fragment alias - * (`countAll`'s `RawQueryFragment` return type carries no alias literal for - * `ExtractRawAliases` to pick up, so `.orderBy({ count: 'desc' })` throws - * "not existing property" at runtime, verified directly) — `Array.prototype - * .sort` is stable (ES2019+), so this produces the identical final row - * order the legacy `ORDER BY count DESC` did. + * ORDER BY count DESC` (PS5). The `ORDER BY` is applied in SQL, by the + * EXPRESSION, not the alias: MikroORM's QueryBuilder cannot order by an + * ad-hoc raw-fragment alias (`countAll`'s `RawQueryFragment` return type + * carries no alias literal for `ExtractRawAliases` to pick up, so + * `.orderBy({ count: 'desc' })` throws "not existing property" at runtime, + * verified directly) — `.orderBy({ [countAllRef(platform)]: 'desc' })` + * orders by `COUNT(*)` itself, which SQLite accepts and which reproduces + * the legacy statement's tie order exactly. A JS + * `Array.prototype.sort((a, b) => b.count - a.count)` does NOT reproduce + * it: SQLite's own sorter is not stable, so on a tie the legacy `ORDER BY + * count DESC` emits ties in DESCENDING source order, while `GROUP BY + * source` hands rows to JS in ASCENDING source order and a stable sort + * leaves ties ascending — verified directly (Task 1 fix review M1). */ async countBySource(): Promise<{ source: string; count: number }[]> { const platform = this.getEntityManager().getPlatform(); - const rows = await this.qb('p') + return this.qb('p') .select(['p.source', countAll(platform, 'count')]) .groupBy('p.source') + .orderBy({ [countAllRef(platform)]: 'desc' }) .execute<{ source: string; count: number }[]>('all', false); - return [...rows].sort((a, b) => b.count - a.count); } /** `SELECT live_rank, COUNT(*) AS count FROM place_shadow_picks GROUP BY live_rank` (PS6) — no ORDER BY, matching the legacy statement; the caller buckets the rows itself. */ diff --git a/server/src/nest/place-photos/place-photo-cache.service.ts b/server/src/nest/place-photos/place-photo-cache.service.ts index 6f19919f8..c9a6d44db 100644 --- a/server/src/nest/place-photos/place-photo-cache.service.ts +++ b/server/src/nest/place-photos/place-photo-cache.service.ts @@ -84,6 +84,16 @@ export class PlacePhotoCacheService { return `/api/maps/place-photo/${encodeURIComponent(placeId)}/bytes`; } + /** + * Non-transactional check-then-act, unchanged (Plan 3c inventory §18.4, + * program rule 11): PP1 (`this.meta.findLive`) → `await storage.exists(...)` + * → PP2 (`this.meta.deleteByPlaceId`) has a real `await` in the window, so + * two concurrent calls for the same never-checked placeId can both read the + * row, both find the storage object missing, and both delete — the second + * delete is a harmless 0-row no-op (`deleteByPlaceId` on an already-gone + * row does not throw), and both callers correctly resolve `null`. Pinned by + * a concurrency test, not fixed. + */ async get(placeId: string): Promise { const row = await this.meta.findLive(placeId); diff --git a/server/tests/e2e/collab.e2e.test.ts b/server/tests/e2e/collab.e2e.test.ts index 51e0e568b..8bad78602 100644 --- a/server/tests/e2e/collab.e2e.test.ts +++ b/server/tests/e2e/collab.e2e.test.ts @@ -64,7 +64,7 @@ const { db } = vi.hoisted(() => { const { canAccessTrip } = vi.hoisted(() => ({ canAccessTrip: vi.fn() })); vi.mock('../../src/db/database', () => ({ - db, canAccessTrip, isOwner: vi.fn(() => true), getPlaceWithTags: vi.fn(), closeDb: () => {}, reinitialize: () => {}, + db, canAccessTrip, getPlaceWithTags: vi.fn(), closeDb: () => {}, reinitialize: () => {}, })); vi.mock('../../src/websocket', () => ({ broadcast: vi.fn() })); diff --git a/server/tests/e2e/files.e2e.test.ts b/server/tests/e2e/files.e2e.test.ts index 3438756d9..e9c154cff 100644 --- a/server/tests/e2e/files.e2e.test.ts +++ b/server/tests/e2e/files.e2e.test.ts @@ -50,7 +50,7 @@ const { db } = vi.hoisted(() => { const { canAccessTrip } = vi.hoisted(() => ({ canAccessTrip: vi.fn() })); vi.mock('../../src/db/database', () => ({ - db, canAccessTrip, isOwner: vi.fn(() => true), getPlaceWithTags: vi.fn(), closeDb: () => {}, reinitialize: () => {}, + db, canAccessTrip, getPlaceWithTags: vi.fn(), closeDb: () => {}, reinitialize: () => {}, })); vi.mock('../../src/websocket', () => ({ broadcast: vi.fn() })); vi.mock('../../src/nest/common/demo', () => ({ isDemoEmail: vi.fn(() => false) })); diff --git a/server/tests/e2e/reservations.e2e.test.ts b/server/tests/e2e/reservations.e2e.test.ts index 6de828f6a..e1b090ce8 100644 --- a/server/tests/e2e/reservations.e2e.test.ts +++ b/server/tests/e2e/reservations.e2e.test.ts @@ -38,7 +38,6 @@ const { canAccessTrip } = vi.hoisted(() => ({ canAccessTrip: vi.fn() })); vi.mock('../../src/db/database', () => ({ db, canAccessTrip, - isOwner: vi.fn(() => true), getPlaceWithTags: vi.fn(), closeDb: () => {}, reinitialize: () => {}, diff --git a/server/tests/e2e/trip-invite.e2e.test.ts b/server/tests/e2e/trip-invite.e2e.test.ts index 752da5521..43a9c9538 100644 --- a/server/tests/e2e/trip-invite.e2e.test.ts +++ b/server/tests/e2e/trip-invite.e2e.test.ts @@ -44,7 +44,7 @@ const { db } = vi.hoisted(() => { const { canAccessTrip } = vi.hoisted(() => ({ canAccessTrip: vi.fn() })); vi.mock('../../src/db/database', () => ({ - db, canAccessTrip, isOwner: vi.fn(() => true), getPlaceWithTags: vi.fn(), closeDb: () => {}, reinitialize: () => {}, + db, canAccessTrip, getPlaceWithTags: vi.fn(), closeDb: () => {}, reinitialize: () => {}, })); import { PermissionsService } from '../../src/nest/permissions/permissions.service'; diff --git a/server/tests/e2e/trips.e2e.test.ts b/server/tests/e2e/trips.e2e.test.ts index 3c65094a1..8f57023b7 100644 --- a/server/tests/e2e/trips.e2e.test.ts +++ b/server/tests/e2e/trips.e2e.test.ts @@ -117,7 +117,7 @@ const { db } = vi.hoisted(() => { const { canAccessTrip } = vi.hoisted(() => ({ canAccessTrip: vi.fn() })); vi.mock('../../src/db/database', () => ({ - db, canAccessTrip, isOwner: vi.fn(() => true), getPlaceWithTags: vi.fn(), closeDb: () => {}, reinitialize: () => {}, + db, canAccessTrip, getPlaceWithTags: vi.fn(), closeDb: () => {}, reinitialize: () => {}, })); vi.mock('../../src/websocket', () => ({ broadcast: vi.fn() })); // The audit domain is DI-native now: writeAudit runs for real against the temp diff --git a/server/tests/unit/db/dialect/sql-functions.test.ts b/server/tests/unit/db/dialect/sql-functions.test.ts index 4cdb5d8ea..7d501bc22 100644 --- a/server/tests/unit/db/dialect/sql-functions.test.ts +++ b/server/tests/unit/db/dialect/sql-functions.test.ts @@ -13,6 +13,7 @@ import { columnIncrementedBy, columnRef, countAll, + countAllRef, currentTimestamp, dateAdd, dateOf, @@ -361,9 +362,12 @@ describe('sql-functions (sqlite)', () => { // "Trying to query by not existing property" at runtime, since `raw()`'s // alias carries no literal type for `ExtractRawAliases` to register). The // repository callers that need "ORDER BY DESC" - // (`PlaceShadowPicksRepository.countBySource`) sort the fetched rows in JS - // instead — `Array.prototype.sort` is stable (ES2019+), so the resulting - // order matches a real `ORDER BY count DESC` byte for byte. + // (`PlaceShadowPicksRepository.countBySource`) order by the `COUNT(*)` + // EXPRESSION instead, via `countAllRef` below (SQLF-032) — NOT a JS + // `Array.prototype.sort` of the fetched rows: that would be stable and + // therefore leave ties in `GROUP BY`'s own (ascending-by-key) row order, + // which is not the same order SQLite's own, non-stable `ORDER BY count + // DESC` sorter produces on a tie (Task 1 fix review M1). it('SQLF-024: countAll composes with GROUP BY, matching a hand-written GROUP BY statement', async () => { createUser(testDb, { role: 'admin' }); createUser(testDb, { role: 'admin' }); @@ -455,4 +459,34 @@ describe('sql-functions (sqlite)', () => { const foreign = new FakePlatform(); expect(() => caseWhenEquals(foreign, 'u.id', 1, 'owner', 'member')).toThrow(/no implementation for platform FakePlatform/); }); + + // Task 1 fix review M1 (`PlaceShadowPicksRepository.countBySource`, PS5): + // `countAll()`'s alias cannot be ordered by (`.orderBy({ count: 'desc' })` + // throws "not existing property" — verified in SQLF-024's comment above); + // `countAllRef` orders by the `COUNT(*)` EXPRESSION instead, which SQLite + // accepts. The full tie-order proof (against the legacy `ORDER BY count + // DESC` statement, on data with tied counts) lives in + // `PlaceShadowPicksRepository.countBySource`'s own test (PSPICKREPO-006b) — + // this pins the fragment's shape and composition with GROUP BY. + it('SQLF-032: countAllRef renders COUNT(*) as an ORDER BY expression, matching a hand-written GROUP BY … ORDER BY statement', async () => { + createUser(testDb, { role: 'admin' }); + createUser(testDb, { role: 'admin' }); + createUser(testDb, { role: 'user' }); + const platform = t.em.getPlatform(); + + const rows = await t.em.createQueryBuilder(Users, 'u') + .select(['u.role', countAll(platform, 'count')]) + .groupBy('u.role') + .orderBy({ [countAllRef(platform)]: 'desc' }) + .execute('all', false); + + const expected = testDb.prepare('SELECT role, COUNT(*) as count FROM users GROUP BY role ORDER BY COUNT(*) DESC').all(); + expect(rows).toEqual(expected); + }); + + it('SQLF-033: an unknown platform fails closed for countAllRef', () => { + class FakePlatform extends Platform {} + const foreign = new FakePlatform(); + expect(() => countAllRef(foreign)).toThrow(/no implementation for platform FakePlatform/); + }); }); diff --git a/server/tests/unit/db/repositories/GooglePlacePhotoMeta.repository.test.ts b/server/tests/unit/db/repositories/GooglePlacePhotoMeta.repository.test.ts index 86ece0895..7141f331b 100644 --- a/server/tests/unit/db/repositories/GooglePlacePhotoMeta.repository.test.ts +++ b/server/tests/unit/db/repositories/GooglePlacePhotoMeta.repository.test.ts @@ -115,9 +115,15 @@ describe('GooglePlacePhotoMetaRepository.listPlaceIds', () => { // (`TrekRepository`'s `disableIdentityMap: true` default), the class of read // program rule 14 targets directly. describe('GooglePlacePhotoMetaRepository — D-shape', () => { - it('GPPMREPO-014: a raw write after findLive is visible in the next findLive call, in one query (disableIdentityMap regression)', async () => { + it('GPPMREPO-014: a raw write after an unrelated identity-map read is visible in the next findLive call, in one query (disableIdentityMap regression)', async () => { testDb.prepare('INSERT INTO google_place_photo_meta (place_id, attribution, fetched_at, error_at) VALUES (?, ?, ?, NULL)').run('p1', 'Old', 1000); - expect(await meta.findLive('p1')).toEqual({ attribution: 'Old' }); // populate the identity map + // rule 20: the FIRST, wider setup read passes `disableIdentityMap: + // false` explicitly and carries the column the later write targets + // (`attribution`) — a PK-only `findOne`, not `findLive`'s own + // `error_at IS NULL`-narrowed filter, matching the shape that makes + // `PlaceDetailsCacheRepository`'s PDCREPO-008 fail under the + // disableIdentityMap-default mutation. + await t.repo(GooglePlacePhotoMeta).findOne({ place_id: 'p1' }, { disableIdentityMap: false }); testDb.prepare('UPDATE google_place_photo_meta SET attribution = ? WHERE place_id = ?').run('New', 'p1'); const connection = t.orm.em.getConnection(); diff --git a/server/tests/unit/db/repositories/PlaceShadowPicks.repository.test.ts b/server/tests/unit/db/repositories/PlaceShadowPicks.repository.test.ts index 6e293ffcf..416d912a1 100644 --- a/server/tests/unit/db/repositories/PlaceShadowPicks.repository.test.ts +++ b/server/tests/unit/db/repositories/PlaceShadowPicks.repository.test.ts @@ -94,6 +94,32 @@ describe('PlaceShadowPicksRepository.totals / countBySource / countByLiveRank', ]); }); + // Task 1 fix review M1: `GROUP BY source` hands rows to JS in ASCENDING + // source order (SQLite's GROUP BY temp b-tree); a stable JS + // `Array.prototype.sort((a, b) => b.count - a.count)` therefore leaves ties + // ascending by source, but SQLite's own (non-stable) `ORDER BY count DESC` + // sorter emits ties in DESCENDING source order on the same data — two + // different final row orders. `countBySource` orders in SQL, by the + // `COUNT(*)` expression, so it reproduces the legacy statement's tie order + // exactly instead of merely matching it on the (untied) case above. + it('PSPICKREPO-006b: countBySource orders ties the same way the legacy ORDER BY count DESC statement does', async () => { + for (const source of ['zeta', 'zeta', 'alpha', 'alpha', 'omega', 'omega', 'mid', 'beta']) { + await picks.insertPick(row({ source })); + } + const rows = await picks.countBySource(); + const legacy = testDb + .prepare('SELECT source, COUNT(*) AS count FROM place_shadow_picks GROUP BY source ORDER BY count DESC') + .all(); + expect(rows).toEqual(legacy); + expect(rows).toEqual([ + { source: 'zeta', count: 2 }, + { source: 'omega', count: 2 }, + { source: 'alpha', count: 2 }, + { source: 'mid', count: 1 }, + { source: 'beta', count: 1 }, + ]); + }); + it('PSPICKREPO-007: countByLiveRank groups with no ORDER BY (caller buckets)', async () => { await picks.insertPick(row({ live_rank: 0 })); await picks.insertPick(row({ live_rank: 0 })); @@ -136,12 +162,19 @@ describe('PlaceShadowPicksRepository.deleteAll / purgeOlderThan', () => { // D-shape (Task 1 brief item): `page`/`totals`/`countBySource`/`countByLiveRank` // all go through `find`/`qb().execute()`, neither of which hydrates a // long-lived entity here — proven anyway for `page` (the one `find()`-based -// read), the honest "not required, proven regardless" shape. +// read), the honest "not required, proven regardless" shape: `page()` itself +// always passes the base's own `disableIdentityMap: true` default (it never +// opts in), so a stale cached entity can't leak into it regardless of what +// the setup read below does. describe('PlaceShadowPicksRepository — D-shape', () => { it('PSPICKREPO-011: a row inserted after an unrelated identity-map read is visible in the FIRST wider projection (page)', async () => { - await t.repo(PlaceShadowPicks).find({}); // populate the identity map with an unrelated (empty) read + await picks.insertPick(row({ query: 'seed' })); + // rule 20: the FIRST, wider setup read passes `disableIdentityMap: false` + // explicitly and carries the column the later write targets (`query`) — + // `find({})` with the base default merged in populates nothing. + await t.repo(PlaceShadowPicks).find({}, { disableIdentityMap: false }); await picks.insertPick(row({ query: 'fresh' })); const page = await picks.page(0, 10); - expect(page.map((r) => r.query)).toEqual(['fresh']); + expect(page.map((r) => r.query)).toEqual(['seed', 'fresh']); }); }); diff --git a/server/tests/unit/nest/place-photos/place-photo-cache.service.test.ts b/server/tests/unit/nest/place-photos/place-photo-cache.service.test.ts index 14a448f32..bc1ae8568 100644 --- a/server/tests/unit/nest/place-photos/place-photo-cache.service.test.ts +++ b/server/tests/unit/nest/place-photos/place-photo-cache.service.test.ts @@ -78,6 +78,24 @@ let t: TestOrm; let metaRepo: GooglePlacePhotoMetaRepository; let placesRepo: PlacesRepository; +// Task 1 fix review L3: this is the one suite in this commit that opts out of +// the request-context ratchet (`allowGlobalContext: true`, disabling +// `TrekRepository.validateRequestContext()` for the whole file). Justified, +// not converted: `PlacePhotoCacheService` is constructed directly below +// (`new PlacePhotoCacheService(...)`), never through Nest DI or an HTTP +// request, and every one of its ~20 cases would otherwise need its own +// `withRequestContext(t.orm, ...)` wrapper around a repository call it makes +// several layers deep inside the service (`get`/`put`/`sweepOrphans`/…) — a +// mechanical, high-diff, low-value change to a file this task does not +// otherwise own. The production ratchet (a service booted outside a request +// context throws) is covered elsewhere, by the seams tests +// (`cron-registrar.service.test.ts`, `tests/unit/nest/database/ +// request-context.test.ts`) and by `PlacePhotoCacheJob`'s own suite +// (`place-photo-cache.job.test.ts`), which DOES use `allowGlobalContext: +// false` + `CronRegistrarService.runOnBoot`'s `withRequestContext` wrapper +// for the one real entrypoint that reaches this service outside a request. +// Recorded so Tasks 4–8 do not copy this suite's `allowGlobalContext: true` +// into a suite that boots a real app. beforeAll(async () => { t = await createTestOrm(testDb, { allowGlobalContext: true }); metaRepo = t.repo(GooglePlacePhotoMeta); @@ -175,6 +193,36 @@ describe.each([ attribution: 'Carol', }); }); + + // Task 1 fix review M3 (Plan 3c inventory §18.4, program rule 11): + // PP1 (`this.meta.findLive`) → `await storage.exists(...)` → PP2 + // (`this.meta.deleteByPlaceId`) is a non-transactional check-then-act with + // a real `await` in the window — flagged, not fixed. This pins today's + // outcome on a genuine race: two concurrent `get()` calls for a + // never-checked placeId whose storage object is missing. + it('PPC-017 (§18.4 concurrency): two concurrent gets on a row whose storage object is missing both resolve null; the loser\'s delete is a harmless no-op', async () => { + testDb + .prepare('INSERT INTO google_place_photo_meta (place_id, attribution, fetched_at) VALUES (?, ?, ?)') + .run('race-place', 'Dana', Date.now()); + const deleteSpy = vi.spyOn(metaRepo, 'deleteByPlaceId'); + + try { + const [a, b] = await Promise.all([cache.get('race-place'), cache.get('race-place')]); + + // Both requests read the row, both find the storage object missing + // (neither had it in `knownOnDisk` yet), so both resolve null and + // both call deleteByPlaceId — the second is a 0-row DELETE, which + // GPPMREPO-008 already proves does not throw. + expect(a).toBeNull(); + expect(b).toBeNull(); + expect(deleteSpy).toHaveBeenCalledTimes(2); + expect(deleteSpy).toHaveBeenCalledWith('race-place'); + // Exactly one row existed to begin with — gone either way, not double-deleted into an error. + expect(testDb.prepare('SELECT 1 FROM google_place_photo_meta WHERE place_id = ?').get('race-place')).toBeUndefined(); + } finally { + deleteSpy.mockRestore(); + } + }); }); describe('removeIfUnreferenced()', () => { diff --git a/server/tests/unit/nest/trips.controller.test.ts b/server/tests/unit/nest/trips.controller.test.ts index 2bce70977..09ee30a6c 100644 --- a/server/tests/unit/nest/trips.controller.test.ts +++ b/server/tests/unit/nest/trips.controller.test.ts @@ -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().mockResolvedValue(null), remove: vi.fn() } as Partial); + const s = svc({ getOwner: vi.fn().mockResolvedValue({ user_id: 2 }), canAccessTrip: vi.fn().mockResolvedValue(null), remove: vi.fn() } as Partial); 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().mockResolvedValue(null), remove, broadcast: vi.fn() } as Partial); + const s = svc({ getOwner: vi.fn().mockResolvedValue({ user_id: 2 }), canAccessTrip: vi.fn().mockResolvedValue(null), remove, broadcast: vi.fn() } as Partial); expect(await tc(s).remove(admin, '9', req)).toEqual({ success: true }); expect(remove).toHaveBeenCalledWith('9', 1, 'admin'); }); diff --git a/server/tests/unit/plugins/plugin-retrust.controller.test.ts b/server/tests/unit/plugins/plugin-retrust.controller.test.ts index 3a58ca2e1..4b1ea77fb 100644 --- a/server/tests/unit/plugins/plugin-retrust.controller.test.ts +++ b/server/tests/unit/plugins/plugin-retrust.controller.test.ts @@ -13,7 +13,7 @@ import { describe, it, expect, beforeEach, vi } from 'vitest'; import { HttpException, type ArgumentsHost } from '@nestjs/common'; -vi.mock('../../../src/db/database', () => ({ db: {}, canAccessTrip: () => undefined })); +vi.mock('../../../src/db/database', () => ({ db: {}, canAccessTrip: async () => undefined })); import { PluginsController } from '../../../src/nest/plugins/plugins.controller'; import { PluginRuntimeService } from '../../../src/nest/plugins/plugin-runtime.service';