mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
refactor(server): delete assignments.bridge via an assignments-domain module split
This commit is contained in:
@@ -0,0 +1,22 @@
|
||||
import { Module } from '@nestjs/common';
|
||||
import { JourneyDomainModule } from '../journey/journey-domain.module';
|
||||
import { PermissionsModule } from '../permissions/permissions.module';
|
||||
import { QueryHelpersModule } from '../query-helpers/query-helpers.module';
|
||||
import { RealtimeModule } from '../realtime/realtime.module';
|
||||
import { AssignmentsService } from './assignments.service';
|
||||
|
||||
/**
|
||||
* The assignments SERVICE, split from the controller/MCP/RPC surfaces (the
|
||||
* journey-domain precedent). AssignmentsService itself never touches
|
||||
* DaysService — only AssignmentsMcp does — so the service can live in a module
|
||||
* that stays off the DaysModule → PlacesModule → AssignmentsModule loop.
|
||||
* That is what lets PlacesModule import this and places.mcp.ts inject the
|
||||
* service, which retired assignments.bridge. Everything here is a leaf:
|
||||
* none of the four imports reaches the days or places domains.
|
||||
*/
|
||||
@Module({
|
||||
imports: [PermissionsModule, QueryHelpersModule, JourneyDomainModule, RealtimeModule],
|
||||
providers: [AssignmentsService],
|
||||
exports: [AssignmentsService],
|
||||
})
|
||||
export class AssignmentsDomainModule {}
|
||||
@@ -1,44 +0,0 @@
|
||||
import { db } from '../../db/database';
|
||||
import { DatabaseService } from '../database/database.service';
|
||||
import { RealtimeService } from '../realtime/realtime.service';
|
||||
import { AssignmentsService } from './assignments.service';
|
||||
import { PermissionsService } from '../permissions/permissions.service';
|
||||
import { QueryHelpersService } from '../query-helpers/query-helpers.service';
|
||||
import { JourneyDomainService } from '../journey/journey-domain.service';
|
||||
import { TrekPhotosRepository } from '../photos/trek-photos.repository';
|
||||
|
||||
/**
|
||||
* Verified-permanent cycle-dodge for the assignments domain. The one consumer
|
||||
* is places.mcp.ts, and the cycle is real: DaysModule → PlacesModule →
|
||||
* AssignmentsModule → DaysModule, so PlacesModule cannot import
|
||||
* AssignmentsModule back (see the note in places.mcp.ts). Everything else —
|
||||
* the DI-discovered assignments.mcp.ts, the plugin RPC surface, ItineraryRpc,
|
||||
* the controllers — injects AssignmentsService. Only the two exports
|
||||
* places.mcp.ts consumes survive; this file dies if that module cycle is ever
|
||||
* broken (e.g. an assignments read-model split), not before.
|
||||
*
|
||||
* The instance is built lazily on first call, NOT at module scope: the
|
||||
* consumer is an in-container module evaluated during container assembly, so
|
||||
* a module-level `new AssignmentsService` would crash with an uninitialized
|
||||
* class binding if an import edge ever forms a cycle through here. By the
|
||||
* time any bridge function is called, all modules have finished evaluating.
|
||||
* (`db` is the reinitialize-proof Proxy onto the shared better-sqlite3
|
||||
* singleton.)
|
||||
*/
|
||||
function journeyDomain(): JourneyDomainService {
|
||||
const dbs = new DatabaseService(db);
|
||||
return new JourneyDomainService(dbs, new RealtimeService(), new TrekPhotosRepository(dbs));
|
||||
}
|
||||
|
||||
let instance: AssignmentsService | undefined;
|
||||
function assignments(): AssignmentsService {
|
||||
return (instance ??= new AssignmentsService(new DatabaseService(db), new PermissionsService(new DatabaseService(db)), new RealtimeService(), new QueryHelpersService(new DatabaseService(db)), journeyDomain()));
|
||||
}
|
||||
|
||||
export function createAssignment(dayId: string | number, placeId: string | number, notes: string | null) {
|
||||
return assignments().createAssignment(dayId, placeId, notes);
|
||||
}
|
||||
|
||||
export function dayExists(dayId: string | number, tripId: string | number) {
|
||||
return assignments().dayExists(dayId, tripId);
|
||||
}
|
||||
@@ -1,10 +1,8 @@
|
||||
import { Module } from '@nestjs/common';
|
||||
import { JourneyDomainModule } from '../journey/journey-domain.module';
|
||||
import { DaysModule } from '../days/days.module';
|
||||
import { PermissionsModule } from '../permissions/permissions.module';
|
||||
import { QueryHelpersModule } from '../query-helpers/query-helpers.module';
|
||||
import { DaysModule } from '../days/days.module';
|
||||
import { DayAssignmentsController, AssignmentOpsController } from './assignments.controller';
|
||||
import { AssignmentsService } from './assignments.service';
|
||||
import { AssignmentsDomainModule } from './assignments-domain.module';
|
||||
import { ItineraryRpc } from './itinerary.rpc';
|
||||
import { PluginGuardsModule } from '../plugins/host/plugin-guards.module';
|
||||
import { RealtimeModule } from '../realtime/realtime.module';
|
||||
@@ -14,14 +12,17 @@ import { AuthModule } from '../auth/auth.module';
|
||||
/**
|
||||
* Assignments domain (S7 — Phase 2 trip sub-domain). The day-assignments mount
|
||||
* sits under the /api/trips/:tripId/days prefix (S6); the per-assignment ops use
|
||||
* the /api/trips/:tripId/assignments prefix. AssignmentsService is exported for
|
||||
* the plugin RPC surface (ItineraryRpc injects it).
|
||||
* the /api/trips/:tripId/assignments prefix. AssignmentsService lives in
|
||||
* AssignmentsDomainModule (re-exported here for ReservationsModule and the
|
||||
* plugin RPC surface) so PlacesModule can reach it without closing the
|
||||
* DaysModule → PlacesModule → AssignmentsModule loop this module is on.
|
||||
*/
|
||||
@Module({
|
||||
// DaysModule: AssignmentsMcp injects DaysService for the target-day checks.
|
||||
imports: [DaysModule, PermissionsModule, QueryHelpersModule, AuthModule, JourneyDomainModule, RealtimeModule, PluginGuardsModule],
|
||||
// PermissionsModule: the controllers' TripAccessGuard injects PermissionsService.
|
||||
imports: [AssignmentsDomainModule, DaysModule, PermissionsModule, AuthModule, RealtimeModule, PluginGuardsModule],
|
||||
controllers: [DayAssignmentsController, AssignmentOpsController],
|
||||
providers: [AssignmentsService, AssignmentsMcp, ItineraryRpc],
|
||||
exports: [AssignmentsService],
|
||||
providers: [AssignmentsMcp, ItineraryRpc],
|
||||
exports: [AssignmentsDomainModule],
|
||||
})
|
||||
export class AssignmentsModule {}
|
||||
|
||||
@@ -19,9 +19,8 @@ type Trip = TripAccess;
|
||||
* uses). Trip access rides DatabaseService.canAccessTrip; mutations use
|
||||
* 'day_edit'. The batch tag/participant loaders are injected as
|
||||
* QueryHelpersService (shared with the day, share and place services).
|
||||
* Non-Nest consumers (the legacy places/reservations MCP registrars) go
|
||||
* through assignments.bridge.ts; the plugin RPC host and the assignments MCP
|
||||
* tools inject this class.
|
||||
* Every consumer injects this class (assignments.bridge.ts is deleted —
|
||||
* PlacesMcp injects it from AssignmentsDomainModule now).
|
||||
*/
|
||||
@Injectable()
|
||||
export class AssignmentsService {
|
||||
|
||||
@@ -6,7 +6,7 @@ import {
|
||||
} from '@trek/nest-mcp';
|
||||
import { z } from 'zod';
|
||||
import { AuthService } from '../auth/auth.service';
|
||||
import { createAssignment, dayExists } from '../assignments/assignments.bridge';
|
||||
import { AssignmentsService } from '../assignments/assignments.service';
|
||||
import { JourneyDomainService } from '../journey/journey-domain.service';
|
||||
import { safeBroadcast, noAccess, hasTripPermission, permissionDenied } from '../../mcp/tools/_shared';
|
||||
import { DatabaseService } from '../database/database.service';
|
||||
@@ -29,13 +29,11 @@ function parseId(value: string | string[]): number | null {
|
||||
* `search_place` stays in this controller even though it calls MapsService:
|
||||
* its legacy gate is `places:read`, not the geo group that maps.mcp.ts carries.
|
||||
*
|
||||
* The two assignments imports stay on assignments.bridge rather than injecting
|
||||
* AssignmentsService, and this is now the ONLY MCP controller left on that
|
||||
* bridge: AssignmentsModule imports DaysModule, and DaysModule imports
|
||||
* PlacesModule for days.mcp.ts's place creation, so injecting here would close
|
||||
* a DaysModule → PlacesModule → AssignmentsModule → DaysModule cycle. A real
|
||||
* cycle, not the module-load problem the others had — reservations.mcp.ts sits
|
||||
* outside that loop and injects the service now.
|
||||
* AssignmentsService is injected from AssignmentsDomainModule — the split that
|
||||
* retired assignments.bridge. The full AssignmentsModule sits on the
|
||||
* DaysModule → PlacesModule → AssignmentsModule loop (its MCP class injects
|
||||
* DaysService), but the service itself never did, so its own module stays off
|
||||
* the loop and PlacesModule can import it.
|
||||
*/
|
||||
@McpController()
|
||||
export class PlacesMcp {
|
||||
@@ -45,6 +43,7 @@ export class PlacesMcp {
|
||||
private readonly db: DatabaseService,
|
||||
private readonly auth: AuthService,
|
||||
private readonly journey: JourneyDomainService,
|
||||
private readonly assignments: AssignmentsService,
|
||||
) {}
|
||||
|
||||
@Tool({
|
||||
@@ -123,11 +122,11 @@ export class PlacesMcp {
|
||||
if (this.auth.isDemoUser(ctx.userId)) return demoDenied();
|
||||
if (!this.db.canAccessTrip(tripId, ctx.userId)) return noAccess();
|
||||
if (!hasTripPermission('place_edit', tripId, ctx.userId)) return permissionDenied();
|
||||
if (!dayExists(dayId, tripId)) return { content: [{ type: 'text' as const, text: 'Day not found.' }], isError: true };
|
||||
if (!this.assignments.dayExists(dayId, tripId)) return { content: [{ type: 'text' as const, text: 'Day not found.' }], isError: true };
|
||||
try {
|
||||
const result = this.db.transaction(() => {
|
||||
const place = this.places.create(String(tripId), { name, description, lat, lng, address, category_id, google_place_id, google_ftid, osm_id, notes: place_notes, website, phone, price, currency });
|
||||
const assignment = createAssignment(dayId, place.id, assignment_notes ?? null);
|
||||
const assignment = this.assignments.createAssignment(dayId, place.id, assignment_notes ?? null);
|
||||
return { place, assignment };
|
||||
});
|
||||
safeBroadcast(tripId, 'place:created', { place: result.place });
|
||||
|
||||
@@ -7,6 +7,7 @@ import { PluginGuardsModule } from '../plugins/host/plugin-guards.module';
|
||||
import { RealtimeModule } from '../realtime/realtime.module';
|
||||
import { PlacesMcp } from './places.mcp';
|
||||
import { PermissionsModule } from '../permissions/permissions.module';
|
||||
import { AssignmentsDomainModule } from '../assignments/assignments-domain.module';
|
||||
import { AppConfigModule } from '../app-config/app-config.module';
|
||||
import { UnsplashModule } from '../unsplash/unsplash.module';
|
||||
import { PlacePhotosModule } from '../place-photos/place-photos.module';
|
||||
@@ -23,7 +24,7 @@ import { AuthModule } from '../auth/auth.module';
|
||||
* is no places.bridge.ts: nothing outside the container consumes this domain.
|
||||
*/
|
||||
@Module({
|
||||
imports: [PermissionsModule, QueryHelpersModule, MapsModule, AuthModule, AppConfigModule, UnsplashModule, PlacePhotosModule, JourneyDomainModule, RealtimeModule, PluginGuardsModule],
|
||||
imports: [PermissionsModule, QueryHelpersModule, MapsModule, AuthModule, AppConfigModule, UnsplashModule, PlacePhotosModule, JourneyDomainModule, RealtimeModule, PluginGuardsModule, AssignmentsDomainModule],
|
||||
controllers: [PlacesController],
|
||||
providers: [PlacesService, PlacesMcp, PlacesRpc],
|
||||
exports: [PlacesService],
|
||||
|
||||
@@ -144,8 +144,7 @@ export function createMcpTestRegistry(): McpRegistry {
|
||||
// one — against the same test DB, which is what makes the `when:` gates
|
||||
// answer truthfully here instead of against the process-wide singleton.
|
||||
const addonsService = new AddonsService(dbService);
|
||||
// One instance, two consumers: AssignmentsMcp and, since it stopped reaching
|
||||
// through assignments.bridge, ReservationsMcp.
|
||||
// One instance, three consumers: AssignmentsMcp, ReservationsMcp and PlacesMcp.
|
||||
const assignmentsService = new AssignmentsService(dbService, permissionsService, realtimeService, queryHelpersService, journeyDomain);
|
||||
return createTestRegistry(
|
||||
[
|
||||
@@ -169,7 +168,7 @@ export function createMcpTestRegistry(): McpRegistry {
|
||||
new TripPromptsMcp(tripsService, readModelService, packingService, addonsService),
|
||||
new ShareMcp(new ShareService(dbService, new SettingsService(dbService), permissionsService, queryHelpersService, placePhotoCache), authService),
|
||||
new MapsMcp(mapsService),
|
||||
new PlacesMcp(placesService, mapsService, dbService, authService, journeyDomain),
|
||||
new PlacesMcp(placesService, mapsService, dbService, authService, journeyDomain, assignmentsService),
|
||||
new CollectionsMcp(new CollectionsService(dbService, permissionsService, realtimeService, notificationsStub()), dbService, authService, addonsService),
|
||||
new TransitMcp(new TransitService(), daysService, reservationsService, dbService, authService),
|
||||
new AtlasMcp(new AtlasService(dbService), addonsService, authService),
|
||||
|
||||
@@ -5,8 +5,8 @@
|
||||
* relocated SQL — MAX(order_index)+1 ordering, post-write re-selects, the
|
||||
* time-driven auto-sort with its '99:99' sentinel — plus the hardening that
|
||||
* followed the move: transactional writes, the DB-derived move source day,
|
||||
* empty-string time clearing, the unified compact tag projection, and the
|
||||
* assignments.bridge delegation). Uses a real in-memory SQLite DB so SQL
|
||||
* empty-string time clearing, and the unified compact tag projection; the
|
||||
* bridge-delegation case died with assignments.bridge). Uses a real in-memory SQLite DB so SQL
|
||||
* logic is exercised faithfully.
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest';
|
||||
@@ -51,7 +51,6 @@ import { createUser, createTrip, addTripMember, createDay, createPlace, createDa
|
||||
import { DatabaseService, type TripAccess } from '../../../src/nest/database/database.service';
|
||||
import { PermissionsService } from '../../../src/nest/permissions/permissions.service';
|
||||
import { AssignmentsService } from '../../../src/nest/assignments/assignments.service';
|
||||
import { createAssignment, dayExists } from '../../../src/nest/assignments/assignments.bridge';
|
||||
import { RealtimeService } from '../../../src/nest/realtime/realtime.service';
|
||||
import { QueryHelpersService } from '../../../src/nest/query-helpers/query-helpers.service';
|
||||
import { JourneyDomainService } from '../../../src/nest/journey/journey-domain.service';
|
||||
@@ -398,13 +397,5 @@ describe('setLegTransportMode', () => {
|
||||
});
|
||||
});
|
||||
|
||||
// ── bridge delegation ─────────────────────────────────────────────────────────
|
||||
|
||||
describe('assignments.bridge', () => {
|
||||
it('ASG-SVC-026: bridge exports delegate to the same SQL over the shared connection', () => {
|
||||
const { trip, day, place } = fixture();
|
||||
expect(dayExists(day.id, trip.id)).toBe(true);
|
||||
const created = createAssignment(day.id, place.id, null);
|
||||
expect(created).toMatchObject({ day_id: day.id, place_id: place.id, order_index: 0 });
|
||||
});
|
||||
});
|
||||
// ASG-SVC-026 pinned the deleted assignments.bridge and died with it — the
|
||||
// same paths are covered on the service above and via places.mcp injection.
|
||||
|
||||
Reference in New Issue
Block a user