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

refactor(server): plan 4 — facade inline sweep part B (rosterUserIds/getPlaceWithTags)

Browse Source 
Inline DatabaseService.rosterUserIds onto TripMembersRepository.rosterUserIds
directly in budget.service.ts, budget.mcp.ts and todo.service.ts, and
DatabaseService.getPlaceWithTags onto PlacesRepository.findWithTagsAndRatings
in accommodations.service.ts (placesRepo was already injected there).

BudgetService/BudgetMcp/TodoService collapse their constructors, dropping
the now-fully-unused DatabaseService param (budget.module.ts/todo.module.ts
register TripMembers on forFeature). PackingService keeps DatabaseService as
a vestigial param behind an optional TripMembersRepository with a fallback
in tripRosterIds: an in-flight Task 5c file
(tests/unit/services/conflictUpdate.test.ts) hand-constructs PackingService
positionally without the new repository, so its arity cannot change this
task; the fallback keeps that file's DatabaseService.rosterUserIds spy
working until Task 4 drops the param with the class itself.
This commit is contained in:
jubnl
2026-09-24 06:18:18 +02:00
parent 1853156da3
commit 8f2fe9d327
8 changed files with 60 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/src/nest/accommodations/accommodations.service.ts
+6 -12
View File
@@ -2,7 +2,6 @@ import { Injectable } from '@nestjs/common';
import { InjectRepository } from '@mikro-orm/nestjs';
import type { RoadtripVia, TrekWsPayload, TrekWsTripEventName } from '@trek/shared';
import { RealtimeService } from '../realtime/realtime.service';
import { DatabaseService, type PlaceWithTags, type TripAccess } from '../database/database.service';
import { UnitOfWork } from '../database/unit-of-work';
import { PermissionsService } from '../permissions/permissions.service';
import { AssignmentsService } from '../assignments/assignments.service';
@@ -12,11 +11,11 @@ import type { DayAccommodationsRepository, DayAccommodationRow } from '../../db/
import { DayAssignments } from '../../db/entities/DayAssignments.entity';
import type { DayAssignmentsRepository } from '../../db/repositories/DayAssignments.repository';
import { Places } from '../../db/entities/Places.entity';
import type { PlacesRepository } from '../../db/repositories/Places.repository';
import type { PlacesRepository, PlaceWithTagsRow as PlaceWithTags } from '../../db/repositories/Places.repository';
import { Days } from '../../db/entities/Days.entity';
import type { DaysRepository } from '../../db/repositories/Days.repository';
import { Trips } from '../../db/entities/Trips.entity';
import type { TripsRepository } from '../../db/repositories/Trips.repository';
import type { TripsRepository, TripAccess } from '../../db/repositories/Trips.repository';
import { RoadtripVias } from '../../db/entities/RoadtripVias.entity';
import type { RoadtripViasRepository } from '../../db/repositories/RoadtripVias.repository';
import { Reservations } from '../../db/entities/Reservations.entity';
@@ -111,9 +110,6 @@ export interface CreateAccommodationData {
@Injectable()
export class AccommodationsService {
constructor(
// `dbs` stays: `stampLodging` still reaches `getPlaceWithTags`
// (Task 3/4's own delegate, not this task's).
private readonly dbs: DatabaseService,
private readonly permissions: PermissionsService,
private readonly realtime: RealtimeService,
private readonly assignments: AssignmentsService,
@@ -131,10 +127,6 @@ export class AccommodationsService {
@InjectRepository(BudgetItems) private readonly budgetItemsRepo: BudgetItemsRepository,
) {}
private get db() {
return this.dbs;
}
/** Owner or member, returning the trip. Takes a number too: the MCP tools pass
* the parsed id, the REST path the raw param. */
async verifyTripAccess(tripId: string | number, userId: number) {
@@ -385,8 +377,10 @@ export class AccommodationsService {
if (!place || place.stop_type) return null;
// AC10
await this.placesRepo.stampHotel(placeId);
// AC11 — primitive, already-converted (`DatabaseService.getPlaceWithTags` → `PlacesRepository.findWithTagsAndRatings`).
return await this.db.getPlaceWithTags(placeId);
// AC11 — Plan 4 Task 3: `DatabaseService.getPlaceWithTags` inlined onto
// `PlacesRepository.findWithTagsAndRatings` directly (`placesRepo` was
// already injected below for other reads).
return await this.placesRepo.findWithTagsAndRatings(placeId);
}
/**
server/src/nest/budget/budget.mcp.ts
+7 -4
View File
@@ -11,7 +11,6 @@ import { DemoService } from '../common/demo.service';
import { ADDON_IDS } from '../../addons';
import { noAccess, permissionDenied } from '../../mcp/tools/_shared';
import { TripMembershipService } from '../trip-membership/trip-membership.service';
import { DatabaseService } from '../database/database.service';
import { BudgetService } from './budget.service';
import { ExchangeRatesService } from './exchange-rates.service';
import { addonGate } from '../addons/addon-gate';
@@ -22,6 +21,8 @@ import { Places } from '../../db/entities/Places.entity';
import type { PlacesRepository } from '../../db/repositories/Places.repository';
import { Trips } from '../../db/entities/Trips.entity';
import type { TripsRepository } from '../../db/repositories/Trips.repository';
import { TripMembers } from '../../db/entities/TripMembers.entity';
import type { TripMembersRepository } from '../../db/repositories/TripMembers.repository';
/** Legacy registrar gate: the whole budget surface rides the budget addon. */
const budgetAddonOn = addonGate(ADDON_IDS.BUDGET);
@@ -85,7 +86,6 @@ export class BudgetMcp {
constructor(
private readonly budget: BudgetService,
private readonly exchangeRates: ExchangeRatesService,
private readonly db: DatabaseService,
private readonly env: RuntimeEnvService,
private readonly membership: TripMembershipService,
readonly addons: AddonsService,
@@ -94,6 +94,9 @@ export class BudgetMcp {
@InjectRepository(Places) private readonly places: PlacesRepository,
@InjectRepository(Trips) private readonly trips: TripsRepository,
private readonly demo: DemoService,
// Plan 4 Task 3 — DatabaseService.rosterUserIds inlined onto
// TripMembersRepository.rosterUserIds directly.
@InjectRepository(TripMembers) private readonly tripMembers: TripMembersRepository,
) {}
/** Plan 3i Task 3: the AuthService.isDemoUser check via the injected DemoService (common/demo.service.ts), not the free-function demo-write.ts helper. */
@@ -132,7 +135,7 @@ export class BudgetMcp {
// here instead would certify a split against a figure the row never receives.
// Negative payers count with their sign (#2176) — the write path stores them.
if (payers !== undefined) {
const roster = await this.db.rosterUserIds(tripId);
const roster = await this.tripMembers.rosterUserIds(tripId);
return sumCents(payers.filter(p => p.amount !== 0 && roster.has(p.user_id)).map(p => p.amount));
}
if (total_price !== undefined) return toCents(total_price);
@@ -154,7 +157,7 @@ export class BudgetMcp {
totalCents: number,
payers?: { user_id: number; amount: number }[],
): Promise<string | null> {
const roster = await this.db.rosterUserIds(tripId);
const roster = await this.tripMembers.rosterUserIds(tripId);
const strangers = members.filter(m => !roster.has(m.user_id)).map(m => m.user_id);
if (strangers.length > 0) {
return `members contains user IDs that are not on this trip: ${strangers.join(', ')}. Resolve them with list_trip_members.`;
server/src/nest/budget/budget.module.ts
+2 -1
View File
@@ -7,6 +7,7 @@ import { BudgetCategoryOrder } from '../../db/entities/BudgetCategoryOrder.entit
import { Reservations } from '../../db/entities/Reservations.entity';
import { Places } from '../../db/entities/Places.entity';
import { Trips } from '../../db/entities/Trips.entity';
import { TripMembers } from '../../db/entities/TripMembers.entity';
import { Module } from '@nestjs/common';
import { BudgetController } from './budget.controller';
import { BudgetService } from './budget.service';
@@ -34,7 +35,7 @@ import { DemoModule } from '../common/demo.module';
* DemoService, and a hand-built TestingModule never imports AppModule, so
* the @Global broadcast never happens unless this module imports it directly. */
@Module({
imports: [McpSharedModule, PermissionsModule, AppConfigModule, DemoModule, RealtimeModule, PluginGuardsModule, AddonsModule, TripMembershipModule, MikroOrmModule.forFeature([BudgetItems, BudgetItemMembers, BudgetItemPayers, BudgetSettlements, BudgetCategoryOrder, Reservations, Places, Trips])],
imports: [McpSharedModule, PermissionsModule, AppConfigModule, DemoModule, RealtimeModule, PluginGuardsModule, AddonsModule, TripMembershipModule, MikroOrmModule.forFeature([BudgetItems, BudgetItemMembers, BudgetItemPayers, BudgetSettlements, BudgetCategoryOrder, Reservations, Places, Trips, TripMembers])],
controllers: [BudgetController],
providers: [BudgetService, ExchangeRatesService, BudgetMcp, ExchangeRatesRpc, CostsRpc],
// For in-container consumers (CostsRpc, TripsService,
server/src/nest/budget/budget.service.ts
+9 -7
View File
@@ -1,6 +1,5 @@
import { Injectable } from '@nestjs/common';
import { InjectRepository } from '@mikro-orm/nestjs';
import { DatabaseService, type TripAccess } from '../database/database.service';
import type { BudgetParticipantFinal, TrekWsPayload, TrekWsTripEventName } from '@trek/shared';
import { RealtimeService } from '../realtime/realtime.service';
import { PermissionsService } from '../permissions/permissions.service';
@@ -23,7 +22,9 @@ import type { ReservationsRepository } from '../../db/repositories/Reservations.
import { Places } from '../../db/entities/Places.entity';
import type { PlacesRepository } from '../../db/repositories/Places.repository';
import { Trips } from '../../db/entities/Trips.entity';
import type { TripsRepository } from '../../db/repositories/Trips.repository';
import type { TripsRepository, TripAccess } from '../../db/repositories/Trips.repository';
import { TripMembers } from '../../db/entities/TripMembers.entity';
import type { TripMembersRepository } from '../../db/repositories/TripMembers.repository';
type Trip = TripAccess;
@@ -126,7 +127,6 @@ function allocateDisplayCents(cents: number[], factor: number, total = Math.roun
@Injectable()
export class BudgetService {
constructor(
private readonly db: DatabaseService,
private readonly permissions: PermissionsService,
private readonly exchangeRates: ExchangeRatesService,
private readonly realtime: RealtimeService,
@@ -139,13 +139,15 @@ export class BudgetService {
@InjectRepository(Reservations) private readonly reservationsRepo: ReservationsRepository,
@InjectRepository(Places) private readonly placesRepo: PlacesRepository,
@InjectRepository(Trips) private readonly tripsRepo: TripsRepository,
// Plan 4 Task 3 — DatabaseService.rosterUserIds inlined onto
// TripMembersRepository.rosterUserIds directly.
@InjectRepository(TripMembers) private readonly tripMembersRepo: TripMembersRepository,
) {}
async verifyTripAccess(tripId: string | number, userId: number) {
// Plan 4 Task 2 — canAccessTrip's own DatabaseService delegation is
// gone: this reuses the TripsRepository already injected for other
// reads and calls findAccessible. `db` stays injected for
// `rosterUserIds` (Task 3's own).
// reads and calls findAccessible.
return await this.tripsRepo.findAccessible(tripId, userId);
}
@@ -208,7 +210,7 @@ export class BudgetService {
private async rosterMemberIds(tripId: string | number, userIds: number[]): Promise<Set<number>> {
const unique = new Set(userIds);
if (unique.size === 0) return new Set();
const roster = await this.db.rosterUserIds(tripId);
const roster = await this.tripMembersRepo.rosterUserIds(tripId);
return new Set([...unique].filter(id => roster.has(id)));
}
@@ -1204,7 +1206,7 @@ export class BudgetService {
* an id it rejected exists at all.
*/
private async settlementPartiesOnTrip(tripId: string | number, data: { from_user_id: number; to_user_id: number }): Promise<boolean> {
const roster = await this.db.rosterUserIds(tripId);
const roster = await this.tripMembersRepo.rosterUserIds(tripId);
return roster.has(data.from_user_id) && roster.has(data.to_user_id);
}
server/src/nest/packing/packing.module.ts
+2 -1
View File
@@ -21,6 +21,7 @@ import { PackingTemplates } from '../../db/entities/PackingTemplates.entity';
import { PackingTemplateCategories } from '../../db/entities/PackingTemplateCategories.entity';
import { PackingTemplateItems } from '../../db/entities/PackingTemplateItems.entity';
import { Trips } from '../../db/entities/Trips.entity';
import { TripMembers } from '../../db/entities/TripMembers.entity';
/** Packing domain (S2 — Phase 2 trip sub-domain). Registered in AppModule.
* Exports PackingService for in-container consumers (TripsService bundle,
@@ -31,7 +32,7 @@ import { Trips } from '../../db/entities/Trips.entity';
// `PackingService.notifyTagged`'s PK91 `getTitle` read only, the same
// "registered here only for one read, never the owning module" shape
// `CollabModule`'s own forFeature list documents).
MikroOrmModule.forFeature([PackingItems, PackingItemContributors, PackingBags, PackingCategoryAssignees, PackingTemplates, PackingTemplateCategories, PackingTemplateItems, Trips]),
MikroOrmModule.forFeature([PackingItems, PackingItemContributors, PackingBags, PackingCategoryAssignees, PackingTemplates, PackingTemplateCategories, PackingTemplateItems, Trips, TripMembers]),
McpSharedModule, NotificationsModule, PermissionsModule, AuthModule, RealtimeModule, PluginGuardsModule, AddonsModule, AuditModule],
controllers: [PackingController, AdminPackingTemplatesController],
providers: [PackingService, PackingMcp, PackingRpc],
server/src/nest/packing/packing.service.ts
+20 -3
View File
@@ -25,6 +25,8 @@ import { PackingTemplateItems } from '../../db/entities/PackingTemplateItems.ent
import type { PackingTemplateItemsRepository } from '../../db/repositories/PackingTemplateItems.repository';
import { Trips } from '../../db/entities/Trips.entity';
import type { TripsRepository } from '../../db/repositories/Trips.repository';
import { TripMembers } from '../../db/entities/TripMembers.entity';
import type { TripMembersRepository } from '../../db/repositories/TripMembers.repository';
/** Privacy fields stamped on a packing item (#858). */
type PrivacyFields = { is_private?: number; owner_id?: number | null };
@@ -95,13 +97,22 @@ export class PackingService {
@InjectRepository(PackingTemplateCategories) private readonly templateCategoriesRepo: PackingTemplateCategoriesRepository,
@InjectRepository(PackingTemplateItems) private readonly templateItemsRepo: PackingTemplateItemsRepository,
@InjectRepository(Trips) private readonly tripsRepo: TripsRepository,
// Plan 4 Task 3: `DatabaseService.rosterUserIds` inlined onto
// `TripMembersRepository.rosterUserIds` — optional (not a trailing
// required param) because an in-flight Task 5c file
// (`tests/unit/services/conflictUpdate.test.ts`, out of this task's
// file-ownership window) hand-constructs `PackingService` positionally
// without it; every real (DI-built) instance gets it via `forFeature`.
// `tripRosterIds` below falls back to the `db` facade only when absent.
// Safe to make required, and drop the fallback + `db`, once that file lands.
@InjectRepository(TripMembers) private readonly tripMembersRepo?: TripMembersRepository,
) {}
async verifyTripAccess(tripId: string | number, userId: number) {
// Plan 4 Task 2 — canAccessTrip's own DatabaseService delegation is
// gone: this reuses the TripsRepository already injected for other
// reads and calls findAccessible. `db` stays injected for
// `rosterUserIds` (Task 3's own).
// reads and calls findAccessible. `db` stays injected only for the
// `tripRosterIds` fallback below (Task 3's own).
return await this.tripsRepo.findAccessible(tripId, userId);
}
@@ -561,7 +572,13 @@ export class PackingService {
* grew up without it.
*/
private async tripRosterIds(tripId: string | number): Promise<Set<number>> {
return await this.db.rosterUserIds(tripId);
// Plan 4 Task 3: `TripMembersRepository.rosterUserIds`, inlined, when a
// real (DI-built) instance has it — falls back to the `DatabaseService`
// facade only for the one hand-built test instance that still omits it
// (see the constructor comment above).
return this.tripMembersRepo
? await this.tripMembersRepo.rosterUserIds(tripId)
: await this.db.rosterUserIds(tripId);
}
async setBagMembers(tripId: string | number, bagId: string | number, userIds: number[]) {
server/src/nest/todo/todo.module.ts
+2 -1
View File
@@ -13,6 +13,7 @@ import { McpSharedModule } from '../mcp-shared/mcp-shared.module';
import { TodoItems } from '../../db/entities/TodoItems.entity';
import { TodoCategoryAssignees } from '../../db/entities/TodoCategoryAssignees.entity';
import { Trips } from '../../db/entities/Trips.entity';
import { TripMembers } from '../../db/entities/TripMembers.entity';
/** To-do domain (S3 — Phase 2 trip sub-domain). Registered in AppModule.
* Exports TodoService for in-container consumers (TripsService bundle).
@@ -23,7 +24,7 @@ import { Trips } from '../../db/entities/Trips.entity';
* (Plan 4 Task 2): `verifyTripAccess`'s own canAccessTrip delegate, now
* TripsRepository directly. */
@Module({
imports: [MikroOrmModule.forFeature([TodoItems, TodoCategoryAssignees, Trips]), McpSharedModule, PermissionsModule, AuthModule, RealtimeModule, PluginGuardsModule, AddonsModule],
imports: [MikroOrmModule.forFeature([TodoItems, TodoCategoryAssignees, Trips, TripMembers]), McpSharedModule, PermissionsModule, AuthModule, RealtimeModule, PluginGuardsModule, AddonsModule],
controllers: [TodoController],
providers: [TodoService, TodoMcp, TodoRpc],
exports: [TodoService],
server/src/nest/todo/todo.service.ts
+12 -9
View File
@@ -4,14 +4,15 @@ import type { TrekWsPayload, TrekWsTripEventName } from '@trek/shared';
import { RealtimeService } from '../realtime/realtime.service';
import { PermissionsService } from '../permissions/permissions.service';
import type { User } from '../../types';
import { DatabaseService, type TripAccess } from '../database/database.service';
import { UnitOfWork } from '../database/unit-of-work';
import { TodoItems } from '../../db/entities/TodoItems.entity';
import type { TodoItemsRepository } from '../../db/repositories/TodoItems.repository';
import { TodoCategoryAssignees } from '../../db/entities/TodoCategoryAssignees.entity';
import type { TodoCategoryAssigneesRepository } from '../../db/repositories/TodoCategoryAssignees.repository';
import { Trips } from '../../db/entities/Trips.entity';
import type { TripsRepository } from '../../db/repositories/Trips.repository';
import type { TripsRepository, TripAccess } from '../../db/repositories/Trips.repository';
import { TripMembers } from '../../db/entities/TripMembers.entity';
import type { TripMembersRepository } from '../../db/repositories/TripMembers.repository';
type Trip = TripAccess;
@@ -21,25 +22,27 @@ type Trip = TripAccess;
* moved 1:1 off the legacy raw statements: identical column sets, the `||`
* falsy-coercion defaults, the bodyKeys sentinel protocol on update and the
* post-write re-selects). Trip access, the 'packing_edit' permission
* (shared with packing), the roster-filter on category assignees (kept
* unconverted on `DatabaseService.rosterUserIds`, the `BudgetService`
* precedent) and the WebSocket broadcast keep their legacy call paths.
* (shared with packing), the roster-filter on category assignees (Plan 4
* Task 3: `TripMembersRepository.rosterUserIds`, injected directly, not
* through `DatabaseService`) and the WebSocket broadcast keep their legacy
* call paths.
* Non-Nest consumers (plugin RPC host, the legacy MCP trips registrar) go
* through todo.bridge.ts instead of importing this class directly.
*/
@Injectable()
export class TodoService {
constructor(
private readonly db: DatabaseService,
private readonly permissions: PermissionsService,
private readonly realtime: RealtimeService,
private readonly uow: UnitOfWork,
@InjectRepository(TodoItems) private readonly todoItemsRepo: TodoItemsRepository,
@InjectRepository(TodoCategoryAssignees) private readonly todoCategoryAssigneesRepo: TodoCategoryAssigneesRepository,
// Plan 4 Task 2 — canAccessTrip's own DatabaseService delegation is
// gone: this injects TripsRepository directly. `db` stays injected for
// `rosterUserIds` (Task 3's own).
// gone: this injects TripsRepository directly.
@InjectRepository(Trips) private readonly tripsRepo: TripsRepository,
// Plan 4 Task 3 — DatabaseService.rosterUserIds inlined onto
// TripMembersRepository.rosterUserIds directly.
@InjectRepository(TripMembers) private readonly tripMembersRepo: TripMembersRepository,
) {}
async verifyTripAccess(tripId: string | number, userId: number) {
@@ -132,7 +135,7 @@ export class TodoService {
// members and reservations filter travellers. Dropped rather than
// rejected: a copied trip carries assignee ids across before its members
// exist, and a 400 would make the picker unusable there.
const roster = await this.db.rosterUserIds(tripId);
const roster = await this.tripMembersRepo.rosterUserIds(tripId);
for (const uid of userIds) if (roster.has(uid)) await this.todoCategoryAssigneesRepo.insertIgnore(tripId, categoryName, uid);
}
});