diff --git a/server/src/nest/assignments/assignments.module.ts b/server/src/nest/assignments/assignments.module.ts index ff590ae9f..e56ef4b42 100644 --- a/server/src/nest/assignments/assignments.module.ts +++ b/server/src/nest/assignments/assignments.module.ts @@ -5,6 +5,9 @@ import { PermissionsModule } from '../permissions/permissions.module'; import { QueryHelpersModule } from '../query-helpers/query-helpers.module'; import { DayAssignmentsController, AssignmentOpsController } from './assignments.controller'; import { AssignmentsService } from './assignments.service'; +import { ItineraryRpc } from './itinerary.rpc'; +import { PluginGuardsModule } from '../plugins/host/plugin-guards.module'; +import { RealtimeModule } from '../realtime/realtime.module'; import { AssignmentsMcp } from './assignments.mcp'; import { AuthModule } from '../auth/auth.module'; @@ -16,9 +19,9 @@ import { AuthModule } from '../auth/auth.module'; */ @Module({ // DaysModule: AssignmentsMcp injects DaysService for the target-day checks. - imports: [DaysModule, PermissionsModule, QueryHelpersModule, AuthModule, JourneyDomainModule], + imports: [DaysModule, PermissionsModule, QueryHelpersModule, AuthModule, JourneyDomainModule, RealtimeModule, PluginGuardsModule], controllers: [DayAssignmentsController, AssignmentOpsController], - providers: [AssignmentsService, AssignmentsMcp], + providers: [AssignmentsService, AssignmentsMcp, ItineraryRpc], exports: [AssignmentsService], }) export class AssignmentsModule {} diff --git a/server/src/nest/assignments/itinerary.rpc.ts b/server/src/nest/assignments/itinerary.rpc.ts new file mode 100644 index 000000000..49982fbc9 --- /dev/null +++ b/server/src/nest/assignments/itinerary.rpc.ts @@ -0,0 +1,61 @@ +import { PluginController, PluginMethod } from '../plugins/host/rpc-kit/decorators'; +import { PluginGuards } from '../plugins/host/plugin-guards.service'; +import { ForbiddenResource } from '../plugins/host/rpc-errors'; +import { num, str } from '../plugins/host/rpc-params'; +import type { PluginRpcContext } from '../plugins/host/rpc-kit/types'; +import { RealtimeService } from '../realtime/realtime.service'; +import { AssignmentsService } from './assignments.service'; + +/** Assigning a place to a day counts as a DAY edit in the app, not an edit of its own. */ +const DAY_EDIT_ACTION = 'day_edit'; + +/** + * The itinerary surface a plugin may reach (#plugins). + * + * Both the day AND the place have to belong to the trip. AssignmentsService does not + * check that itself, the controllers do, so it is reproduced here: without it a + * plugin could cross-link another trip's rows. + */ +@PluginController() +export class ItineraryRpc { + constructor( + private readonly assignments: AssignmentsService, + private readonly realtime: RealtimeService, + private readonly guards: PluginGuards, + ) {} + + @PluginMethod('itinerary.assign', { permission: 'db:write:itinerary' }) + assign(params: Record, ctx: PluginRpcContext): unknown { + const tripId = num(params.tripId, 'tripId'); + const dayId = num(params.dayId, 'dayId'); + const placeId = num(params.placeId, 'placeId'); + const actor = this.guards.requireActor(ctx, 'itinerary'); + const notes = params.notes === undefined || params.notes === null ? null : str(params.notes, 'notes'); + this.guards.requireTripEdit(tripId, actor, DAY_EDIT_ACTION); + 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); + this.realtime.broadcast(tripId, 'assignment:created', { assignment }); + this.assignments.reconcile(tripId); + return assignment; + } + + @PluginMethod('itinerary.unassign', { permission: 'db:write:itinerary' }) + unassign(params: Record, ctx: PluginRpcContext): unknown { + const tripId = num(params.tripId, 'tripId'); + const assignmentId = num(params.assignmentId, 'assignmentId'); + const actor = this.guards.requireActor(ctx, 'itinerary'); + this.guards.requireTripEdit(tripId, actor, DAY_EDIT_ACTION); + const existing = this.assignments.getAssignmentForTrip(assignmentId, tripId); + if (!existing) throw new ForbiddenResource(`no assignment ${assignmentId} on trip ${tripId}`); + this.assignments.deleteAssignment(assignmentId); + // The dayId is what the client reducer keys the eviction on. Without it nothing + // leaves the day, so keep the payload shape identical to the REST/MCP delete. + this.realtime.broadcast(tripId, 'assignment:deleted', { assignmentId, dayId: existing.day_id }); + // Create, delete, move and time re-mirror the linked journey skeletons afterwards, + // in the controller and the MCP tool alike; reorder, transport and participants do + // not. Without it an open journey keeps the removed place until the next reload. + this.assignments.reconcile(tripId); + return { deleted: true }; + } +} diff --git a/server/src/nest/days/days.module.ts b/server/src/nest/days/days.module.ts index d61c054a0..aad299796 100644 --- a/server/src/nest/days/days.module.ts +++ b/server/src/nest/days/days.module.ts @@ -5,6 +5,7 @@ import { DaysMcp } from './days.mcp'; import { DayNotesController } from './day-notes.controller'; import { DayNotesService } from './day-notes.service'; import { DayNotesRpc } from './day-notes.rpc'; +import { DaysRpc } from './days.rpc'; import { PluginGuardsModule } from '../plugins/host/plugin-guards.module'; import { RealtimeModule } from '../realtime/realtime.module'; import { DayNotesMcp } from './day-notes.mcp'; @@ -25,7 +26,7 @@ import { AuthModule } from '../auth/auth.module'; @Module({ imports: [PermissionsModule, QueryHelpersModule, PlacesModule, AuthModule, RealtimeModule, PluginGuardsModule], controllers: [DaysController, DayNotesController], - providers: [DaysService, DaysMcp, DayNotesService, DayNotesMcp, DayNotesRpc], + providers: [DaysService, DaysMcp, DayNotesService, DayNotesMcp, DayNotesRpc, DaysRpc], exports: [DaysService, DayNotesService], }) export class DaysModule {} diff --git a/server/src/nest/days/days.rpc.ts b/server/src/nest/days/days.rpc.ts new file mode 100644 index 000000000..5aa995fd8 --- /dev/null +++ b/server/src/nest/days/days.rpc.ts @@ -0,0 +1,66 @@ +import { dayCreateRequestSchema, dayUpdateRequestSchema } from '@trek/shared'; +import { PluginController, PluginMethod } from '../plugins/host/rpc-kit/decorators'; +import { PluginGuards } from '../plugins/host/plugin-guards.service'; +import { BadParams, ForbiddenResource } from '../plugins/host/rpc-errors'; +import { num } from '../plugins/host/rpc-params'; +import type { PluginRpcContext } from '../plugins/host/rpc-kit/types'; +import { RealtimeService } from '../realtime/realtime.service'; +import { DaysService } from './days.service'; + +const DAY_EDIT_ACTION = 'day_edit'; + +/** + * The day surface a plugin may reach (#plugins). Every write scopes the day to the + * trip before touching it, so a plugin cannot edit another trip's day by naming its + * id. The itinerary half lives in assignments/itinerary.rpc.ts, because + * AssignmentsModule already imports DaysModule and the reverse would be a cycle. + */ +@PluginController() +export class DaysRpc { + constructor( + private readonly days: DaysService, + private readonly realtime: RealtimeService, + private readonly guards: PluginGuards, + ) {} + + @PluginMethod('days.create', { permission: 'db:write:days' }) + create(params: Record, ctx: PluginRpcContext): unknown { + const tripId = num(params.tripId, 'tripId'); + const actor = this.guards.requireActor(ctx, 'day'); + const parsed = dayCreateRequestSchema.safeParse(params.input); + if (!parsed.success) throw new BadParams(`invalid day: ${parsed.error.issues[0]?.message ?? 'bad input'}`); + this.guards.requireTripEdit(tripId, actor, DAY_EDIT_ACTION); + const input = parsed.data as { date?: string; notes?: string }; + const day = this.days.create(tripId, input.date, input.notes); + this.realtime.broadcast(tripId, 'day:created', { day }); + return day; + } + + @PluginMethod('days.update', { permission: 'db:write:days' }) + update(params: Record, ctx: PluginRpcContext): unknown { + const tripId = num(params.tripId, 'tripId'); + const dayId = num(params.dayId, 'dayId'); + const actor = this.guards.requireActor(ctx, 'day'); + const parsed = dayUpdateRequestSchema.safeParse(params.input); + if (!parsed.success) throw new BadParams(`invalid day: ${parsed.error.issues[0]?.message ?? 'bad input'}`); + this.guards.requireTripEdit(tripId, actor, DAY_EDIT_ACTION); + // getDay scopes the row to the trip before the write touches it. + const current = this.days.getDay(dayId, tripId); + if (!current) throw new ForbiddenResource(`no day ${dayId} on trip ${tripId}`); + const day = this.days.update(dayId, current, parsed.data as { notes?: string; title?: string | null }); + this.realtime.broadcast(tripId, 'day:updated', { day }); + return day; + } + + @PluginMethod('days.delete', { permission: 'db:write:days' }) + delete(params: Record, ctx: PluginRpcContext): unknown { + const tripId = num(params.tripId, 'tripId'); + const dayId = num(params.dayId, 'dayId'); + const actor = this.guards.requireActor(ctx, 'day'); + this.guards.requireTripEdit(tripId, actor, DAY_EDIT_ACTION); + if (!this.days.getDay(dayId, tripId)) throw new ForbiddenResource(`no day ${dayId} on trip ${tripId}`); + this.days.remove(dayId); + this.realtime.broadcast(tripId, 'day:deleted', { dayId }); + return { deleted: true }; + } +} diff --git a/server/src/nest/places/places.module.ts b/server/src/nest/places/places.module.ts index c3278cc3d..36c790ec0 100644 --- a/server/src/nest/places/places.module.ts +++ b/server/src/nest/places/places.module.ts @@ -2,6 +2,9 @@ import { Module } from '@nestjs/common'; import { JourneyDomainModule } from '../journey/journey-domain.module'; import { PlacesController } from './places.controller'; import { PlacesService } from './places.service'; +import { PlacesRpc } from './places.rpc'; +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 { AppConfigModule } from '../app-config/app-config.module'; @@ -20,9 +23,9 @@ 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], + imports: [PermissionsModule, QueryHelpersModule, MapsModule, AuthModule, AppConfigModule, UnsplashModule, PlacePhotosModule, JourneyDomainModule, RealtimeModule, PluginGuardsModule], controllers: [PlacesController], - providers: [PlacesService, PlacesMcp], + providers: [PlacesService, PlacesMcp, PlacesRpc], exports: [PlacesService], }) export class PlacesModule {} diff --git a/server/src/nest/places/places.rpc.ts b/server/src/nest/places/places.rpc.ts new file mode 100644 index 000000000..3847c7380 --- /dev/null +++ b/server/src/nest/places/places.rpc.ts @@ -0,0 +1,98 @@ +import { placeCreateRequestSchema, placeUpdateRequestSchema } from '@trek/shared'; +import { PluginController, PluginMethod } from '../plugins/host/rpc-kit/decorators'; +import { PluginGuards } from '../plugins/host/plugin-guards.service'; +import { BadParams, ForbiddenResource } from '../plugins/host/rpc-errors'; +import { num } from '../plugins/host/rpc-params'; +import type { PluginRpcContext } from '../plugins/host/rpc-kit/types'; +import { RealtimeService } from '../realtime/realtime.service'; +import { JourneyDomainService } from '../journey/journey-domain.service'; +import { PlacesService } from './places.service'; +import type { PlaceCreateInput, PlaceUpdateInput } from './places.service'; + +const PLACE_EDIT_ACTION = 'place_edit'; + +/** + * The @trek/shared write schemas carry no string-length caps, so the plugin path + * mirrors the ones the REST controller adds. Without them a plugin could store a + * field the web app refuses with a 400, e.g. a 100k-character place name. + */ +const PLACE_STR_LIMITS: Record = { name: 200, description: 2000, address: 500, notes: 2000 }; + +/** + * The place surface a plugin may reach (#plugins). + * + * Journey mirroring is best-effort on every path that triggers it, in the REST + * controller and the MCP tool alike, so a broken journey link can never turn a + * successful write into an RPC error. + */ +@PluginController() +export class PlacesRpc { + constructor( + private readonly places: PlacesService, + private readonly journey: JourneyDomainService, + private readonly realtime: RealtimeService, + private readonly guards: PluginGuards, + ) {} + + @PluginMethod('places.create', { permission: 'db:write:places' }) + create(params: Record, ctx: PluginRpcContext): unknown { + const tripId = num(params.tripId, 'tripId'); + const actor = this.guards.requireActor(ctx, 'place'); + const parsed = placeCreateRequestSchema.safeParse(params.input); + if (!parsed.success) throw new BadParams(`invalid place: ${parsed.error.issues[0]?.message ?? 'bad input'}`); + this.guards.capStrings(parsed.data as Record, PLACE_STR_LIMITS); + this.guards.requireTripEdit(tripId, actor, PLACE_EDIT_ACTION); + const place = this.places.create(String(tripId), parsed.data as unknown as PlaceCreateInput); + this.realtime.broadcast(tripId, 'place:created', { place }); + this.mirrorJourneys(() => this.journey.onPlaceCreated(tripId, place.id)); + return place; + } + + @PluginMethod('places.update', { permission: 'db:write:places' }) + update(params: Record, ctx: PluginRpcContext): unknown { + const tripId = num(params.tripId, 'tripId'); + const placeId = num(params.placeId, 'placeId'); + const actor = this.guards.requireActor(ctx, 'place'); + const parsed = placeUpdateRequestSchema.safeParse(params.input); + if (!parsed.success) throw new BadParams(`invalid place: ${parsed.error.issues[0]?.message ?? 'bad input'}`); + this.guards.capStrings(parsed.data as Record, PLACE_STR_LIMITS); + this.guards.requireTripEdit(tripId, actor, PLACE_EDIT_ACTION); + const place = this.places.update(String(tripId), String(placeId), parsed.data as PlaceUpdateInput); + if (place === null) throw new ForbiddenResource(`no place ${placeId} on trip ${tripId}`); + this.realtime.broadcast(tripId, 'place:updated', { place }); + this.mirrorJourneys(() => this.journey.onPlaceUpdated(placeId)); + return place; + } + + @PluginMethod('places.delete', { permission: 'db:write:places' }) + delete(params: Record, ctx: PluginRpcContext): unknown { + const tripId = num(params.tripId, 'tripId'); + const placeId = num(params.placeId, 'placeId'); + const actor = this.guards.requireActor(ctx, 'place'); + this.guards.requireTripEdit(tripId, actor, PLACE_EDIT_ACTION); + // Scope the id to the trip before anything else: onPlaceDeleted keys on the place + // alone, so an id belonging to a foreign trip would detach THAT trip's journey + // entries even though the delete below refuses it. + if (!this.places.get(String(tripId), String(placeId))) { + throw new ForbiddenResource(`no place ${placeId} on trip ${tripId}`); + } + // Ahead of the DELETE, like the REST route and the MCP tool: + // journey_entries.source_place_id is ON DELETE SET NULL, so afterwards the hook + // finds nothing left to detach and the entries linger as orphans. + this.mirrorJourneys(() => this.journey.onPlaceDeleted(placeId)); + if (!this.places.remove(String(tripId), String(placeId))) { + throw new ForbiddenResource(`no place ${placeId} on trip ${tripId}`); + } + this.realtime.broadcast(tripId, 'place:deleted', { placeId }); + return { deleted: true }; + } + + /** Journey mirroring never fails a write that already succeeded. */ + private mirrorJourneys(run: () => void): void { + try { + run(); + } catch { + /* non-fatal */ + } + } +} 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 c358fc55d..9ee6e6272 100644 --- a/server/src/nest/plugins/host/plugin-host-deps.factory.ts +++ b/server/src/nest/plugins/host/plugin-host-deps.factory.ts @@ -367,82 +367,11 @@ export class PluginHostDepsFactory { // without the hooks it keeps the old title/location — or a dead entry — until // somebody reloads it. --- canEditPlaces: (tripId, userId) => this.canEditTripAs('place_edit', tripId, userId), - createPlace: (tripId, input) => { - const place = this.places.create(String(tripId), input as unknown as PlaceCreateInput); - this.realtime.broadcast(tripId, 'place:created', { place }); - mirrorJourneys(() => this.journey.onPlaceCreated(Number(tripId), place.id)); - return place; - }, - updatePlace: (tripId, placeId, input) => { - const place = this.places.update(String(tripId), String(placeId), input as PlaceUpdateInput); - if (place === null) throw new ForbiddenResource(`no place ${placeId} on trip ${tripId}`); - this.realtime.broadcast(tripId, 'place:updated', { place }); - mirrorJourneys(() => this.journey.onPlaceUpdated(Number(placeId))); - return place; - }, - deletePlace: (tripId, placeId) => { - // Scope the id to the trip before anything else: onPlaceDeleted keys on the - // place alone, so an id belonging to a foreign trip would detach that trip's - // journey entries even though the delete below refuses it. - if (!this.places.get(String(tripId), String(placeId))) throw new ForbiddenResource(`no place ${placeId} on trip ${tripId}`); - // Ahead of the DELETE, like the REST route and the MCP tool: journey_entries - // .source_place_id is ON DELETE SET NULL, so afterwards the hook finds nothing - // left to detach and the entries linger as orphans. - mirrorJourneys(() => this.journey.onPlaceDeleted(Number(placeId))); - const deleted = this.places.remove(String(tripId), String(placeId)); - if (!deleted) throw new ForbiddenResource(`no place ${placeId} on trip ${tripId}`); - this.realtime.broadcast(tripId, 'place:deleted', { placeId }); - return { deleted: true }; - }, // --- Days (day_edit). getDay scopes the row to the trip before any write. --- canEditDays: (tripId, userId) => this.canEditTripAs('day_edit', tripId, userId), - createDay: (tripId, input) => { - const i = input as { date?: string; notes?: string }; - const day = this.days.create(tripId, i.date, i.notes); - this.realtime.broadcast(tripId, 'day:created', { day }); - return day; - }, - updateDay: (tripId, dayId, input) => { - const current = this.days.getDay(dayId, tripId); - if (!current) throw new ForbiddenResource(`no day ${dayId} on trip ${tripId}`); - const day = this.days.update(dayId, current, input as { notes?: string; title?: string | null }); - this.realtime.broadcast(tripId, 'day:updated', { day }); - return day; - }, - deleteDay: (tripId, dayId) => { - const current = this.days.getDay(dayId, tripId); - if (!current) throw new ForbiddenResource(`no day ${dayId} on trip ${tripId}`); - this.days.remove(dayId); - this.realtime.broadcast(tripId, 'day:deleted', { dayId }); - 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 (AssignmentsService doesn't // self-check this — the controllers do, so we reproduce it here). --- - assignPlaceToDay: (tripId, 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); - this.realtime.broadcast(tripId, 'assignment:created', { assignment }); - this.assignments.reconcile(tripId); - return assignment; - }, - unassignPlace: (tripId, 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. The dayId is what the client - // reducer keys the eviction on — without it nothing leaves the day. - this.realtime.broadcast(tripId, 'assignment:deleted', { assignmentId, dayId: existing.day_id }); - // Create, delete, move and time re-mirror the linked journey skeletons - // afterwards, in the controller and in the MCP tool alike (reorder, - // transport and participants don't). Without it an open journey keeps - // the removed place until the next reload. No sid — a plugin has no - // originating socket. - this.assignments.reconcile(tripId); - return { deleted: true }; - }, // --- Trip creation (trip_create; owner = acting user). No broadcast — a new trip // is only visible to its owner, who refetches (same as the REST POST). --- canCreateTrip: (userId) => { diff --git a/server/src/nest/plugins/host/rpc-host.ts b/server/src/nest/plugins/host/rpc-host.ts index d88cbf8f8..71db0de80 100644 --- a/server/src/nest/plugins/host/rpc-host.ts +++ b/server/src/nest/plugins/host/rpc-host.ts @@ -166,18 +166,10 @@ export interface HostDeps { deleteCost(tripId: number, itemId: number): unknown; // --- Places (the 'place_edit' permission) --- canEditPlaces(tripId: number, userId: number): boolean; - createPlace(tripId: number, input: Record): unknown; - updatePlace(tripId: number, placeId: number, input: Record): unknown; - deletePlace(tripId: number, placeId: number): unknown; // --- Days + itinerary (the 'day_edit' permission) --- canEditDays(tripId: number, userId: number): boolean; - createDay(tripId: number, input: Record): unknown; - updateDay(tripId: number, dayId: number, input: Record): unknown; - deleteDay(tripId: number, dayId: number): unknown; /** Assign a place to a day (both trip-scoped by the wiring); returns the assignment. */ - assignPlaceToDay(tripId: number, dayId: number, placeId: number, notes: string | null): unknown; /** Remove a day-assignment (trip-scoped by the wiring). */ - unassignPlace(tripId: number, assignmentId: number): unknown; // --- Trip (the 'trip_edit' permission) --- canEditTrip(tripId: number, userId: number): boolean; updateTrip(tripId: number, userId: number, input: Record): unknown; @@ -573,83 +565,11 @@ export class PluginRpcHost { // trip access + the entity's edit permission for the HOST-bound acting user // (a job/onLoad has no user, so its writes are refused). The delegating deps // reuse the real services + broadcast the same events, so the app stays live. --- - if (has('db:write:places')) { - this.methods.set('places.create', (p, uid) => { - const tripId = num(p.tripId, 'tripId'); - const actor = this.requireActor(uid, 'place'); - const parsed = placeCreateRequestSchema.safeParse(p.input); - if (!parsed.success) throw new BadParams(`invalid place: ${parsed.error.issues[0]?.message ?? 'bad input'}`); - this.capStrings(parsed.data as Record, PLACE_STR_LIMITS); - this.requireTripEdit(tripId, actor, deps.canEditPlaces); - return deps.createPlace(tripId, parsed.data as Record); - }); - this.methods.set('places.update', (p, uid) => { - const tripId = num(p.tripId, 'tripId'); - const placeId = num(p.placeId, 'placeId'); - const actor = this.requireActor(uid, 'place'); - const parsed = placeUpdateRequestSchema.safeParse(p.input); - if (!parsed.success) throw new BadParams(`invalid place: ${parsed.error.issues[0]?.message ?? 'bad input'}`); - this.capStrings(parsed.data as Record, PLACE_STR_LIMITS); - this.requireTripEdit(tripId, actor, deps.canEditPlaces); - return deps.updatePlace(tripId, placeId, parsed.data as Record); - }); - this.methods.set('places.delete', (p, uid) => { - const tripId = num(p.tripId, 'tripId'); - const placeId = num(p.placeId, 'placeId'); - const actor = this.requireActor(uid, 'place'); - this.requireTripEdit(tripId, actor, deps.canEditPlaces); - return deps.deletePlace(tripId, placeId); - }); - } + // places.* now lives in src/nest/places/places.rpc.ts. - if (has('db:write:days')) { - this.methods.set('days.create', (p, uid) => { - const tripId = num(p.tripId, 'tripId'); - const actor = this.requireActor(uid, 'day'); - const parsed = dayCreateRequestSchema.safeParse(p.input); - if (!parsed.success) throw new BadParams(`invalid day: ${parsed.error.issues[0]?.message ?? 'bad input'}`); - this.requireTripEdit(tripId, actor, deps.canEditDays); - return deps.createDay(tripId, parsed.data as Record); - }); - this.methods.set('days.update', (p, uid) => { - const tripId = num(p.tripId, 'tripId'); - const dayId = num(p.dayId, 'dayId'); - const actor = this.requireActor(uid, 'day'); - const parsed = dayUpdateRequestSchema.safeParse(p.input); - if (!parsed.success) throw new BadParams(`invalid day: ${parsed.error.issues[0]?.message ?? 'bad input'}`); - this.requireTripEdit(tripId, actor, deps.canEditDays); - return deps.updateDay(tripId, dayId, parsed.data as Record); - }); - this.methods.set('days.delete', (p, uid) => { - const tripId = num(p.tripId, 'tripId'); - const dayId = num(p.dayId, 'dayId'); - const actor = this.requireActor(uid, 'day'); - this.requireTripEdit(tripId, actor, deps.canEditDays); - return deps.deleteDay(tripId, dayId); - }); - } + // days.* now lives in src/nest/days/days.rpc.ts. - if (has('db:write:itinerary')) { - // Assigning/removing a place on a day is a DAY edit in the app (day_edit), so - // gate it with canEditDays; the wiring also checks the day AND place belong to - // the trip so a plugin can't cross-link another trip's rows. - this.methods.set('itinerary.assign', (p, uid) => { - const tripId = num(p.tripId, 'tripId'); - const dayId = num(p.dayId, 'dayId'); - const placeId = num(p.placeId, 'placeId'); - const actor = this.requireActor(uid, 'itinerary'); - const notes = p.notes === undefined || p.notes === null ? null : str(p.notes, 'notes'); - this.requireTripEdit(tripId, actor, deps.canEditDays); - return deps.assignPlaceToDay(tripId, dayId, placeId, notes); - }); - this.methods.set('itinerary.unassign', (p, uid) => { - const tripId = num(p.tripId, 'tripId'); - const assignmentId = num(p.assignmentId, 'assignmentId'); - const actor = this.requireActor(uid, 'itinerary'); - this.requireTripEdit(tripId, actor, deps.canEditDays); - return deps.unassignPlace(tripId, assignmentId); - }); - } + // itinerary.* now lives in src/nest/assignments/itinerary.rpc.ts. if (has('db:write:trips')) { this.methods.set('trips.update', (p, uid) => { diff --git a/server/tests/helpers/rpc-host-deps.ts b/server/tests/helpers/rpc-host-deps.ts index 9c4153886..5b6e7d5f9 100644 --- a/server/tests/helpers/rpc-host-deps.ts +++ b/server/tests/helpers/rpc-host-deps.ts @@ -8,6 +8,9 @@ import { TodoRpc } from '../../src/nest/todo/todo.rpc'; import { DayNotesRpc } from '../../src/nest/days/day-notes.rpc'; import { PackingRpc } from '../../src/nest/packing/packing.rpc'; import { FilesRpc } from '../../src/nest/files/files.rpc'; +import { PlacesRpc } from '../../src/nest/places/places.rpc'; +import { DaysRpc } from '../../src/nest/days/days.rpc'; +import { ItineraryRpc } from '../../src/nest/assignments/itinerary.rpc'; /** * The stubbed HostDeps every plugin router test builds on. It used to live inside @@ -190,5 +193,8 @@ export function allRpcControllers(): object[] { new DayNotesRpc(anyService(), anyService(), anyService()), new PackingRpc(anyService(), anyService(), anyService()), new FilesRpc(anyService(), anyService(), anyService(), anyService()), + new PlacesRpc(anyService(), anyService(), anyService(), anyService()), + new DaysRpc(anyService(), anyService(), anyService()), + new ItineraryRpc(anyService(), anyService(), anyService()), ]; } diff --git a/server/tests/unit/days/days-itinerary.rpc.test.ts b/server/tests/unit/days/days-itinerary.rpc.test.ts new file mode 100644 index 000000000..6a1da64ab --- /dev/null +++ b/server/tests/unit/days/days-itinerary.rpc.test.ts @@ -0,0 +1,145 @@ +/** + * The day and itinerary plugin surfaces after they moved onto @PluginMethod. + * + * They are tested together because they share the gate: assigning a place to a day + * counts as a DAY edit in the app, so itinerary.* runs under day_edit rather than a + * permission of its own, and both halves scope their rows to the trip before writing. + */ +import { describe, it, expect, vi } from 'vitest'; +import { PluginRpcHost } from '../../../src/nest/plugins/host/rpc-host'; +import { createTestPluginRegistry } from '../../../src/nest/plugins/host/rpc-kit/testing'; +import { PluginGuards } from '../../../src/nest/plugins/host/plugin-guards.service'; +import { DaysRpc } from '../../../src/nest/days/days.rpc'; +import { DaysModule } from '../../../src/nest/days/days.module'; +import { ItineraryRpc } from '../../../src/nest/assignments/itinerary.rpc'; +import { AssignmentsModule } from '../../../src/nest/assignments/assignments.module'; +import type { DaysService } from '../../../src/nest/days/days.service'; +import type { AssignmentsService } from '../../../src/nest/assignments/assignments.service'; +import type { RealtimeService } from '../../../src/nest/realtime/realtime.service'; +import type { DatabaseService } from '../../../src/nest/database/database.service'; +import type { PermissionsService } from '../../../src/nest/permissions/permissions.service'; +import type { AddonsService } from '../../../src/nest/addons/addons.service'; +import type { RpcRequest, RpcError } from '../../../src/nest/plugins/protocol/envelope'; +import { makeDeps } from '../../helpers/rpc-host-deps'; + +const req = (method: string, params: Record = {}): RpcRequest => ({ k: 'req', id: 'x', method, params }); + +/** Trip 1 belongs to user 42; day 3, place 7 and assignment 30 sit on it. */ +function build(opts: { canEdit?: boolean } = {}) { + const days = { + create: vi.fn((tripId: number, date?: string) => ({ id: 20, trip_id: tripId, date })), + getDay: vi.fn((dayId: number, tripId: number) => (dayId === 3 && tripId === 1 ? { id: 3 } : undefined)), + update: vi.fn((id: number) => ({ id })), + remove: vi.fn(), + } as unknown as DaysService & Record>; + const assignments = { + dayExists: vi.fn((dayId: number, tripId: number) => dayId === 3 && tripId === 1), + placeExists: vi.fn((placeId: number, tripId: number) => placeId === 7 && tripId === 1), + createAssignment: vi.fn(() => ({ id: 30, day_id: 3, place_id: 7 })), + getAssignmentForTrip: vi.fn((id: number, tripId: number) => (id === 30 && tripId === 1 ? { id: 30, day_id: 3 } : undefined)), + deleteAssignment: vi.fn(), + reconcile: vi.fn(), + } as unknown as AssignmentsService & Record>; + const realtime = { broadcast: vi.fn() } as unknown as RealtimeService & { broadcast: ReturnType }; + const guards = new PluginGuards( + { + canAccessTrip: vi.fn((tripId: number, userId: number) => (tripId === 1 && userId === 42 ? { id: 1, user_id: 42 } : undefined)), + prepare: vi.fn(() => ({ get: () => ({ role: 'user' }) })), + } as unknown as DatabaseService, + { checkPermission: vi.fn(() => opts.canEdit ?? true) } as unknown as PermissionsService, + { isAddonEnabled: vi.fn(() => true) } as unknown as AddonsService, + ); + const registry = createTestPluginRegistry([new DaysRpc(days, realtime, guards), new ItineraryRpc(assignments, realtime, guards)]); + const host = (...grants: string[]) => new PluginRpcHost('p', new Set(grants), makeDeps(), registry); + return { days, assignments, realtime, host }; +} + +describe('DaysRpc', () => { + it('DAYS-RPC-001 writes need the grant, the edit right and a bound user', async () => { + const f = build(); + const host = f.host('db:write:days'); + expect((await host.dispatch(req('days.create', { tripId: 1, input: { date: '2027-01-01' } }), 42)).ok).toBe(true); + expect(((await host.dispatch(req('days.create', { tripId: 2, input: {} }), 42)) as RpcError).error.code).toBe('RESOURCE_FORBIDDEN'); + const noUser = (await host.dispatch(req('days.create', { tripId: 1, input: {} }), undefined)) as RpcError; + expect(noUser.error.message).toBe('day writes require an authenticated user context'); + }); + + it('DAYS-RPC-002 a day on another trip is refused for update and delete alike', async () => { + const f = build(); + const host = f.host('db:write:days'); + expect(((await host.dispatch(req('days.update', { tripId: 1, dayId: 404, input: {} }), 42)) as RpcError).error.message) + .toBe('no day 404 on trip 1'); + expect(((await host.dispatch(req('days.delete', { tripId: 1, dayId: 404 }), 42)) as RpcError).error.message) + .toBe('no day 404 on trip 1'); + expect(f.days.remove).not.toHaveBeenCalled(); + }); + + it('DAYS-RPC-003 every write broadcasts its event', async () => { + const f = build(); + const host = f.host('db:write:days'); + await host.dispatch(req('days.create', { tripId: 1, input: {} }), 42); + await host.dispatch(req('days.update', { tripId: 1, dayId: 3, input: { notes: 'x' } }), 42); + await host.dispatch(req('days.delete', { tripId: 1, dayId: 3 }), 42); + expect(f.realtime.broadcast.mock.calls.map((c) => c[1])).toEqual(['day:created', 'day:updated', 'day:deleted']); + }); + + it('DAYS-RPC-004 the class is listed in its module providers', () => { + expect(Reflect.getMetadata('providers', DaysModule) as unknown[]).toContain(DaysRpc); + }); +}); + +describe('ItineraryRpc', () => { + it('ITIN-RPC-001 assign links a day and a place that both belong to the trip', async () => { + const f = build(); + const res = await f.host('db:write:itinerary').dispatch(req('itinerary.assign', { tripId: 1, dayId: 3, placeId: 7 }), 42); + expect(res.ok).toBe(true); + expect(f.assignments.createAssignment).toHaveBeenCalledWith(3, 7, null); + }); + + it('ITIN-RPC-002 a day or place from another trip is refused, so nothing cross-links', async () => { + const f = build(); + const host = f.host('db:write:itinerary'); + expect(((await host.dispatch(req('itinerary.assign', { tripId: 1, dayId: 404, placeId: 7 }), 42)) as RpcError).error.message) + .toBe('no day 404 on trip 1'); + expect(((await host.dispatch(req('itinerary.assign', { tripId: 1, dayId: 3, placeId: 404 }), 42)) as RpcError).error.message) + .toBe('no place 404 on trip 1'); + expect(f.assignments.createAssignment).not.toHaveBeenCalled(); + }); + + it('ITIN-RPC-003 it runs under day_edit, not an itinerary permission of its own', async () => { + const f = build({ canEdit: false }); + const res = (await f.host('db:write:itinerary').dispatch(req('itinerary.assign', { tripId: 1, dayId: 3, placeId: 7 }), 42)) as RpcError; + expect(res.error.message).toBe('no permission to edit trip 1'); + }); + + it('ITIN-RPC-004 notes are optional and pass through as null when absent', async () => { + const f = build(); + await f.host('db:write:itinerary').dispatch(req('itinerary.assign', { tripId: 1, dayId: 3, placeId: 7, notes: 'lunch' }), 42); + expect(f.assignments.createAssignment).toHaveBeenCalledWith(3, 7, 'lunch'); + }); + + it('ITIN-RPC-005 unassign carries the dayId, which the client keys its eviction on', async () => { + const f = build(); + await f.host('db:write:itinerary').dispatch(req('itinerary.unassign', { tripId: 1, assignmentId: 30 }), 42); + expect(f.realtime.broadcast).toHaveBeenCalledWith(1, 'assignment:deleted', { assignmentId: 30, dayId: 3 }); + }); + + it('ITIN-RPC-006 both writes re-mirror the journey skeletons afterwards', async () => { + const f = build(); + const host = f.host('db:write:itinerary'); + await host.dispatch(req('itinerary.assign', { tripId: 1, dayId: 3, placeId: 7 }), 42); + await host.dispatch(req('itinerary.unassign', { tripId: 1, assignmentId: 30 }), 42); + expect(f.assignments.reconcile).toHaveBeenCalledTimes(2); + }); + + it('ITIN-RPC-007 an assignment on another trip is refused and nothing is deleted', async () => { + const f = build(); + const res = (await f.host('db:write:itinerary').dispatch(req('itinerary.unassign', { tripId: 1, assignmentId: 404 }), 42)) as RpcError; + expect(res.error.message).toBe('no assignment 404 on trip 1'); + expect(f.assignments.deleteAssignment).not.toHaveBeenCalled(); + }); + + it('ITIN-RPC-008 the class is listed in its module providers', () => { + expect(Reflect.getMetadata('providers', AssignmentsModule) as unknown[]).toContain(ItineraryRpc); + }); +}); diff --git a/server/tests/unit/places/places.rpc.test.ts b/server/tests/unit/places/places.rpc.test.ts new file mode 100644 index 000000000..5f2e2a7a0 --- /dev/null +++ b/server/tests/unit/places/places.rpc.test.ts @@ -0,0 +1,136 @@ +/** + * The place plugin surface after it moved onto @PluginMethod. + * + * The delete path carries an ordering constraint that is easy to lose in a move and + * silent when broken, so it gets its own cases: the journey hook has to run BEFORE + * the row is deleted, and the id has to be scoped to the trip before the hook runs + * at all. + */ +import { describe, it, expect, vi } from 'vitest'; +import { PluginRpcHost } from '../../../src/nest/plugins/host/rpc-host'; +import { createTestPluginRegistry } from '../../../src/nest/plugins/host/rpc-kit/testing'; +import { PluginGuards } from '../../../src/nest/plugins/host/plugin-guards.service'; +import { PlacesRpc } from '../../../src/nest/places/places.rpc'; +import { PlacesModule } from '../../../src/nest/places/places.module'; +import type { PlacesService } from '../../../src/nest/places/places.service'; +import type { JourneyDomainService } from '../../../src/nest/journey/journey-domain.service'; +import type { RealtimeService } from '../../../src/nest/realtime/realtime.service'; +import type { DatabaseService } from '../../../src/nest/database/database.service'; +import type { PermissionsService } from '../../../src/nest/permissions/permissions.service'; +import type { AddonsService } from '../../../src/nest/addons/addons.service'; +import type { RpcRequest, RpcError } from '../../../src/nest/plugins/protocol/envelope'; +import { makeDeps } from '../../helpers/rpc-host-deps'; + +const req = (method: string, params: Record = {}): RpcRequest => ({ k: 'req', id: 'x', method, params }); + +/** Trip 1 belongs to user 42; place 7 sits on it. */ +function build(opts: { canEdit?: boolean; journeyThrows?: boolean } = {}) { + const places = { + create: vi.fn((_t: string, i: Record) => ({ id: 10, ...i })), + update: vi.fn((_t: string, id: string) => (id === '7' ? { id: 7, name: 'updated' } : null)), + get: vi.fn((_t: string, id: string) => (id === '7' ? { id: 7 } : undefined)), + remove: vi.fn((_t: string, id: string) => id === '7'), + } as unknown as PlacesService & Record>; + const journey = { + onPlaceCreated: vi.fn(() => { if (opts.journeyThrows) throw new Error('journey down'); }), + onPlaceUpdated: vi.fn(() => { if (opts.journeyThrows) throw new Error('journey down'); }), + onPlaceDeleted: vi.fn(() => { if (opts.journeyThrows) throw new Error('journey down'); }), + } as unknown as JourneyDomainService & Record>; + const realtime = { broadcast: vi.fn() } as unknown as RealtimeService & { broadcast: ReturnType }; + const guards = new PluginGuards( + { + canAccessTrip: vi.fn((tripId: number, userId: number) => (tripId === 1 && userId === 42 ? { id: 1, user_id: 42 } : undefined)), + prepare: vi.fn(() => ({ get: () => ({ role: 'user' }) })), + } as unknown as DatabaseService, + { checkPermission: vi.fn(() => opts.canEdit ?? true) } as unknown as PermissionsService, + { isAddonEnabled: vi.fn(() => true) } as unknown as AddonsService, + ); + const rpc = new PlacesRpc(places, journey, realtime, guards); + const host = (...grants: string[]) => new PluginRpcHost('p', new Set(grants), makeDeps(), createTestPluginRegistry([rpc])); + return { places, journey, realtime, host }; +} + +describe('PlacesRpc', () => { + it('PLACES-RPC-001 writes need the grant, the edit right and a bound user', async () => { + const f = build(); + const host = f.host('db:write:places'); + expect((await host.dispatch(req('places.create', { tripId: 1, input: { name: 'Shrine' } }), 42)).ok).toBe(true); + expect(((await host.dispatch(req('places.create', { tripId: 2, input: { name: 'x' } }), 42)) as RpcError).error.code).toBe('RESOURCE_FORBIDDEN'); + const noUser = (await host.dispatch(req('places.create', { tripId: 1, input: { name: 'x' } }), undefined)) as RpcError; + expect(noUser.error.message).toBe('place writes require an authenticated user context'); + const denied = (await f.host().dispatch(req('places.create', { tripId: 1, input: { name: 'x' } }), 42)) as RpcError; + expect(denied.error.code).toBe('PERMISSION_DENIED'); + }); + + it('PLACES-RPC-002 an invalid place is BAD_PARAMS with the schema message', async () => { + const f = build(); + const res = (await f.host('db:write:places').dispatch(req('places.create', { tripId: 1, input: {} }), 42)) as RpcError; + expect(res.error.code).toBe('BAD_PARAMS'); + expect(res.error.message).toMatch(/^invalid place: /); + }); + + it('PLACES-RPC-003 an oversized field is capped like the REST controller caps it', async () => { + const f = build(); + const res = (await f.host('db:write:places').dispatch( + req('places.create', { tripId: 1, input: { name: 'x'.repeat(201) } }), 42, + )) as RpcError; + expect(res.error.message).toBe('name must be 200 characters or fewer'); + expect(f.places.create).not.toHaveBeenCalled(); + }); + + it('PLACES-RPC-004 a place on another trip is refused, naming it', async () => { + const f = build(); + const host = f.host('db:write:places'); + expect(((await host.dispatch(req('places.update', { tripId: 1, placeId: 404, input: { name: 'x' } }), 42)) as RpcError).error.message) + .toBe('no place 404 on trip 1'); + expect(((await host.dispatch(req('places.delete', { tripId: 1, placeId: 404 }), 42)) as RpcError).error.message) + .toBe('no place 404 on trip 1'); + }); + + it('PLACES-RPC-005 delete runs the journey hook BEFORE the row goes', async () => { + const f = build(); + await f.host('db:write:places').dispatch(req('places.delete', { tripId: 1, placeId: 7 }), 42); + // journey_entries.source_place_id is ON DELETE SET NULL. Run the hook afterwards + // and it finds nothing left to detach, leaving the entries as orphans. + expect(f.journey.onPlaceDeleted.mock.invocationCallOrder[0]).toBeLessThan(f.places.remove.mock.invocationCallOrder[0]); + }); + + it('PLACES-RPC-006 a foreign place id never reaches the journey hook', async () => { + const f = build(); + await f.host('db:write:places').dispatch(req('places.delete', { tripId: 1, placeId: 404 }), 42); + // The hook keys on the place alone, so an unscoped id would detach ANOTHER + // trip's journey entries even though the delete itself is refused. + expect(f.journey.onPlaceDeleted).not.toHaveBeenCalled(); + }); + + it('PLACES-RPC-007 a broken journey link never fails a write that already succeeded', async () => { + const f = build({ journeyThrows: true }); + const host = f.host('db:write:places'); + expect((await host.dispatch(req('places.create', { tripId: 1, input: { name: 'Shrine' } }), 42)).ok).toBe(true); + expect((await host.dispatch(req('places.update', { tripId: 1, placeId: 7, input: { name: 'x' } }), 42)).ok).toBe(true); + expect((await host.dispatch(req('places.delete', { tripId: 1, placeId: 7 }), 42)).ok).toBe(true); + }); + + it('PLACES-RPC-008 every write broadcasts its event, a refused one broadcasts nothing', async () => { + const f = build(); + const host = f.host('db:write:places'); + await host.dispatch(req('places.create', { tripId: 1, input: { name: 'Shrine' } }), 42); + await host.dispatch(req('places.update', { tripId: 1, placeId: 7, input: { name: 'x' } }), 42); + await host.dispatch(req('places.delete', { tripId: 1, placeId: 7 }), 42); + expect(f.realtime.broadcast.mock.calls.map((c) => c[1])).toEqual(['place:created', 'place:updated', 'place:deleted']); + f.realtime.broadcast.mockClear(); + await host.dispatch(req('places.delete', { tripId: 1, placeId: 404 }), 42); + expect(f.realtime.broadcast).not.toHaveBeenCalled(); + }); + + it('PLACES-RPC-009 without the edit right nothing is written', async () => { + const f = build({ canEdit: false }); + const res = (await f.host('db:write:places').dispatch(req('places.create', { tripId: 1, input: { name: 'x' } }), 42)) as RpcError; + expect(res.error.message).toBe('no permission to edit trip 1'); + expect(f.places.create).not.toHaveBeenCalled(); + }); + + it('PLACES-RPC-010 the class is listed in its module providers', () => { + expect(Reflect.getMetadata('providers', PlacesModule) as unknown[]).toContain(PlacesRpc); + }); +}); 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 77d928f04..6973f5bd1 100644 --- a/server/tests/unit/plugins/plugin-host-deps.factory.test.ts +++ b/server/tests/unit/plugins/plugin-host-deps.factory.test.ts @@ -441,93 +441,16 @@ describe('host-deps factory — planner write + metadata deps', () => { beforeEach(() => { checkPermission.mockReset(); checkPermission.mockReturnValue(true); }); afterAll(() => closePluginDataDb('writer')); - it('places.create/update/delete delegate + broadcast; a missing place is RESOURCE_FORBIDDEN', async () => { - const h = host('db:write:places'); - expect((await call(h, 'places.create', { tripId: 1, input: { name: 'P' } })).ok).toBe(true); - // Write deps re-emit the SAME core event the controllers do (not the plugin: namespace). - expect(broadcast).toHaveBeenCalledWith(1, 'place:created', expect.anything()); - expect((await call(h, 'places.update', { tripId: 1, placeId: 5, input: { name: 'Q' } })).ok).toBe(true); - expect((await call(h, 'places.update', { tripId: 1, placeId: 99, input: {} })).error.code).toBe('RESOURCE_FORBIDDEN'); - expect((await call(h, 'places.delete', { tripId: 1, placeId: 5 })).ok).toBe(true); - expect((await call(h, 'places.delete', { tripId: 1, placeId: 99 })).error.code).toBe('RESOURCE_FORBIDDEN'); - }); - + // places, days and itinerary left this factory with the decorator migration; the + // cases now run in tests/unit/places/ and tests/unit/days/. // #1705: a place write from a plugin has to update a linked journey the same way // the REST route does, otherwise the journey shows the old title (or an entry for // a place that no longer exists) until it is reloaded. - it('places writes fire the journey hooks, delete ahead of the row, and skip a foreign id', async () => { - const h = host('db:write:places'); - const created = vi.mocked(onPlaceCreated); - const updated = vi.mocked(onPlaceUpdated); - const deleted = vi.mocked(onPlaceDeleted); - const removePlace = removePlaceStub; - [created, updated, deleted, removePlace].forEach((m) => m.mockClear()); - - expect((await call(h, 'places.create', { tripId: 1, input: { name: 'P' } })).ok).toBe(true); - expect(created).toHaveBeenCalledWith(1, 10); - - expect((await call(h, 'places.update', { tripId: 1, placeId: 5, input: { name: 'Q' } })).ok).toBe(true); - expect(updated).toHaveBeenCalledWith(5); - - expect((await call(h, 'places.delete', { tripId: 1, placeId: 5 })).ok).toBe(true); - expect(deleted).toHaveBeenCalledWith(5); - // source_place_id is ON DELETE SET NULL — after the row is gone the hook has - // nothing left to detach, so the order is part of the fix. - expect(deleted.mock.invocationCallOrder[0]).toBeLessThan(removePlace.mock.invocationCallOrder[0]); - - // A place on another trip: refused before the hook can touch that trip's journeys. - deleted.mockClear(); - updated.mockClear(); - expect((await call(h, 'places.delete', { tripId: 1, placeId: 99 })).error.code).toBe('RESOURCE_FORBIDDEN'); - expect((await call(h, 'places.update', { tripId: 1, placeId: 99, input: {} })).error.code).toBe('RESOURCE_FORBIDDEN'); - expect(deleted).not.toHaveBeenCalled(); - expect(updated).not.toHaveBeenCalled(); - }); - // A journey link that blows up must not turn a successful place write into an // RPC error — every other caller of these hooks swallows them too. - it('places writes survive a throwing journey hook', async () => { - const h = host('db:write:places'); - const updated = vi.mocked(onPlaceUpdated); - updated.mockImplementationOnce(() => { throw new Error('journey db locked'); }); - expect((await call(h, 'places.update', { tripId: 1, placeId: 5, input: { name: 'Q' } })).ok).toBe(true); - }); - - it('days + itinerary delegate; a day/place/assignment outside the trip is refused', async () => { - const h = host('db:write:days', 'db:write:itinerary'); - expect((await call(h, 'days.create', { tripId: 1, input: { notes: 'n' } })).ok).toBe(true); - expect((await call(h, 'days.update', { tripId: 1, dayId: 3, input: { notes: 'x' } })).ok).toBe(true); - expect((await call(h, 'days.delete', { tripId: 1, dayId: 3 })).ok).toBe(true); - expect((await call(h, 'days.update', { tripId: 1, dayId: 99, input: {} })).error.code).toBe('RESOURCE_FORBIDDEN'); - expect((await call(h, 'itinerary.assign', { tripId: 1, dayId: 3, placeId: 7 })).ok).toBe(true); - expect((await call(h, 'itinerary.assign', { tripId: 1, dayId: 99, placeId: 7 })).error.code).toBe('RESOURCE_FORBIDDEN'); - expect((await call(h, 'itinerary.assign', { tripId: 1, dayId: 3, placeId: 99 })).error.code).toBe('RESOURCE_FORBIDDEN'); - expect((await call(h, 'itinerary.unassign', { tripId: 1, assignmentId: 30 })).ok).toBe(true); - expect((await call(h, 'itinerary.unassign', { tripId: 1, assignmentId: 99 })).error.code).toBe('RESOURCE_FORBIDDEN'); - }); - // #1705: a plugin itinerary write has to reach open sessions exactly like the REST // route. The delete payload needs the dayId the client reducer evicts by, and both // directions run the same journey-skeleton reconcile the controller/MCP tool run. - it('itinerary writes carry the dayId the client evicts by and re-mirror linked journeys', async () => { - const h = host('db:write:itinerary'); - const reconcile = vi.mocked(assignmentsStub.reconcile); - - reconcile.mockClear(); - expect((await call(h, 'itinerary.assign', { tripId: 1, dayId: 3, placeId: 7 })).ok).toBe(true); - expect(reconcile).toHaveBeenCalledWith(1); - - reconcile.mockClear(); - expect((await call(h, 'itinerary.unassign', { tripId: 1, assignmentId: 30 })).ok).toBe(true); - expect(broadcast).toHaveBeenCalledWith(1, 'assignment:deleted', { assignmentId: 30, dayId: 3 }); - expect(reconcile).toHaveBeenCalledWith(1); - - // A refused unassign deletes nothing, so it must not touch the journeys either. - reconcile.mockClear(); - expect((await call(h, 'itinerary.unassign', { tripId: 1, assignmentId: 99 })).error.code).toBe('RESOURCE_FORBIDDEN'); - expect(reconcile).not.toHaveBeenCalled(); - }); - it('trips.update: archive/cover need their own permission; service errors map to RPC codes', async () => { const h = host('db:write:trips'); expect((await call(h, 'trips.update', { tripId: 1, input: { title: 'T' } })).ok).toBe(true); diff --git a/server/tests/unit/plugins/rpc-coverage.test.ts b/server/tests/unit/plugins/rpc-coverage.test.ts index d846d56de..483df3313 100644 --- a/server/tests/unit/plugins/rpc-coverage.test.ts +++ b/server/tests/unit/plugins/rpc-coverage.test.ts @@ -54,8 +54,8 @@ describe('plugin RPC coverage ledger', () => { // Update these two numbers in every rollout PR. They make the diff state the size // of the move out loud, which is the number a reviewer wants to see. const total = KNOWN_METHODS.length + UNCONDITIONAL_METHODS.length; - expect(fromRegistry.size).toBe(30); - expect(legacy.size).toBe(total - 30); + expect(fromRegistry.size).toBe(38); + expect(legacy.size).toBe(total - 38); }); it('RPCLEDGER-006 the migrated methods are exactly the ones this PR claims', () => { @@ -65,12 +65,17 @@ describe('plugin RPC coverage ledger', () => { 'daynotes.delete', 'daynotes.list', 'daynotes.update', + 'days.create', + 'days.delete', + 'days.update', 'files.create', 'files.createLink', 'files.getContent', 'files.list', 'files.softDelete', 'files.update', + 'itinerary.assign', + 'itinerary.unassign', 'packing.create', 'packing.createBag', 'packing.delete', @@ -80,6 +85,9 @@ describe('plugin RPC coverage ledger', () => { 'packing.setBagMembers', 'packing.update', 'packing.updateBag', + 'places.create', + 'places.delete', + 'places.update', 'rates.get', 'tags.create', 'tags.delete', diff --git a/server/tests/unit/plugins/rpc-host.test.ts b/server/tests/unit/plugins/rpc-host.test.ts index 54c93639e..634567034 100644 --- a/server/tests/unit/plugins/rpc-host.test.ts +++ b/server/tests/unit/plugins/rpc-host.test.ts @@ -662,53 +662,13 @@ describe('PluginRpcHost — capability enforcement', () => { }); // --- Planner writes (#1429) --- - it('db:write:places creates a place on a trip the acting user may edit', async () => { - const host = new PluginRpcHost('p', new Set(['db:write:places']), deps); - const res = await host.dispatch(req('places.create', { tripId: 1, input: { name: 'Fushimi Inari' } }), 42); - expect(ok(res)).toBe(true); - expect(deps.createPlace).toHaveBeenCalledWith(1, expect.objectContaining({ name: 'Fushimi Inari' })); - }); - - it('places.create rejects an over-long string field (mirrors the REST STRING_LIMITS)', async () => { - const host = new PluginRpcHost('p', new Set(['db:write:places']), deps); - const res = await host.dispatch(req('places.create', { tripId: 1, input: { name: 'A'.repeat(201) } }), 42); - expect((res as RpcError).error.code).toBe('BAD_PARAMS'); - expect(deps.createPlace).not.toHaveBeenCalled(); - }); - + // The places, days and itinerary cases moved to their own domain suites. it('places.create is PERMISSION_DENIED without db:write:places', async () => { const host = new PluginRpcHost('p', new Set(['db:read:trips']), deps); const res = await host.dispatch(req('places.create', { tripId: 1, input: { name: 'X' } }), 42); expect((res as RpcError).error.code).toBe('PERMISSION_DENIED'); }); - it('places.create is RESOURCE_FORBIDDEN on a trip the acting user cannot edit', async () => { - const host = new PluginRpcHost('p', new Set(['db:write:places']), deps); - const res = await host.dispatch(req('places.create', { tripId: 2, input: { name: 'X' } }), 42); - expect((res as RpcError).error.code).toBe('RESOURCE_FORBIDDEN'); - expect(deps.createPlace).not.toHaveBeenCalled(); - }); - - it('places.create with no bound acting user is RESOURCE_FORBIDDEN (jobs / forged calls)', async () => { - const host = new PluginRpcHost('p', new Set(['db:write:places']), deps); - const res = await host.dispatch(req('places.create', { tripId: 1, input: { name: 'X' } }), undefined); - expect((res as RpcError).error.code).toBe('RESOURCE_FORBIDDEN'); - }); - - it('places.create with an invalid payload (no name) is BAD_PARAMS', async () => { - const host = new PluginRpcHost('p', new Set(['db:write:places']), deps); - const res = await host.dispatch(req('places.create', { tripId: 1, input: {} }), 42); - expect((res as RpcError).error.code).toBe('BAD_PARAMS'); - expect(deps.createPlace).not.toHaveBeenCalled(); - }); - - it('db:write:itinerary assigns a place to a day (day_edit gated)', async () => { - const host = new PluginRpcHost('p', new Set(['db:write:itinerary']), deps); - const res = await host.dispatch(req('itinerary.assign', { tripId: 1, dayId: 3, placeId: 10 }), 42); - expect(ok(res)).toBe(true); - expect(deps.assignPlaceToDay).toHaveBeenCalledWith(1, 3, 10, null); - }); - it('db:write:trips updates a trip the acting user may edit', async () => { const host = new PluginRpcHost('p', new Set(['db:write:trips']), deps); const res = await host.dispatch(req('trips.update', { tripId: 1, input: { title: 'Renamed' } }), 42);