From 262dd97679b1f1d69653c7f286204faaf5b59bdd Mon Sep 17 00:00:00 2001 From: Maurice Date: Thu, 22 Oct 2026 12:14:25 +0200 Subject: [PATCH] fix(realtime): place writes over MCP and plugins send what REST sends --- server/src/nest/places/places.mcp.ts | 14 +- server/src/nest/places/places.module.ts | 6 +- .../broadcast-parity/plan.cases.ts | 395 ++++++++++++++++-- server/tests/unit/mcp/tools-places.test.ts | 109 +++++ .../nest/collections.copy-to-trip.test.ts | 113 +++++ .../plugins/addon-domains-errors.rpc.test.ts | 11 + 6 files changed, 610 insertions(+), 38 deletions(-) create mode 100644 server/tests/unit/nest/collections.copy-to-trip.test.ts diff --git a/server/src/nest/places/places.mcp.ts b/server/src/nest/places/places.mcp.ts index c67c35124..6feda0732 100644 --- a/server/src/nest/places/places.mcp.ts +++ b/server/src/nest/places/places.mcp.ts @@ -109,6 +109,8 @@ export class PlacesMcp { if (!(await this.guards.hasTripPermission('place_edit', tripId, ctx.userId))) return permissionDenied(); const place = await this.places.create(String(tripId), { name, description, lat, lng, address, category_id, google_place_id, google_ftid, osm_id, amap_poi_id, notes, website, phone, email, opening_hours: hoursText(opening_hours), image_url, price, currency, stop_type }); this.guards.safeBroadcast(tripId, 'place:created', { place }); + // The journey hook the REST route and the plugin RPC run after the same write. + await this.places.onCreated(String(tripId), place.id); return ok({ place }); } @@ -220,8 +222,11 @@ export class PlacesMcp { if (!(await this.tripsRepo.findAccessible(tripId, ctx.userId))) return noAccess(); if (!(await this.guards.hasTripPermission('place_edit', tripId, ctx.userId))) return permissionDenied(); const place = await this.places.update(String(tripId), String(placeId), { name, description, lat, lng, address, category_id, price, currency, place_time, end_time, duration_minutes, notes, website, phone, email, opening_hours: opening_hours === undefined ? undefined : hoursText(opening_hours), image_url, transport_mode, osm_id, google_place_id, google_ftid, amap_poi_id, stop_type, fill_percent }); - if (!place) return { content: [{ type: 'text' as const, text: 'Place not found.' }], isError: true }; + // No base version is sent, so the update never answers a conflict; the guard only narrows the type. + if (!place || isUpdateConflict(place)) return { content: [{ type: 'text' as const, text: 'Place not found.' }], isError: true }; this.guards.safeBroadcast(tripId, 'place:updated', { place }); + // As on REST: the journey entries made from this place follow its new name, address and time. + await this.places.onUpdated(place.id); return ok({ place }); } @@ -248,6 +253,7 @@ export class PlacesMcp { if (result === 'too_large') return errorResult('That image is too large.'); if (!result || isUpdateConflict(result)) return errorResult('Place not found.'); this.guards.safeBroadcast(tripId, 'place:updated', { place: result }); + await this.places.onUpdated(result.id); return ok({ place: result }); } @@ -521,7 +527,11 @@ export class PlacesMcp { } const updated = await this.places.updateMany(String(tripId), placeIds, fields); - for (const place of updated) this.guards.safeBroadcast(tripId, 'place:updated', { place }); + // Per place, the event and then the journey hook, in the order POST /places/bulk-update keeps. + for (const place of updated) { + this.guards.safeBroadcast(tripId, 'place:updated', { place }); + await this.places.onUpdated(place.id); + } return ok({ count: updated.length, updatedIds: updated.map(p => p.id), skipped: placeIds.length - updated.length }); } diff --git a/server/src/nest/places/places.module.ts b/server/src/nest/places/places.module.ts index f5a779375..0ef103589 100644 --- a/server/src/nest/places/places.module.ts +++ b/server/src/nest/places/places.module.ts @@ -82,9 +82,9 @@ import { MulterModule } from '@nestjs/platform-express'; // PL33's `importKmlPlaces` folder → category lookup. Plan 3h Task 6 adds // `CollectionPlaces` for SV-PI2 (`reclaimPlaceImage`'s second existence // check) — a read-only cross-domain repository reference, not a - // `CollectionsModule` import, so it creates no cycle (`CollectionsModule` - // does not import `PlacesModule` either — both sides reach the other's - // table only through `forFeature`). + // `CollectionsModule` import, so it creates no cycle: `CollectionsModule` + // imports this module for copyToTrip's journey hook, and this side reaches + // the collections table only through `forFeature`. MikroOrmModule.forFeature([ Places, Tags, diff --git a/server/tests/integration/broadcast-parity/plan.cases.ts b/server/tests/integration/broadcast-parity/plan.cases.ts index 582bd0940..72118db85 100644 --- a/server/tests/integration/broadcast-parity/plan.cases.ts +++ b/server/tests/integration/broadcast-parity/plan.cases.ts @@ -1,7 +1,93 @@ -import type { CaseGroup } from './types'; +import { DayAssignments } from '../../../src/db/entities/DayAssignments.entity'; +import { RoadtripDayBoundaries } from '../../../src/db/entities/RoadtripDayBoundaries.entity'; +import { TripFiles } from '../../../src/db/entities/TripFiles.entity'; +import type { ListImportResult } from '../../../src/nest/places/places.helpers'; +import { PlacesService } from '../../../src/nest/places/places.service'; +import { StorageService } from '../../../src/nest/storage/storage.service'; +import { makeDayAssignment } from '../../helpers/factories/itinerary'; +import { insertRow, updateRows } from '../../helpers/factories/rows'; +import type { CaseGroup, Scene } from './types'; +import { MikroORM } from '@mikro-orm/core'; +import type { INestApplication } from '@nestjs/common'; + +import { Readable } from 'node:stream'; +import { vi } from 'vitest'; const CATALOG = "the admin's category catalog is global: no trip room, and the registry has no event for it"; +const placesPath = (s: Scene) => `/api/trips/${s.trip.id}/places`; +const daysPath = (s: Scene) => `/api/trips/${s.trip.id}/days`; +/** The assignments of one of the scene's days. */ +const dayPlanPath = (s: Scene, day: 0 | 1 | 2) => `/api/trips/${s.trip.id}/days/${s.days[day].id}/assignments`; +/** The per-assignment routes of the scene's assignment. */ +const assignmentPath = (s: Scene) => `/api/trips/${s.trip.id}/assignments/${s.assignment.id}`; +const dayNotesPath = (s: Scene) => `/api/trips/${s.trip.id}/days/${s.days[0].id}/notes`; +const onAssignment = (s: Scene) => ({ tripId: s.trip.id, assignmentId: s.assignment.id }); + +/** One waypoint, the smallest file the importer turns into a place. */ +const GPX = [ + '', + '', + 'Pont des Arts', + '', +].join(''); + +/** + * A shared Google Maps list without the network: the list reader answers with + * one place, written the way the importer writes it, so the route and the tool + * announce a real row. + */ +function stubGoogleList(app: INestApplication): () => void { + const places = app.get(PlacesService); + const spy = vi + .spyOn(places, 'importGoogleList') + .mockImplementation(async (tripId: string): Promise => { + const place = await places.create(tripId, { name: 'Shakespeare and Company', lat: 48.8526, lng: 2.3471 }); + return { places: [place], listName: 'Left bank', skipped: 0 }; + }); + return () => spy.mockRestore(); +} + +/** The scene's file becomes a picture, with its bytes in storage, so a place can take it as its image. */ +async function fileIsPicture(s: Scene, app: INestApplication): Promise { + const name = `poster-${s.n}.jpg`; + await app.get(StorageService).put('files', name, Readable.from(Buffer.from('jpg')), { contentType: 'image/jpeg' }); + await updateRows( + app.get(MikroORM), + TripFiles, + { id: s.file.id }, + { filename: name, original_name: name, mime_type: 'image/jpeg', file_size: 3 }, + ); +} + +/** A second stop on the first day, behind the scene's assignment. */ +async function secondStop(s: Scene, app: INestApplication): Promise { + await makeDayAssignment(app.get(MikroORM), s.days[0].id, s.place2.id); +} + +/** + * The first day in time order, 07:00 then 08:00, with the scene's via after + * the first stop: moving the first stop to 09:00 sorts the day and moves the + * via with its stop. + */ +async function timedDay(s: Scene, app: INestApplication): Promise { + const orm = app.get(MikroORM); + await updateRows(orm, DayAssignments, { id: s.assignment.id }, { assignment_time: '07:00' }); + await makeDayAssignment(orm, s.days[0].id, s.place2.id, { assignment_time: '08:00' }); +} + +/** A road trip day boundary on the second day, after a stop of its own there. */ +async function boundaryOnSecondDay(s: Scene, app: INestApplication): Promise { + const orm = app.get(MikroORM); + const stop = await makeDayAssignment(orm, s.days[1].id, s.place.id); + await insertRow(orm, RoadtripDayBoundaries, { + trip: s.trip.id, + day_number: 2, + fromAssignment: stop.id, + fraction: 0.5, + }); +} + /** * The planner: places, the day plan, days, day notes, categories, and the * collection places copied into a trip. @@ -15,7 +101,7 @@ export const plan: CaseGroup = { args: (s) => ({ tripId: s.trip.id, name: 'Sacré-Cœur', lat: 48.8867, lng: 2.3431 }), rest: { method: 'post', - path: (s) => `/api/trips/${s.trip.id}/places`, + path: placesPath, body: () => ({ name: 'Sacré-Cœur', lat: 48.8867, lng: 2.3431 }), }, expect: ['place:created'], @@ -46,6 +132,280 @@ export const plan: CaseGroup = { { event: 'assignment:reordered', all: true }, ], }, + { + tool: 'create_and_assign_place', + args: (s) => ({ tripId: s.trip.id, dayId: s.days[1].id, name: 'Café de Flore', lat: 48.854, lng: 2.3326 }), + twin: 'POST /api/trips/:tripId/places, then POST /api/trips/:tripId/days/:dayId/assignments', + expect: ['place:created', 'assignment:created'], + }, + { + tool: 'update_place', + needs: ['place'], + args: (s) => ({ tripId: s.trip.id, placeId: s.place.id, name: 'Musée du Louvre' }), + rest: { method: 'put', path: (s) => `${placesPath(s)}/${s.place.id}`, body: () => ({ name: 'Musée du Louvre' }) }, + expect: ['place:updated'], + }, + { + tool: 'set_place_image_from_file', + needs: ['place', 'file'], + setup: fileIsPicture, + args: (s) => ({ tripId: s.trip.id, placeId: s.place.id, fileId: s.file.id }), + rest: { + method: 'put', + path: (s) => `${placesPath(s)}/${s.place.id}/image/from-file`, + body: (s) => ({ file_id: s.file.id }), + }, + expect: ['place:updated'], + }, + { + tool: 'rate_place', + needs: ['place'], + args: (s) => ({ tripId: s.trip.id, placeId: s.place.id, rating: 4 }), + rest: { method: 'put', path: (s) => `${placesPath(s)}/${s.place.id}/rating`, body: () => ({ rating: 4 }) }, + expect: ['place:updated'], + }, + { + // The hotel of a stay: the night and its partner booking go with the place, + // and the deleting tab hears that too, since it only removed the place. + tool: 'delete_place', + needs: ['stay'], + args: (s) => ({ tripId: s.trip.id, placeId: s.stay.placeId }), + rest: { method: 'delete', path: (s) => `${placesPath(s)}/${s.stay.placeId}` }, + expect: ['place:deleted', { event: 'reservation:deleted', all: true }], + }, + { + tool: 'import_places_from_url', + args: (s) => ({ tripId: s.trip.id, url: 'https://maps.app.goo.gl/parityList', source: 'google-list' }), + rest: { + method: 'post', + path: (s) => `${placesPath(s)}/import/google-list`, + body: () => ({ url: 'https://maps.app.goo.gl/parityList' }), + }, + stub: stubGoogleList, + expect: ['place:created'], + }, + { + tool: 'import_trip_gpx', + args: (s) => ({ tripId: s.trip.id, gpx: GPX, name: 'Paris walk', importRoutes: false, importTracks: false }), + twin: 'POST /api/trips/:tripId/places/import/gpx, a multipart upload of the same file', + expect: ['place:created'], + }, + { + tool: 'bulk_delete_places', + needs: ['place', 'place2'], + args: (s) => ({ tripId: s.trip.id, placeIds: [s.place.id, s.place2.id] }), + rest: { + method: 'post', + path: (s) => `${placesPath(s)}/bulk-delete`, + body: (s) => ({ ids: [s.place.id, s.place2.id] }), + }, + expect: ['place:deleted', 'place:deleted'], + }, + { + // REST takes the category alone; the tool's other fields are not a difference in events. + tool: 'bulk_update_places', + needs: ['place', 'place2', 'category'], + args: (s) => ({ tripId: s.trip.id, placeIds: [s.place.id, s.place2.id], category_id: s.category.id }), + rest: { + method: 'post', + path: (s) => `${placesPath(s)}/bulk-update`, + body: (s) => ({ ids: [s.place.id, s.place2.id], category_id: s.category.id }), + }, + expect: ['place:updated', 'place:updated'], + }, + { + tool: 'assign_place_to_day', + needs: ['place'], + args: (s) => ({ tripId: s.trip.id, dayId: s.days[1].id, placeId: s.place.id }), + rest: { method: 'post', path: (s) => dayPlanPath(s, 1), body: (s) => ({ place_id: s.place.id }) }, + expect: ['assignment:created'], + }, + { + tool: 'set_assignment_end_day', + needs: ['assignment'], + args: (s) => ({ ...onAssignment(s), end_day: true }), + rest: { method: 'put', path: (s) => `${assignmentPath(s)}/end-day`, body: () => ({ end_day: true }) }, + expect: ['assignment:updated'], + }, + { + tool: 'unassign_place', + needs: ['assignment'], + args: (s) => ({ ...onAssignment(s), dayId: s.days[0].id }), + rest: { method: 'delete', path: (s) => `${dayPlanPath(s, 0)}/${s.assignment.id}` }, + expect: ['assignment:deleted'], + }, + { + tool: 'clear_day_assignments', + needs: ['assignment', 'place2'], + setup: secondStop, + args: (s) => ({ tripId: s.trip.id, dayId: s.days[0].id }), + rest: { method: 'delete', path: (s) => dayPlanPath(s, 0) }, + expect: ['assignment:deleted', 'assignment:deleted'], + }, + { + // A start that moves sorts the day: the new order and the vias pinned to it + // reach the writing tab too, which routes on them before its reload. + tool: 'update_assignment_time', + needs: ['assignment', 'place2', 'via'], + setup: timedDay, + args: (s) => ({ ...onAssignment(s), place_time: '09:00' }), + rest: { method: 'put', path: (s) => `${assignmentPath(s)}/time`, body: () => ({ place_time: '09:00' }) }, + expect: [ + 'assignment:updated', + { event: 'assignment:reordered', all: true }, + { event: 'roadtripVia:changed', all: true }, + ], + }, + { + tool: 'update_assignment_notes', + needs: ['assignment'], + args: (s) => ({ ...onAssignment(s), notes: 'Tickets at the side door' }), + rest: { + method: 'put', + path: (s) => `${assignmentPath(s)}/notes`, + body: () => ({ notes: 'Tickets at the side door' }), + }, + expect: ['assignment:updated'], + }, + { + tool: 'set_leg_transport_mode', + needs: ['assignment'], + args: (s) => ({ ...onAssignment(s), transport_mode: 'walking' }), + rest: { + method: 'put', + path: (s) => `${assignmentPath(s)}/transport`, + body: () => ({ transport_mode: 'walking' }), + }, + expect: ['assignment:updated'], + }, + { + tool: 'set_assignment_route_excluded', + needs: ['assignment'], + args: (s) => ({ ...onAssignment(s), excluded: true }), + rest: { method: 'put', path: (s) => `${assignmentPath(s)}/route`, body: () => ({ excluded: true }) }, + expect: ['assignment:updated'], + }, + { + tool: 'move_assignment', + needs: ['assignment'], + args: (s) => ({ ...onAssignment(s), newDayId: s.days[1].id }), + rest: { + method: 'put', + path: (s) => `${assignmentPath(s)}/move`, + body: (s) => ({ new_day_id: s.days[1].id, order_index: 0 }), + }, + expect: ['assignment:moved'], + }, + { + tool: 'set_assignment_participants', + needs: ['assignment', 'member'], + args: (s) => ({ ...onAssignment(s), userIds: [s.member.id] }), + rest: { + method: 'put', + path: (s) => `${assignmentPath(s)}/participants`, + body: (s) => ({ user_ids: [s.member.id] }), + }, + expect: ['assignment:participants'], + }, + { + tool: 'reorder_day_assignments', + needs: ['assignment'], + args: (s) => ({ tripId: s.trip.id, dayId: s.days[0].id, assignmentIds: [s.assignment.id] }), + rest: { + method: 'put', + path: (s) => `${dayPlanPath(s, 0)}/reorder`, + body: (s) => ({ orderedIds: [s.assignment.id] }), + }, + expect: ['assignment:reordered'], + }, + { + // Closed gap G3: the copies used to reach no open planner, on any surface. + tool: 'copy_collection_places_to_trip', + needs: ['collection'], + args: (s) => ({ trip_id: s.trip.id, place_ids: [s.collectionPlace.id] }), + rest: { + method: 'post', + path: () => '/api/addons/collections/copy-to-trip', + body: (s) => ({ trip_id: s.trip.id, place_ids: [s.collectionPlace.id] }), + }, + expect: ['place:created'], + }, + { + tool: 'create_day_note', + args: (s) => ({ tripId: s.trip.id, dayId: s.days[0].id, text: 'Bring the museum pass' }), + rest: { method: 'post', path: dayNotesPath, body: () => ({ text: 'Bring the museum pass' }) }, + expect: ['dayNote:created'], + }, + { + tool: 'update_day_note', + needs: ['dayNote'], + args: (s) => ({ tripId: s.trip.id, dayId: s.days[0].id, noteId: s.dayNote.id, text: 'Pass in the blue bag' }), + rest: { + method: 'put', + path: (s) => `${dayNotesPath(s)}/${s.dayNote.id}`, + body: () => ({ text: 'Pass in the blue bag' }), + }, + expect: ['dayNote:updated'], + }, + { + tool: 'delete_day_note', + needs: ['dayNote'], + args: (s) => ({ tripId: s.trip.id, dayId: s.days[0].id, noteId: s.dayNote.id }), + rest: { method: 'delete', path: (s) => `${dayNotesPath(s)}/${s.dayNote.id}` }, + expect: ['dayNote:deleted'], + }, + { + tool: 'update_day', + args: (s) => ({ tripId: s.trip.id, dayId: s.days[1].id, title: 'Montmartre' }), + rest: { method: 'put', path: (s) => `${daysPath(s)}/${s.days[1].id}`, body: () => ({ title: 'Montmartre' }) }, + expect: ['day:updated'], + }, + { + // The calendar day after the last date: the list and the longer trip. Every + // day of the scene has a date, so no undated day moves back with a road + // trip boundary on it, and no boundary event follows. + tool: 'create_day', + args: (s) => ({ tripId: s.trip.id, dated: true }), + rest: { method: 'post', path: daysPath, body: () => ({ dated: true }) }, + expect: ['day:reordered', 'trip:updated'], + }, + { + tool: 'reorder_days', + args: (s) => ({ tripId: s.trip.id, orderedIds: [s.days[1].id, s.days[0].id, s.days[2].id] }), + rest: { + method: 'put', + path: (s) => `${daysPath(s)}/reorder`, + body: (s) => ({ orderedIds: [s.days[1].id, s.days[0].id, s.days[2].id] }), + }, + expect: ['day:reordered'], + }, + { + // The check-in day of a stay: the stay and its booking go with it, and the + // boundary on the second day moves up with its day, for every tab. + tool: 'delete_day', + needs: ['stay', 'place'], + setup: boundaryOnSecondDay, + args: (s) => ({ tripId: s.trip.id, dayId: s.days[0].id }), + rest: { method: 'delete', path: (s) => `${daysPath(s)}/${s.days[0].id}` }, + expect: [ + 'day:deleted', + 'day:reordered', + { event: 'reservation:deleted', all: true }, + 'accommodation:deleted', + { event: 'roadtripBoundary:changed', all: true }, + 'trip:updated', + ], + }, + { + tool: 'set_day_default_transport_mode', + args: (s) => ({ tripId: s.trip.id, dayId: s.days[0].id, transport_mode: 'walking' }), + rest: { + method: 'put', + path: (s) => `${daysPath(s)}/${s.days[0].id}/transport`, + body: () => ({ transport_mode: 'walking' }), + }, + expect: ['day:updated'], + }, ], exempt: [ { @@ -69,35 +429,4 @@ export const plan: CaseGroup = { args: (s) => ({ categoryId: s.category.id }), }, ], - pending: [ - 'assign_place_to_day', - 'set_assignment_end_day', - 'unassign_place', - 'clear_day_assignments', - 'update_assignment_time', - 'update_assignment_notes', - 'set_leg_transport_mode', - 'set_assignment_route_excluded', - 'move_assignment', - 'set_assignment_participants', - 'reorder_day_assignments', - 'copy_collection_places_to_trip', - 'create_day_note', - 'update_day_note', - 'delete_day_note', - 'update_day', - 'create_day', - 'reorder_days', - 'delete_day', - 'set_day_default_transport_mode', - 'create_and_assign_place', - 'update_place', - 'set_place_image_from_file', - 'rate_place', - 'delete_place', - 'import_places_from_url', - 'import_trip_gpx', - 'bulk_delete_places', - 'bulk_update_places', - ], }; diff --git a/server/tests/unit/mcp/tools-places.test.ts b/server/tests/unit/mcp/tools-places.test.ts index a85f1b900..a8984a5bd 100644 --- a/server/tests/unit/mcp/tools-places.test.ts +++ b/server/tests/unit/mcp/tools-places.test.ts @@ -16,8 +16,10 @@ import { PlaceRatings } from '../../../src/db/entities/PlaceRatings.entity'; import { PlaceRegions } from '../../../src/db/entities/PlaceRegions.entity'; import { Places } from '../../../src/db/entities/Places.entity'; import { DomainError } from '../../../src/nest/common/domain-error'; +import { JourneyDomainService } from '../../../src/nest/journey/journey-domain.service'; import { MapsService } from '../../../src/nest/maps/maps.service'; import { PlacesService } from '../../../src/nest/places/places.service'; +import { StorageService } from '../../../src/nest/storage/storage.service'; import { createUser, createTrip, @@ -33,6 +35,7 @@ import { linkTripToJourney } from '../../helpers/factories/journeys'; import { countRows, findRow, findRows, insertRow, updateRows } from '../../helpers/factories/rows'; import { FakeRealtimeService } from '../../helpers/fake-realtime'; import { createMcpHarness, parseToolResult, parseResourceResult, type McpHarness } from '../../helpers/mcp-harness'; +import { bootTestApp } from '../../helpers/test-app'; // PlacesMcp and PlacesService both inject MapsService (search_place and the // import enrichment), so the geo calls are stubbed on the prototype — see // beforeEach — rather than through a module mock. @@ -40,6 +43,7 @@ import { createMcpHarness, parseToolResult, parseResourceResult, type McpHarness import { resetTestDb } from '../../helpers/test-db'; import { createTestOrm, type TestOrm } from '../../helpers/test-orm'; +import { Readable } from 'node:stream'; import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest'; vi.mock('../../../src/db/database', async () => { @@ -1490,6 +1494,111 @@ describe('journey hooks on the MCP delete paths', () => { }); }); +// --------------------------------------------------------------------------- +// Journey hooks on the MCP create and update paths: the same ones REST runs +// --------------------------------------------------------------------------- + +describe('journey hooks on the MCP create and update paths', () => { + /** A place planned on a day of a trip with a linked journey, and the skeleton entry made from it. */ + async function plannedWithSkeleton(placeName: string, skeletonTitle = placeName) { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const place = createPlace(testDb, trip.id, { name: placeName }); + createDayAssignment(testDb, day.id, place.id); + const journey = createJourney(testDb, user.id); + await linkJourney(journey.id, trip.id); + await seedSkeleton(journey.id, trip.id, place.id, user.id, skeletonTitle); + return { user, trip, day, place, journey }; + } + + it('MCP-PLACES-JRN-001: update_place carries a rename into the journey entry made from the place', async () => { + const { user, trip, place, journey } = await plannedWithSkeleton('Musée'); + await withHarness(user.id, async (h) => { + const result = await h.client.callTool({ + name: 'update_place', + arguments: { tripId: trip.id, placeId: place.id, name: "Musée d'Orsay" }, + }); + expect(result.isError).toBeFalsy(); + }); + expect((await skeletonFor(journey.id, place.id))!.title).toBe("Musée d'Orsay"); + }); + + it('MCP-PLACES-JRN-002: bulk_update_places carries a new time into the journey entry of every place', async () => { + const { user, trip, day, place, journey } = await plannedWithSkeleton('Market'); + const second = createPlace(testDb, trip.id, { name: 'Harbour' }); + createDayAssignment(testDb, day.id, second.id); + await seedSkeleton(journey.id, trip.id, second.id, user.id, 'Harbour'); + await withHarness(user.id, async (h) => { + const result = await h.client.callTool({ + name: 'bulk_update_places', + arguments: { tripId: trip.id, placeIds: [place.id, second.id], place_time: '10:30' }, + }); + expect((parseToolResult(result) as { count: number }).count).toBe(2); + }); + expect((await skeletonFor(journey.id, place.id))!.entry_time).toBe('10:30'); + expect((await skeletonFor(journey.id, second.id))!.entry_time).toBe('10:30'); + }); + + it('MCP-PLACES-JRN-003: set_place_image_from_file runs the journey hook after the new picture', async () => { + // The entry still carries a title the place no longer has; the hook writes the place's own back. + const { user, trip, place, journey } = await plannedWithSkeleton('Lighthouse', 'Old title'); + const storage = (await bootTestApp(realtime)).get(StorageService); + await storage.put('files', 'beacon.jpg', Readable.from(Buffer.from('img')), { contentType: 'image/jpeg' }); + const file = await makeTripFile(orm, trip.id, { + filename: 'beacon.jpg', + original_name: 'beacon.jpg', + mime_type: 'image/jpeg', + file_size: 3, + }); + await withHarness(user.id, async (h) => { + const result = await h.client.callTool({ + name: 'set_place_image_from_file', + arguments: { tripId: trip.id, placeId: place.id, fileId: file.id }, + }); + expect(result.isError).toBeFalsy(); + }); + expect(await imageOf(place.id)).toMatch(/^\/uploads\/places\/[0-9a-f-]+\.jpg$/); + expect((await skeletonFor(journey.id, place.id))!.title).toBe('Lighthouse'); + }); + + it('MCP-PLACES-JRN-004: create_place hands the new place to the journey hook, as POST /places does', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const hook = vi.spyOn(JourneyDomainService.prototype, 'onPlaceCreated'); + try { + await withHarness(user.id, async (h) => { + const created = parseToolResult( + await h.client.callTool({ name: 'create_place', arguments: { tripId: trip.id, name: 'Night market' } }), + ) as { place: { id: number } }; + expect(hook).toHaveBeenCalledWith(trip.id, created.place.id); + }); + } finally { + hook.mockRestore(); + } + }); + + it('MCP-PLACES-JRN-005: update_place on a missing place sends nothing and runs no journey hook', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const hook = vi.spyOn(JourneyDomainService.prototype, 'onPlaceUpdated'); + broadcastMock.mockClear(); + try { + await withHarness(user.id, async (h) => { + const result = await h.client.callTool({ + name: 'update_place', + arguments: { tripId: trip.id, placeId: 99999, name: 'Nowhere' }, + }); + expect(result.isError).toBe(true); + }); + expect(hook).not.toHaveBeenCalled(); + expect(broadcastMock.mock.calls.filter(([, event]) => event === 'place:updated')).toHaveLength(0); + } finally { + hook.mockRestore(); + } + }); +}); + // --------------------------------------------------------------------------- // A website without a scheme (#2483): the tools parse it the way REST does // --------------------------------------------------------------------------- diff --git a/server/tests/unit/nest/collections.copy-to-trip.test.ts b/server/tests/unit/nest/collections.copy-to-trip.test.ts new file mode 100644 index 000000000..41251a2f1 --- /dev/null +++ b/server/tests/unit/nest/collections.copy-to-trip.test.ts @@ -0,0 +1,113 @@ +/** + * CollectionsService.copyToTrip, the trip side (G3): after the commit every copied + * place goes out to the trip room as `place:created`, shaped as the place routes + * send it, and runs the places domain's journey hook, so open planners show the + * copies without a reload. A skipped duplicate and a refused copy send nothing. + */ +import { Places } from '../../../src/db/entities/Places.entity'; +import { CollectionsModule } from '../../../src/nest/collections/collections.module'; +import { CollectionsService } from '../../../src/nest/collections/collections.service'; +import { PlacesService } from '../../../src/nest/places/places.service'; +import { createSnapshotTestDb } from '../../helpers/db-mock'; +import { makeCategory, makePlace, makeTag } from '../../helpers/factories/places'; +import { findRow } from '../../helpers/factories/rows'; +import { makeTrip } from '../../helpers/factories/trips'; +import { makeUser } from '../../helpers/factories/users'; +import { resetTestDb } from '../../helpers/test-db'; +import { createTestModule, type TestModule } from '../../helpers/test-module'; +import { createTestOrm, type TestOrm } from '../../helpers/test-orm'; + +import { afterAll, beforeAll, beforeEach, describe, expect, it, vi, type MockInstance } from 'vitest'; + +const db = createSnapshotTestDb(); +let t: TestModule; +let orm: TestOrm; +let svc: CollectionsService; +let onCreated: MockInstance; + +beforeAll(async () => { + t = await createTestModule({ db, imports: [CollectionsModule] }); + svc = t.get(CollectionsService); + onCreated = vi.spyOn(t.get(PlacesService), 'onCreated'); + orm = await createTestOrm(db); +}); +beforeEach(() => { + resetTestDb(db); + orm.clear(); + t.realtime.reset(); + onCreated.mockClear(); +}); +afterAll(async () => { + onCreated.mockRestore(); + await orm.close(); + await t.close(); + db.close(); +}); + +/** The place events of the trip room, as [room, payload, left-out socket]. */ +const placeCreated = () => + t.realtime.broadcastMock.mock.calls + .filter(([, event]) => event === 'place:created') + .map(([room, , payload, sid]) => [room, payload, sid]); + +describe('CollectionsService.copyToTrip: the trip hears its new places', () => { + it('COLLECTIONS-SVC-215: each copied place goes out as place:created with its tags, then runs the journey hook; a skipped one sends nothing', async () => { + const { user } = await makeUser(orm); + await makeCategory(orm); + const trip = await makeTrip(orm, user.id); + const tag = await makeTag(orm, user.id, { name: 'rooftop' }); + const col = await svc.createCollection(user.id, { name: 'Evenings' }); + const bar = (await svc.savePlace(user.id, { collection_id: col.id, name: 'Sky Bar', tag_ids: [tag.id] })).place!; + await makePlace(orm, trip.id, { name: 'Pantheon' }); + const dup = (await svc.savePlace(user.id, { collection_id: col.id, name: 'Pantheon' })).place!; + t.realtime.broadcastMock.mockClear(); + + const res = await svc.copyToTrip( + user.id, + { trip_id: trip.id, place_ids: [bar.id, dup.id] }, + svc.tripEvents(trip.id, 'sock-copy'), + ); + + expect(res).toEqual({ copied: 1, skipped: [{ id: dup.id, name: 'Pantheon' }] }); + const copy = (await findRow(orm, Places, { trip: trip.id, name: 'Sky Bar' }))!; + const sent = placeCreated(); + expect(sent).toHaveLength(1); + // The trip room, the REST socket left out, the place as the place routes send it. + const [room, payload, sid] = sent[0]; + expect([room, sid]).toEqual([trip.id, 'sock-copy']); + expect((payload as { place: { id: number; tags: { id: number }[] } }).place).toMatchObject({ + id: copy.id, + tags: [expect.objectContaining({ id: tag.id })], + }); + expect(onCreated.mock.calls).toEqual([[String(trip.id), copy.id]]); + }); + + it('COLLECTIONS-SVC-216: a refused copy sends nothing and runs no hook', async () => { + const { user: owner } = await makeUser(orm); + const { user: stranger } = await makeUser(orm); + await makeCategory(orm); + const col = await svc.createCollection(owner.id, { name: 'Mine' }); + const gem = (await svc.savePlace(owner.id, { collection_id: col.id, name: 'Hidden gem' })).place!; + const theirs = await makeTrip(orm, stranger.id); + t.realtime.broadcastMock.mockClear(); + + await expect( + svc.copyToTrip(owner.id, { trip_id: theirs.id, place_ids: [gem.id] }, svc.tripEvents(theirs.id, undefined)), + ).rejects.toThrow(); + expect(placeCreated()).toEqual([]); + expect(onCreated).not.toHaveBeenCalled(); + }); + + it('COLLECTIONS-SVC-225: a plugin copy has no socket, so no tab of the trip is left out', async () => { + const { user } = await makeUser(orm); + await makeCategory(orm); + const trip = await makeTrip(orm, user.id); + const col = await svc.createCollection(user.id, { name: 'Plugin' }); + const spot = (await svc.savePlace(user.id, { collection_id: col.id, name: 'Viewpoint' })).place!; + t.realtime.broadcastMock.mockClear(); + + await svc.copyToTrip(user.id, { trip_id: trip.id, place_ids: [spot.id] }, svc.tripEvents(trip.id, undefined)); + + expect(placeCreated().map(([room, , sid]) => [room, sid])).toEqual([[trip.id, undefined]]); + }); +}); diff --git a/server/tests/unit/plugins/addon-domains-errors.rpc.test.ts b/server/tests/unit/plugins/addon-domains-errors.rpc.test.ts index 18a22f65c..09c0db0d2 100644 --- a/server/tests/unit/plugins/addon-domains-errors.rpc.test.ts +++ b/server/tests/unit/plugins/addon-domains-errors.rpc.test.ts @@ -126,6 +126,7 @@ function build( updateCollection: vi.fn(() => ({})), savePlace: vi.fn(() => ({})), copyToTrip: vi.fn(() => ({})), + tripEvents: vi.fn(() => ({ emit: vi.fn(), emitAll: vi.fn() })), deletePlace: vi.fn(() => Promise.resolve(undefined)), ...overrides.collections, } as never, @@ -451,6 +452,16 @@ describe('addon-gated domains validate their input', () => { expect(updateCollection).toHaveBeenCalled(); }); + it('ADDONERR-010b2 a copy hands the service the target trip and no socket, so every open planner hears it', async () => { + const sink = { emit: vi.fn(), emitAll: vi.fn() }; + const tripEvents = vi.fn(() => sink); + const copyToTrip = vi.fn(() => ({ copied: 1, skipped: [] })); + const host = build({ collections: { tripEvents, copyToTrip } }); + await host.dispatch(req('collections.copyToTrip', { input: { trip_id: 7, place_ids: [3] } }), 42); + expect(tripEvents).toHaveBeenCalledWith(7, undefined); + expect(copyToTrip).toHaveBeenCalledWith(42, { trip_id: 7, place_ids: [3] }, sink); + }); + it('ADDONERR-010c a journal entry update carries a non-object input through asPayload', async () => { const updateEntry = vi.fn(() => ({ id: 1 })); const host = build({ journey: { updateEntry } });