From 164f5152be02791bd4bb63d34744cc6e8e47c096 Mon Sep 17 00:00:00 2001 From: jubnl Date: Sun, 26 Jul 2026 20:33:31 +0200 Subject: [PATCH] chore: move assignments backend to proper DI and harden its writes --- migrate.md | 41 ++ server/src/mcp/tools.ts | 4 +- server/src/mcp/tools/assignments.ts | 194 --------- server/src/mcp/tools/places.ts | 2 +- server/src/mcp/tools/reservations.ts | 2 +- server/src/nest/README.md | 11 +- .../nest/assignments/assignments.bridge.ts | 45 ++ .../src/nest/assignments/assignments.mcp.ts | 209 ++++++++++ .../nest/assignments/assignments.module.ts | 7 +- .../nest/assignments/assignments.service.ts | 274 +++++++++++-- server/src/nest/plugins/DI-MIGRATION.md | 13 +- .../plugins/host/plugin-host-deps.factory.ts | 28 +- server/src/nest/plugins/plugins.module.ts | 3 +- server/src/services/assignmentService.ts | 229 ----------- server/tests/helpers/mcp-test-controllers.ts | 3 + server/tests/helpers/plugin-host.ts | 2 + .../unit/nest/assignments.service.test.ts | 388 ++++++++++++++++++ .../plugins/plugin-host-deps.factory.test.ts | 17 +- 18 files changed, 974 insertions(+), 498 deletions(-) create mode 100644 migrate.md delete mode 100644 server/src/mcp/tools/assignments.ts create mode 100644 server/src/nest/assignments/assignments.bridge.ts create mode 100644 server/src/nest/assignments/assignments.mcp.ts delete mode 100644 server/src/services/assignmentService.ts create mode 100644 server/tests/unit/nest/assignments.service.test.ts diff --git a/migrate.md b/migrate.md new file mode 100644 index 000000000..824a34c67 --- /dev/null +++ b/migrate.md @@ -0,0 +1,41 @@ +Recommended order + +Wave 1 — pilots (establish the template on easy wins) + +1. tagService (26 lines, zero deps, only consumers: TagsModule + 1 MCP tool) — the ideal first migration. Small enough that the whole pattern fits in one PR: move the SQL into TagsService with injected DatabaseService, retire the + legacy file, convert its tests to overrideProvider(DATABASE_CONNECTION). +2. categoryService (31 lines, same shape) — immediate repeat to confirm the template. +3. todoService (120 lines, classic CRUD with transactions + broadcast) — first "full-shaped" domain: prepared statements, db.transaction, WebSocket broadcast, 3 MCP tools. After this one, the recipe is settled for everything else. + +The pilots deliberately include MCP consumers, because they force the one open design decision early: MCP tools run outside the Nest container, so either the migrated module keeps thin function exports delegating to a shared instance, +or the MCP layer gets a handle to the Nest app (app.get(TodoService)). Better to settle that on a 26-line service than on authService. + +Wave 2 — cross-cutting leverage (parallel to wave 3, high payoff) + +4. permissions (149 lines, zero deps, 17 Nest consumers) → an injectable PermissionsService. Every module wrapper already calls checkPermission; this converts the single most-imported seam in one move. +5. auditLog (171 lines, 13 Nest consumers) → AuditService. +6. tripAccess (9 lines) — don't migrate, delete: it's a wrapper around canAccessTrip, which DatabaseService now exposes. Absorb its 11 call sites opportunistically. + +Wave 3 — domain services with existing modules, low fan-in + +7. dayNoteService (done 2026-07), tripInviteService (done 2026-07), assignmentService (done 2026-07), shareService, settingsService, fileService, collabService, packingService (done 2026-07) — each maps 1:1 to a Nest module that's already a thin delegate; none is imported by more than one + other legacy service. + +Wave 4 — the coupled cluster (order matters here) + +8. dayService → budgetService → reservationService in that order: reservations depend on both (budget-item linking, day resolution), so migrating the dependencies first lets ReservationsService inject the finished versions instead of + importing legacy functions. +9. Then placeService (7 internal deps, mostly on things migrated by now) and tripService (10 deps — the biggest hub; last in this wave, since nearly everything it needs will already be injectable). + +Wave 5 — the heavyweights, last + +10. adminService (14 Nest + 11 MCP consumers), journeyService/atlasService/vacayService/collectionsService/mapsService (big but self-contained), and finally the auth stack (authService at 1494 lines with 18 MCP consumers, plus + oauthService/oidcService/passkeyService) — highest risk, most consumers, security-sensitive; do it when the pattern is thoroughly proven. backupService also stays late: it owns closeDb/reinitialize lifecycle, which is deliberately + outside the DI wrapper. + +Two special cases to keep in mind: airportService is lazily required by db/database.ts at boot (the flight-endpoint backfill) — whenever it migrates, that call should move into Nest's bootstrap instead, or you recreate the circular +import; and notifications/notificationService are imported by 6+ other services, so like the wave-4 cluster they should migrate before their dependents or keep function shims in the interim. + +The through-line: each finished migration lets that domain's tests drop vi.mock('db/database') for overrideProvider(DATABASE_CONNECTION), so the mock seam shrinks wave by wave and disappears with the auth stack. Want me to start on +tagService as the pilot? + diff --git a/server/src/mcp/tools.ts b/server/src/mcp/tools.ts index 191689587..bdbe2a72a 100644 --- a/server/src/mcp/tools.ts +++ b/server/src/mcp/tools.ts @@ -1,5 +1,4 @@ import { McpServer } from '@modelcontextprotocol/sdk/server/mcp'; -import { registerAssignmentTools } from './tools/assignments'; import { registerJourneyTools } from './tools/journey'; import { registerReservationTools } from './tools/reservations'; import { registerMapsWeatherTools } from './tools/mapsWeather'; @@ -33,7 +32,8 @@ export function registerTools(server: McpServer, userId: number, scopes: string[ registerDayTools(server, userId, scopes); - registerAssignmentTools(server, userId, scopes); + // The assignment tools moved to the DI-discovered src/nest/assignments/ + // assignments.mcp.ts (@McpController, attached via the nest-mcp registry below). registerMapsWeatherTools(server, userId, scopes); diff --git a/server/src/mcp/tools/assignments.ts b/server/src/mcp/tools/assignments.ts deleted file mode 100644 index ea1a764c5..000000000 --- a/server/src/mcp/tools/assignments.ts +++ /dev/null @@ -1,194 +0,0 @@ -import { McpServer } from '@modelcontextprotocol/sdk/server/mcp'; -import { z } from 'zod'; -import { canAccessTrip } from '../../db/database'; -import { isDemoUser } from '../../services/authService'; -import { - dayExists, placeExists, createAssignment, assignmentExistsInDay, - deleteAssignment, reorderAssignments, getAssignmentForTrip, updateTime, - moveAssignment, - getParticipants as getAssignmentParticipants, - setParticipants as setAssignmentParticipants, -} from '../../services/assignmentService'; -import { getDay } from '../../services/dayService'; -import { reconcileTripSkeletons } from '../../services/journeyService'; -import { - safeBroadcast, TOOL_ANNOTATIONS_READONLY, TOOL_ANNOTATIONS_WRITE, TOOL_ANNOTATIONS_DELETE, - TOOL_ANNOTATIONS_NON_IDEMPOTENT, - demoDenied, noAccess, ok, hasTripPermission, permissionDenied, -} from './_shared'; -import { canRead, canWrite } from '../scopes'; - -export function registerAssignmentTools(server: McpServer, userId: number, scopes: string[] | null): void { - const R = canRead(scopes, 'places'); - const W = canWrite(scopes, 'places'); - - // --- ASSIGNMENTS --- - - if (W) server.registerTool( - 'assign_place_to_day', - { - description: 'Assign a place to a specific day in a trip.', - inputSchema: { - tripId: z.number().int().positive(), - dayId: z.number().int().positive(), - placeId: z.number().int().positive(), - notes: z.string().max(500).optional(), - }, - annotations: TOOL_ANNOTATIONS_NON_IDEMPOTENT, - }, - async ({ tripId, dayId, placeId, notes }) => { - if (isDemoUser(userId)) return demoDenied(); - if (!canAccessTrip(tripId, userId)) return noAccess(); - if (!hasTripPermission('day_edit', tripId, userId)) return permissionDenied(); - if (!dayExists(dayId, tripId)) return { content: [{ type: 'text' as const, text: 'Day not found.' }], isError: true }; - if (!placeExists(placeId, tripId)) return { content: [{ type: 'text' as const, text: 'Place not found.' }], isError: true }; - const assignment = createAssignment(dayId, placeId, notes || null); - safeBroadcast(tripId, 'assignment:created', { assignment }); - try { reconcileTripSkeletons(tripId); } catch { /* non-fatal */ } - return ok({ assignment }); - } - ); - - if (W) server.registerTool( - 'unassign_place', - { - description: 'Remove a place assignment from a day.', - inputSchema: { - tripId: z.number().int().positive(), - dayId: z.number().int().positive(), - assignmentId: z.number().int().positive(), - }, - annotations: TOOL_ANNOTATIONS_DELETE, - }, - async ({ tripId, dayId, assignmentId }) => { - if (isDemoUser(userId)) return demoDenied(); - if (!canAccessTrip(tripId, userId)) return noAccess(); - if (!hasTripPermission('day_edit', tripId, userId)) return permissionDenied(); - if (!assignmentExistsInDay(assignmentId, dayId, tripId)) - return { content: [{ type: 'text' as const, text: 'Assignment not found.' }], isError: true }; - deleteAssignment(assignmentId); - safeBroadcast(tripId, 'assignment:deleted', { assignmentId, dayId }); - try { reconcileTripSkeletons(tripId); } catch { /* non-fatal */ } - return ok({ success: true }); - } - ); - - if (W) server.registerTool( - 'update_assignment_time', - { - description: 'Set the start and/or end time for a place assignment on a day (e.g. "09:00", "11:30"). Pass null to clear a time.', - inputSchema: { - tripId: z.number().int().positive(), - assignmentId: z.number().int().positive(), - place_time: z.string().max(50).nullable().optional().describe('Start time (e.g. "09:00"), or null to clear'), - end_time: z.string().max(50).nullable().optional().describe('End time (e.g. "11:00"), or null to clear'), - }, - annotations: TOOL_ANNOTATIONS_WRITE, - }, - async ({ tripId, assignmentId, place_time, end_time }) => { - if (isDemoUser(userId)) return demoDenied(); - if (!canAccessTrip(tripId, userId)) return noAccess(); - if (!hasTripPermission('day_edit', tripId, userId)) return permissionDenied(); - const existing = getAssignmentForTrip(assignmentId, tripId); - if (!existing) return { content: [{ type: 'text' as const, text: 'Assignment not found.' }], isError: true }; - const assignment = updateTime( - assignmentId, - place_time !== undefined ? place_time : (existing as any).assignment_time, - end_time !== undefined ? end_time : (existing as any).assignment_end_time - ); - safeBroadcast(tripId, 'assignment:updated', { assignment }); - try { reconcileTripSkeletons(tripId); } catch { /* non-fatal */ } - return ok({ assignment }); - } - ); - - if (W) server.registerTool( - 'move_assignment', - { - description: 'Move a place assignment to a different day.', - inputSchema: { - tripId: z.number().int().positive(), - assignmentId: z.number().int().positive(), - newDayId: z.number().int().positive(), - oldDayId: z.number().int().positive(), - orderIndex: z.number().int().min(0).optional().default(0), - }, - annotations: TOOL_ANNOTATIONS_WRITE, - }, - async ({ tripId, assignmentId, newDayId, oldDayId, orderIndex }) => { - if (isDemoUser(userId)) return demoDenied(); - if (!canAccessTrip(tripId, userId)) return noAccess(); - if (!hasTripPermission('day_edit', tripId, userId)) return permissionDenied(); - if (!getAssignmentForTrip(assignmentId, tripId)) return { content: [{ type: 'text' as const, text: 'Assignment not found.' }], isError: true }; - if (!getDay(newDayId, tripId)) return { content: [{ type: 'text' as const, text: 'Day not found.' }], isError: true }; - const result = moveAssignment(assignmentId, newDayId, orderIndex ?? 0, oldDayId); - safeBroadcast(tripId, 'assignment:moved', { assignment: result.assignment, oldDayId: result.oldDayId }); - try { reconcileTripSkeletons(tripId); } catch { /* non-fatal */ } - return ok({ assignment: result.assignment }); - } - ); - - if (R) server.registerTool( - 'get_assignment_participants', - { - description: 'Get the list of users participating in a specific place assignment.', - inputSchema: { - tripId: z.number().int().positive(), - assignmentId: z.number().int().positive(), - }, - annotations: TOOL_ANNOTATIONS_READONLY, - }, - async ({ tripId, assignmentId }) => { - if (!canAccessTrip(tripId, userId)) return noAccess(); - if (!getAssignmentForTrip(assignmentId, tripId)) return { content: [{ type: 'text' as const, text: 'Assignment not found.' }], isError: true }; - const participants = getAssignmentParticipants(assignmentId); - return ok({ participants }); - } - ); - - if (W) server.registerTool( - 'set_assignment_participants', - { - description: 'Set the participants for a place assignment (replaces current list).', - inputSchema: { - tripId: z.number().int().positive(), - assignmentId: z.number().int().positive(), - userIds: z.array(z.number().int().positive()).describe('User IDs to set as participants; empty array clears all'), - }, - annotations: TOOL_ANNOTATIONS_WRITE, - }, - async ({ tripId, assignmentId, userIds }) => { - if (isDemoUser(userId)) return demoDenied(); - if (!canAccessTrip(tripId, userId)) return noAccess(); - if (!hasTripPermission('day_edit', tripId, userId)) return permissionDenied(); - if (!getAssignmentForTrip(assignmentId, tripId)) return { content: [{ type: 'text' as const, text: 'Assignment not found.' }], isError: true }; - const participants = setAssignmentParticipants(assignmentId, userIds); - safeBroadcast(tripId, 'assignment:participants', { assignmentId, participants }); - return ok({ participants }); - } - ); - - // --- REORDER --- - - if (W) server.registerTool( - 'reorder_day_assignments', - { - description: 'Reorder places within a day by providing the assignment IDs in the desired order.', - inputSchema: { - tripId: z.number().int().positive(), - dayId: z.number().int().positive(), - assignmentIds: z.array(z.number().int().positive()).min(1).max(200).describe('Assignment IDs in desired display order'), - }, - annotations: TOOL_ANNOTATIONS_WRITE, - }, - async ({ tripId, dayId, assignmentIds }) => { - if (isDemoUser(userId)) return demoDenied(); - if (!canAccessTrip(tripId, userId)) return noAccess(); - if (!hasTripPermission('day_edit', tripId, userId)) return permissionDenied(); - if (!getDay(dayId, tripId)) return { content: [{ type: 'text' as const, text: 'Day not found.' }], isError: true }; - reorderAssignments(dayId, assignmentIds); - safeBroadcast(tripId, 'assignment:reordered', { dayId, assignmentIds }); - return ok({ success: true, dayId, order: assignmentIds }); - } - ); -} diff --git a/server/src/mcp/tools/places.ts b/server/src/mcp/tools/places.ts index 2b2f731c9..f8d2afac1 100644 --- a/server/src/mcp/tools/places.ts +++ b/server/src/mcp/tools/places.ts @@ -3,7 +3,7 @@ import { z } from 'zod'; import { canAccessTrip, db } from '../../db/database'; import { isDemoUser } from '../../services/authService'; import { deletePlacesMany, updatePlacesMany, importGoogleList, importNaverList, listPlaces, createPlace, updatePlace, deletePlace, ratePlace } from '../../services/placeService'; -import { createAssignment, dayExists } from '../../services/assignmentService'; +import { createAssignment, dayExists } from '../../nest/assignments/assignments.bridge'; import { onPlaceDeleted, reconcileTripSkeletons } from '../../services/journeyService'; import { searchPlaces } from '../../services/mapsService'; import { diff --git a/server/src/mcp/tools/reservations.ts b/server/src/mcp/tools/reservations.ts index b78e0c08c..44337c313 100644 --- a/server/src/mcp/tools/reservations.ts +++ b/server/src/mcp/tools/reservations.ts @@ -8,7 +8,7 @@ import { } from '../../services/reservationService'; import { linkBudgetItemToReservation } from '../../services/budgetService'; import { getDay } from '../../services/dayService'; -import { placeExists, getAssignmentForTrip } from '../../services/assignmentService'; +import { placeExists, getAssignmentForTrip } from '../../nest/assignments/assignments.bridge'; import { safeBroadcast, TOOL_ANNOTATIONS_WRITE, TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT, diff --git a/server/src/nest/README.md b/server/src/nest/README.md index 8807cb377..8d4e420e4 100644 --- a/server/src/nest/README.md +++ b/server/src/nest/README.md @@ -27,7 +27,8 @@ mount to Nest and leaves the sibling trip routes (days, places, ...) on Express. categories, tags, notifications, atlas. - **Phase 2 (trip sub-domains):** vacay (addon), packing, todo. - **DI-native services (legacy `src/services/*` deleted):** tags, categories, - todo, packing, day-notes — see the migration recipe below. + todo, packing, day-notes, trip-invite, assignments — see the migration recipe + below. ## Cross-cutting Foundation pieces @@ -106,8 +107,12 @@ migration needing **no bridge at all**: after its three tools + resource moved to `day-notes.mcp.ts` and the plugin host injection, nothing outside the container consumed it); trip-invite followed (the smallest port: no MCP surface, no plugin-host import and no bridge — the SQL folded straight into -`trip-invite.service.ts`). Repeat these steps per -service (next up: assignmentService). This is a **pure relocation** — byte-identical +`trip-invite.service.ts`); assignments followed (a 7-tool `assignments.mcp.ts`, +the plugin-host swap, and a bridge kept only for the two legacy registrars — +places and reservations — that borrow its existence checks; the batch loaders +stay in `services/queryHelpers.ts`, shared with the unmigrated day/share/place +services). Repeat these steps per +service (next up: shareService). This is a **pure relocation** — byte-identical SQL, statuses, bodies, and error strings. The plugin RPC host is **no longer a bridge consumer**: since Option A of `src/nest/plugins/DI-MIGRATION.md` it injects domain services via `PluginHostDepsFactory`, so a migrated domain adds diff --git a/server/src/nest/assignments/assignments.bridge.ts b/server/src/nest/assignments/assignments.bridge.ts new file mode 100644 index 000000000..273e2f21a --- /dev/null +++ b/server/src/nest/assignments/assignments.bridge.ts @@ -0,0 +1,45 @@ +import { db } from '../../db/database'; +import { DatabaseService } from '../database/database.service'; +import { AssignmentsService } from './assignments.service'; + +/** + * Non-Nest entry point for the assignments domain — for code running OUTSIDE + * the Nest container (the legacy places and reservations MCP registrars in + * src/mcp/tools/places.ts and src/mcp/tools/reservations.ts; the assignment + * MCP tools moved to the DI-discovered assignments.mcp.ts, and the plugin RPC + * host injects AssignmentsService via PluginHostDepsFactory). Exports only the + * legacy services/assignmentService names still consumed outside the + * container, 1:1, so repointing a consumer is an import-path-only diff. Inside + * the container, inject AssignmentsService instead. Delete this file when + * those legacy consumers migrate. + * + * Unlike todo/packing, the instance is built lazily on first call, NOT at + * module scope: this bridge sits on the mcp tools fan-out while its service + * has a deep legacy-services import closure, so a module-level + * `new AssignmentsService` would crash with an uninitialized class binding if + * a services→mcp import edge ever forms a cycle through here (one existed via + * adminService importing the ../mcp barrel until that was rerouted to + * mcp/sessionManager). 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.) + */ +let instance: AssignmentsService | undefined; +function assignments(): AssignmentsService { + return (instance ??= new AssignmentsService(new DatabaseService(db))); +} + +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); +} + +export function placeExists(placeId: string | number, tripId: string | number) { + return assignments().placeExists(placeId, tripId); +} + +export function getAssignmentForTrip(id: string | number, tripId: string | number) { + return assignments().getAssignmentForTrip(id, tripId); +} diff --git a/server/src/nest/assignments/assignments.mcp.ts b/server/src/nest/assignments/assignments.mcp.ts new file mode 100644 index 000000000..45bfe33cd --- /dev/null +++ b/server/src/nest/assignments/assignments.mcp.ts @@ -0,0 +1,209 @@ +import { + McpController, Tool, type McpContext, + TOOL_ANNOTATIONS_READONLY, TOOL_ANNOTATIONS_WRITE, + TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT, + demoDenied, errorResult, ok, +} from '@trek/nest-mcp'; +import { z } from 'zod'; +import { isDemoUser } from '../../services/authService'; +import { getDay } from '../../services/dayService'; +import { safeBroadcast, noAccess, hasTripPermission, permissionDenied } from '../../mcp/tools/_shared'; +import { AssignmentsService } from './assignments.service'; + +/** + * Assignment MCP surface — ported 1:1 from the legacy registrar in + * src/mcp/tools/assignments.ts (identical names, descriptions, schemas, + * annotations, error/payload shapes and broadcasts). The legacy `if (R)` / + * `if (W)` registration gates on the 'places' scope group map to the + * declarative access markers (resolved by trekMcpAccessPolicy); there is no + * addon gate, so no `when`. The target-day check on move keeps its legacy + * dayService.getDay call path. + */ +@McpController() +export class AssignmentsMcp { + constructor(private readonly assignments: AssignmentsService) {} + + @Tool({ + name: 'assign_place_to_day', + description: 'Assign a place to a specific day in a trip.', + inputSchema: { + tripId: z.number().int().positive(), + dayId: z.number().int().positive(), + placeId: z.number().int().positive(), + notes: z.string().max(500).optional(), + }, + annotations: TOOL_ANNOTATIONS_NON_IDEMPOTENT, + access: { group: 'places', mode: 'write' }, + }) + async assignPlaceToDay( + { tripId, dayId, placeId, notes }: { tripId: number; dayId: number; placeId: number; notes?: string }, + ctx: McpContext, + ) { + if (isDemoUser(ctx.userId)) return demoDenied(); + if (!this.assignments.verifyTripAccess(tripId, ctx.userId)) return noAccess(); + if (!hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.assignments.dayExists(dayId, tripId)) return errorResult('Day not found.'); + if (!this.assignments.placeExists(placeId, tripId)) return errorResult('Place not found.'); + const assignment = this.assignments.createAssignment(dayId, placeId, notes || null); + safeBroadcast(tripId, 'assignment:created', { assignment }); + this.assignments.reconcile(tripId); + return ok({ assignment }); + } + + @Tool({ + name: 'unassign_place', + description: 'Remove a place assignment from a day.', + inputSchema: { + tripId: z.number().int().positive(), + dayId: z.number().int().positive(), + assignmentId: z.number().int().positive(), + }, + annotations: TOOL_ANNOTATIONS_DELETE, + access: { group: 'places', mode: 'write' }, + }) + async unassignPlace( + { tripId, dayId, assignmentId }: { tripId: number; dayId: number; assignmentId: number }, + ctx: McpContext, + ) { + if (isDemoUser(ctx.userId)) return demoDenied(); + if (!this.assignments.verifyTripAccess(tripId, ctx.userId)) return noAccess(); + if (!hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.assignments.assignmentExistsInDay(assignmentId, dayId, tripId)) + return errorResult('Assignment not found.'); + this.assignments.deleteAssignment(assignmentId); + safeBroadcast(tripId, 'assignment:deleted', { assignmentId, dayId }); + this.assignments.reconcile(tripId); + return ok({ success: true }); + } + + @Tool({ + name: 'update_assignment_time', + description: 'Set the start and/or end time for a place assignment on a day (e.g. "09:00", "11:30"). Pass null to clear a time.', + inputSchema: { + tripId: z.number().int().positive(), + assignmentId: z.number().int().positive(), + place_time: z.string().max(50).nullable().optional().describe('Start time (e.g. "09:00"), or null to clear'), + end_time: z.string().max(50).nullable().optional().describe('End time (e.g. "11:00"), or null to clear'), + }, + annotations: TOOL_ANNOTATIONS_WRITE, + access: { group: 'places', mode: 'write' }, + }) + async updateAssignmentTime( + { tripId, assignmentId, place_time, end_time }: { + tripId: number; assignmentId: number; place_time?: string | null; end_time?: string | null; + }, + ctx: McpContext, + ) { + if (isDemoUser(ctx.userId)) return demoDenied(); + if (!this.assignments.verifyTripAccess(tripId, ctx.userId)) return noAccess(); + if (!hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); + const existing = this.assignments.getAssignmentForTrip(assignmentId, tripId); + if (!existing) return errorResult('Assignment not found.'); + const assignment = this.assignments.updateTime( + assignmentId, + place_time !== undefined ? place_time : existing.assignment_time, + end_time !== undefined ? end_time : existing.assignment_end_time + ); + safeBroadcast(tripId, 'assignment:updated', { assignment }); + this.assignments.reconcile(tripId); + return ok({ assignment }); + } + + @Tool({ + name: 'move_assignment', + description: 'Move a place assignment to a different day.', + inputSchema: { + tripId: z.number().int().positive(), + assignmentId: z.number().int().positive(), + newDayId: z.number().int().positive(), + oldDayId: z.number().int().positive().optional().describe('Deprecated and ignored — the server derives the source day from the assignment'), + orderIndex: z.number().int().min(0).optional().default(0), + }, + annotations: TOOL_ANNOTATIONS_WRITE, + access: { group: 'places', mode: 'write' }, + }) + async moveAssignment( + { tripId, assignmentId, newDayId, orderIndex }: { + tripId: number; assignmentId: number; newDayId: number; oldDayId?: number; orderIndex?: number; + }, + ctx: McpContext, + ) { + if (isDemoUser(ctx.userId)) return demoDenied(); + if (!this.assignments.verifyTripAccess(tripId, ctx.userId)) return noAccess(); + if (!hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.assignments.getAssignmentForTrip(assignmentId, tripId)) return errorResult('Assignment not found.'); + if (!getDay(newDayId, tripId)) return errorResult('Day not found.'); + const result = this.assignments.moveAssignment(assignmentId, newDayId, orderIndex ?? 0); + safeBroadcast(tripId, 'assignment:moved', { assignment: result.assignment, oldDayId: result.oldDayId }); + this.assignments.reconcile(tripId); + return ok({ assignment: result.assignment }); + } + + @Tool({ + name: 'get_assignment_participants', + description: 'Get the list of users participating in a specific place assignment.', + inputSchema: { + tripId: z.number().int().positive(), + assignmentId: z.number().int().positive(), + }, + annotations: TOOL_ANNOTATIONS_READONLY, + access: { group: 'places', mode: 'read' }, + }) + async getAssignmentParticipants( + { tripId, assignmentId }: { tripId: number; assignmentId: number }, + ctx: McpContext, + ) { + if (!this.assignments.verifyTripAccess(tripId, ctx.userId)) return noAccess(); + if (!this.assignments.getAssignmentForTrip(assignmentId, tripId)) return errorResult('Assignment not found.'); + const participants = this.assignments.getParticipants(assignmentId); + return ok({ participants }); + } + + @Tool({ + name: 'set_assignment_participants', + description: 'Set the participants for a place assignment (replaces current list).', + inputSchema: { + tripId: z.number().int().positive(), + assignmentId: z.number().int().positive(), + userIds: z.array(z.number().int().positive()).describe('User IDs to set as participants; empty array clears all'), + }, + annotations: TOOL_ANNOTATIONS_WRITE, + access: { group: 'places', mode: 'write' }, + }) + async setAssignmentParticipants( + { tripId, assignmentId, userIds }: { tripId: number; assignmentId: number; userIds: number[] }, + ctx: McpContext, + ) { + if (isDemoUser(ctx.userId)) return demoDenied(); + if (!this.assignments.verifyTripAccess(tripId, ctx.userId)) return noAccess(); + if (!hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); + if (!this.assignments.getAssignmentForTrip(assignmentId, tripId)) return errorResult('Assignment not found.'); + const participants = this.assignments.setParticipants(assignmentId, userIds); + safeBroadcast(tripId, 'assignment:participants', { assignmentId, participants }); + return ok({ participants }); + } + + @Tool({ + name: 'reorder_day_assignments', + description: 'Reorder places within a day by providing the assignment IDs in the desired order.', + inputSchema: { + tripId: z.number().int().positive(), + dayId: z.number().int().positive(), + assignmentIds: z.array(z.number().int().positive()).min(1).max(200).describe('Assignment IDs in desired display order'), + }, + annotations: TOOL_ANNOTATIONS_WRITE, + access: { group: 'places', mode: 'write' }, + }) + async reorderDayAssignments( + { tripId, dayId, assignmentIds }: { tripId: number; dayId: number; assignmentIds: number[] }, + ctx: McpContext, + ) { + if (isDemoUser(ctx.userId)) return demoDenied(); + if (!this.assignments.verifyTripAccess(tripId, ctx.userId)) return noAccess(); + if (!hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); + if (!getDay(dayId, tripId)) return errorResult('Day not found.'); + this.assignments.reorderAssignments(dayId, assignmentIds); + safeBroadcast(tripId, 'assignment:reordered', { dayId, assignmentIds }); + return ok({ success: true, dayId, order: assignmentIds }); + } +} diff --git a/server/src/nest/assignments/assignments.module.ts b/server/src/nest/assignments/assignments.module.ts index 908a12836..b192a74ed 100644 --- a/server/src/nest/assignments/assignments.module.ts +++ b/server/src/nest/assignments/assignments.module.ts @@ -1,14 +1,17 @@ import { Module } from '@nestjs/common'; import { DayAssignmentsController, AssignmentOpsController } from './assignments.controller'; import { AssignmentsService } from './assignments.service'; +import { AssignmentsMcp } from './assignments.mcp'; /** * 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. + * the /api/trips/:tripId/assignments prefix. AssignmentsService is exported for + * the plugin RPC host (PluginHostDepsFactory injects it). */ @Module({ controllers: [DayAssignmentsController, AssignmentOpsController], - providers: [AssignmentsService], + providers: [AssignmentsService, AssignmentsMcp], + exports: [AssignmentsService], }) export class AssignmentsModule {} diff --git a/server/src/nest/assignments/assignments.service.ts b/server/src/nest/assignments/assignments.service.ts index 9b1070a25..977190695 100644 --- a/server/src/nest/assignments/assignments.service.ts +++ b/server/src/nest/assignments/assignments.service.ts @@ -2,23 +2,30 @@ import { Injectable } from '@nestjs/common'; import { broadcast } from '../../websocket'; import { DatabaseService } from '../database/database.service'; import { checkPermission } from '../../services/permissions'; -import type { User } from '../../types'; -import * as svc from '../../services/assignmentService'; +import { loadTagsByPlaceIds, loadParticipantsByAssignmentIds, formatAssignmentWithPlace } from '../../services/queryHelpers'; +import type { AssignmentRow, DayAssignment, User } from '../../types'; import { reconcileTripSkeletons } from '../../services/journeyService'; type Trip = { user_id: number }; /** - * Thin Nest wrapper around the existing assignment service. Trip access mirrors - * the requireTripAccess middleware (canAccessTrip); mutations use 'day_edit'. - * The SQL, the move/reorder logic and the journey skeleton reconcile reuse the - * legacy code unchanged. + * Assignments domain service — owns the day-assignment SQL (relocated from the + * legacy services/assignmentService.ts, then hardened: every multi-statement + * write runs in a transaction, moveAssignment derives the source day from the + * row instead of trusting the caller, empty-string times clear like null, and + * single-assignment reads embed the same compact tag projection the list path + * uses). Trip access mirrors the requireTripAccess middleware (canAccessTrip); + * mutations use 'day_edit'. The batch tag/participant loaders stay in + * services/queryHelpers (shared with the unmigrated day/share/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. */ @Injectable() export class AssignmentsService { constructor(private readonly dbs: DatabaseService) {} - verifyTripAccess(tripId: string, userId: number) { + verifyTripAccess(tripId: string | number, userId: number) { return this.dbs.canAccessTrip(Number(tripId), userId) as Trip | null | undefined; } @@ -30,64 +37,247 @@ export class AssignmentsService { broadcast(tripId, event, payload, socketId); } - dayExists(dayId: string, tripId: string) { - return svc.dayExists(dayId, tripId); - } - - placeExists(placeId: unknown, tripId: string) { - return svc.placeExists(placeId as never, tripId); - } - - listDayAssignments(dayId: string) { - return svc.listDayAssignments(dayId); - } - - createAssignment(dayId: string, placeId: unknown, notes?: string | null) { - return svc.createAssignment(dayId, placeId as never, notes as never); - } - /** * Re-mirror the trip's day-assigned places onto every linked journey's skeleton * suggestions. Called after any assignment mutation (create/delete/move/time) so * the journey stays in sync. Non-fatal, like the route's try/catch. */ - reconcile(tripId: string, socketId?: string): void { + reconcile(tripId: string | number, socketId?: string): void { try { reconcileTripSkeletons(Number(tripId), socketId); } catch { /* non-fatal */ } } - assignmentExistsInDay(id: string, dayId: string, tripId: string) { - return svc.assignmentExistsInDay(id, dayId, tripId); + private getAssignmentWithPlace(assignmentId: number | bigint) { + const a = this.dbs.get(` + SELECT da.*, p.id as place_id, p.name as place_name, p.description as place_description, + p.lat, p.lng, p.address, p.category_id, p.price, p.currency as place_currency, + COALESCE(da.assignment_time, p.place_time) as place_time, + COALESCE(da.assignment_end_time, p.end_time) as end_time, + p.duration_minutes, p.notes as place_notes, + p.image_url, p.transport_mode, p.google_place_id, p.google_ftid, p.osm_id, p.website, p.phone, + c.name as category_name, c.color as category_color, c.icon as category_icon + FROM day_assignments da + JOIN places p ON da.place_id = p.id + LEFT JOIN categories c ON p.category_id = c.id + WHERE da.id = ? + `, assignmentId); + + if (!a) return null; + + // Same compact tag projection as listDayAssignments, so an assignment has + // one wire shape regardless of which read path produced it. + const tags = loadTagsByPlaceIds([a.place_id], { compact: true })[a.place_id] || []; + + const participants = this.dbs.all(` + SELECT ap.user_id, COALESCE(u.display_name, u.username) AS username, u.avatar + FROM assignment_participants ap + JOIN users u ON ap.user_id = u.id + WHERE ap.assignment_id = ? + `, a.id); + + return { + id: a.id, + day_id: a.day_id, + place_id: a.place_id, + order_index: a.order_index, + notes: a.notes, + assignment_time: a.assignment_time ?? null, + assignment_end_time: a.assignment_end_time ?? null, + leg_transport_mode: a.leg_transport_mode ?? null, + participants, + created_at: a.created_at, + place: { + id: a.place_id, + name: a.place_name, + description: a.place_description, + lat: a.lat, + lng: a.lng, + address: a.address, + category_id: a.category_id, + price: a.price, + currency: a.place_currency, + place_time: a.place_time, + end_time: a.end_time, + duration_minutes: a.duration_minutes, + notes: a.place_notes, + image_url: a.image_url, + transport_mode: a.transport_mode, + google_place_id: a.google_place_id, + google_ftid: a.google_ftid, + osm_id: a.osm_id, + website: a.website, + phone: a.phone, + category: a.category_id ? { + id: a.category_id, + name: a.category_name, + color: a.category_color, + icon: a.category_icon, + } : null, + tags, + } + }; } - deleteAssignment(id: string): void { - svc.deleteAssignment(id); + listDayAssignments(dayId: string | number) { + const assignments = this.dbs.all(` + SELECT da.*, p.id as place_id, p.name as place_name, p.description as place_description, + p.lat, p.lng, p.address, p.category_id, p.price, p.currency as place_currency, + COALESCE(da.assignment_time, p.place_time) as place_time, + COALESCE(da.assignment_end_time, p.end_time) as end_time, + p.duration_minutes, p.notes as place_notes, + p.image_url, p.transport_mode, p.google_place_id, p.google_ftid, p.osm_id, p.website, p.phone, + c.name as category_name, c.color as category_color, c.icon as category_icon + FROM day_assignments da + JOIN places p ON da.place_id = p.id + LEFT JOIN categories c ON p.category_id = c.id + WHERE da.day_id = ? + ORDER BY da.order_index ASC, da.created_at ASC + `, dayId); + + const placeIds = [...new Set(assignments.map(a => a.place_id))]; + const tagsByPlaceId = loadTagsByPlaceIds(placeIds, { compact: true }); + + const assignmentIds = assignments.map(a => a.id); + const participantsByAssignment = loadParticipantsByAssignmentIds(assignmentIds); + + return assignments.map(a => { + return formatAssignmentWithPlace(a, tagsByPlaceId[a.place_id] || [], participantsByAssignment[a.id] || []); + }); } - reorderAssignments(dayId: string, orderedIds: number[]): void { - svc.reorderAssignments(dayId, orderedIds as never); + dayExists(dayId: string | number, tripId: string | number) { + return !!this.dbs.get('SELECT id FROM days WHERE id = ? AND trip_id = ?', dayId, tripId); } - getAssignmentForTrip(id: string, tripId: string) { - return svc.getAssignmentForTrip(id, tripId); + placeExists(placeId: unknown, tripId: string | number) { + return !!this.dbs.get('SELECT id FROM places WHERE id = ? AND trip_id = ?', placeId, tripId); } - moveAssignment(id: string, newDayId: unknown, orderIndex: number | undefined, oldDayId: unknown) { - return svc.moveAssignment(id, newDayId as never, orderIndex as never, oldDayId as never); + createAssignment(dayId: string | number, placeId: unknown, notes?: string | null) { + const result = this.dbs.transaction(() => { + const maxOrder = this.dbs.get<{ max: number | null }>('SELECT MAX(order_index) as max FROM day_assignments WHERE day_id = ?', dayId)!; + const orderIndex = (maxOrder.max !== null ? maxOrder.max : -1) + 1; + + return this.dbs.run( + 'INSERT INTO day_assignments (day_id, place_id, order_index, notes) VALUES (?, ?, ?, ?)', + dayId, placeId, orderIndex, notes || null + ); + }); + + return this.getAssignmentWithPlace(result.lastInsertRowid); } - getParticipants(id: string) { - return svc.getParticipants(id); + assignmentExistsInDay(id: string | number, dayId: string | number, tripId: string | number) { + return !!this.dbs.get( + 'SELECT da.id FROM day_assignments da JOIN days d ON da.day_id = d.id WHERE da.id = ? AND da.day_id = ? AND d.trip_id = ?', + id, dayId, tripId + ); } - updateTime(id: string, placeTime: unknown, endTime: unknown) { - return svc.updateTime(id, placeTime as never, endTime as never); + deleteAssignment(id: string | number): void { + this.dbs.run('DELETE FROM day_assignments WHERE id = ?', id); } - setLegTransportMode(id: string, mode: string | null) { - return svc.setLegTransportMode(id, mode); + reorderAssignments(dayId: string | number, orderedIds: number[]): void { + const update = this.dbs.prepare('UPDATE day_assignments SET order_index = ? WHERE id = ? AND day_id = ?'); + this.dbs.transaction(() => { + orderedIds.forEach((id: number, index: number) => { + update.run(index, id, dayId); + }); + }); } - setParticipants(id: string, userIds: number[]) { - return svc.setParticipants(id, userIds); + getAssignmentForTrip(id: string | number, tripId: string | number) { + return this.dbs.get(` + SELECT da.* FROM day_assignments da + JOIN days d ON da.day_id = d.id + WHERE da.id = ? AND d.trip_id = ? + `, id, tripId); + } + + moveAssignment(id: string | number, newDayId: unknown, orderIndex: number | null | undefined) { + // The source day comes from the row, not the caller — callers can't lie + // about (or race on) where the assignment was. + const oldDayId = this.dbs.transaction(() => { + const row = this.dbs.get<{ day_id: number }>('SELECT day_id FROM day_assignments WHERE id = ?', id); + this.dbs.run('UPDATE day_assignments SET day_id = ?, order_index = ? WHERE id = ?', newDayId, orderIndex ?? 0, id); + return row?.day_id; + }); + const updated = this.getAssignmentWithPlace(Number(id)); + return { assignment: updated, oldDayId }; + } + + getParticipants(assignmentId: string | number) { + return this.dbs.all(` + SELECT ap.user_id, COALESCE(u.display_name, u.username) AS username, u.avatar + FROM assignment_participants ap + JOIN users u ON ap.user_id = u.id + WHERE ap.assignment_id = ? + `, assignmentId); + } + + updateTime(id: string | number, placeTime: unknown, endTime: unknown) { + this.dbs.transaction(() => { + // Falsy times (null, undefined, '') all clear the override — an empty + // string is a clear, not a stored value. + this.dbs.run('UPDATE day_assignments SET assignment_time = ?, assignment_end_time = ? WHERE id = ?', + placeTime || null, endTime || null, id); + + // Auto-sort: reorder timed assignments chronologically within the day + if (placeTime) { + const assignment = this.dbs.get<{ day_id: number }>('SELECT day_id FROM day_assignments WHERE id = ?', id); + if (assignment) { + const dayAssignments = this.dbs.all<{ id: number; effective_time: string | null }>(` + SELECT da.id, COALESCE(da.assignment_time, p.place_time) as effective_time + FROM day_assignments da + JOIN places p ON da.place_id = p.id + WHERE da.day_id = ? + ORDER BY da.order_index ASC + `, assignment.day_id); + + // Separate timed and untimed, sort timed by time + const timed = dayAssignments.filter(a => a.effective_time).sort((a, b) => { + const ta = a.effective_time!.includes(':') ? a.effective_time! : '99:99'; + const tb = b.effective_time!.includes(':') ? b.effective_time! : '99:99'; + return ta.localeCompare(tb); + }); + const untimed = dayAssignments.filter(a => !a.effective_time); + + // Interleave: timed in chronological order, untimed keep relative position + const reordered = [...timed, ...untimed]; + const update = this.dbs.prepare('UPDATE day_assignments SET order_index = ? WHERE id = ?'); + reordered.forEach((a, i) => update.run(i, a.id)); + } + } + }); + + return this.getAssignmentWithPlace(Number(id)); + } + + /** + * Set the travel mode of the leg leaving this stop (#1281). null clears the + * override so the leg falls back to the day's default_transport_mode. This is + * sticky by design: changing the whole-day default never touches a leg that + * carries its own explicit mode. + */ + setLegTransportMode(id: string | number, mode: string | null) { + this.dbs.run('UPDATE day_assignments SET leg_transport_mode = ? WHERE id = ?', mode ?? null, id); + return this.getAssignmentWithPlace(Number(id)); + } + + setParticipants(assignmentId: string | number, userIds: number[]) { + this.dbs.transaction(() => { + this.dbs.run('DELETE FROM assignment_participants WHERE assignment_id = ?', assignmentId); + if (userIds.length > 0) { + const insert = this.dbs.prepare('INSERT OR IGNORE INTO assignment_participants (assignment_id, user_id) VALUES (?, ?)'); + for (const userId of userIds) insert.run(assignmentId, userId); + } + }); + + return this.dbs.all(` + SELECT ap.user_id, COALESCE(u.display_name, u.username) AS username, u.avatar + FROM assignment_participants ap + JOIN users u ON ap.user_id = u.id + WHERE ap.assignment_id = ? + `, assignmentId); } } diff --git a/server/src/nest/plugins/DI-MIGRATION.md b/server/src/nest/plugins/DI-MIGRATION.md index 480b2bcb8..f59e4d2bc 100644 --- a/server/src/nest/plugins/DI-MIGRATION.md +++ b/server/src/nest/plugins/DI-MIGRATION.md @@ -100,7 +100,9 @@ import for one injected service (+ module export/import) — **no more bridge files for the plugin host, ever**. The factory's import list shrinks monotonically as the recipe in `src/nest/README.md` proceeds (packing and day-notes swapped in 2026-07; trip-invite migrated without touching the -factory — it was never imported here; next factory swap: assignmentService). +factory — it was never imported here; assignments swapped in 2026-07; next +factory swap: whichever Wave-3 domain the factory imports migrates next — +shareService/fileService/collabService are all imported here). ### Test impact (as landed) @@ -108,7 +110,8 @@ factory — it was never imported here; next factory swap: assignmentService). `PluginRpcHost` directly with hand-built `HostDeps`). - `tests/unit/plugins/plugin-host-deps.factory.test.ts` (was `create-rpc-host.test.ts`) — the six DI-domain path mocks became constructor - stubs; the ~25 legacy-service path mocks remain; a file-local shim keeps the + stubs (nine stubs as of the 2026-07 assignments migration); the ~25 + legacy-service path mocks remain; a file-local shim keeps the historical `createRealRpcHost(id, granted)` call sites and supplies a default no-op router. - Bridge delegation tests (TAG-SVC-016..020, CAT-SVC-016) — deleted with the @@ -216,9 +219,9 @@ none of it is throwaway. 1. ~~**Now / opportunistically — Option A.**~~ **Done (2026-07).** Injectable factory, tags/categories bridges deleted, wiring test rewritten against stubs (see §2). -2. **Per domain, as the service migrations proceed** (`packingService` and - `dayNoteService` done 2026-07; `tripInviteService` done 2026-07 with no - factory impact — it was never imported here; `assignmentService` next, per +2. **Per domain, as the service migrations proceed** (`packingService`, + `dayNoteService` and `assignmentService` done 2026-07; `tripInviteService` + done 2026-07 with no factory impact — it was never imported here; next per `src/nest/README.md`): migrate the service to DI as usual; the factory swaps one legacy import for one injected service. Optionally pilot `tags.rpc.ts` here — tags is small and diff --git a/server/src/nest/plugins/host/plugin-host-deps.factory.ts b/server/src/nest/plugins/host/plugin-host-deps.factory.ts index 4f1da5bda..7223af094 100644 --- a/server/src/nest/plugins/host/plugin-host-deps.factory.ts +++ b/server/src/nest/plugins/host/plugin-host-deps.factory.ts @@ -21,7 +21,6 @@ import { checkPermission } from '../../../services/permissions'; import { listTrips, updateTrip, createTrip, removeMember as removeTripMemberSvc, NotFoundError, ValidationError } from '../../../services/tripService'; import { createPlace, updatePlace, deletePlace } from '../../../services/placeService'; import { createDay, getDay, updateDay, deleteDay, listDays, listAccommodations, validateAccommodationRefs, createAccommodation as createAccommodationSvc, getAccommodation, updateAccommodation as updateAccommodationSvc, deleteAccommodation as deleteAccommodationSvc } from '../../../services/dayService'; -import { createAssignment, deleteAssignment, dayExists, placeExists, getAssignmentForTrip } from '../../../services/assignmentService'; import { isAddonEnabled } from '../../../services/adminService'; import { isDemoEmail } from '../../../services/demo'; import { ADDON_IDS } from '../../../addons'; @@ -36,6 +35,7 @@ import { CategoriesService } from '../../categories/categories.service'; import { TodoService } from '../../todo/todo.service'; import { PackingService } from '../../packing/packing.service'; import { DayNotesService } from '../../days/day-notes.service'; +import { AssignmentsService } from '../../assignments/assignments.service'; import { notifyBookingChange } from '../../../services/reservationService'; import { PluginRpcHost, ForbiddenResource, BadParams } from './rpc-host'; import { appendAudit } from './plugin-audit'; @@ -150,10 +150,10 @@ export interface PluginCallRouter { * `plugin:{id}:{event}` so a plugin can't forge a core event. * * DI-native domain services (budget, reservations, tags, categories, todo, - * packing, day-notes, oauth) are constructor-injected; legacy `services/*` - * domains stay plain function imports until their own migration lands - * (DI-MIGRATION.md), at which point each swaps one import for one injected - * service. + * packing, day-notes, assignments, oauth) are constructor-injected; legacy + * `services/*` domains stay plain function imports until their own migration + * lands (DI-MIGRATION.md), at which point each swaps one import for one + * injected service. */ @Injectable() export class PluginHostDepsFactory { @@ -166,6 +166,7 @@ export class PluginHostDepsFactory { private readonly packing: PackingService, private readonly oauth: PluginOAuthService, private readonly dayNotes: DayNotesService, + private readonly assignments: AssignmentsService, ) {} create(id: string, granted: ReadonlySet, router: PluginCallRouter): PluginRpcHost { @@ -490,19 +491,22 @@ export class PluginHostDepsFactory { return { deleted: true }; }, // --- Itinerary (day_edit). Both the day AND the place must belong to the trip, - // so a plugin can't cross-link another trip's rows (assignmentService doesn't + // so a plugin can't cross-link another trip's rows (AssignmentsService doesn't // self-check this — the controllers do, so we reproduce it here). --- assignPlaceToDay: (tripId, dayId, placeId, notes) => { - if (!dayExists(dayId, tripId)) throw new ForbiddenResource(`no day ${dayId} on trip ${tripId}`); - if (!placeExists(placeId, tripId)) throw new ForbiddenResource(`no place ${placeId} on trip ${tripId}`); - const assignment = createAssignment(dayId, placeId, notes); + if (!this.assignments.dayExists(dayId, tripId)) throw new ForbiddenResource(`no day ${dayId} on trip ${tripId}`); + if (!this.assignments.placeExists(placeId, tripId)) throw new ForbiddenResource(`no place ${placeId} on trip ${tripId}`); + const assignment = this.assignments.createAssignment(dayId, placeId, notes); broadcast(tripId, 'assignment:created', { assignment }); return assignment; }, unassignPlace: (tripId, assignmentId) => { - if (!getAssignmentForTrip(assignmentId, tripId)) throw new ForbiddenResource(`no assignment ${assignmentId} on trip ${tripId}`); - deleteAssignment(assignmentId); - broadcast(tripId, 'assignment:deleted', { assignmentId }); + const existing = this.assignments.getAssignmentForTrip(assignmentId, tripId); + if (!existing) throw new ForbiddenResource(`no assignment ${assignmentId} on trip ${tripId}`); + this.assignments.deleteAssignment(assignmentId); + // Same payload shape as the REST/MCP delete, so client stores can + // evict the assignment from the right day. + broadcast(tripId, 'assignment:deleted', { assignmentId, dayId: existing.day_id }); return { deleted: true }; }, // --- Trip creation (trip_create; owner = acting user). No broadcast — a new trip diff --git a/server/src/nest/plugins/plugins.module.ts b/server/src/nest/plugins/plugins.module.ts index 4a6aa7526..1b5fd9a22 100644 --- a/server/src/nest/plugins/plugins.module.ts +++ b/server/src/nest/plugins/plugins.module.ts @@ -31,6 +31,7 @@ import { ReservationsModule } from '../reservations/reservations.module'; import { TodoModule } from '../todo/todo.module'; import { PackingModule } from '../packing/packing.module'; import { DaysModule } from '../days/days.module'; +import { AssignmentsModule } from '../assignments/assignments.module'; /** * Plugin system (#plugins). M0 read side + M2 isolated runtime + M3 frontend: @@ -42,7 +43,7 @@ import { DaysModule } from '../days/days.module'; @Module({ // The DI-native domain services the plugin host wiring injects // (PluginHostDepsFactory); DatabaseModule is @Global, so not listed. - imports: [TagsModule, CategoriesModule, BudgetModule, ReservationsModule, TodoModule, PackingModule, DaysModule], + imports: [TagsModule, CategoriesModule, BudgetModule, ReservationsModule, TodoModule, PackingModule, DaysModule, AssignmentsModule], controllers: [PluginsController, PluginsFeedController, PluginsProxyController, PluginFrameController, PlaceDetailsController, TripWarningsController, ViewContributionsController, TripCardContributionsController, PluginPhotosController, PluginCalendarController, MapMarkersController, MapLayersController, PluginRoutesController, DayScheduleController, PdfSectionsController, AtlasLayersController, JournalEntryRowsController, PluginUserSettingsController, PluginOAuthController, PluginActivityController], providers: [PluginsService, PluginRuntimeService, PluginRegistryService, PluginOAuthService, PluginHostDepsFactory], // Exported so the admin addon-toggle handler can cascade-disable plugins whose diff --git a/server/src/services/assignmentService.ts b/server/src/services/assignmentService.ts deleted file mode 100644 index a3712bef4..000000000 --- a/server/src/services/assignmentService.ts +++ /dev/null @@ -1,229 +0,0 @@ -import { db } from '../db/database'; -import { loadTagsByPlaceIds, loadParticipantsByAssignmentIds, formatAssignmentWithPlace } from './queryHelpers'; -import { AssignmentRow, DayAssignment } from '../types'; - -export function getAssignmentWithPlace(assignmentId: number | bigint) { - const a = db.prepare(` - SELECT da.*, p.id as place_id, p.name as place_name, p.description as place_description, - p.lat, p.lng, p.address, p.category_id, p.price, p.currency as place_currency, - COALESCE(da.assignment_time, p.place_time) as place_time, - COALESCE(da.assignment_end_time, p.end_time) as end_time, - p.duration_minutes, p.notes as place_notes, - p.image_url, p.transport_mode, p.google_place_id, p.google_ftid, p.osm_id, p.website, p.phone, - c.name as category_name, c.color as category_color, c.icon as category_icon - FROM day_assignments da - JOIN places p ON da.place_id = p.id - LEFT JOIN categories c ON p.category_id = c.id - WHERE da.id = ? - `).get(assignmentId) as AssignmentRow | undefined; - - if (!a) return null; - - const tags = db.prepare(` - SELECT t.* FROM tags t - JOIN place_tags pt ON t.id = pt.tag_id - WHERE pt.place_id = ? - `).all(a.place_id); - - const participants = db.prepare(` - SELECT ap.user_id, COALESCE(u.display_name, u.username) AS username, u.avatar - FROM assignment_participants ap - JOIN users u ON ap.user_id = u.id - WHERE ap.assignment_id = ? - `).all(a.id); - - return { - id: a.id, - day_id: a.day_id, - place_id: a.place_id, - order_index: a.order_index, - notes: a.notes, - assignment_time: a.assignment_time ?? null, - assignment_end_time: a.assignment_end_time ?? null, - leg_transport_mode: a.leg_transport_mode ?? null, - participants, - created_at: a.created_at, - place: { - id: a.place_id, - name: a.place_name, - description: a.place_description, - lat: a.lat, - lng: a.lng, - address: a.address, - category_id: a.category_id, - price: a.price, - currency: a.place_currency, - place_time: a.place_time, - end_time: a.end_time, - duration_minutes: a.duration_minutes, - notes: a.place_notes, - image_url: a.image_url, - transport_mode: a.transport_mode, - google_place_id: a.google_place_id, - google_ftid: a.google_ftid, - osm_id: a.osm_id, - website: a.website, - phone: a.phone, - category: a.category_id ? { - id: a.category_id, - name: a.category_name, - color: a.category_color, - icon: a.category_icon, - } : null, - tags, - } - }; -} - -export function listDayAssignments(dayId: string | number) { - const assignments = db.prepare(` - SELECT da.*, p.id as place_id, p.name as place_name, p.description as place_description, - p.lat, p.lng, p.address, p.category_id, p.price, p.currency as place_currency, - COALESCE(da.assignment_time, p.place_time) as place_time, - COALESCE(da.assignment_end_time, p.end_time) as end_time, - p.duration_minutes, p.notes as place_notes, - p.image_url, p.transport_mode, p.google_place_id, p.google_ftid, p.osm_id, p.website, p.phone, - c.name as category_name, c.color as category_color, c.icon as category_icon - FROM day_assignments da - JOIN places p ON da.place_id = p.id - LEFT JOIN categories c ON p.category_id = c.id - WHERE da.day_id = ? - ORDER BY da.order_index ASC, da.created_at ASC - `).all(dayId) as AssignmentRow[]; - - const placeIds = [...new Set(assignments.map(a => a.place_id))]; - const tagsByPlaceId = loadTagsByPlaceIds(placeIds, { compact: true }); - - const assignmentIds = assignments.map(a => a.id); - const participantsByAssignment = loadParticipantsByAssignmentIds(assignmentIds); - - return assignments.map(a => { - return formatAssignmentWithPlace(a, tagsByPlaceId[a.place_id] || [], participantsByAssignment[a.id] || []); - }); -} - -export function dayExists(dayId: string | number, tripId: string | number) { - return !!db.prepare('SELECT id FROM days WHERE id = ? AND trip_id = ?').get(dayId, tripId); -} - -export function placeExists(placeId: string | number, tripId: string | number) { - return !!db.prepare('SELECT id FROM places WHERE id = ? AND trip_id = ?').get(placeId, tripId); -} - -export function createAssignment(dayId: string | number, placeId: string | number, notes: string | null) { - const maxOrder = db.prepare('SELECT MAX(order_index) as max FROM day_assignments WHERE day_id = ?').get(dayId) as { max: number | null }; - const orderIndex = (maxOrder.max !== null ? maxOrder.max : -1) + 1; - - const result = db.prepare( - 'INSERT INTO day_assignments (day_id, place_id, order_index, notes) VALUES (?, ?, ?, ?)' - ).run(dayId, placeId, orderIndex, notes || null); - - return getAssignmentWithPlace(result.lastInsertRowid); -} - -export function assignmentExistsInDay(id: string | number, dayId: string | number, tripId: string | number) { - return !!db.prepare( - 'SELECT da.id FROM day_assignments da JOIN days d ON da.day_id = d.id WHERE da.id = ? AND da.day_id = ? AND d.trip_id = ?' - ).get(id, dayId, tripId); -} - -export function deleteAssignment(id: string | number) { - db.prepare('DELETE FROM day_assignments WHERE id = ?').run(id); -} - -export function reorderAssignments(dayId: string | number, orderedIds: number[]) { - const update = db.prepare('UPDATE day_assignments SET order_index = ? WHERE id = ? AND day_id = ?'); - db.exec('BEGIN'); - try { - orderedIds.forEach((id: number, index: number) => { - update.run(index, id, dayId); - }); - db.exec('COMMIT'); - } catch (e) { - db.exec('ROLLBACK'); - throw e; - } -} - -export function getAssignmentForTrip(id: string | number, tripId: string | number) { - return db.prepare(` - SELECT da.* FROM day_assignments da - JOIN days d ON da.day_id = d.id - WHERE da.id = ? AND d.trip_id = ? - `).get(id, tripId) as DayAssignment | undefined; -} - -export function moveAssignment(id: string | number, newDayId: string | number, orderIndex: number, oldDayId: number) { - db.prepare('UPDATE day_assignments SET day_id = ?, order_index = ? WHERE id = ?').run(newDayId, orderIndex || 0, id); - const updated = getAssignmentWithPlace(Number(id)); - return { assignment: updated, oldDayId }; -} - -export function getParticipants(assignmentId: string | number) { - return db.prepare(` - SELECT ap.user_id, COALESCE(u.display_name, u.username) AS username, u.avatar - FROM assignment_participants ap - JOIN users u ON ap.user_id = u.id - WHERE ap.assignment_id = ? - `).all(assignmentId); -} - -export function updateTime(id: string | number, placeTime: string | null, endTime: string | null) { - db.prepare('UPDATE day_assignments SET assignment_time = ?, assignment_end_time = ? WHERE id = ?') - .run(placeTime ?? null, endTime ?? null, id); - - // Auto-sort: reorder timed assignments chronologically within the day - if (placeTime) { - const assignment = db.prepare('SELECT day_id FROM day_assignments WHERE id = ?').get(id) as { day_id: number } | undefined; - if (assignment) { - const dayAssignments = db.prepare(` - SELECT da.id, COALESCE(da.assignment_time, p.place_time) as effective_time - FROM day_assignments da - JOIN places p ON da.place_id = p.id - WHERE da.day_id = ? - ORDER BY da.order_index ASC - `).all(assignment.day_id) as { id: number; effective_time: string | null }[]; - - // Separate timed and untimed, sort timed by time - const timed = dayAssignments.filter(a => a.effective_time).sort((a, b) => { - const ta = a.effective_time!.includes(':') ? a.effective_time! : '99:99'; - const tb = b.effective_time!.includes(':') ? b.effective_time! : '99:99'; - return ta.localeCompare(tb); - }); - const untimed = dayAssignments.filter(a => !a.effective_time); - - // Interleave: timed in chronological order, untimed keep relative position - const reordered = [...timed, ...untimed]; - const update = db.prepare('UPDATE day_assignments SET order_index = ? WHERE id = ?'); - reordered.forEach((a, i) => update.run(i, a.id)); - } - } - - return getAssignmentWithPlace(Number(id)); -} - -/** - * Set the travel mode of the leg leaving this stop (#1281). null clears the - * override so the leg falls back to the day's default_transport_mode. This is - * sticky by design: changing the whole-day default never touches a leg that - * carries its own explicit mode. - */ -export function setLegTransportMode(id: string | number, mode: string | null) { - db.prepare('UPDATE day_assignments SET leg_transport_mode = ? WHERE id = ?').run(mode ?? null, id); - return getAssignmentWithPlace(Number(id)); -} - -export function setParticipants(assignmentId: string | number, userIds: number[]) { - db.prepare('DELETE FROM assignment_participants WHERE assignment_id = ?').run(assignmentId); - if (userIds.length > 0) { - const insert = db.prepare('INSERT OR IGNORE INTO assignment_participants (assignment_id, user_id) VALUES (?, ?)'); - for (const userId of userIds) insert.run(assignmentId, userId); - } - - return db.prepare(` - SELECT ap.user_id, COALESCE(u.display_name, u.username) AS username, u.avatar - FROM assignment_participants ap - JOIN users u ON ap.user_id = u.id - WHERE ap.assignment_id = ? - `).all(assignmentId); -} diff --git a/server/tests/helpers/mcp-test-controllers.ts b/server/tests/helpers/mcp-test-controllers.ts index 343b374d8..d7edbe99a 100644 --- a/server/tests/helpers/mcp-test-controllers.ts +++ b/server/tests/helpers/mcp-test-controllers.ts @@ -1,6 +1,8 @@ import { createTestRegistry, type McpRegistry } from '@trek/nest-mcp'; import { db } from '../../src/db/database'; import { trekMcpAccessPolicy } from '../../src/mcp/nest-mcp-policy'; +import { AssignmentsMcp } from '../../src/nest/assignments/assignments.mcp'; +import { AssignmentsService } from '../../src/nest/assignments/assignments.service'; import { CategoriesMcp } from '../../src/nest/categories/categories.mcp'; import { CategoriesService } from '../../src/nest/categories/categories.service'; import { DatabaseService } from '../../src/nest/database/database.service'; @@ -29,6 +31,7 @@ export function createMcpTestRegistry(): McpRegistry { new TodoMcp(new TodoService(dbService)), new PackingMcp(new PackingService(dbService)), new DayNotesMcp(new DayNotesService(dbService)), + new AssignmentsMcp(new AssignmentsService(dbService)), ], { accessPolicy: trekMcpAccessPolicy }, ); diff --git a/server/tests/helpers/plugin-host.ts b/server/tests/helpers/plugin-host.ts index 1a6e6ffc7..2df1c21d3 100644 --- a/server/tests/helpers/plugin-host.ts +++ b/server/tests/helpers/plugin-host.ts @@ -10,6 +10,7 @@ import { CategoriesService } from '../../src/nest/categories/categories.service' import { TodoService } from '../../src/nest/todo/todo.service'; import { PackingService } from '../../src/nest/packing/packing.service'; import { DayNotesService } from '../../src/nest/days/day-notes.service'; +import { AssignmentsService } from '../../src/nest/assignments/assignments.service'; /** * Hand-wired counterpart of the PluginsModule DI graph for no-Nest tests @@ -27,6 +28,7 @@ export function createHostDepsFactory(dbs: DatabaseService): PluginHostDepsFacto new PackingService(dbs), new PluginOAuthService(dbs), new DayNotesService(dbs), + new AssignmentsService(dbs), ); } diff --git a/server/tests/unit/nest/assignments.service.test.ts b/server/tests/unit/nest/assignments.service.test.ts new file mode 100644 index 000000000..1227a1c79 --- /dev/null +++ b/server/tests/unit/nest/assignments.service.test.ts @@ -0,0 +1,388 @@ +/** + * Unit tests for the DI-native AssignmentsService — ASG-SVC-001 through + * ASG-SVC-028 (written for the DI migration: the legacy + * services/assignmentService.ts had no dedicated unit suite, so these pin the + * 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 + * logic is exercised faithfully. + */ +import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest'; + +// ── DB setup ────────────────────────────────────────────────────────────────── + +const { testDb, dbMock } = vi.hoisted(() => { + const Database = require('better-sqlite3'); + const db = new Database(':memory:'); + db.exec('PRAGMA journal_mode = WAL'); + db.exec('PRAGMA foreign_keys = ON'); + db.exec('PRAGMA busy_timeout = 5000'); + const mock = { + db, + closeDb: () => {}, + reinitialize: () => {}, + getPlaceWithTags: () => null, + canAccessTrip: (tripId: unknown, userId: number) => + db.prepare(` + SELECT t.id, t.user_id FROM trips t + LEFT JOIN trip_members m ON m.trip_id = t.id AND m.user_id = ? + WHERE t.id = ? AND (t.user_id = ? OR m.user_id IS NOT NULL) + `).get(userId, tripId, userId), + isOwner: (tripId: unknown, userId: number) => + !!db.prepare('SELECT id FROM trips WHERE id = ? AND user_id = ?').get(tripId, userId), + }; + return { testDb: db, dbMock: mock }; +}); + +vi.mock('../../../src/db/database', () => dbMock); +vi.mock('../../../src/config', () => ({ + JWT_SECRET: 'test-secret', + ENCRYPTION_KEY: 'a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2', + updateJwtSecret: () => {}, +})); +vi.mock('../../../src/websocket', () => ({ broadcast: vi.fn() })); + +import { createTables } from '../../../src/db/schema'; +import { runMigrations } from '../../../src/db/migrations'; +import { resetTestDb } from '../../helpers/test-db'; +import { createUser, createTrip, addTripMember, createDay, createPlace, createDayAssignment, createTag } from '../../helpers/factories'; +import { DatabaseService } from '../../../src/nest/database/database.service'; +import { AssignmentsService } from '../../../src/nest/assignments/assignments.service'; +import { createAssignment, dayExists, placeExists, getAssignmentForTrip } from '../../../src/nest/assignments/assignments.bridge'; + +const svc = new AssignmentsService(new DatabaseService(testDb)); + +beforeAll(() => { + createTables(testDb); + runMigrations(testDb); +}); + +beforeEach(() => { + resetTestDb(testDb); +}); + +afterAll(() => { + testDb.close(); +}); + +/** owner + trip + one day + one place — the minimal itinerary fixture. */ +function fixture() { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const place = createPlace(testDb, trip.id, { name: 'Louvre' }); + return { user, trip, day, place }; +} + +// ── verifyTripAccess / canEdit ──────────────────────────────────────────────── + +describe('verifyTripAccess', () => { + it('ASG-SVC-001: returns trip for owner', () => { + const { user, trip } = fixture(); + const result = svc.verifyTripAccess(trip.id, user.id); + expect(result).toBeDefined(); + expect((result as { user_id: number }).user_id).toBe(user.id); + }); + + it('ASG-SVC-002: returns falsy for non-member', () => { + const { trip } = fixture(); + const { user: stranger } = createUser(testDb); + expect(svc.verifyTripAccess(trip.id, stranger.id)).toBeFalsy(); + }); +}); + +describe('canEdit', () => { + it('ASG-SVC-003: owner and member pass day_edit (default trip_member)', () => { + const { user, trip } = fixture(); + const { user: member } = createUser(testDb); + addTripMember(testDb, trip.id, member.id); + expect(svc.canEdit({ user_id: trip.user_id }, user as never)).toBe(true); + expect(svc.canEdit({ user_id: trip.user_id }, member as never)).toBe(true); + }); +}); + +// ── existence checks ────────────────────────────────────────────────────────── + +describe('dayExists / placeExists / assignmentExistsInDay', () => { + it('ASG-SVC-004: dayExists is trip-scoped', () => { + const { trip, day } = fixture(); + const { user: other } = createUser(testDb); + const otherTrip = createTrip(testDb, other.id); + expect(svc.dayExists(day.id, trip.id)).toBe(true); + expect(svc.dayExists(day.id, otherTrip.id)).toBe(false); + expect(svc.dayExists(9999, trip.id)).toBe(false); + }); + + it('ASG-SVC-005: placeExists is trip-scoped', () => { + const { trip, place } = fixture(); + const { user: other } = createUser(testDb); + const otherTrip = createTrip(testDb, other.id); + expect(svc.placeExists(place.id, trip.id)).toBe(true); + expect(svc.placeExists(place.id, otherTrip.id)).toBe(false); + }); + + it('ASG-SVC-006: assignmentExistsInDay requires matching day AND trip', () => { + const { trip, day, place } = fixture(); + const otherDay = createDay(testDb, trip.id); + const a = createDayAssignment(testDb, day.id, place.id); + expect(svc.assignmentExistsInDay(a.id, day.id, trip.id)).toBe(true); + expect(svc.assignmentExistsInDay(a.id, otherDay.id, trip.id)).toBe(false); + expect(svc.assignmentExistsInDay(a.id, day.id, trip.id + 1)).toBe(false); + }); +}); + +// ── createAssignment ────────────────────────────────────────────────────────── + +describe('createAssignment', () => { + it('ASG-SVC-007: first assignment gets order_index 0, then MAX+1 appends', () => { + const { day, place } = fixture(); + const first = svc.createAssignment(day.id, place.id, null); + const second = svc.createAssignment(day.id, place.id, null); + expect(first!.order_index).toBe(0); + expect(second!.order_index).toBe(1); + }); + + it('ASG-SVC-008: empty-string notes coerce to null (`notes || null`)', () => { + const { day, place } = fixture(); + const a = svc.createAssignment(day.id, place.id, ''); + expect(a!.notes).toBeNull(); + }); + + it('ASG-SVC-009: returns the re-selected nested shape with place, category and compact tags', () => { + const { user, trip, day, place } = fixture(); + const tag = createTag(testDb, user.id, { name: 'museum' }); + testDb.prepare('INSERT INTO place_tags (place_id, tag_id) VALUES (?, ?)').run(place.id, tag.id); + const a = svc.createAssignment(day.id, place.id, 'skip the line'); + expect(a).toMatchObject({ + day_id: day.id, + place_id: place.id, + notes: 'skip the line', + assignment_time: null, + assignment_end_time: null, + leg_transport_mode: null, + participants: [], + place: { id: place.id, name: 'Louvre' }, + }); + // The same compact tag projection as listDayAssignments — id/name/color/ + // created_at only, no user_id — so both read paths share one wire shape. + expect(a!.place.tags).toEqual([{ id: tag.id, name: 'museum', color: '#10b981', created_at: (tag as { created_at?: string }).created_at }]); + void trip; + }); +}); + +// ── listDayAssignments ──────────────────────────────────────────────────────── + +describe('listDayAssignments', () => { + it('ASG-SVC-010: orders by order_index and formats with compact tags + participants', () => { + const { user, trip, day, place } = fixture(); + const second = createPlace(testDb, trip.id, { name: 'Orsay' }); + const tag = createTag(testDb, user.id, { name: 'art' }); + testDb.prepare('INSERT INTO place_tags (place_id, tag_id) VALUES (?, ?)').run(place.id, tag.id); + const a1 = createDayAssignment(testDb, day.id, place.id, { order_index: 1 }); + const a2 = createDayAssignment(testDb, day.id, second.id, { order_index: 0 }); + svc.setParticipants(a1.id, [user.id]); + + const list = svc.listDayAssignments(day.id); + expect(list.map(a => a.id)).toEqual([a2.id, a1.id]); + // Compact tag projection (id/name/color/created_at only — no user_id key). + expect(list[1].place.tags).toEqual([{ id: tag.id, name: 'art', color: '#10b981', created_at: (tag as { created_at?: string }).created_at }]); + expect(list[1].participants).toEqual([{ user_id: user.id, username: user.username, avatar: null }]); + expect(list[0].participants).toEqual([]); + expect(list[0].place.tags).toEqual([]); + }); + + it('ASG-SVC-011: returns [] for an empty day', () => { + const { day } = fixture(); + expect(svc.listDayAssignments(day.id)).toEqual([]); + }); +}); + +// ── delete / reorder ────────────────────────────────────────────────────────── + +describe('deleteAssignment / reorderAssignments', () => { + it('ASG-SVC-012: deleteAssignment removes the row', () => { + const { day, place } = fixture(); + const a = createDayAssignment(testDb, day.id, place.id); + svc.deleteAssignment(a.id); + expect(testDb.prepare('SELECT id FROM day_assignments WHERE id = ?').get(a.id)).toBeUndefined(); + }); + + it('ASG-SVC-013: reorderAssignments applies positional order, scoped to the day', () => { + const { trip, day, place } = fixture(); + const otherDay = createDay(testDb, trip.id); + const a1 = createDayAssignment(testDb, day.id, place.id, { order_index: 0 }); + const a2 = createDayAssignment(testDb, day.id, place.id, { order_index: 1 }); + const foreign = createDayAssignment(testDb, otherDay.id, place.id, { order_index: 5 }); + + svc.reorderAssignments(day.id, [a2.id, a1.id, foreign.id]); + + const order = (id: number) => (testDb.prepare('SELECT order_index FROM day_assignments WHERE id = ?').get(id) as { order_index: number }).order_index; + expect(order(a2.id)).toBe(0); + expect(order(a1.id)).toBe(1); + // The day-scoped WHERE leaves another day's assignment untouched. + expect(order(foreign.id)).toBe(5); + }); +}); + +// ── getAssignmentForTrip / moveAssignment ───────────────────────────────────── + +describe('getAssignmentForTrip', () => { + it('ASG-SVC-014: returns the raw row for the trip, undefined cross-trip', () => { + const { trip, day, place } = fixture(); + const a = createDayAssignment(testDb, day.id, place.id); + expect(svc.getAssignmentForTrip(a.id, trip.id)).toMatchObject({ id: a.id, day_id: day.id }); + expect(svc.getAssignmentForTrip(a.id, trip.id + 1)).toBeUndefined(); + }); +}); + +describe('moveAssignment', () => { + it('ASG-SVC-015: moves to the new day, re-selects the nested shape, derives oldDayId from the row', () => { + const { trip, day, place } = fixture(); + const target = createDay(testDb, trip.id); + const a = createDayAssignment(testDb, day.id, place.id); + const result = svc.moveAssignment(a.id, target.id, 2); + expect(result.assignment).toMatchObject({ id: a.id, day_id: target.id, order_index: 2 }); + expect(result.oldDayId).toBe(day.id); + }); + + it('ASG-SVC-016: nullish orderIndex coerces to 0; explicit 0 stays 0', () => { + const { trip, day, place } = fixture(); + const target = createDay(testDb, trip.id); + const a = createDayAssignment(testDb, day.id, place.id, { order_index: 7 }); + expect(svc.moveAssignment(a.id, target.id, undefined).assignment!.order_index).toBe(0); + expect(svc.moveAssignment(a.id, day.id, null).assignment!.order_index).toBe(0); + expect(svc.moveAssignment(a.id, target.id, 0).assignment!.order_index).toBe(0); + }); +}); + +// ── participants ────────────────────────────────────────────────────────────── + +describe('getParticipants / setParticipants', () => { + it('ASG-SVC-017: setParticipants replaces the list and returns the joined rows', () => { + const { user, day, place } = fixture(); + const { user: peer } = createUser(testDb); + const a = createDayAssignment(testDb, day.id, place.id); + svc.setParticipants(a.id, [user.id]); + const replaced = svc.setParticipants(a.id, [peer.id]); + expect(replaced).toEqual([{ user_id: peer.id, username: peer.username, avatar: null }]); + expect(svc.getParticipants(a.id)).toEqual(replaced); + }); + + it('ASG-SVC-018: empty array clears all participants', () => { + const { user, day, place } = fixture(); + const a = createDayAssignment(testDb, day.id, place.id); + svc.setParticipants(a.id, [user.id]); + expect(svc.setParticipants(a.id, [])).toEqual([]); + expect(svc.getParticipants(a.id)).toEqual([]); + }); + + it('ASG-SVC-019: username COALESCEs display_name over username', () => { + const { user, day, place } = fixture(); + testDb.prepare('UPDATE users SET display_name = ? WHERE id = ?').run('Fancy Name', user.id); + const a = createDayAssignment(testDb, day.id, place.id); + const rows = svc.setParticipants(a.id, [user.id]); + expect(rows).toEqual([{ user_id: user.id, username: 'Fancy Name', avatar: null }]); + }); + + it('ASG-SVC-020: duplicate user ids collapse via INSERT OR IGNORE', () => { + const { user, day, place } = fixture(); + const a = createDayAssignment(testDb, day.id, place.id); + const rows = svc.setParticipants(a.id, [user.id, user.id]); + expect(rows).toHaveLength(1); + }); + + it('ASG-SVC-028: a failing insert rolls back the whole replace (delete included)', () => { + const { user, day, place } = fixture(); + const a = createDayAssignment(testDb, day.id, place.id); + svc.setParticipants(a.id, [user.id]); + // 999999 violates the users FK — the transaction must undo the delete too. + expect(() => svc.setParticipants(a.id, [999999])).toThrow(); + expect(svc.getParticipants(a.id)).toEqual([{ user_id: user.id, username: user.username, avatar: null }]); + }); +}); + +// ── updateTime / setLegTransportMode ────────────────────────────────────────── + +describe('updateTime', () => { + it('ASG-SVC-021: persists both times and re-selects the nested shape', () => { + const { day, place } = fixture(); + const a = createDayAssignment(testDb, day.id, place.id); + const updated = svc.updateTime(a.id, '09:00', '11:30'); + expect(updated).toMatchObject({ id: a.id, assignment_time: '09:00', assignment_end_time: '11:30' }); + }); + + it('ASG-SVC-022: a truthy place_time auto-sorts the day chronologically (untimed keep relative position)', () => { + const { day, place } = fixture(); + const a = createDayAssignment(testDb, day.id, place.id, { order_index: 0 }); + const b = createDayAssignment(testDb, day.id, place.id, { order_index: 1 }); + const c = createDayAssignment(testDb, day.id, place.id, { order_index: 2 }); + + svc.updateTime(c.id, '08:00', null); + const order = (id: number) => (testDb.prepare('SELECT order_index FROM day_assignments WHERE id = ?').get(id) as { order_index: number }).order_index; + expect([order(c.id), order(a.id), order(b.id)]).toEqual([0, 1, 2]); + + svc.updateTime(a.id, '07:00', null); + expect([order(a.id), order(c.id), order(b.id)]).toEqual([0, 1, 2]); + }); + + it('ASG-SVC-023: clearing with null persists but skips the auto-sort (falsy gate)', () => { + const { day, place } = fixture(); + const a = createDayAssignment(testDb, day.id, place.id, { order_index: 0 }); + const b = createDayAssignment(testDb, day.id, place.id, { order_index: 1 }); + svc.updateTime(b.id, '06:00', null); // sorts b first + const updated = svc.updateTime(b.id, null, null); // clear — no re-sort + expect(updated!.assignment_time).toBeNull(); + const order = (id: number) => (testDb.prepare('SELECT order_index FROM day_assignments WHERE id = ?').get(id) as { order_index: number }).order_index; + expect(order(b.id)).toBe(0); + expect(order(a.id)).toBe(1); + }); + + it('ASG-SVC-027: an empty-string time clears the override like null (and skips the auto-sort)', () => { + const { day, place } = fixture(); + const a = createDayAssignment(testDb, day.id, place.id, { order_index: 0 }); + const b = createDayAssignment(testDb, day.id, place.id, { order_index: 1 }); + svc.updateTime(b.id, '06:00', '07:00'); // sorts b first + const updated = svc.updateTime(b.id, '', ''); + expect(updated!.assignment_time).toBeNull(); + expect(updated!.assignment_end_time).toBeNull(); + expect(testDb.prepare('SELECT assignment_time FROM day_assignments WHERE id = ?').get(b.id)).toEqual({ assignment_time: null }); + const order = (id: number) => (testDb.prepare('SELECT order_index FROM day_assignments WHERE id = ?').get(id) as { order_index: number }).order_index; + expect(order(b.id)).toBe(0); + expect(order(a.id)).toBe(1); + }); + + it('ASG-SVC-024: a time without ":" sorts last among timed via the 99:99 sentinel', () => { + const { day, place } = fixture(); + const a = createDayAssignment(testDb, day.id, place.id, { order_index: 0 }); + const b = createDayAssignment(testDb, day.id, place.id, { order_index: 1 }); + svc.updateTime(a.id, 'morning', null); // no colon → '99:99' in the sort + svc.updateTime(b.id, '13:00', null); + const order = (id: number) => (testDb.prepare('SELECT order_index FROM day_assignments WHERE id = ?').get(id) as { order_index: number }).order_index; + expect(order(b.id)).toBe(0); + expect(order(a.id)).toBe(1); + }); +}); + +describe('setLegTransportMode', () => { + it('ASG-SVC-025: sets and clears the leg override (sticky per #1281)', () => { + const { day, place } = fixture(); + const a = createDayAssignment(testDb, day.id, place.id); + expect(svc.setLegTransportMode(a.id, 'cycling')!.leg_transport_mode).toBe('cycling'); + expect(svc.setLegTransportMode(a.id, null)!.leg_transport_mode).toBeNull(); + }); +}); + +// ── 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); + expect(placeExists(place.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 }); + expect(getAssignmentForTrip(created!.id, trip.id)).toMatchObject({ id: created!.id, day_id: day.id }); + }); +}); diff --git a/server/tests/unit/plugins/plugin-host-deps.factory.test.ts b/server/tests/unit/plugins/plugin-host-deps.factory.test.ts index d05505940..5fec0ddf7 100644 --- a/server/tests/unit/plugins/plugin-host-deps.factory.test.ts +++ b/server/tests/unit/plugins/plugin-host-deps.factory.test.ts @@ -4,8 +4,8 @@ * per-plugin data db is cached, a granted db:own call works through the wired * host, and trip broadcasts are force-namespaced to plugin:{id}:{event}. * DI-native domains (budget/reservations/tags/categories/todo/packing/ - * day-notes/oauth) are constructor-injected stubs; legacy services/* domains - * stay path-mocked until their own DI migration lands. + * day-notes/assignments/oauth) are constructor-injected stubs; legacy + * services/* domains stay path-mocked until their own DI migration lands. */ import { describe, it, expect, beforeAll, beforeEach, afterAll, vi } from 'vitest'; import fs from 'node:fs'; @@ -109,13 +109,14 @@ vi.mock('../../../src/services/dayService', () => ({ updateAccommodation: vi.fn((id: number, _existing: unknown, fields: Record) => ({ id: Number(id), ...fields })), deleteAccommodation: vi.fn((id: number) => (Number(id) === 61 ? { linkedReservationId: 40, deletedBudgetItemId: 9 } : { linkedReservationId: null, deletedBudgetItemId: null })), })); -vi.mock('../../../src/services/assignmentService', () => ({ +// Assignments is a constructor-injected stub (same behaviors as the old path mock). +const assignmentsStub = { createAssignment: vi.fn((dayId: number, placeId: number, notes: string | null) => ({ id: 30, day_id: dayId, place_id: placeId, notes })), deleteAssignment: vi.fn(), dayExists: vi.fn((dayId: number) => dayId === 3), placeExists: vi.fn((placeId: number) => placeId === 7), - getAssignmentForTrip: vi.fn((id: number) => (id === 99 ? undefined : { id })), -})); + getAssignmentForTrip: vi.fn((id: number) => (id === 99 ? undefined : { id, day_id: 3 })), +} as unknown as AssignmentsService; // The extra named exports satisfy the real reservations.service.ts module (now // loaded un-mocked as a factory constructor dep); its instance is stubbed, so // these are never called. @@ -235,6 +236,9 @@ const reservationsStub = { syncBudgetOnUpdate() {}, } as unknown as ReservationsService; vi.mock('../../../src/services/journeyService', () => ({ + // Consumed by the real assignments.service.ts module (loaded un-mocked as a + // factory constructor dep); the instance is stubbed, so this is never called. + reconcileTripSkeletons: vi.fn(), listJourneys: vi.fn((uid: number) => [{ id: 1, owner: uid }]), // journeyId 88 = no access (listEntries self-gates to null); else returns entries listEntries: vi.fn((journeyId: number, uid: number) => (journeyId === 88 ? null : [{ id: 10, journey_id: journeyId, author_id: uid }])), @@ -293,12 +297,13 @@ import type { CategoriesService } from '../../../src/nest/categories/categories. import type { TodoService } from '../../../src/nest/todo/todo.service'; import type { PackingService } from '../../../src/nest/packing/packing.service'; import type { DayNotesService } from '../../../src/nest/days/day-notes.service'; +import type { AssignmentsService } from '../../../src/nest/assignments/assignments.service'; import type { PluginOAuthService } from '../../../src/nest/plugins/plugin-oauth.service'; // The factory under test, wired exactly like PluginsModule does — but with the // DI-native domain services replaced by the stubs above. The shim keeps the // ~45 historical call sites unchanged and supplies a default no-op router. -const factory = new PluginHostDepsFactory(budgetStub, reservationsStub, tagsStub, categoriesStub, todoStub, packingStub, oauthStub, dayNotesStub); +const factory = new PluginHostDepsFactory(budgetStub, reservationsStub, tagsStub, categoriesStub, todoStub, packingStub, oauthStub, dayNotesStub, assignmentsStub); const stubRouter: PluginCallRouter = { callPlugin: async () => undefined, emitPluginEvent: () => {} }; const createRealRpcHost = (id: string, granted: ReadonlySet, router: PluginCallRouter = stubRouter) => factory.create(id, granted, router);