refactor(trips): trips through repositories

This commit is contained in:
jubnl
2026-09-23 09:33:48 +02:00
parent f19face405
commit 18a4784ebd
19 changed files with 1244 additions and 218 deletions
+1 -1
View File
@@ -166,7 +166,7 @@ export async function createPluginRpcHostFactory(dbs: DatabaseService): Promise<
new DaysRpc(days, realtime, guards),
new AccommodationsRpc(accommodations, realtime, guards),
new ItineraryRpc(assignments, realtime, guards),
new TripsRpc(trips, reservations, days, membership, dbs, realtime, guards, accommodations, members),
new TripsRpc(trips, reservations, days, membership, realtime, guards, accommodations, members, (await sharedTestOrm(dbs.connection)).em),
new CostsRpc(budget, dbs, realtime, guards, membership),
new ReservationsRpc(reservations, realtime, guards),
new CollabRpc(collab, realtime, guards),
@@ -2,7 +2,7 @@ import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vites
import { createSnapshotTestDb } from '../../../helpers/db-mock';
import { resetTestDb } from '../../../helpers/test-db';
import { createTestOrm, type TestOrm } from '../../../helpers/test-orm';
import { createDay, createTrip, createUser } from '../../../helpers/factories';
import { createDay, createDayAccommodation, createDayAssignment, createDayNote, createPlace, createTrip, createUser } from '../../../helpers/factories';
import { Days } from '../../../../src/db/entities/Days.entity';
import type { DaysRepository } from '../../../../src/db/repositories/Days.repository';
@@ -226,4 +226,73 @@ describe('DaysRepository — Task 2 additions', () => {
expect(await days.existsInTrip('not-a-number', trip.id)).toBe(false);
});
});
// ── Plan 3c Task 7 (TripsService.generateDays: TP3, TP6, TP12) ─────────────
describe('clearDate (TP3)', () => {
it('DAYREPO-021: nullifies the date without deleting the row — assignments/notes/accommodations survive', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
const day = createDay(testDb, trip.id, { date: '2026-01-01' });
const place = createPlace(testDb, trip.id);
createDayAssignment(testDb, day.id, place.id);
await days.clearDate(day.id);
const row = testDb.prepare('SELECT date FROM days WHERE id = ?').get(day.id) as { date: string | null };
expect(row.date).toBeNull();
expect(testDb.prepare('SELECT id FROM day_assignments WHERE day_id = ?').get(day.id)).toBeDefined();
});
});
describe('isEmptyDay (TP12) — three counts across day_assignments/day_notes/day_accommodations', () => {
it('DAYREPO-022: true for a day with nothing on it; false once it holds an assignment, a note, or an accommodation (start OR end)', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
const empty = createDay(testDb, trip.id);
const withAssignment = createDay(testDb, trip.id);
const withNote = createDay(testDb, trip.id);
const withAccomStart = createDay(testDb, trip.id);
const withAccomEnd = createDay(testDb, trip.id);
const place = createPlace(testDb, trip.id);
createDayAssignment(testDb, withAssignment.id, place.id);
createDayNote(testDb, withNote.id, trip.id);
createDayAccommodation(testDb, trip.id, place.id, withAccomStart.id, withAccomStart.id);
createDayAccommodation(testDb, trip.id, place.id, withAccomEnd.id, withAccomEnd.id);
expect(await days.isEmptyDay(empty.id)).toBe(true);
expect(await days.isEmptyDay(withAssignment.id)).toBe(false);
expect(await days.isEmptyDay(withNote.id)).toBe(false);
expect(await days.isEmptyDay(withAccomStart.id)).toBe(false);
expect(await days.isEmptyDay(withAccomEnd.id)).toBe(false);
});
});
describe('listTrailingEmptyIds (TP6) — ORDER BY day_number DESC, first `limit` empty days', () => {
it('DAYREPO-023: a non-empty day in the middle is skipped, not a stopping point — matches the legacy WHERE-filter-then-LIMIT semantics', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
const d1 = createDay(testDb, trip.id, { day_number: 1 }); // empty
const d2 = createDay(testDb, trip.id, { day_number: 2 }); // holds a note — excluded, not a stop
const d3 = createDay(testDb, trip.id, { day_number: 3 }); // empty
const d4 = createDay(testDb, trip.id, { day_number: 4 }); // empty
createDayNote(testDb, d2.id, trip.id);
// limit=2: stops as soon as 2 empty days are collected, in day_number
// DESC order — d2 is never even reached.
expect(await days.listTrailingEmptyIds(trip.id, 2)).toEqual([d4.id, d3.id]);
// limit=3: d4, d3 collected; d2 examined and SKIPPED (not empty, and
// not a stopping point either); d1 examined and collected — matching
// the legacy `WHERE NOT EXISTS(...) ×3 ORDER BY day_number DESC LIMIT
// 3` exactly (it filters every day up front, then takes the top 3 of
// what's left — d2 was never a candidate to begin with).
expect(await days.listTrailingEmptyIds(trip.id, 3)).toEqual([d4.id, d3.id, d1.id]);
});
it('DAYREPO-024: a limit of 0 returns nothing without querying', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
createDay(testDb, trip.id);
expect(await days.listTrailingEmptyIds(trip.id, 0)).toEqual([]);
});
});
});
@@ -466,7 +466,7 @@ describe('PlacesRepository.listForTrip (PL3) — filter fragments', () => {
expect(rows.map((r) => r.id)).toEqual([newer.id, older.id]);
});
it('PLACEREPO-028: searchPattern matches name/address/description (already-escaped, wrapped by the caller)', async () => {
it('PLACEREPO-028: searchPattern matches name/address/description (already-escaped, wrapped by the caller — NOT a test of the ESCAPE clause itself: `%Eiffel%` has no literal `%`/`_` to escape. `ESCAPE \'\\\'` is proven by `PLACE-SVC-068`/PLACEREPO-028b below, which do carry a literal wildcard character)', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
const hit = createPlace(testDb, trip.id, { name: 'Eiffel Tower' });
@@ -476,6 +476,19 @@ describe('PlacesRepository.listForTrip (PL3) — filter fragments', () => {
void miss;
});
it('PLACEREPO-028b (M2 — the ESCAPE clause this repository method actually relies on): a literal `%` in the already-escaped pattern matches literally, not as a wildcard', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
// escapeLikePattern (the service) would have produced this pattern for a
// literal search term "50%": the `%` the user typed is backslash-escaped,
// and the wrapping `%…%` are the real wildcards this method adds.
const hit = createPlace(testDb, trip.id, { name: '50% off tour' });
const miss = createPlace(testDb, trip.id, { name: '50X off tour' });
const rows = await places.listForTrip(String(trip.id), { searchPattern: '%50\\%%' });
expect(rows.map((r) => r.id)).toEqual([hit.id]);
void miss;
});
it('PLACEREPO-029: category filters to that category_id only', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
@@ -533,6 +546,99 @@ describe('PlacesRepository.listForTrip (PL3) — filter fragments', () => {
});
});
/**
* PL3 parity (Task 4 review M2/rule 19): the base `94c6efbbc` raw statement
* (`places.service.ts:180-217` at that commit — reproduced verbatim below,
* the same `let query += …` composition, the same param order), run over
* every `search × category × tag × assignment` combination
* (2 × 2 × 2 × 4 = 32, undefined/present for the first three, {undefined,
* 'all', 'assigned', 'unassigned'} for the fourth). Each cell asserts
* `toEqual` on the FULL row set from both statements, not just the id list —
* a dropped/renamed column would pass an id-only comparison silently.
*/
describe('PlacesRepository.listForTrip (PL3) — toEqual(legacy) over all 32 filter combinations', () => {
function legacyListForTrip(
tripId: string,
filters: { searchPattern?: string; category?: string; tag?: string; assignment?: 'all' | 'unassigned' | 'assigned' },
): unknown[] {
let query = `
SELECT DISTINCT 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.trip_id = ?
`;
const params: (string | number)[] = [tripId];
if (filters.searchPattern) {
query += " AND (p.name LIKE ? ESCAPE '\\' OR p.address LIKE ? ESCAPE '\\' OR p.description LIKE ? ESCAPE '\\')";
params.push(filters.searchPattern, filters.searchPattern, filters.searchPattern);
}
if (filters.category) {
query += ' AND p.category_id = ?';
params.push(filters.category);
}
if (filters.tag) {
query += ' AND p.id IN (SELECT place_id FROM place_tags WHERE tag_id = ?)';
params.push(filters.tag);
}
if (filters.assignment === 'unassigned') {
query += ' AND p.id NOT IN (SELECT da.place_id FROM day_assignments da JOIN days d ON da.day_id = d.id WHERE d.trip_id = ?)';
params.push(tripId);
} else if (filters.assignment === 'assigned') {
query += ' AND p.id IN (SELECT da.place_id FROM day_assignments da JOIN days d ON da.day_id = d.id WHERE d.trip_id = ?)';
params.push(tripId);
}
query += ' ORDER BY p.created_at DESC';
return testDb.prepare(query).all(...params);
}
it('PLACEREPO-032b (PL3, M2): every row, every column, on all 32 combinations, byte-identical to the legacy raw statement', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
const otherTrip = createTrip(testDb, user.id);
const cat = createCategory(testDb, { name: 'Museums' });
const tag = createTag(testDb, user.id, { name: 'Must see' });
const day = createDay(testDb, trip.id);
// p1: category + tag + assigned + matches the search term.
const p1 = createPlace(testDb, trip.id, { name: 'Central Park', category_id: cat.id });
testDb.prepare('INSERT INTO place_tags (place_id, tag_id) VALUES (?, ?)').run(p1.id, tag.id);
createDayAssignment(testDb, day.id, p1.id);
// p2: category, no tag, unassigned, matches the search term (a near-miss on 3 of 4 axes).
createPlace(testDb, trip.id, { name: 'Central Station', category_id: cat.id });
// p3: no category, tag + assigned, does NOT match the search term.
const p3 = createPlace(testDb, trip.id, { name: 'Louvre' });
testDb.prepare('INSERT INTO place_tags (place_id, tag_id) VALUES (?, ?)').run(p3.id, tag.id);
createDayAssignment(testDb, day.id, p3.id);
// p4: no category, no tag, unassigned, does NOT match the search term.
createPlace(testDb, trip.id, { name: 'Eiffel Tower' });
// Another trip's place, named to collide with the search term — must
// never appear in either statement's output (both scope by trip_id).
createPlace(testDb, otherTrip.id, { name: 'Central Perk' });
const searchOptions: (string | undefined)[] = [undefined, '%Central%'];
const categoryOptions: (string | undefined)[] = [undefined, String(cat.id)];
const tagOptions: (string | undefined)[] = [undefined, String(tag.id)];
const assignmentOptions: (('all' | 'unassigned' | 'assigned') | undefined)[] = [undefined, 'all', 'assigned', 'unassigned'];
let combinations = 0;
for (const searchPattern of searchOptions) {
for (const category of categoryOptions) {
for (const tag_ of tagOptions) {
for (const assignment of assignmentOptions) {
combinations++;
const filters = { searchPattern, category, tag: tag_, assignment };
const actual = await places.listForTrip(String(trip.id), filters);
const legacy = legacyListForTrip(String(trip.id), filters);
expect(actual).toEqual(legacy);
}
}
}
}
expect(combinations).toBe(32);
});
});
// ---------------------------------------------------------------------------
// Plan 3c Task 5 — the four importers' `insertPlace` column sets, the
// colorize read+write transaction, the ftid backfill, and the
@@ -793,3 +899,29 @@ describe('PlacesRepository.fillIfEmpty (PL43/PL44/PL46) — three call shapes ov
expect((testDb.prepare('SELECT address FROM places WHERE id = ?').get(place.id) as { address: string | null }).address).toBeNull();
});
});
// ── Plan 3c Task 7 (trips.rpc.ts::getPlaces, RP2) — additive ────────────────
describe('PlacesRepository.listForTripOrdered (RP2)', () => {
it('PLACEREPO-046: every place of the trip, ORDER BY created_at DESC, scoped to the trip', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
const otherTrip = createTrip(testDb, user.id);
const p1 = createPlace(testDb, trip.id, { name: 'First' });
testDb.prepare('UPDATE places SET created_at = ? WHERE id = ?').run('2026-01-01 00:00:00', p1.id);
const p2 = createPlace(testDb, trip.id, { name: 'Second' });
testDb.prepare('UPDATE places SET created_at = ? WHERE id = ?').run('2026-02-01 00:00:00', p2.id);
createPlace(testDb, otherTrip.id, { name: 'Elsewhere' });
const legacy = testDb.prepare('SELECT * FROM places WHERE trip_id = ? ORDER BY created_at DESC').all(trip.id);
const rows = await places.listForTripOrdered(trip.id);
expect(rows).toEqual(legacy);
expect(rows.map((r) => r.id)).toEqual([p2.id, p1.id]);
});
it('PLACEREPO-047: an empty trip returns an empty array, not a throw', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
expect(await places.listForTripOrdered(trip.id)).toEqual([]);
});
});
@@ -340,3 +340,41 @@ describe('NaN user_id — rule 15 (a non-numeric route id parses to NaN, must no
await expect(tripMembers.isGuestOfTrip(trip.id, NaN)).resolves.toBe(false);
});
});
// ── Plan 3c Task 7 (trips.rpc.ts::members, RP3) — additive ──────────────────
describe('TripMembersRepository.listRawUsernameAndDisplayName (RP3)', () => {
it('TMEMREPO-026: byte-identical to the legacy statement — raw username AND display_name, NOT TM2\'s COALESCE', async () => {
const { user: owner } = createUser(testDb);
const { user: named } = createUser(testDb, { username: 'bare-name' });
testDb.prepare('UPDATE users SET display_name = ?, avatar = ? WHERE id = ?').run('Displayed', 'a.png', named.id);
const { user: bare } = createUser(testDb, { username: 'no-display' });
const trip = createTrip(testDb, owner.id);
addTripMember(testDb, trip.id, named.id);
addTripMember(testDb, trip.id, bare.id);
const legacy = testDb.prepare(
'SELECT u.id, u.username, u.display_name, u.avatar FROM trip_members tm JOIN users u ON u.id = tm.user_id WHERE tm.trip_id = ?',
).all(trip.id);
const rows = await tripMembers.listRawUsernameAndDisplayName(trip.id);
expect(rows).toEqual(legacy);
const byId = new Map(rows.map((r) => [r.id, r]));
// Raw username, not COALESCEd with display_name — TM2's shape is different on purpose (§18.10).
expect(byId.get(named.id)).toEqual({ id: named.id, username: 'bare-name', display_name: 'Displayed', avatar: 'a.png' });
expect(byId.get(bare.id)).toEqual({ id: bare.id, username: 'no-display', display_name: null, avatar: null });
});
it('TMEMREPO-027: an owner-only trip (no trip_members rows) returns an empty array', async () => {
const { user: owner } = createUser(testDb);
const trip = createTrip(testDb, owner.id);
expect(await tripMembers.listRawUsernameAndDisplayName(trip.id)).toEqual([]);
});
it('TMEMREPO-028: raw-bind — a string trip id binds unconverted, same as a real number', async () => {
const { user: owner } = createUser(testDb);
const { user: member } = createUser(testDb);
const trip = createTrip(testDb, owner.id);
addTripMember(testDb, trip.id, member.id);
expect((await tripMembers.listRawUsernameAndDisplayName(String(trip.id))).map((r) => r.id)).toEqual([member.id]);
});
});
@@ -2,7 +2,7 @@ import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest';
import { createSnapshotTestDb } from '../../../helpers/db-mock';
import { CAN_ACCESS_TRIP_SQL, resetTestDb } from '../../../helpers/test-db';
import { createTestOrm, type TestOrm } from '../../../helpers/test-orm';
import { addTripMember, createTrip, createUser } from '../../../helpers/factories';
import { addTripMember, createDay, createPlace, createTrip, createUser } from '../../../helpers/factories';
import { Trips } from '../../../../src/db/entities/Trips.entity';
import type { TripsRepository } from '../../../../src/db/repositories/Trips.repository';
@@ -235,3 +235,251 @@ describe('TripsRepository — TripMembersService / TripReadModelService (Plan 3c
});
});
});
// ── Plan 3c Task 7 — TRIP_SELECT's ONE builder, generateDays helpers, CRUD ───
//
// `TRIP_SELECT` (trips.service.ts): the shared projection every one of
// `findForViewer`/`listForUser`/`activeTrip` builds on (`tripSelectQuery`).
// Byte-diffed here against the legacy statement run raw, on a FULLY seeded
// trip (days, places, a second member, an owner) with every nullable column
// both null and set (rule 19).
const LEGACY_TRIP_SELECT = `
SELECT t.*,
NULL AS feed_token,
(SELECT COUNT(*) FROM days d WHERE d.trip_id = t.id) as day_count,
(SELECT COUNT(*) FROM places p WHERE p.trip_id = t.id) as place_count,
CASE WHEN t.user_id = :userId THEN 1 ELSE 0 END as is_owner,
u.username as owner_username,
(SELECT COUNT(*) FROM trip_members tm WHERE tm.trip_id = t.id) as shared_count
FROM trips t
JOIN users u ON u.id = t.user_id
`;
describe('TripsRepository.findForViewer / listForUser / activeTrip (Plan 3c Task 7, TRIP_SELECT)', () => {
describe('findForViewer (TP20, also serves TP19/TP29 and TripMembersService.getTripForViewer)', () => {
it('TRIPREPO-023: byte-identical to the legacy TRIP_SELECT statement on a FULLY seeded trip (days, places, a member, feed_token set in the DB)', async () => {
const { user: owner } = createUser(testDb, { username: 'owner-handle' });
const { user: member } = createUser(testDb);
const trip = createTrip(testDb, owner.id, {
title: 'Full Row', description: 'A real description', start_date: '2026-03-01', end_date: '2026-03-05',
});
testDb.prepare('UPDATE trips SET feed_token = ?, cover_image = ? WHERE id = ?').run('super-secret', 'cover.png', trip.id);
addTripMember(testDb, trip.id, member.id);
createDay(testDb, trip.id, { date: '2026-03-01' });
createDay(testDb, trip.id, { date: '2026-03-02' });
createPlace(testDb, trip.id);
createPlace(testDb, trip.id);
createPlace(testDb, trip.id);
const legacyOwner = testDb.prepare(`
${LEGACY_TRIP_SELECT}
LEFT JOIN trip_members m ON m.trip_id = t.id AND m.user_id = :userId
WHERE t.id = :tripId AND (t.user_id = :userId OR m.user_id IS NOT NULL)
`)
.get({ userId: owner.id, tripId: trip.id });
const row = await trips.findForViewer(trip.id, owner.id);
expect(row).toEqual(legacyOwner);
// createTrip's own start_date/end_date span already generates 5 days;
// the two explicit createDay calls above add 2 more, for 7 total.
expect(row).toMatchObject({
day_count: 7, place_count: 3, shared_count: 1, is_owner: 1, owner_username: 'owner-handle', feed_token: null,
});
// The real column is set in the DB — the wire value is blanked by the
// SQL-side `NULL AS feed_token` trick, not merely absent from a
// hand-picked column list (§18.5).
expect((testDb.prepare('SELECT feed_token FROM trips WHERE id = ?').get(trip.id) as { feed_token: string }).feed_token).toBe('super-secret');
const legacyMember = testDb.prepare(`
${LEGACY_TRIP_SELECT}
LEFT JOIN trip_members m ON m.trip_id = t.id AND m.user_id = :userId
WHERE t.id = :tripId AND (t.user_id = :userId OR m.user_id IS NOT NULL)
`)
.get({ userId: member.id, tripId: trip.id });
const memberRow = await trips.findForViewer(trip.id, member.id);
expect(memberRow).toEqual(legacyMember);
expect(memberRow).toMatchObject({ is_owner: 0, day_count: 7, place_count: 3, shared_count: 1 });
});
it('TRIPREPO-024: NULL-seeded nullable columns come back null on the wire; a stranger and a nonexistent trip both get undefined', async () => {
const { user: owner } = createUser(testDb);
const { user: stranger } = createUser(testDb);
const trip = createTrip(testDb, owner.id); // description/start_date/end_date/cover_image all NULL by default
const legacy = testDb.prepare(`
${LEGACY_TRIP_SELECT}
LEFT JOIN trip_members m ON m.trip_id = t.id AND m.user_id = :userId
WHERE t.id = :tripId AND (t.user_id = :userId OR m.user_id IS NOT NULL)
`)
.get({ userId: owner.id, tripId: trip.id });
const row = await trips.findForViewer(trip.id, owner.id);
expect(row).toEqual(legacy);
expect(row).toMatchObject({ description: null, start_date: null, end_date: null, cover_image: null, feed_token: null });
expect(await trips.findForViewer(trip.id, stranger.id)).toBeUndefined();
expect(await trips.findForViewer(999999, owner.id)).toBeUndefined();
});
it('TRIPREPO-025: binds a string trip id raw, the seam every other method here preserves', async () => {
const { user: owner } = createUser(testDb);
const trip = createTrip(testDb, owner.id, { title: 'Stringy' });
expect((await trips.findForViewer(String(trip.id), owner.id))?.title).toBe('Stringy');
});
// Rule 21: `findForViewer` is a Kysely read, which never consults the
// identity map — the "fresh after a raw UPDATE" shape applies instead of
// a D-shape identity-map test.
it('TRIPREPO-026 (fresh after a raw UPDATE): a title changed outside the ORM is visible on the next read', async () => {
const { user: owner } = createUser(testDb);
const trip = createTrip(testDb, owner.id, { title: 'Stale' });
expect((await trips.findForViewer(trip.id, owner.id))?.title).toBe('Stale');
testDb.prepare('UPDATE trips SET title = ? WHERE id = ?').run('Fresh', trip.id);
expect((await trips.findForViewer(trip.id, owner.id))?.title).toBe('Fresh');
});
});
describe('listForUser (TP16/TP17)', () => {
it('TRIPREPO-027: byte-identical to the legacy statement, archived === null (TP16, no filter)', async () => {
const { user: owner } = createUser(testDb);
const { user: member } = createUser(testDb);
const owned = createTrip(testDb, owner.id, { title: 'Owned' });
const shared = createTrip(testDb, member.id, { title: 'Shared' });
addTripMember(testDb, shared.id, owner.id);
const archived = createTrip(testDb, owner.id, { title: 'Archived' });
testDb.prepare('UPDATE trips SET is_archived = 1 WHERE id = ?').run(archived.id);
const legacy = testDb.prepare(`
${LEGACY_TRIP_SELECT}
LEFT JOIN trip_members m ON m.trip_id = t.id AND m.user_id = :userId
WHERE (t.user_id = :userId OR m.user_id IS NOT NULL)
ORDER BY t.created_at DESC
`).all({ userId: owner.id });
const rows = await trips.listForUser(owner.id, null);
expect(rows).toEqual(legacy);
expect(rows.map((r) => r.id).sort()).toEqual([owned.id, shared.id, archived.id].sort());
});
it('TRIPREPO-028: byte-identical to the legacy statement, archived filtered (TP17)', async () => {
const { user: owner } = createUser(testDb);
const active = createTrip(testDb, owner.id, { title: 'Active' });
const archived = createTrip(testDb, owner.id, { title: 'Archived' });
testDb.prepare('UPDATE trips SET is_archived = 1 WHERE id = ?').run(archived.id);
const legacyArchived = testDb.prepare(`
${LEGACY_TRIP_SELECT}
LEFT JOIN trip_members m ON m.trip_id = t.id AND m.user_id = :userId
WHERE (t.user_id = :userId OR m.user_id IS NOT NULL) AND t.is_archived = :archived
ORDER BY t.created_at DESC
`).all({ userId: owner.id, archived: 1 });
expect(await trips.listForUser(owner.id, 1)).toEqual(legacyArchived);
expect((await trips.listForUser(owner.id, 1)).map((r) => r.id)).toEqual([archived.id]);
expect((await trips.listForUser(owner.id, 0)).map((r) => r.id)).toEqual([active.id]);
});
});
describe('activeTrip (TP21) — the triple CASE WHEN relevance projection and the double-CASE WHEN ORDER BY', () => {
const LEGACY_ACTIVE_TRIP = `
SELECT t.id, t.title, t.start_date, t.end_date,
CASE
WHEN t.start_date IS NOT NULL AND t.end_date IS NOT NULL AND t.start_date <= :today AND t.end_date >= :today THEN 0
WHEN t.start_date IS NOT NULL AND t.start_date >= :today THEN 1
ELSE 2
END AS relevance
FROM trips t
LEFT JOIN trip_members m ON m.trip_id = t.id AND m.user_id = :userId
WHERE (t.user_id = :userId OR m.user_id IS NOT NULL) AND t.is_archived = 0
ORDER BY relevance ASC,
CASE WHEN relevance < 2 THEN t.start_date END ASC,
CASE WHEN relevance = 2 THEN t.start_date END DESC
LIMIT 1
`;
it('TRIPREPO-029: a running-today trip (relevance 0) wins over an upcoming or past one, matching the legacy statement result AND ordering', async () => {
const { user } = createUser(testDb);
const today = '2026-06-10';
createTrip(testDb, user.id, { title: 'Past', start_date: '2026-01-01', end_date: '2026-01-05' });
const running = createTrip(testDb, user.id, { title: 'Running', start_date: '2026-06-08', end_date: '2026-06-12' });
createTrip(testDb, user.id, { title: 'Future', start_date: '2026-07-01', end_date: '2026-07-05' });
const legacy = testDb.prepare(LEGACY_ACTIVE_TRIP).get({ userId: user.id, today });
const row = await trips.activeTrip(user.id, today);
expect(row).toEqual(legacy);
expect(row).toMatchObject({ id: running.id, title: 'Running', relevance: 0 });
});
it('TRIPREPO-030: no running trip — the next upcoming one wins, earliest first (relevance 1)', async () => {
const { user } = createUser(testDb);
const today = '2026-06-10';
const soonest = createTrip(testDb, user.id, { title: 'Soonest', start_date: '2026-07-01', end_date: '2026-07-05' });
createTrip(testDb, user.id, { title: 'Later', start_date: '2026-08-01', end_date: '2026-08-05' });
const legacy = testDb.prepare(LEGACY_ACTIVE_TRIP).get({ userId: user.id, today });
const row = await trips.activeTrip(user.id, today);
expect(row).toEqual(legacy);
expect(row).toMatchObject({ id: soonest.id, relevance: 1 });
});
it('TRIPREPO-031: nothing running or upcoming — the most recently started trip wins (relevance 2), archived trips excluded', async () => {
const { user } = createUser(testDb);
const today = '2026-06-10';
createTrip(testDb, user.id, { title: 'Long ago', start_date: '2026-01-01', end_date: '2026-01-05' });
const recent = createTrip(testDb, user.id, { title: 'Recent', start_date: '2026-05-01', end_date: '2026-05-05' });
const archived = createTrip(testDb, user.id, { title: 'Archived-but-recent', start_date: '2026-06-01', end_date: '2026-06-05' });
testDb.prepare('UPDATE trips SET is_archived = 1 WHERE id = ?').run(archived.id);
const legacy = testDb.prepare(LEGACY_ACTIVE_TRIP).get({ userId: user.id, today });
const row = await trips.activeTrip(user.id, today);
expect(row).toEqual(legacy);
expect(row).toMatchObject({ id: recent.id, relevance: 2 });
});
it('TRIPREPO-032: nothing at all returns undefined', async () => {
const { user } = createUser(testDb);
expect(await trips.activeTrip(user.id, '2026-06-10')).toBeUndefined();
});
});
});
describe('TripsRepository.insertTrip / updateTripRow / setCoverImage / deleteById (Plan 3c Task 7)', () => {
it('TRIPREPO-033: insertTrip (TP18) writes the given column set verbatim and returns the generated id', async () => {
const { user } = createUser(testDb);
const id = await trips.insertTrip({
user_id: user.id, title: 'New Trip', description: null, start_date: null, end_date: null, currency: 'EUR', reminder_days: 3,
});
const row = testDb.prepare('SELECT * FROM trips WHERE id = ?').get(id) as Record<string, unknown>;
expect(row).toMatchObject({ user_id: user.id, title: 'New Trip', currency: 'EUR', reminder_days: 3 });
});
it('TRIPREPO-034: updateTripRow (TP25) writes every column and stamps updated_at', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id, { title: 'Before' });
await trips.updateTripRow(trip.id, {
title: 'After', description: 'd', start_date: '2026-01-01', end_date: '2026-01-05',
currency: 'USD', is_archived: 1, cover_image: 'c.png', reminder_days: 7,
});
const row = testDb.prepare('SELECT * FROM trips WHERE id = ?').get(trip.id) as Record<string, unknown>;
expect(row).toMatchObject({
title: 'After', description: 'd', start_date: '2026-01-01', end_date: '2026-01-05',
currency: 'USD', is_archived: 1, cover_image: 'c.png', reminder_days: 7,
});
// CURRENT_TIMESTAMP has one-second resolution — a same-second before/after
// comparison is flaky, not a real assertion; the column being non-null and
// in the CURRENT_TIMESTAMP text shape is what `updateTripRow` promises.
expect(row.updated_at).toMatch(/^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}$/);
});
it('TRIPREPO-035: setCoverImage (TP35) writes cover_image and stamps updated_at', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
await trips.setCoverImage(trip.id, '/uploads/covers/x.jpg');
expect((testDb.prepare('SELECT cover_image FROM trips WHERE id = ?').get(trip.id) as { cover_image: string }).cover_image).toBe('/uploads/covers/x.jpg');
});
it('TRIPREPO-036: deleteById (TP34, security-sensitive) removes exactly the given trip', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
const other = createTrip(testDb, user.id);
await trips.deleteById(trip.id);
expect(testDb.prepare('SELECT id FROM trips WHERE id = ?').get(trip.id)).toBeUndefined();
expect(testDb.prepare('SELECT id FROM trips WHERE id = ?').get(other.id)).toBeDefined();
});
});
@@ -836,6 +836,17 @@ describe('UsersRepository — TripMembersService (Plan 3c Task 6)', () => {
it('USERSREPO-065: no match returns null, not a throw', async () => {
expect(await users.findInvitableByEmailOrUsername('nobody@example.test')).toBeNull();
});
// Task 6 review, L3: `findInvitableByEmailOrUsername` is `findOne`-based
// (unlike the qb-based projections above), so rule 21's D-shape test
// applies for real — this is the guest-exclusion guard (#1362), so
// staleness here would re-open the bug the guard exists to close.
it('USERSREPO-072 (D-shape): an is_guest flip written after an unrelated identity-map read is visible in the FIRST wider projection', async () => {
const { user } = createUser(testDb, { email: 'flip@example.test', username: 'flip-handle' });
await t.repo(Users).find({}, { disableIdentityMap: false }); // populate the identity map with an unrelated read
testDb.prepare('UPDATE users SET is_guest = 1 WHERE id = ?').run(user.id);
expect(await users.findInvitableByEmailOrUsername('flip@example.test')).toBeNull();
});
});
describe('findIdEmailGuest (TM10)', () => {
@@ -63,6 +63,10 @@ describe('members', () => {
const s = svc({ addMember, notifyInvite } as Partial<TripMembersService>);
expect(await tc(s).addMember(user, '9', { identifier: 'bob@x.y' })).toEqual({ member: { id: 2, email: 'bob@x.y' } });
expect(notifyInvite).toHaveBeenCalledWith('9', user, 2, 'T', 'bob@x.y');
// Task 6 review, L1: the pre-existing REST↔MCP asymmetry (§13b) — these five
// controller mutations broadcast NOTHING while the MCP twins all emit
// member:added/member:removed. Pinned here rather than "fixed".
expect(s.broadcast).not.toHaveBeenCalled();
});
it('POST 404 without trip access', async () => {
@@ -90,6 +94,8 @@ describe('members', () => {
// self-removal (targetId === user.id) bypasses the permission check
expect(await tc(s).removeMember(user, '9', '1')).toEqual({ success: true });
expect(await thrown(() => tc(s).removeMember(user, '9', '2'))).toEqual({ status: 403, body: { error: 'No permission to remove members' } });
// Task 6 review, L1: see the addMember case above.
expect(s.broadcast).not.toHaveBeenCalled();
});
});
@@ -136,6 +142,8 @@ describe('guests (#1362)', () => {
const s = svc({ createGuest } as Partial<TripMembersService>);
expect(await tc(s).createGuest(user, '9', { name: 'Anna' })).toEqual({ member: { id: 7, username: 'Anna', is_guest: true } });
expect(createGuest).toHaveBeenCalledWith('9', 'Anna', user.id);
// Task 6 review, L1: see the addMember case above.
expect(s.broadcast).not.toHaveBeenCalled();
});
it('rename: 404 when the guest is missing, else success', async () => {
@@ -143,6 +151,8 @@ describe('guests (#1362)', () => {
expect(await thrown(() => tc(miss).renameGuest(user, '9', '7', { name: 'Bob' }))).toEqual({ status: 404, body: { error: 'Guest not found' } });
const ok = svc({ renameGuest: vi.fn().mockReturnValue(true) } as Partial<TripMembersService>);
expect(await tc(ok).renameGuest(user, '9', '7', { name: 'Bob' })).toEqual({ success: true });
// Task 6 review, L1: see the addMember case above.
expect(ok.broadcast).not.toHaveBeenCalled();
});
it('delete: 404 when the guest is missing, else success', async () => {
@@ -150,6 +160,8 @@ describe('guests (#1362)', () => {
expect(await thrown(() => tc(miss).deleteGuest(user, '9', '7'))).toEqual({ status: 404, body: { error: 'Guest not found' } });
const ok = svc({ deleteGuest: vi.fn().mockResolvedValue(true) } as Partial<TripMembersService>);
expect(await tc(ok).deleteGuest(user, '9', '7')).toEqual({ success: true });
// Task 6 review, L1: see the addMember case above.
expect(ok.broadcast).not.toHaveBeenCalled();
});
it('maps a ValidationError from createGuest to 400', async () => {
@@ -1,5 +1,7 @@
/**
* Unit tests for TripMembersService — MEMBERS-SVC-001 through MEMBERS-SVC-015.
* Unit tests for TripMembersService — MEMBERS-SVC-001 through MEMBERS-SVC-018
* (016-018 added by the Task 6 review's M1/L2 items: transferOwnership/
* createGuest transaction-rollback proofs and an addMember concurrency case).
*
* The roster cases that came over with the split still run in
* tests/unit/nest/trips.service.test.ts (TRIP-SVC-020…023, 030…034, 049, 052,
@@ -69,6 +71,7 @@ import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, c
import type { EntityManager } from '@mikro-orm/core';
import type { TripsRepository } from '../../../src/db/repositories/Trips.repository';
import type { UsersRepository } from '../../../src/db/repositories/Users.repository';
import type { TripMembersRepository } from '../../../src/db/repositories/TripMembers.repository';
// Plan 3c Task 0b: `dbsEm` is resolved once, at the top of the `beforeAll`
// below, before any `dbs()` call — `canAccessTrip`/`isOwner`/`rosterUserIds`/
@@ -83,17 +86,24 @@ let budgetSvc: BudgetService;
let roster: TripMembersService;
let tripsRepo: TripsRepository;
let usersRepo: UsersRepository;
// Task 6 review, M1: hoisted (was constructed inline) so the mutation-rollback
// tests below can `vi.spyOn` the SAME instance the service holds — the shared
// EM caches repositories, so a fresh `createTestTripMembersRepo(testDb)` call
// returns this identical object (Task 6 review's own confirmation for `tripsRepo`/
// `usersRepo`'s existing spies applies here too).
let tripMembersRepo: TripMembersRepository;
beforeAll(async () => {
dbsEm = (await sharedTestOrm(testDb)).em;
tripsRepo = await createTestTripsRepo(testDb);
usersRepo = await createTestUsersRepo(testDb);
tripMembersRepo = await createTestTripMembersRepo(testDb);
budgetSvc = new BudgetService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), new ExchangeRatesService(), new RealtimeService(), await createTestUnitOfWork(dbs().connection));
roster = new TripMembersService(
dbs(), budgetSvc,
new UserCleanupService(dbs(), budgetSvc, await createTestUnitOfWork(dbs().connection), usersRepo),
new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)),
new RealtimeService(), notificationsStub(notifySend), await createTestUnitOfWork(dbs().connection),
tripsRepo, await createTestTripMembersRepo(testDb), usersRepo,
tripsRepo, tripMembersRepo, usersRepo,
);
});
@@ -160,9 +170,9 @@ describe('TripMembersService delegation', () => {
// The handover broadcast hands the raw :id route param straight in, so the
// named-parameter query has to keep matching a string id against the INTEGER
// column — and the payload the clients re-read must carry their own is_owner.
const asOwner = await roster.getTripForViewer(String(trip.id), owner.id) as Record<string, unknown>;
const asOwner = await roster.getTripForViewer(String(trip.id), owner.id) as unknown as Record<string, unknown>;
expect(asOwner).toMatchObject({ id: trip.id, title: 'Handover', is_owner: 1, owner_username: owner.username, shared_count: 1 });
const asMember = await roster.getTripForViewer(trip.id, member.id) as Record<string, unknown>;
const asMember = await roster.getTripForViewer(trip.id, member.id) as unknown as Record<string, unknown>;
expect(asMember.is_owner).toBe(0);
expect(await roster.getTripForViewer(999999, owner.id)).toBeUndefined();
});
@@ -375,3 +385,77 @@ describe('listMembers shaping', () => {
expect(ownerRow.is_guest).toBe(false);
});
});
// ── Task 6 review items (M1, L2) ─────────────────────────────────────────────
//
// task-6-review.md M1: neither `transferOwnership` nor `createGuest` had a
// test proving their `uow.transactional` block actually rolls back when its
// LAST statement fails — a mutation moving `addIgnoringConflict`/`addMember`
// outside the transaction survived the whole suite. Shaped like TRIP-SVC-052
// (`vi.spyOn` the repository the shared EM caches, not a hand-rolled fault
// injection). L2: `addMember`'s TM5→TM6 check-then-act (§18.4) had no
// concurrency test of its own — `joinTripAsMember`'s TRIP-JOIN-005 covers the
// identical shape one method over; this copies it.
describe('Task 6 review items — rollback and concurrency', () => {
it('MEMBERS-SVC-016 (mutation-proved): transferOwnership rolls back when the final INSERT rejects — the owner pointer and the new owner\'s membership both survive', async () => {
const { user: owner } = createUser(testDb);
const { user: member } = createUser(testDb);
const trip = createTrip(testDb, owner.id);
await roster.addMember(trip.id, member.email, owner.id, owner.id);
const spy = vi.spyOn(tripMembersRepo, 'addIgnoringConflict').mockRejectedValueOnce(new Error('boom'));
try {
await expect(roster.transferOwnership(trip.id, member.id, owner.id)).rejects.toThrow('boom');
} finally {
spy.mockRestore();
}
// setOwner (TM13) ran, then remove (TM14) ran, then addIgnoringConflict (TM15)
// rejected — a fix that moved TM15 outside the transaction would leave TM13/TM14
// committed while this assertion still expects them rolled back.
const tripRow = testDb.prepare('SELECT user_id FROM trips WHERE id = ?').get(trip.id) as { user_id: number };
expect(tripRow.user_id).toBe(owner.id);
expect(testDb.prepare('SELECT id FROM trip_members WHERE trip_id = ? AND user_id = ?').get(trip.id, member.id)).toBeDefined();
});
it('MEMBERS-SVC-017 (mutation-proved): createGuest rolls back when the membership INSERT rejects — no orphan guest user row', async () => {
const { user: owner } = createUser(testDb);
const trip = createTrip(testDb, owner.id);
const before = (testDb.prepare('SELECT COUNT(*) AS n FROM users').get() as { n: number }).n;
const spy = vi.spyOn(tripMembersRepo, 'addMember').mockRejectedValueOnce(new Error('boom'));
try {
await expect(roster.createGuest(trip.id, 'Rollback Rae', owner.id)).rejects.toThrow('boom');
} finally {
spy.mockRestore();
}
// insertGuest (TM16) ran, then addMember (TM17) rejected — a fix that moved
// TM17 outside the transaction would leave the guest's `users` row behind.
const after = (testDb.prepare('SELECT COUNT(*) AS n FROM users').get() as { n: number }).n;
expect(after).toBe(before);
});
it('MEMBERS-SVC-018 (concurrency, §18.4, copies TRIP-JOIN-005): two concurrent addMember calls for the same (trip, user) race the TM5→TM6 check-then-act window', async () => {
const { user: owner } = createUser(testDb);
const { user: invitee } = createUser(testDb);
const trip = createTrip(testDb, owner.id);
const results = await Promise.allSettled([
roster.addMember(trip.id, invitee.email, owner.id, owner.id),
roster.addMember(trip.id, invitee.email, owner.id, owner.id),
]);
// Today's actual outcome, run under real concurrency, not assumed: the
// method performs no locking/serialization of its own (TM5 `exists` →
// TM6 `addMember`, unchanged by this conversion, §18.4). Whichever call
// loses the race either throws the app-level 'User already has access'
// (TM5 caught it) or rejects on the table's own UNIQUE(trip_id, user_id)
// constraint (TM6 raced past TM5) — both are acceptable, unchanged
// outcomes; what must hold is that at most one membership row exists.
const fulfilled = results.filter((r) => r.status === 'fulfilled');
expect(fulfilled.length).toBeGreaterThanOrEqual(1);
const count = testDb.prepare('SELECT COUNT(*) as n FROM trip_members WHERE trip_id = ? AND user_id = ?').get(trip.id, invitee.id) as { n: number };
expect(count.n).toBe(1);
});
});
+18 -36
View File
@@ -1224,41 +1224,18 @@ describe('folded quirk branches', () => {
// ── Post-fold quirk fixes (transactions, owner display name) ─────────────────
describe('quirk fixes', () => {
/** A TripsService whose connection throws when preparing SQL matching `match`. */
function failingConnection(match: string) {
const conn = new Proxy(testDb, {
get(target, prop) {
if (prop === 'prepare') {
return (sql: string) => {
if (sql.includes(match)) throw new Error('boom');
return target.prepare(sql);
};
}
const v = (target as any)[prop];
return typeof v === 'function' ? v.bind(target) : v;
},
});
return { connection: conn, canAccessTrip: dbMock.canAccessTrip, isOwner: dbMock.isOwner } as unknown as import('../../../src/nest/database/database.service').DatabaseService;
}
// Plan 3c Task 7 (R8): the SQL-text-keyed `failingConnection`/`failingTrips`
// Proxy this test used to build a one-off TripsService around is rewritten
// as a repository-level fault, the same shape TRIP-SVC-052 below already
// uses for `deleteGuest` — `remove`'s TP32-TP34 (journey_entries stay raw;
// `deleteById` is TP34, `TripsRepository`'s) are the statements this test
// pins, and TP34 is now a repository method to `vi.spyOn`, not SQL text to
// pattern-match. The Proxy could never trigger any more: no statement text
// matching `'DELETE FROM trips WHERE id = ?'` runs through `this.db`
// inside `remove`'s transaction any more (TP32/TP33 are the only survivors
// there, and their own text is untouched).
async function failingTrips(match: string) {
const fdbs = failingConnection(match);
return new TripsService(
fdbs,
new ReservationsService(dbs(), new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)), budgetSvc, new RealtimeService(), notificationsStub(), new ReservationsReadRepository(dbs()), accommodationsSvc, await createTestUnitOfWork(dbs().connection)),
daysSvc,
new PermissionsService(await createTestAppSettingsRepo(dbs().connection), await createTestUnitOfWork(dbs().connection)),
budgetSvc,
new VacayService(dbs(), new RealtimeService(), notificationsStub(), await createTestUnitOfWork(dbs().connection)),
new RealtimeService(),
undefined as never,
coversFx.storage,
await createTestUnitOfWork(dbs().connection),
(await sharedTestOrm(testDb)).em,
);
}
it('TRIP-SVC-051: remove is atomic — a failed trip DELETE keeps the journey entries intact', async () => {
it('TRIP-SVC-051 (mutation-proved): remove is atomic — a failed trip DELETE keeps the journey entries intact', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
const journeyId = Number(testDb.prepare(
@@ -1268,8 +1245,13 @@ describe('quirk fixes', () => {
"INSERT INTO journey_entries (journey_id, source_trip_id, author_id, type, title, entry_date, created_at, updated_at) VALUES (?, ?, ?, 'skeleton', 'S', '2025-06-01', 0, 0)",
).run(journeyId, trip.id, user.id);
const broken = await failingTrips('DELETE FROM trips WHERE id = ?');
await expect(broken.remove(trip.id, user.id, 'user')).rejects.toThrow('boom');
const tripsRepo = await createTestTripsRepo(dbs().connection);
const spy = vi.spyOn(tripsRepo, 'deleteById').mockRejectedValueOnce(new Error('boom'));
try {
await expect(svc.remove(trip.id, user.id, 'user')).rejects.toThrow('boom');
} finally {
spy.mockRestore();
}
// The skeleton cleanup rolled back with the failed delete.
expect(testDb.prepare("SELECT id FROM journey_entries WHERE type = 'skeleton'").get()).toBeDefined();
@@ -23,6 +23,7 @@ import { ReservationsRpc } from '../../../src/nest/reservations/reservations.rpc
import { DaysRpc } from '../../../src/nest/days/days.rpc';
import { VacayRpc } from '../../../src/nest/vacay/vacay.rpc';
import { NotFoundError, ValidationError } from '../../../src/nest/trips/trips.service';
import type { EntityManager } from '@mikro-orm/core';
import type { DatabaseService } from '../../../src/nest/database/database.service';
import type { PermissionsService } from '../../../src/nest/permissions/permissions.service';
import type { AddonsService } from '../../../src/nest/addons/addons.service';
@@ -43,8 +44,23 @@ function guardsFor() {
canAccessTrip: vi.fn(async () => ({ id: 1, user_id: 42 })),
prepare: vi.fn(() => ({ get: () => ({ role: 'user' }), all: () => [] })),
} as unknown as DatabaseService;
// Plan 3c Task 7: TripsRpc's RP1-RP6 resolve `EntityManager.getRepository(...)`
// now, not `db.prepare(...)` — a generic fake repository (same shape regardless
// of which entity class was requested) covers every RP method these fallback
// cases exercise, none of which assert on the repository's specific return value.
const em = {
getRepository: () => ({
getRole: vi.fn(async () => 'user'),
findIdAndEmail: vi.fn(async () => ({ id: 2, email: 'x@example.test' })),
getOwnerId: vi.fn(async () => 1),
findRaw: vi.fn(async () => ({ id: 1, title: 'Japan', feed_token: null })),
listForTripOrdered: vi.fn(async () => []),
listRawUsernameAndDisplayName: vi.fn(async () => []),
}),
} as unknown as EntityManager;
return {
db,
em,
guards: new PluginGuards(
db,
{ checkPermission: vi.fn(() => true) } as unknown as PermissionsService,
@@ -141,14 +157,14 @@ describe('a service that says no becomes a refusal, not a crash', () => {
describe('service errors are translated into the RPC taxonomy', () => {
function tripsHost(updateThrows?: Error, createThrows?: Error) {
const { guards, db } = guardsFor();
const { guards, em } = guardsFor();
const trips = {
list: vi.fn(() => []),
updateTrip: vi.fn(() => { if (updateThrows) throw updateThrows; return { updatedTrip: { id: 1 } }; }),
create: vi.fn(() => { if (createThrows) throw createThrows; return { trip: { id: 99 } }; }),
removeMember: vi.fn(),
} as never;
const rpc = new TripsRpc(trips, {} as never, {} as never, {} as never, db, realtime(), guards, {} as never, {} as never);
const rpc = new TripsRpc(trips, {} as never, {} as never, {} as never, realtime(), guards, {} as never, {} as never, em);
return new PluginRpcHost('p', ALL, makeDeps(), createTestPluginRegistry([rpc]));
}
@@ -245,13 +261,13 @@ const SCHEMA_REJECTS: Array<[string, Record<string, unknown>]> = [
describe('every schema-validated method rejects a payload its schema refuses', () => {
function everything() {
const { guards, db } = guardsFor();
const { guards, em } = guardsFor();
const anything = new Proxy({}, { get: () => vi.fn(() => ({ id: 1, trip: { id: 1 }, updatedTrip: { id: 1 }, reservation: { id: 1 } })) }) as never;
const registry = createTestPluginRegistry([
new PlacesRpc(anything, anything, realtime(), guards),
new DaysRpc(anything, realtime(), guards),
new PackingRpc(anything, realtime(), guards),
new TripsRpc(anything, anything, anything, anything, db, realtime(), guards, anything, anything),
new TripsRpc(anything, anything, anything, anything, realtime(), guards, anything, anything, em),
new AccommodationsRpc(anything, realtime(), guards),
new ReservationsRpc(anything, realtime(), guards),
]);
@@ -17,6 +17,7 @@ import type { RealtimeService } from '../../../src/nest/realtime/realtime.servic
import { TripsRpc } from '../../../src/nest/trips/trips.rpc';
import { DbRpc } from '../../../src/nest/plugins/host/rpc/db.rpc';
import { PluginGuards } from '../../../src/nest/plugins/host/plugin-guards.service';
import type { EntityManager } from '@mikro-orm/core';
import type { DatabaseService } from '../../../src/nest/database/database.service';
import type { PluginUserSettingsService } from '../../../src/nest/plugins/plugin-user-settings.service';
@@ -39,8 +40,13 @@ const tripsRegistry = () => {
prepare: () => ({ get: () => ({ id: 1, title: 'Japan' }), all: () => [] }),
} as unknown as DatabaseService;
const guards = new PluginGuards(db, {} as never, {} as never);
// Plan 3c Task 7: trips.getById (RP1) now reads through
// `EntityManager.getRepository(Trips).findRaw(...)`, not `db.prepare(...)`.
const em = {
getRepository: () => ({ findRaw: async () => ({ id: 1, title: 'Japan', feed_token: null }) }),
} as unknown as EntityManager;
return createTestPluginRegistry([
new TripsRpc({} as never, {} as never, {} as never, {} as never, db, {} as never, guards, {} as never, {} as never),
new TripsRpc({} as never, {} as never, {} as never, {} as never, {} as never, guards, {} as never, {} as never, em),
]);
};
+43 -9
View File
@@ -8,6 +8,7 @@
* ACCESS, which is why it sits behind its own permission rather than any other write.
*/
import { describe, it, expect, vi } from 'vitest';
import type { EntityManager } from '@mikro-orm/core';
import { expectRegisteredProvider } from '../../helpers/module-providers';
import { PluginRpcHost } from '../../../src/nest/plugins/host/rpc-host';
import { createTestPluginRegistry } from '../../../src/nest/plugins/host/rpc-kit/testing';
@@ -28,6 +29,10 @@ import type { PermissionsService } from '../../../src/nest/permissions/permissio
import type { AddonsService } from '../../../src/nest/addons/addons.service';
import type { RpcRequest, RpcError } from '../../../src/nest/plugins/protocol/envelope';
import { makeDeps } from '../../helpers/rpc-host-deps';
import { Trips } from '../../../src/db/entities/Trips.entity';
import { Places } from '../../../src/db/entities/Places.entity';
import { Users } from '../../../src/db/entities/Users.entity';
import { TripMembers } from '../../../src/db/entities/TripMembers.entity';
const req = (method: string, params: Record<string, unknown> = {}): RpcRequest => ({ k: 'req', id: 'x', method, params });
@@ -60,21 +65,50 @@ export function build(opts: { allow?: (action: string) => boolean; updateThrows?
removeMember: vi.fn(),
} 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>>;
// `PluginGuards.canEditAs` (nest/plugins/host/, out of this task's scope)
// still calls `db.prepare('SELECT role FROM users WHERE id = ?')` for its
// own per-field permission role lookup — unrelated to RP4's now-converted
// `SELECT role FROM users WHERE id = ?` inside `TripsRpc.update` itself.
const db = {
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' };
if (sql.includes('SELECT user_id FROM trips')) return { user_id: 42 };
return { id: 1, title: 'Japan' };
},
all: () => [{ id: 7, name: 'Place' }],
})),
prepare: vi.fn(() => ({ get: () => ({ role: 'user' }) })),
} as unknown as DatabaseService;
// Plan 3c Task 7: RP1-RP6's SQL-text-keyed `db.prepare` stub is gone —
// every raw statement moved to a repository method, so this is a
// repository-level stub, keyed on the entity CLASS `em.getRepository(...)`
// was asked for, matching the shape the legacy `db.prepare(sql)` branch
// covered: `Trips.findRaw` (RP1/getById), `Places.listForTripOrdered`
// (RP2/getPlaces), `TripMembers.listRawUsernameAndDisplayName`
// (RP3/members), `Users.getRole` (RP4/update) and `.findIdAndEmail`
// (RP5/addMember — id 404 is the "unknown user" case every test below
// exercises), `Trips.getOwnerId` (RP6/removeMember — trip 1's owner is 42).
const em = {
getRepository: vi.fn((entity: unknown) => {
if (entity === Trips) {
return {
findRaw: vi.fn(async (id: number) => ({ id, title: 'Japan', feed_token: null })),
getOwnerId: vi.fn(async () => 42),
};
}
if (entity === Places) {
return { listForTripOrdered: vi.fn(async () => [{ id: 7, name: 'Place' }]) };
}
if (entity === Users) {
return {
getRole: vi.fn(async () => 'user'),
findIdAndEmail: vi.fn(async (id: number) => (id === 404 ? null : { id, email: `u${id}@example.test` })),
};
}
if (entity === TripMembers) {
return { listRawUsernameAndDisplayName: vi.fn(async () => [{ id: 7, username: 'bob', display_name: null, avatar: null }]) };
}
throw new Error(`build(): unexpected entity ${String(entity)}`);
}),
} as unknown as EntityManager;
const permissions = { checkPermission: vi.fn((a: string) => (opts.allow ? opts.allow(a) : true)) } as unknown as PermissionsService;
const realtime = { broadcast: vi.fn() } as unknown as RealtimeService & { broadcast: ReturnType<typeof vi.fn> };
const guards = new PluginGuards(db, permissions, { isAddonEnabled: vi.fn(() => true) } as unknown as AddonsService);
const rpc = new TripsRpc(trips, reservations, days, membership, db, realtime, guards, accommodations, roster);
const rpc = new TripsRpc(trips, reservations, days, membership, realtime, guards, accommodations, roster, em);
const host = (...grants: string[]) =>
new PluginRpcHost('p', new Set(grants.length ? grants : ALL_TRIP_GRANTS), makeDeps(), createTestPluginRegistry([rpc]));
return { trips, reservations, days, accommodations, roster, membership, realtime, permissions, host };