k1nq/TREK
1
0
Fork 0
mirror of https://github.com/mauriceboe/TREK.git synced 2026-10-18 21:16:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(settings): the planner views ask trip membership which trips a user can open, and their tests seed through the factories

Browse Source
This commit is contained in:
Maurice
2026-10-12 21:13:57 +00:00
parent 4a6ee6819c
commit 15c9503295
8 changed files with 66 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/src/nest/settings/settings.module.ts
+5 -4
View File
@@ -1,8 +1,8 @@
import { AppSettings } from '../../db/entities/AppSettings.entity';
import { Settings } from '../../db/entities/Settings.entity';
import { Trips } from '../../db/entities/Trips.entity';
import { AppConfigModule } from '../app-config/app-config.module';
import { AuditModule } from '../audit/audit.module';
import { TripMembershipModule } from '../trip-membership/trip-membership.module';
import { AdminDefaultUserSettingsController, SettingsController } from './settings.controller';
import { SettingsMcp } from './settings.mcp';
import { SettingsService } from './settings.service';
@@ -20,9 +20,10 @@ import { Module } from '@nestjs/common';
// (nothing resolves it off an ambient MikroORM request context — an
// earlier design tried that and broke ~185 hand-built unit tests, see
// task-5-review.md §4), so nothing in this module ever needs
// `@InjectRepository(Users)`. Trips is here for the planner view row (#2639),
// which keeps only trips the user can still open.
imports: [AppConfigModule, AuditModule, MikroOrmModule.forFeature([AppSettings, Settings, Trips])],
// `@InjectRepository(Users)`. TripMembershipModule is here for the planner
// view row (#2639), which keeps only trips the user can still open: the
// trips table belongs to another domain, so the ids come from its service.
imports: [AppConfigModule, AuditModule, TripMembershipModule, MikroOrmModule.forFeature([AppSettings, Settings])],
controllers: [SettingsController, AdminDefaultUserSettingsController],
providers: [SettingsService, SettingsMcp],
exports: [SettingsService],
server/src/nest/settings/settings.service.ts
+3 -4
View File
@@ -1,12 +1,11 @@
import { readEnv } from '../../app-config';
import { AppSettings } from '../../db/entities/AppSettings.entity';
import { Settings } from '../../db/entities/Settings.entity';
import { Trips } from '../../db/entities/Trips.entity';
import type { AppSettingsRepository } from '../../db/repositories/AppSettings.repository';
import type { SettingsRepository } from '../../db/repositories/Settings.repository';
import type { TripsRepository } from '../../db/repositories/Trips.repository';
import { decrypt_api_key, maybe_encrypt_api_key } from '../common/crypto/apiKeyCrypto';
import { UnitOfWork } from '../database/unit-of-work';
import { TripMembershipService } from '../trip-membership/trip-membership.service';
import { InjectRepository } from '@mikro-orm/nestjs';
import { Injectable } from '@nestjs/common';
import {
@@ -180,7 +179,7 @@ export class SettingsService {
private readonly uow: UnitOfWork,
@InjectRepository(AppSettings) private readonly appSettings: AppSettingsRepository,
@InjectRepository(Settings) private readonly settings: SettingsRepository,
@InjectRepository(Trips) private readonly trips: TripsRepository,
private readonly membership: TripMembershipService,
) {}
/**
@@ -196,7 +195,7 @@ export class SettingsService {
if (!parsed.success) return {};
const views = compactPlannerTripViews(parsed.data);
if (Object.keys(views).length === 0) return views;
const openable = new Set((await this.trips.listAccessibleIds(userId)).map(String));
const openable = new Set((await this.membership.listAccessibleTripIds(userId)).map(String));
return Object.fromEntries(Object.entries(views).filter(([tripId]) => openable.has(tripId)));
}
server/tests/e2e/settings.e2e.test.ts
+15 -10
View File
@@ -7,6 +7,8 @@
*/
import { AppSettings } from '../../src/db/entities/AppSettings.entity';
import { Settings } from '../../src/db/entities/Settings.entity';
import { TripMembers } from '../../src/db/entities/TripMembers.entity';
import { Trips } from '../../src/db/entities/Trips.entity';
import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter';
import { ZodValidationPipe } from '../../src/nest/common/zod-validation.pipe';
import { SettingsModule } from '../../src/nest/settings/settings.module';
@@ -33,11 +35,13 @@ const { db } = vi.hoisted(() => {
key TEXT NOT NULL, value TEXT, UNIQUE(user_id, key));`);
tmp.exec('CREATE TABLE app_settings (key TEXT PRIMARY KEY, value TEXT);');
// Only what the planner view row (#2639) asks: which trips the user can open.
// An ORM insert names every column the entity gives a default (currency,
// is_archived, reminder_days, the timestamps and added_at), so those are here too.
tmp.exec(`CREATE TABLE trips (id INTEGER PRIMARY KEY AUTOINCREMENT, user_id INTEGER NOT NULL,
title TEXT NOT NULL DEFAULT 'Trip', created_at DATETIME DEFAULT CURRENT_TIMESTAMP);`);
tmp.exec(
'CREATE TABLE trip_members (id INTEGER PRIMARY KEY AUTOINCREMENT, trip_id INTEGER NOT NULL, user_id INTEGER NOT NULL);',
);
title TEXT NOT NULL DEFAULT 'Trip', currency TEXT, is_archived INTEGER DEFAULT 0, reminder_days INTEGER DEFAULT 3,
created_at DATETIME DEFAULT CURRENT_TIMESTAMP, updated_at DATETIME DEFAULT CURRENT_TIMESTAMP);`);
tmp.exec(`CREATE TABLE trip_members (id INTEGER PRIMARY KEY AUTOINCREMENT, trip_id INTEGER NOT NULL,
user_id INTEGER NOT NULL, added_at DATETIME DEFAULT CURRENT_TIMESTAMP);`);
return { db: tmp };
});
@@ -335,12 +339,13 @@ describe('Settings e2e (real auth guard + temp SQLite)', () => {
});
describe('planner_trip_views (#2639)', () => {
beforeEach(() => {
db.exec('DELETE FROM trip_members; DELETE FROM trips;');
db.prepare('INSERT INTO trips (id, user_id) VALUES (?, ?)').run(41, 1);
db.prepare('INSERT INTO trips (id, user_id) VALUES (?, ?)').run(42, 2);
db.prepare('INSERT INTO trips (id, user_id) VALUES (?, ?)').run(43, 2);
db.prepare('INSERT INTO trip_members (trip_id, user_id) VALUES (?, ?)').run(43, 1);
beforeEach(async () => {
await deleteRows(orm, TripMembers);
await deleteRows(orm, Trips);
await insertRow(orm, Trips, { id: 41, user: 1, title: 'Trip' });
await insertRow(orm, Trips, { id: 42, user: 2, title: 'Trip' });
await insertRow(orm, Trips, { id: 43, user: 2, title: 'Trip' });
await insertRow(orm, TripMembers, { trip: 43, user: 1 });
});
it('PUT keeps the switches of trips the user can open and GET hands them back', async () => {
server/tests/helpers/plugin-host.ts
+2 -12
View File
@@ -413,12 +413,7 @@ export async function createPluginRpcHostParts(
// Plan 4 Task 1 constructor-ripple: LlmConfigResolver's addon-row read
// moved off DatabaseService onto AddonsRepository.
const llmConfig = new LlmConfigResolver(
new SettingsService(
await createTestUnitOfWork(db),
appSettings,
await createTestSettingsRepo(db),
await createTestTripsRepo(db),
),
new SettingsService(await createTestUnitOfWork(db), appSettings, await createTestSettingsRepo(db), membership),
(await sharedTestOrm(db)).repo(Addons),
addons,
);
@@ -512,12 +507,7 @@ export async function createPluginRpcHostParts(
generalStorage,
await createTestUnitOfWork(db),
(await sharedTestOrm(db)).em,
new SettingsService(
await createTestUnitOfWork(db),
appSettings,
await createTestSettingsRepo(db),
await createTestTripsRepo(db),
),
new SettingsService(await createTestUnitOfWork(db), appSettings, await createTestSettingsRepo(db), membership),
);
// Plan 4 Task 4: UserCleanupService's UC1 now goes through a directly-injected
// EntityManager (MaintenanceRepository.deletePluginUserData), not DatabaseService.
server/tests/unit/nest/journey-share.service.test.ts
+9 -1
View File
@@ -22,6 +22,8 @@ import { JourneyShareService } from '../../../src/nest/journey/journey-share.ser
import { TrekPhotoRegistrationService } from '../../../src/nest/photos/trek-photo-registration.service';
import { RealtimeService } from '../../../src/nest/realtime/realtime.service';
import { SettingsService } from '../../../src/nest/settings/settings.service';
import { TripMembershipModule } from '../../../src/nest/trip-membership/trip-membership.module';
import { TripMembershipService } from '../../../src/nest/trip-membership/trip-membership.service';
import { createUser, createJourney, createJourneyEntry, addJourneyContributor } from '../../helpers/factories';
import { countRows, findRow, insertRow, insertRowIgnoringConflict, updateRows } from '../../helpers/factories/rows';
import { setAppSetting, setUserSetting } from '../../helpers/factories/settings';
@@ -36,6 +38,7 @@ import {
import { createTestJourneyShareTokensRepo } from '../../helpers/journey-share-repos';
import { GALLERY_CHRONOLOGICAL_ORDER } from '../../helpers/legacy-gallery-order';
import { resetTestDb } from '../../helpers/test-db';
import { createTestModule, type TestModule } from '../../helpers/test-module';
import type { TestOrm } from '../../helpers/test-orm';
import {
sharedTestOrm,
@@ -66,6 +69,7 @@ vi.mock('../../../src/db/database', async () => {
let svc: JourneyShareService;
let t: TestOrm;
let membership: TestModule;
// `t`, `uow` and the settings repositories all derive from the SAME
// `sharedTestOrm(testDb)` (task-2-review.md I2) — see settings.service.test.ts's
@@ -76,6 +80,9 @@ beforeAll(async () => {
const uow = await createTestUnitOfWork(testDb);
t = await sharedTestOrm(testDb);
const journeysRepo = await createTestJourneysRepo(testDb);
// SettingsService reads the trips a planner view may keep (#2639) through
// TripMembershipService, taken from its own module.
membership = await createTestModule({ db: testDb, imports: [TripMembershipModule] });
svc = new JourneyShareService(
new JourneyDomainService(
new RealtimeService(),
@@ -99,7 +106,7 @@ beforeAll(async () => {
uow,
await createTestAppSettingsRepo(testDb),
await createTestSettingsRepo(testDb),
await createTestTripsRepo(testDb),
membership.get(TripMembershipService),
),
// Plan 3g Task 3: JourneyShareTokensRepository (JS1-JS15) + the
// already-built JourneysRepository (JS8/JS12), same instance the domain
@@ -124,6 +131,7 @@ beforeEach(() => {
});
afterAll(async () => {
await membership.close();
await t.close();
testDb.close();
});
server/tests/unit/nest/settings.service.test.ts
+13 -4
View File
@@ -9,18 +9,21 @@
*/
import { db as testDb } from '../../../src/db/database';
import { Settings } from '../../../src/db/entities/Settings.entity';
import { Trips } from '../../../src/db/entities/Trips.entity';
import { SettingsService } from '../../../src/nest/settings/settings.service';
import { TripMembershipModule } from '../../../src/nest/trip-membership/trip-membership.module';
import { TripMembershipService } from '../../../src/nest/trip-membership/trip-membership.service';
import { addTripMember, createTrip, createUser } from '../../helpers/factories';
import { findRows } from '../../helpers/factories/rows';
import { deleteRows, findRows } from '../../helpers/factories/rows';
import { readUserSetting, setAppSetting, setUserSetting } from '../../helpers/factories/settings';
import { resetTestDb } from '../../helpers/test-db';
import { createTestModule, type TestModule } from '../../helpers/test-module';
import type { TestOrm } from '../../helpers/test-orm';
import {
sharedTestOrm,
createTestUnitOfWork,
createTestAppSettingsRepo,
createTestSettingsRepo,
createTestTripsRepo,
} from '../../helpers/test-uow';
import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest';
@@ -49,6 +52,7 @@ vi.mock('../../../src/nest/common/crypto/apiKeyCrypto', () => ({
let svc: SettingsService;
let t: TestOrm;
let membership: TestModule;
// `t`, `uow` and the two repositories all derive from the SAME `sharedTestOrm(testDb)`
// (task-2-review.md I2): a second, independent `createTestOrm(testDb)` here would give
@@ -56,13 +60,17 @@ let t: TestOrm;
// opens its transaction on — a repository write inside setAdminUserDefaults/
// bulkUpsertSettings's transaction would then run outside it (or contend on a second
// mutex over the same connection), exactly the trap the shared-ORM helpers exist to avoid.
// TripMembershipService, which lists the trips the planner view row (#2639) may keep, comes
// out of its own module instead of a constructor call here. Its container brings an ORM of
// its own, which is safe for it: that service only reads.
beforeAll(async () => {
t = await sharedTestOrm(testDb);
membership = await createTestModule({ db: testDb, imports: [TripMembershipModule] });
svc = new SettingsService(
await createTestUnitOfWork(testDb),
await createTestAppSettingsRepo(testDb),
await createTestSettingsRepo(testDb),
await createTestTripsRepo(testDb),
membership.get(TripMembershipService),
);
});
@@ -72,6 +80,7 @@ beforeEach(() => {
});
afterAll(async () => {
await membership.close();
await t.close();
testDb.close();
});
@@ -435,7 +444,7 @@ describe('planner_trip_views', () => {
const own = createTrip(testDb, user.id);
const foreign = createTrip(testDb, other.id);
const gone = createTrip(testDb, user.id);
testDb.prepare('DELETE FROM trips WHERE id = ?').run(gone.id);
await deleteRows(t, Trips, { id: gone.id });
await svc.upsertSetting(user.id, 'planner_trip_views', {
[own.id]: { roadtrip: true },
[foreign.id]: { roadtrip: true },
server/tests/unit/nest/share.service.test.ts
+9 -2
View File
@@ -23,6 +23,8 @@ import type { PlacePhotoCacheService } from '../../../src/nest/place-photos/plac
import { QueryHelpersService } from '../../../src/nest/query-helpers/query-helpers.service';
import { SettingsService } from '../../../src/nest/settings/settings.service';
import { ShareService, publicReservationMetadata } from '../../../src/nest/share/share.service';
import { TripMembershipModule } from '../../../src/nest/trip-membership/trip-membership.module';
import { TripMembershipService } from '../../../src/nest/trip-membership/trip-membership.service';
import type { User } from '../../../src/types';
import {
createUser,
@@ -39,13 +41,13 @@ import { deleteRows, findRow, insertRow, updateRows } from '../../helpers/factor
import { setAppSetting, setUserSetting } from '../../helpers/factories/settings';
import { shareServiceRepoArgs } from '../../helpers/share-repos';
import { resetTestDb } from '../../helpers/test-db';
import { createTestModule, type TestModule } from '../../helpers/test-module';
import type { TestOrm } from '../../helpers/test-orm';
import {
sharedTestOrm,
createTestUnitOfWork,
createTestAppSettingsRepo,
createTestSettingsRepo,
createTestTripsRepo,
createTestTagsRepo,
createTestPlaceRatingsRepo,
createTestAssignmentParticipantsRepo,
@@ -72,18 +74,22 @@ const photoCacheStub = { serveKey } as unknown as PlacePhotoCacheService;
let svc: ShareService;
let t: TestOrm;
let membership: TestModule;
// `t`, `uow` and the settings repositories all derive from the SAME
// `sharedTestOrm(testDb)` (task-2-review.md I2) — see settings.service.test.ts's
// own comment on this pattern.
beforeAll(async () => {
t = await sharedTestOrm(testDb);
// SettingsService reads the trips a planner view may keep (#2639) through
// TripMembershipService, taken from its own module.
membership = await createTestModule({ db: testDb, imports: [TripMembershipModule] });
svc = new ShareService(
new SettingsService(
await createTestUnitOfWork(testDb),
await createTestAppSettingsRepo(testDb),
await createTestSettingsRepo(testDb),
await createTestTripsRepo(testDb),
membership.get(TripMembershipService),
),
permissionsStub,
new QueryHelpersService(
@@ -105,6 +111,7 @@ beforeEach(() => {
});
afterAll(async () => {
await membership.close();
await t.close();
testDb.close();
});
server/tests/unit/nest/trips.service.test.ts
+10 -2
View File
@@ -146,6 +146,9 @@ import { Trips } from '../../../src/db/entities/Trips.entity';
import { Users } from '../../../src/db/entities/Users.entity';
import { legacyBoundIntegerText } from '../../../src/nest/common/row-id';
import { TripAccessService } from '../../../src/nest/trip-membership/trip-access.service';
import { TripMembershipModule } from '../../../src/nest/trip-membership/trip-membership.module';
import { TripMembershipService } from '../../../src/nest/trip-membership/trip-membership.service';
import { createTestModule, type TestModule } from '../../helpers/test-module';
import { FakeRealtimeService } from '../../helpers/fake-realtime';
const realtime = new FakeRealtimeService();
@@ -226,6 +229,7 @@ let placesSvc: PlacesService;
let svc: TripsService;
let membersSvc: TripMembersService;
let readModelSvc: TripReadModelService;
let membership: TestModule;
beforeAll(async () => {
dbsEm = (await sharedTestOrm(testDb)).em;
photoCache = new PlacePhotoCacheService(
@@ -277,6 +281,9 @@ beforeAll(async () => {
await createTestCollectionPlacesRepo(testDb),
buildPlaceImportService(),
);
// SettingsService reads the trips a planner view may keep (#2639) through TripMembershipService,
// which comes out of its own module instead of a constructor call here.
membership = await createTestModule({ db: testDb, imports: [TripMembershipModule] });
svc = new TripsService(
new ReservationsService(new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), budgetSvc, realtime, notificationsStub(), new ReservationsReadService(await createTestReservationsRepo(testDb), await createTestReservationEndpointsRepo(testDb), await createTestReservationTravelersRepo(testDb), await createTestReservationDayPositionsRepo(testDb)), accommodationsSvc, await createTestUnitOfWork(testDb), await createTestReservationsRepo(testDb), await createTestReservationEndpointsRepo(testDb), await createTestReservationTravelersRepo(testDb), await createTestReservationDayPositionsRepo(testDb), await createTestDayAccommodationsRepo(testDb), await createTestDaysRepo(testDb), await createTestPlacesRepo(testDb), await createTestDayAssignmentsRepo(testDb), await createTestTripMembersRepo(testDb), await createTestUsersRepo(testDb), await createTestTripsRepo(testDb), await createTestBudgetItemsRepo(testDb)),
daysSvc,
@@ -296,7 +303,7 @@ beforeAll(async () => {
coversFx.storage,
await createTestUnitOfWork(testDb),
(await sharedTestOrm(testDb)).em,
new SettingsService(await createTestUnitOfWork(testDb), await createTestAppSettingsRepo(testDb), await createTestSettingsRepo(testDb), await createTestTripsRepo(testDb)),
new SettingsService(await createTestUnitOfWork(testDb), await createTestAppSettingsRepo(testDb), await createTestSettingsRepo(testDb), membership.get(TripMembershipService)),
);
membersSvc = new TripMembersService(budgetSvc, new UserCleanupService(new MaintenanceRepository(dbsEm!), budgetSvc, await createTestUnitOfWork(testDb), await createTestUsersRepo(testDb), await createTestTripMembersRepo(testDb), await createTestBudgetItemsRepo(testDb), await createTestBudgetSettlementsRepo(testDb), await createTestJourneyShareTokensRepo(testDb), await createTestJourneysRepo(testDb), await createTestJourneyEntriesRepo(testDb), await createTestJourneyContributorsRepo(testDb), await createTestShareTokensRepo(testDb), await createTestPluginsRepo(testDb), await createTestPluginUserErasureQueueRepo(testDb)), new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), realtime, notificationsStub(), await createTestUnitOfWork(testDb), await createTestTripsRepo(testDb), await createTestTripMembersRepo(testDb), await createTestUsersRepo(testDb));
readModelSvc = new TripReadModelService(
@@ -347,7 +354,8 @@ beforeEach(() => {
resetTestDb(testDb);
});
afterAll(() => {
afterAll(async () => {
await membership.close();
testDb.close();
});