fix(realtime): road trip vias and day boundaries send the same events everywhere

This commit is contained in:
Maurice
2026-10-22 12:14:26 +02:00
parent dbb70fc525
commit 588ec8feb1
10 changed files with 863 additions and 221 deletions
+17 -42
View File
@@ -1,6 +1,8 @@
import { ADDON_IDS } from '../../addons';
import type { User } from '../../types';
import { AddonGuard } from '../addons/addon.guard';
import { RequireAddon } from '../addons/require-addon.decorator';
import { CurrentUser } from '../auth-core/current-user.decorator';
import { JwtAuthGuard } from '../auth-core/jwt-auth.guard';
import { RequirePermission, TripAccessGuard } from '../permissions/trip-access.guard';
import {
@@ -37,6 +39,10 @@ import type { RoadtripDayTrack, RoadtripVia } from '@trek/shared';
* one road trip: they look at the same line on the same map, and a reshaped drive moves
* every arrival time after it. Waiting for the other side's next reload is not "eventual
* consistency", it is two people editing a route neither can see the other change.
*
* The events are the service's (RoadtripService.addVia and its siblings), the same for
* these routes and the road trip tools; a route hands it a writer that leaves out the
* socket that sent the request.
*/
@Controller('api/trips/:tripId/roadtrip')
// AddonGuard FIRST: @RequireAddon is metadata and does nothing on its own, and the guard
@@ -63,14 +69,14 @@ export class RoadtripController {
@RequirePermission('day_edit')
@Post('days/:dayId/vias')
async create(
@CurrentUser() user: User,
@Param('tripId') tripId: string,
@Param('dayId') dayId: string,
@Body() body: RoadtripViaCreateDto,
@Headers('x-socket-id') socketId?: string,
): Promise<{ via: RoadtripVia }> {
await this.requireDay(dayId, tripId);
const via = await this.roadtrip.create(dayId, body);
await this.announce(tripId, dayId, socketId);
const via = await this.roadtrip.addVia(dayId, body, this.roadtrip.restWriter(tripId, user, socketId));
return { via };
}
@@ -86,6 +92,7 @@ export class RoadtripController {
@Post('days/:dayId/vias/batch')
@HttpCode(200)
async createMany(
@CurrentUser() user: User,
@Param('tripId') tripId: string,
@Param('dayId') dayId: string,
@Body() body: RoadtripViaBatchDto,
@@ -98,20 +105,7 @@ export class RoadtripController {
if (body.track && !(await this.roadtrip.trackExists(body.track.place_id, tripId))) {
throw new HttpException({ error: 'Track not found' }, 404);
}
const vias = await this.roadtrip.createMany(dayId, body);
await this.announce(tripId, dayId, socketId);
// A batch is how a day starts following a recorded track, so the track it now follows
// is news in its own right: the rail draws a badge for it and the map a line.
const tracks = await this.roadtrip.tracksForTrip(tripId);
this.roadtrip.broadcast(
tripId,
'roadtripTrack:changed',
{
dayId,
track: tracks.find((t) => String(t.day_id) === String(dayId)) ?? null,
},
socketId,
);
const vias = await this.roadtrip.addVias(tripId, dayId, body, this.roadtrip.restWriter(tripId, user, socketId));
return { vias };
}
@@ -126,14 +120,14 @@ export class RoadtripController {
@Put('days/:dayId/vias')
@HttpCode(200)
async reanchor(
@CurrentUser() user: User,
@Param('tripId') tripId: string,
@Param('dayId') dayId: string,
@Body() body: RoadtripViaReanchorDto,
@Headers('x-socket-id') socketId?: string,
): Promise<{ vias: RoadtripVia[] }> {
await this.requireDay(dayId, tripId);
const vias = await this.roadtrip.reanchor(dayId, body);
await this.announce(tripId, dayId, socketId);
const vias = await this.roadtrip.reanchorVias(dayId, body, this.roadtrip.restWriter(tripId, user, socketId));
return { vias };
}
@@ -141,6 +135,7 @@ export class RoadtripController {
@Put('days/:dayId/vias/:id')
@HttpCode(200)
async update(
@CurrentUser() user: User,
@Param('tripId') tripId: string,
@Param('dayId') dayId: string,
@Param('id') id: string,
@@ -148,48 +143,28 @@ export class RoadtripController {
@Headers('x-socket-id') socketId?: string,
): Promise<{ via: RoadtripVia }> {
await this.requireDay(dayId, tripId);
const via = await this.roadtrip.move(id, dayId, body.lat, body.lng, body.after_order_index);
const writer = this.roadtrip.restWriter(tripId, user, socketId);
const via = await this.roadtrip.moveVia(dayId, id, body, writer);
if (!via) throw new HttpException({ error: 'Via not found' }, 404);
await this.announce(tripId, dayId, socketId);
return { via };
}
@RequirePermission('day_edit')
@Delete('days/:dayId/vias/:id')
async remove(
@CurrentUser() user: User,
@Param('tripId') tripId: string,
@Param('dayId') dayId: string,
@Param('id') id: string,
@Headers('x-socket-id') socketId?: string,
): Promise<{ success: true }> {
await this.requireDay(dayId, tripId);
if (!(await this.roadtrip.remove(id, dayId))) {
if (!(await this.roadtrip.removeVia(dayId, id, this.roadtrip.restWriter(tripId, user, socketId)))) {
throw new HttpException({ error: 'Via not found' }, 404);
}
await this.announce(tripId, dayId, socketId);
return { success: true };
}
/**
* Says what this day's drive is routed through now.
*
* Read back rather than assembled from what the write returned: a reanchor rewrites the
* whole set, a batch may clear legs before filling them, and one shape for all five
* routes means a client applies them all the same way. The originating socket is
* excluded, so the person dragging does not get their own point handed back mid-drag.
*/
private async announce(tripId: string, dayId: string, socketId: string | undefined): Promise<void> {
this.roadtrip.broadcast(
tripId,
'roadtripVia:changed',
{
dayId,
vias: await this.roadtrip.listForDay(dayId),
},
socketId,
);
}
/**
* The guard proves the caller may reach the trip; this proves the day is part of it.
* Without it a valid day id from someone else's trip would be editable through a trip
+13 -40
View File
@@ -41,6 +41,8 @@ const BOOKEND_VIA_NOTE =
* the controller demands, so an assistant can never do through a tool what a person
* cannot do through the UI. Reads go through the same day-belongs-to-trip check, so a
* valid day id from another trip is not reachable by way of one the caller can see.
* The writes and their events are the service's use cases the routes call too; a tool
* call has no socket of its own, so the tab that asked hears the change like everyone else.
*/
@McpController()
export class RoadtripMcp {
@@ -99,8 +101,8 @@ export class RoadtripMcp {
if (!(await this.tripsRepo.findAccessible(tripId, ctx.userId))) return noAccess();
if (!(await this.guards.hasTripPermission('day_edit', tripId, ctx.userId))) return permissionDenied();
if (!(await this.roadtrip.dayExists(dayId, tripId))) return noAccess();
const via = await this.roadtrip.create(dayId, { after_order_index, lat, lng });
await this.announce(tripId, dayId);
const writer = await this.guards.tripWriter(tripId, ctx.userId);
const via = await this.roadtrip.addVia(dayId, { after_order_index, lat, lng }, writer);
return ok({ via });
}
@@ -173,18 +175,8 @@ export class RoadtripMcp {
// a place id from another trip, or a place that is not a track at all, become this
// day's label.
if (track && !(await this.roadtrip.trackExists(track.place_id, tripId))) return noAccess();
const made = await this.roadtrip.createMany(dayId, { vias, replace_legs, track });
await this.announce(tripId, dayId);
const tracks = await this.roadtrip.tracksForTrip(String(tripId));
this.roadtrip.broadcast(
String(tripId),
'roadtripTrack:changed',
{
dayId,
track: tracks.find((t) => String(t.day_id) === String(dayId)) ?? null,
},
undefined,
);
const writer = await this.guards.tripWriter(tripId, ctx.userId);
const made = await this.roadtrip.addVias(tripId, dayId, { vias, replace_legs, track }, writer);
return ok({ vias: made });
}
@@ -231,8 +223,8 @@ export class RoadtripMcp {
if (!(await this.tripsRepo.findAccessible(tripId, ctx.userId))) return noAccess();
if (!(await this.guards.hasTripPermission('day_edit', tripId, ctx.userId))) return permissionDenied();
if (!(await this.roadtrip.dayExists(dayId, tripId))) return noAccess();
const next = await this.roadtrip.reanchor(dayId, { vias, remove });
await this.announce(tripId, dayId);
const writer = await this.guards.tripWriter(tripId, ctx.userId);
const next = await this.roadtrip.reanchorVias(dayId, { vias, remove }, writer);
return ok({ vias: next });
}
@@ -252,8 +244,8 @@ export class RoadtripMcp {
if (!(await this.tripsRepo.findAccessible(tripId, ctx.userId))) return noAccess();
if (!(await this.guards.hasTripPermission('day_edit', tripId, ctx.userId))) return permissionDenied();
if (!(await this.roadtrip.dayExists(dayId, tripId))) return noAccess();
if (!(await this.roadtrip.remove(viaId, dayId))) return noAccess();
await this.announce(tripId, dayId);
const writer = await this.guards.tripWriter(tripId, ctx.userId);
if (!(await this.roadtrip.removeVia(dayId, viaId, writer))) return noAccess();
return ok({ success: true });
}
@@ -270,29 +262,10 @@ export class RoadtripMcp {
if (!(await this.tripsRepo.findAccessible(input.tripId, ctx.userId))) return noAccess();
if (!(await this.guards.hasTripPermission('day_edit', input.tripId, ctx.userId))) return permissionDenied();
if (!(await this.roadtrip.dayExists(input.dayId, input.tripId))) return noAccess();
const via = await this.roadtrip.move(input.viaId, input.dayId, input.lat, input.lng, input.after_order_index);
const writer = await this.guards.tripWriter(input.tripId, ctx.userId);
const { lat, lng, after_order_index } = input;
const via = await this.roadtrip.moveVia(input.dayId, input.viaId, { lat, lng, after_order_index }, writer);
if (!via) return noAccess();
await this.announce(input.tripId, input.dayId);
return ok({ via });
}
/**
* Says what this day's drive is routed through now.
*
* The same announcement the REST routes make, because it is the same change: a via laid
* by an assistant moves the line on everybody's map exactly as one dragged by hand.
* No originating socket to exclude here — a tool call has no socket of its own, so the
* client that asked for it hears about it like everyone else.
*/
private async announce(tripId: number, dayId: number): Promise<void> {
this.roadtrip.broadcast(
String(tripId),
'roadtripVia:changed',
{
dayId,
vias: await this.roadtrip.listForDay(String(dayId)),
},
undefined,
);
}
}
+119 -15
View File
@@ -6,13 +6,31 @@ import type { DaysRepository } from '../../db/repositories/Days.repository';
import type { PlacesRepository } from '../../db/repositories/Places.repository';
import type { RoadtripDayTracksRepository } from '../../db/repositories/RoadtripDayTracks.repository';
import type { RoadtripViasRepository } from '../../db/repositories/RoadtripVias.repository';
import type { User } from '../../types';
import { DomainError } from '../common/domain-error';
import { toRowId } from '../common/row-id';
import { restTripWriter, type TripWriter } from '../common/trip-writer';
import { UnitOfWork } from '../database/unit-of-work';
import { RealtimeService } from '../realtime/realtime.service';
import { InjectRepository } from '@mikro-orm/nestjs';
import { Injectable } from '@nestjs/common';
import type { RoadtripDayTrack, RoadtripVia, TrekWsPayload, TrekWsTripEventName } from '@trek/shared';
import { Injectable, Logger } from '@nestjs/common';
import type { RoadtripDayTrack, RoadtripVia } from '@trek/shared';
/** A via as a write places it: after which stop of the day, and where. */
type ViaInput = { after_order_index: number; lat: number; lng: number };
/** A chain of vias laid on one day, the legs it clears first and the track it was fitted to. */
type ViaChainInput = {
vias: ViaInput[];
replace_legs?: number[];
track?: { place_id: number; stray_km?: number | null } | null;
};
/** The vias a re-anchoring moves to other legs, and those whose leg is gone. */
type ViaReanchorInput = { vias: { id: number; after_order_index: number }[]; remove?: number[] };
/** Where a dragged via lands, and the leg it now follows when the drag crossed a stop. */
type ViaMove = { lat: number; lng: number; after_order_index?: number };
/**
* Via points: the places a day's drive is made to pass through without stopping.
@@ -30,6 +48,8 @@ import type { RoadtripDayTrack, RoadtripVia, TrekWsPayload, TrekWsTripEventName
*/
@Injectable()
export class RoadtripService {
private readonly logger = new Logger(RoadtripService.name);
constructor(
private readonly realtime: RealtimeService,
private readonly uow: UnitOfWork,
@@ -39,24 +59,98 @@ export class RoadtripService {
@InjectRepository(RoadtripDayTracks) private readonly tracksRepo: RoadtripDayTracksRepository,
) {}
/** The REST caller of a via write: its events leave out the socket that sent the request. */
restWriter(tripId: string, user: User, socketId: string | undefined): TripWriter {
return restTripWriter(this.realtime, tripId, user, socketId);
}
/**
* Tells the trip's other clients what the drive looks like now.
* The via writes below, one use case each for the routes and the tools alike. The caller
* has checked that the day belongs to the trip and that the writer may edit its days; each
* writes, then tells the trip what the day's drive runs through now, after the commit.
*/
async addVia(dayId: string | number, input: ViaInput, writer: TripWriter): Promise<RoadtripVia> {
const via = await this.create(dayId, input);
await this.announceDay(dayId, writer);
return via;
}
/**
* A chain laid on one day, then the track the day follows now: a batch is how a day starts
* following a recorded track, so the rail draws a badge for it and the map a line. The
* track event goes out even when the batch named none, as it always has.
*/
async addVias(
tripId: string | number,
dayId: string | number,
input: ViaChainInput,
writer: TripWriter,
): Promise<RoadtripVia[]> {
const vias = await this.createMany(dayId, input);
await this.announceDay(dayId, writer);
try {
const tracks = await this.tracksForTrip(tripId);
writer.events.emit('roadtripTrack:changed', {
dayId: String(dayId),
track: tracks.find((t) => String(t.day_id) === String(dayId)) ?? null,
});
} catch {
this.logger.warn(`Committed vias for day ${String(dayId)}: track notification failed`);
}
return vias;
}
async reanchorVias(dayId: string | number, input: ViaReanchorInput, writer: TripWriter): Promise<RoadtripVia[]> {
const vias = await this.reanchor(dayId, input);
await this.announceDay(dayId, writer);
return vias;
}
/** Null, and nothing said, when the via is not on this day. */
async moveVia(
dayId: string | number,
id: string | number,
to: ViaMove,
writer: TripWriter,
): Promise<RoadtripVia | null> {
const via = await this.move(id, dayId, to.lat, to.lng, to.after_order_index);
if (via) await this.announceDay(dayId, writer);
return via;
}
/** False, and nothing said, when the via is not on this day. */
async removeVia(dayId: string | number, id: string | number, writer: TripWriter): Promise<boolean> {
const removed = await this.remove(id, dayId);
if (removed) await this.announceDay(dayId, writer);
return removed;
}
/**
* Tells the trip what this day's drive is routed through now.
*
* Two people planning a road trip look at the same line on the same map, and a reshaped
* drive moves every arrival time after it — so this is not "how one person draws their
* drive moves every arrival time after it, so this is not "how one person draws their
* route", which is what it was taken for when these routes were written silent.
*
* The whole day's list goes out rather than the one point that changed: a via carries no
* identity anybody reads, the client holds them per day, and a drag is a burst of writes
* whose only interesting state is the one that lands last.
* The whole day's list goes out, read back rather than assembled from the write: a via
* carries no identity anybody reads, the client holds them per day, a reanchor rewrites
* the set and a batch may clear legs before filling them. REST leaves out the socket that
* made the change, so the person dragging does not get their point handed back mid-drag;
* a tool call has no socket, so the tab that asked hears it like everyone else.
*
* The day id goes out as text, the way the routes have always sent it, on every surface.
* Other senders of this event (assignments, accommodations, hike plans) still send a
* number, as their own routes always did; the client reads it through `Number()` either way.
*
* The write is committed by the time this runs, so a failed read or send is logged and
* swallowed: the caller gets the row it wrote, not a 500 for a change that landed.
*/
broadcast<E extends TrekWsTripEventName>(
tripId: string,
event: E,
payload: TrekWsPayload<E>,
socketId: string | undefined,
): void {
this.realtime.broadcast(tripId, event, payload, socketId);
private async announceDay(dayId: string | number, writer: TripWriter): Promise<void> {
try {
writer.events.emit('roadtripVia:changed', { dayId: String(dayId), vias: await this.listForDay(dayId) });
} catch {
this.logger.warn(`Committed vias for day ${String(dayId)}: realtime notification failed`);
}
}
/**
@@ -110,6 +204,7 @@ export class RoadtripService {
return await this.viasRepo.listForTrip(tripIdNum);
}
/** Silent: writes and tells nobody. Callers that change a trip go through `addVia`. */
async create(
dayId: string | number,
input: { after_order_index: number; lat: number; lng: number; sequence?: number },
@@ -140,6 +235,8 @@ export class RoadtripService {
*
* `replace_legs` clears by leg rather than by day, so vias the traveller placed by hand
* on other legs survive a track being laid on this one.
*
* Silent: writes and tells nobody. Callers that change a trip go through `addVias`.
*/
async createMany(
dayId: string | number,
@@ -202,7 +299,11 @@ export class RoadtripService {
return await this.placesRepo.isTrackInTrip(placeId, tripIdNum);
}
/** Moving a via is the whole edit; where it sits in the chain does not change. */
/**
* Moving a via is the whole edit; where it sits in the chain does not change.
*
* Silent: writes and tells nobody. Callers that change a trip go through `moveVia`.
*/
async move(
id: string | number,
dayId: string | number,
@@ -243,6 +344,8 @@ export class RoadtripService {
* Silent about ids it does not find. The caller computed this list from a snapshot of
* the day, and a via somebody else deleted in the meantime is not a failure of the
* re-anchoring; failing the batch over it would leave the rest of the day mis-pinned.
*
* Silent: writes and tells nobody. Callers that change a trip go through `reanchorVias`.
*/
async reanchor(
dayId: string | number,
@@ -300,6 +403,7 @@ export class RoadtripService {
});
}
/** Silent: writes and tells nobody. Callers that change a trip go through `removeVia`. */
async remove(id: string | number, dayId: string | number): Promise<boolean> {
const idNum = toRowId(id);
const dayIdNum = toRowId(dayId);
@@ -1,34 +1,464 @@
import type { CaseGroup } from './types';
import { CollectionTours } from '../../../src/db/entities/CollectionTours.entity';
import { Places } from '../../../src/db/entities/Places.entity';
import { TourChangeSteps } from '../../../src/db/entities/TourChangeSteps.entity';
import { TourWaypoints } from '../../../src/db/entities/TourWaypoints.entity';
import { Tours } from '../../../src/db/entities/Tours.entity';
import { RoadtripRouterService } from '../../../src/nest/roadtrip/roadtrip-router.service';
import { proposalIdOf } from '../../../src/nest/tours/tour-change-proposal';
import { makePlace } from '../../helpers/factories/places';
import { createRow, deleteRows, updateRows } from '../../helpers/factories/rows';
import { addTourWaypoints, linkTourPacking, makeTour, makeTourActivity } from '../../helpers/factories/tours';
import type { CaseGroup, Scene } from './types';
import { MikroORM } from '@mikro-orm/core';
import type { INestApplication } from '@nestjs/common';
import type { TourChange } from '@trek/shared';
/** Tours and road trips: city walks, hikes and their library, route vias, day boundaries and the Google route import. */
import { vi } from 'vitest';
const ormOf = (app: INestApplication) => app.get(MikroORM);
/**
* The row a case's setup made beyond its scene (a recorded track, a kept
* tour, a saved step), by the scene's number, for the arguments to point at.
*/
const madeFor = new Map<number, number>();
function made(s: Scene): number {
const id = madeFor.get(s.n);
if (id === undefined) throw new Error(`scene ${s.n}: the case's setup made no row to point at`);
return id;
}
/**
* No routing host in a test: the router that plans a city walk's and a
* hike's way on foot fails as a router that is down does, so each is saved
* with its points only, the way both tools save one when the router is away.
*/
function routerDown(app: INestApplication): () => void {
const route = vi.spyOn(app.get(RoadtripRouterService), 'route');
route.mockRejectedValue(new Error('no routing host in the broadcast parity test'));
return () => route.mockRestore();
}
/** A hike's line with its heights, as the planner saves one. */
const HIKE_LINE: [number, number, number][] = [
[47, 11, 600],
[47.01, 11.01, 700],
[47.02, 11.02, 650],
];
/** The planner's save of a hike, for the create and edit routes. */
const hikeBody = (name: string) => ({
name,
route_geometry: HIKE_LINE,
waypoints: [
{ lat: 47, lng: 11, role: 'start', sequence: 0 },
{ lat: 47.02, lng: 11.02, role: 'end', sequence: 1 },
],
max_hiking_difficulty: 2,
duration_seconds: 3600,
});
/** The scene's hike with its line and its start and end, as the planner saves one. */
async function planHike(s: Scene, app: INestApplication): Promise<void> {
const orm = ormOf(app);
await updateRows(orm, Places, { id: s.tourPlace.id }, { route_geometry: JSON.stringify(HIKE_LINE) });
await addTourWaypoints(orm, s.tourPlace.id, [
{ lat: 47, lng: 11 },
{ lat: 47.02, lng: 11.02 },
]);
}
/** The stations of the scene's tour as a city walk: a gate, a market and a park. */
const STATIONS = [
{ lat: 48.1, lng: 11.5, station_key: 'walk-gate', name: 'Gate' },
{ lat: 48.104, lng: 11.5, station_key: 'walk-market', name: 'Market' },
{ lat: 48.108, lng: 11.5, station_key: 'walk-park', name: 'Park' },
];
/** The scene's tour made a city walk of three stations, without a line yet. */
async function planWalk(s: Scene, app: INestApplication): Promise<void> {
const orm = ormOf(app);
await updateRows(orm, Tours, { place: s.tourPlace.id }, { tourTypeRef: 'walk' });
await addTourWaypoints(orm, s.tourPlace.id, STATIONS);
}
/** The market left out of the walk. */
const WITHOUT_MARKET: TourChange[] = [{ op: 'remove', key: 'walk-market' }];
/** The planner's save of the walk from the gate to the park, against the revision it was opened at. */
const walkBody = (revision: number) => ({
name: 'Old town',
tour_type: 'walk',
route_geometry: [
[48.1, 11.5, 520],
[48.108, 11.5, 525],
],
waypoints: [
{ lat: 48.1, lng: 11.5, role: 'start', sequence: 0, key: 'walk-gate', name: 'Gate' },
{ lat: 48.108, lng: 11.5, role: 'end', sequence: 1, key: 'walk-park', name: 'Park' },
],
expected_revision: revision,
});
/**
* The walk after a saved step took the market out: the step at revision 2
* keeps the three stations as they were before it, and the walk holds two.
* Written row by row, since the save that records a step tells the trip.
*/
async function walkWithSavedStep(s: Scene, app: INestApplication): Promise<void> {
const orm = ormOf(app);
await planWalk(s, app);
const before = STATIONS.map(({ station_key, ...station }, sequence) => ({
...station,
key: station_key,
role: sequence === 0 ? 'start' : sequence === STATIONS.length - 1 ? 'end' : 'via',
sequence,
}));
const step = await createRow(orm, TourChangeSteps, {
place: s.tourPlace.id,
proposal_id: `parity-step-${s.n}`,
base_revision: 1,
revision: 2,
changes: JSON.stringify(WITHOUT_MARKET),
before_state: JSON.stringify({
route_geometry: null,
waypoints: before,
distance: null,
duration: null,
elevation_gain: null,
elevation_loss: null,
}),
});
await deleteRows(orm, TourWaypoints, { place: s.tourPlace.id, station_key: 'walk-market' });
await updateRows(orm, Tours, { place: s.tourPlace.id }, { revision: 2 });
madeFor.set(s.n, step.id);
}
/** A recorded track of the scene's trip (a GPX import, not planned over), for the hike to be done with. */
async function recordedTrack(s: Scene, app: INestApplication): Promise<number> {
const orm = ormOf(app);
const track = await makePlace(orm, s.trip.id, {
name: `Recorded ${s.n}`,
lat: 47,
lng: 11,
route_geometry: JSON.stringify(HIKE_LINE),
});
await makeTour(orm, track.id, { origin: 'gpx' });
return track.id;
}
/** The scene's collection keeping a hike of its own, with its line and its two points. */
async function keptTour(s: Scene, app: INestApplication): Promise<void> {
const row = await createRow(ormOf(app), CollectionTours, {
collection: s.collection.id,
name: `Kept ridge ${s.n}`,
route_geometry: JSON.stringify(HIKE_LINE),
waypoints: JSON.stringify(hikeBody('').waypoints),
tourTypeRef: 'hike',
origin: 'trek',
source_trip_title: `Alps ${s.n}`,
source_tour_name: `Ridge ${s.n}`,
});
madeFor.set(s.n, row.id);
}
/** Two stops of a Google Maps directions link, reviewed. */
const GOOGLE_STOPS = [
{ name: 'Brandenburger Tor', lat: 52.5163, lng: 13.3777 },
{ name: 'Alexanderplatz', lat: 52.5219, lng: 13.4132 },
];
/** A GPX file with one track and its heights. */
const gpxOf = (s: Scene) =>
`<gpx><trk><name>Ridge ${s.n}</name><trkseg><trkpt lat="47.6" lon="11.3"><ele>800</ele></trkpt>` +
'<trkpt lat="47.61" lon="11.3"><ele>1000</ele></trkpt></trkseg></trk></gpx>';
const roadtripDay = (s: Scene) => `/api/trips/${s.trip.id}/roadtrip/days/${s.days[0].id}`;
const tourPath = (s: Scene) => `/api/trips/${s.trip.id}/tours/${s.tourPlace.id}`;
/**
* Tours and road trips: city walks, hikes and their library, the tours kept in
* collections, recorded tracks, the packing link, route vias, day boundaries,
* the road trip settings and the Google route import. Paddling, skiing and
* charging plan and read only; they have no write tool.
*/
export const toursRoadtrip: CaseGroup = {
name: 'tours-roadtrip',
prefix: 'BCP-TOURS',
cases: [],
exempt: [],
pending: [
'set_day_boundary',
'import_google_maps_route',
'update_roadtrip_settings',
'add_route_via',
'add_route_vias',
'reanchor_route_vias',
'remove_route_via',
'update_route_via',
'create_city_walk',
'save_tour_to_collection',
'take_over_collection_tour',
'delete_collection_tour',
'assign_tour_track',
'unassign_tour_track',
'set_tour_activity_excluded',
'undo_tour_change',
'save_tour_change',
'take_over_tour',
'link_tour_packing',
'unlink_tour_packing',
'create_hike',
'update_tour',
'import_tour_gpx',
prefix: 'BCP-TOUR',
cases: [
{
tool: 'set_day_boundary',
needs: ['assignment'],
args: (s) => ({
tripId: s.trip.id,
dayNumber: 1,
boundary: { day_number: 1, from_assignment_id: s.assignment.id, to_assignment_id: null, fraction: 1 },
}),
rest: {
method: 'put',
path: (s) => `/api/trips/${s.trip.id}/roadtrip/day-boundaries`,
body: (s) => ({ day_number: 1, from_assignment_id: s.assignment.id, to_assignment_id: null, fraction: 1 }),
},
expect: ['roadtripBoundary:changed'],
},
{
tool: 'import_google_maps_route',
args: (s) => ({ tripId: s.trip.id, dayId: s.days[0].id, stops: GOOGLE_STOPS }),
rest: {
method: 'post',
path: (s) => `/api/trips/${s.trip.id}/roadtrip/google-maps-import`,
body: (s) => ({ dayId: s.days[0].id, stops: GOOGLE_STOPS }),
},
// Each reviewed stop as a place and its visit, in the order of the link.
expect: ['place:created', 'assignment:created', 'place:created', 'assignment:created'],
},
{
tool: 'update_roadtrip_settings',
args: (s) => ({ tripId: s.trip.id, settings: { roadtrip_hotel_bookends: true } }),
rest: {
method: 'put',
path: (s) => `/api/trips/${s.trip.id}/roadtrip/preferences`,
body: () => ({ roadtrip_hotel_bookends: true }),
},
expect: ['roadtripPreferences:changed'],
},
{
// G8: the day id goes out as text on both surfaces now, as the route always sent it.
tool: 'add_route_via',
args: (s) => ({ tripId: s.trip.id, dayId: s.days[0].id, after_order_index: 0, lat: 48.86, lng: 2.34 }),
rest: {
method: 'post',
path: (s) => `${roadtripDay(s)}/vias`,
body: () => ({ after_order_index: 0, lat: 48.86, lng: 2.34 }),
},
expect: ['roadtripVia:changed'],
},
{
tool: 'add_route_vias',
args: (s) => ({
tripId: s.trip.id,
dayId: s.days[0].id,
vias: [{ after_order_index: 0, lat: 48.86, lng: 2.34 }],
}),
rest: {
method: 'post',
path: (s) => `${roadtripDay(s)}/vias/batch`,
body: () => ({ vias: [{ after_order_index: 0, lat: 48.86, lng: 2.34 }] }),
},
// The vias first, then the track the day follows now (none here), as the batch always sent them.
expect: ['roadtripVia:changed', 'roadtripTrack:changed'],
},
{
tool: 'reanchor_route_vias',
needs: ['via'],
args: (s) => ({ tripId: s.trip.id, dayId: s.days[0].id, vias: [{ id: s.via.id, after_order_index: 1 }] }),
rest: {
method: 'put',
path: (s) => `${roadtripDay(s)}/vias`,
body: (s) => ({ vias: [{ id: s.via.id, after_order_index: 1 }] }),
},
expect: ['roadtripVia:changed'],
},
{
tool: 'remove_route_via',
needs: ['via'],
args: (s) => ({ tripId: s.trip.id, dayId: s.days[0].id, viaId: s.via.id }),
rest: { method: 'delete', path: (s) => `${roadtripDay(s)}/vias/${s.via.id}` },
expect: ['roadtripVia:changed'],
},
{
tool: 'update_route_via',
needs: ['via'],
args: (s) => ({ tripId: s.trip.id, dayId: s.days[0].id, viaId: s.via.id, lat: 48.87, lng: 2.35 }),
rest: {
method: 'put',
path: (s) => `${roadtripDay(s)}/vias/${s.via.id}`,
body: () => ({ lat: 48.87, lng: 2.35 }),
},
expect: ['roadtripVia:changed'],
},
{
tool: 'create_city_walk',
args: (s) => ({
tripId: s.trip.id,
name: 'Old town',
stations: [
{ lat: 48.1, lng: 11.5, name: 'Gate' },
{ lat: 48.108, lng: 11.5, name: 'Park' },
],
}),
// The planner saves a new walk through the tours route, with the line it planned in the browser.
rest: { method: 'post', path: (s) => `/api/trips/${s.trip.id}/tours`, body: () => walkBody(1) },
stub: routerDown,
expect: ['tours:changed', 'place:created'],
},
{
tool: 'create_hike',
args: (s) => ({
tripId: s.trip.id,
name: 'Ridge walk',
waypoints: [
{ lat: 47, lng: 11 },
{ lat: 47.02, lng: 11.02 },
],
}),
rest: { method: 'post', path: (s) => `/api/trips/${s.trip.id}/tours`, body: () => hikeBody('Ridge walk') },
stub: routerDown,
expect: ['tours:changed', 'place:created'],
},
{
// Only the name changes, so the hike keeps its way and the router is not asked.
tool: 'update_tour',
needs: ['tour'],
setup: planHike,
args: (s) => ({ tripId: s.trip.id, placeId: s.tourPlace.id, expected_revision: 1, name: 'Renamed ridge' }),
rest: {
method: 'put',
path: tourPath,
body: () => ({ ...hikeBody('Renamed ridge'), expected_revision: 1 }),
},
// Not needed while only the name changes, but a hike edit must never reach a real router.
stub: routerDown,
expect: ['tours:changed', 'place:updated'],
},
{
tool: 'import_tour_gpx',
args: (s) => ({ tripId: s.trip.id, gpx: gpxOf(s), file_name: 'ridge.gpx' }),
// A multipart upload, which the parity harness does not send.
twin: 'POST /api/trips/:tripId/tours/import/gpx with the file',
expect: ['tours:changed', 'place:created'],
},
{
tool: 'save_tour_change',
needs: ['tour'],
setup: planWalk,
args: (s) => ({
tripId: s.trip.id,
placeId: s.tourPlace.id,
changes: WITHOUT_MARKET,
expected_revision: 1,
proposal_id: proposalIdOf({ tripId: s.trip.id, placeId: s.tourPlace.id, revision: 1, changes: WITHOUT_MARKET }),
confirmed: true,
}),
// The planner saves the changed walk through the same route write.
rest: { method: 'put', path: tourPath, body: () => walkBody(1) },
stub: routerDown,
expect: ['tours:changed', 'place:updated'],
},
{
tool: 'undo_tour_change',
needs: ['tour'],
setup: walkWithSavedStep,
args: (s) => ({
tripId: s.trip.id,
placeId: s.tourPlace.id,
step_id: made(s),
expected_revision: 2,
confirmed: true,
}),
// Taking a step back writes the walk as it was through the route write the planner's save uses.
rest: { method: 'put', path: tourPath, body: () => walkBody(2) },
expect: ['tours:changed', 'place:updated'],
},
{
tool: 'take_over_tour',
needs: ['tour'],
setup: planHike,
args: (s) => ({ tripId: s.trip.id, source_trip_id: s.trip.id, source_place_id: s.tourPlace.id }),
rest: {
method: 'post',
path: (s) => `/api/trips/${s.trip.id}/tours/take-over`,
body: (s) => ({ source_trip_id: s.trip.id, source_place_id: s.tourPlace.id }),
},
expect: ['tours:changed', 'place:created'],
},
{
tool: 'assign_tour_track',
needs: ['tour'],
setup: async (s, app) => {
madeFor.set(s.n, await recordedTrack(s, app));
},
args: (s) => ({ tripId: s.trip.id, placeId: s.tourPlace.id, track_place_id: made(s), assignment_id: null }),
rest: {
method: 'put',
path: (s) => `${tourPath(s)}/activities`,
body: (s) => ({ track_place_id: made(s), assignment_id: null }),
},
expect: ['tours:changed'],
},
{
tool: 'unassign_tour_track',
needs: ['tour'],
setup: async (s, app) => {
const activity = await makeTourActivity(ormOf(app), s.tourPlace.id, await recordedTrack(s, app));
madeFor.set(s.n, activity.id);
},
args: (s) => ({ tripId: s.trip.id, placeId: s.tourPlace.id, activityId: made(s) }),
rest: { method: 'delete', path: (s) => `${tourPath(s)}/activities/${made(s)}` },
expect: ['tours:changed'],
},
{
tool: 'link_tour_packing',
needs: ['tour'],
args: (s) => ({ tripId: s.trip.id, placeId: s.tourPlace.id, category: null }),
rest: { method: 'put', path: (s) => `${tourPath(s)}/packing-link`, body: () => ({ category: null }) },
expect: ['tours:changed'],
},
{
tool: 'unlink_tour_packing',
needs: ['tour'],
setup: (s, app) => linkTourPacking(ormOf(app), s.tourPlace.id, null, s.owner.id),
args: (s) => ({ tripId: s.trip.id, placeId: s.tourPlace.id }),
rest: { method: 'delete', path: (s) => `${tourPath(s)}/packing-link` },
expect: ['tours:changed'],
},
{
// A collection is the user's, not the trip's: its people hear it on their own sockets.
tool: 'save_tour_to_collection',
needs: ['collection', 'tour'],
setup: planHike,
args: (s) => ({ collection_id: s.collection.id, source_trip_id: s.trip.id, source_place_id: s.tourPlace.id }),
rest: {
method: 'post',
path: (s) => `/api/addons/collections/${s.collection.id}/tours`,
body: (s) => ({ source_trip_id: s.trip.id, source_place_id: s.tourPlace.id }),
},
expect: [{ event: 'collections:updated', user: 'actor' }],
},
{
tool: 'take_over_collection_tour',
needs: ['collection'],
setup: keptTour,
args: (s) => ({
tripId: s.trip.id,
collection_id: s.collection.id,
collection_tour_id: made(s),
include_waypoints: true,
}),
rest: {
method: 'post',
path: (s) => `/api/trips/${s.trip.id}/tours/from-collection`,
body: (s) => ({ collection_id: s.collection.id, collection_tour_id: made(s), include_waypoints: true }),
},
expect: ['tours:changed', 'place:created'],
},
{
tool: 'delete_collection_tour',
needs: ['collection'],
setup: keptTour,
args: (s) => ({ collection_id: s.collection.id, collection_tour_id: made(s) }),
rest: { method: 'delete', path: (s) => `/api/addons/collections/${s.collection.id}/tours/${made(s)}` },
expect: [{ event: 'collections:updated', user: 'actor' }],
},
],
exempt: [
{
tool: 'set_tour_activity_excluded',
reason:
"the user's own activity sums, read fresh when the tours statistics open; both surfaces are silent, and the registry has no event for them yet",
needs: ['tour'],
args: (s) => ({ place_id: s.tourPlace.id, excluded: true }),
},
],
};
+29 -1
View File
@@ -15,6 +15,7 @@ import { RoadtripPreferences } from '../../../src/db/entities/RoadtripPreference
import { RoadtripVias } from '../../../src/db/entities/RoadtripVias.entity';
import { DEMO_EMAIL_PRIMARY } from '../../../src/nest/common/demo';
import { PermissionsService } from '../../../src/nest/permissions/permissions.service';
import { FakeRealtimeService } from '../../helpers/fake-realtime';
import { createUser, createTrip, createDay, createPlace } from '../../helpers/factories';
import { addTripMember } from '../../helpers/factories';
import { countRows, findRow, updateRows } from '../../helpers/factories/rows';
@@ -31,6 +32,8 @@ vi.mock('../../../src/db/database', async () => {
});
let orm: TestOrm;
// Every tool here broadcasts through this one, so a case can read what went out.
const realtime = new FakeRealtimeService();
// The permissions cache is module-scoped, so a write through any instance is
// what the tool's own check reads back.
@@ -46,6 +49,7 @@ beforeAll(async () => {
beforeEach(() => {
resetTestDb(testDb);
realtime.reset();
// resetTestDb leaves the addons table alone, so each run restates it. The
// addon ships disabled, which is why the tools were invisible before.
setAddonEnabled(testDb, ADDON_IDS.ROADTRIP, true);
@@ -57,7 +61,7 @@ afterAll(async () => {
});
async function withHarness(userId: number, fn: (h: McpHarness) => Promise<void>) {
const h = await createMcpHarness({ userId, withResources: false, scopes: null });
const h = await createMcpHarness({ realtime, userId, withResources: false, scopes: null });
try {
await fn(h);
} finally {
@@ -394,6 +398,30 @@ describe('road-trip MCP tools', () => {
});
});
it('MCP-ROADTRIP-015: adding a via tells the trip once, with the day id as text and no socket left out', async () => {
// G8: the tool used to send the day id as a number where the route sent text. It now runs
// the same use case through the MCP writer, so the payload matches the route's.
const { user, trip, day } = await scenario();
await withHarness(user.id, async (h) => {
const added = await h.client.callTool({
name: 'add_route_via',
arguments: { tripId: trip.id, dayId: day.id, after_order_index: 0, lat: 53, lng: 10 },
});
expect(added.isError).toBeFalsy();
const { via } = parseToolResult(added) as { via: { id: number } };
const sent = realtime.broadcastMock.mock.calls.filter(([, event]) => event === 'roadtripVia:changed');
expect(sent).toHaveLength(1);
// Three arguments: room-wide, with no socket to leave out.
expect(sent[0]).toHaveLength(3);
const [tripId, , payload] = sent[0];
expect(String(tripId)).toBe(String(trip.id));
expect(payload).toMatchObject({ dayId: String(day.id), _source: 'mcp' });
expect((payload.vias as { id: number }[]).map((v) => v.id)).toEqual([via.id]);
});
});
it('MCP-ROADTRIP-009: removing one that is not on the day is refused, not silently ignored', async () => {
const { user, trip, day } = await scenario();
@@ -157,9 +157,18 @@ it('MCP checks demo, trip access and edit permission before saving', async () =>
s.guards.hasTripPermission.mockReturnValue(false);
await s.mcp.save(request, ctx);
expect(await s.service.list(s.tripA.id)).toEqual([]);
// A refused call tells nobody anything.
expect(s.realtime.broadcast).not.toHaveBeenCalled();
s.guards.hasTripPermission.mockReturnValue(true);
await s.mcp.save(request, ctx);
expect(await s.service.list(s.tripA.id)).toEqual([boundary]);
// Clearing a boundary goes through `remove`, which the parity case never runs, so the
// event it sends is pinned here: once, with the list as it is now.
s.realtime.broadcast.mockClear();
await s.mcp.save({ ...request, boundary: null }, ctx);
expect(await s.service.list(s.tripA.id)).toEqual([]);
expect(s.realtime.broadcast).toHaveBeenCalledTimes(1);
expect(s.realtime.broadcast).toHaveBeenCalledWith(String(s.tripA.id), 'roadtripBoundary:changed', {
boundaries: [],
});
});
@@ -496,11 +496,11 @@ describe('Roadtrip MCP registration and search', () => {
it('checks visit editing permission and ownership before moving a via', async () => {
const service = {
dayExists: vi.fn(() => true),
move: vi.fn(() => ({ id: 4 })),
listForDay: () => [],
broadcast: vi.fn(),
moveVia: vi.fn(() => ({ id: 4 })),
};
const guards = { hasTripPermission: vi.fn(() => false) };
// The move and what it tells the trip are the service's; the tool hands it the MCP writer.
const writer = { userId: 5, surface: 'mcp' };
const guards = { hasTripPermission: vi.fn(() => false), tripWriter: vi.fn(async () => writer) };
const mcp = new RoadtripMcp(
service as never,
{ findAccessible: () => true } as never,
@@ -509,15 +509,15 @@ describe('Roadtrip MCP registration and search', () => {
);
const input = { tripId: 1, dayId: 2, viaId: 4, lat: 48, lng: 10 };
await mcp.updateVia(input, ctx);
expect(service.move).not.toHaveBeenCalled();
expect(service.moveVia).not.toHaveBeenCalled();
guards.hasTripPermission.mockReturnValue(true);
service.dayExists.mockReturnValue(false);
await mcp.updateVia(input, ctx);
expect(service.move).not.toHaveBeenCalled();
expect(service.moveVia).not.toHaveBeenCalled();
service.dayExists.mockReturnValue(true);
await mcp.updateVia(input, ctx);
expect(service.move).toHaveBeenCalledWith(4, 2, 48, 10, undefined);
expect(service.broadcast).toHaveBeenCalled();
expect(guards.tripWriter).toHaveBeenCalledWith(1, 5);
expect(service.moveVia).toHaveBeenCalledWith(2, 4, { lat: 48, lng: 10, after_order_index: undefined }, writer);
});
});
@@ -534,15 +534,14 @@ describe('RoadtripMcp — non-member refusal, one case per tool (Plan 3d Task 7
listForTrip: vi.fn(async () => []),
tracksForTrip: vi.fn(async () => []),
trackExists: vi.fn(async () => true),
create: vi.fn(async () => ({ id: 1 })),
createMany: vi.fn(async () => []),
reanchor: vi.fn(async () => []),
remove: vi.fn(async () => true),
move: vi.fn(async () => ({ id: 1 })),
broadcast: vi.fn(),
addVia: vi.fn(async () => ({ id: 1 })),
addVias: vi.fn(async () => []),
reanchorVias: vi.fn(async () => []),
removeVia: vi.fn(async () => true),
moveVia: vi.fn(async () => ({ id: 1 })),
};
const tripsRepo = { findAccessible: vi.fn(async () => findAccessible) };
const guards = { hasTripPermission: vi.fn(async () => true) };
const guards = { hasTripPermission: vi.fn(async () => true), tripWriter: vi.fn(async () => ({ userId: 5 })) };
const mcp = new RoadtripMcp(service as never, tripsRepo as never, guards as never, {} as never);
return { mcp, service, tripsRepo };
}
@@ -559,28 +558,28 @@ describe('RoadtripMcp — non-member refusal, one case per tool (Plan 3d Task 7
const { mcp, service } = build(false);
const res = await mcp.addVia({ tripId: 10, dayId: 1, after_order_index: 0, lat: 48, lng: 10 }, ctx);
expect(res.isError).toBe(true);
expect(service.create).not.toHaveBeenCalled();
expect(service.addVia).not.toHaveBeenCalled();
});
it('add_route_vias refuses a non-member trip', async () => {
const { mcp, service } = build(false);
const res = await mcp.addVias({ tripId: 10, dayId: 1, vias: [] }, ctx);
expect(res.isError).toBe(true);
expect(service.createMany).not.toHaveBeenCalled();
expect(service.addVias).not.toHaveBeenCalled();
});
it('reanchor_route_vias refuses a non-member trip', async () => {
const { mcp, service } = build(false);
const res = await mcp.reanchorVias({ tripId: 10, dayId: 1, vias: [] }, ctx);
expect(res.isError).toBe(true);
expect(service.reanchor).not.toHaveBeenCalled();
expect(service.reanchorVias).not.toHaveBeenCalled();
});
it('remove_route_via refuses a non-member trip', async () => {
const { mcp, service } = build(false);
const res = await mcp.removeVia({ tripId: 10, dayId: 1, viaId: 4 }, ctx);
expect(res.isError).toBe(true);
expect(service.remove).not.toHaveBeenCalled();
expect(service.removeVia).not.toHaveBeenCalled();
});
});
@@ -13,18 +13,27 @@
* AddonGuard again. `@RequireAddon` is metadata and does nothing on its own, so
* dropping the guard from the list is a silent regression that opens all seven
* routes on an instance that switched the addon off.
*
* What a write tells the trip is the service's (RoadtripService.addVia and its
* siblings, pinned in roadtrip.service.test.ts); here only that every write
* hands it the REST writer of this trip, user and socket.
*/
import { AddonGuard } from '../../../src/nest/addons/addon.guard';
import { JwtAuthGuard } from '../../../src/nest/auth-core/jwt-auth.guard';
import type { TripWriter } from '../../../src/nest/common/trip-writer';
import { TripAccessGuard } from '../../../src/nest/permissions/trip-access.guard';
import { RoadtripController } from '../../../src/nest/roadtrip/roadtrip.controller';
import type { RoadtripService } from '../../../src/nest/roadtrip/roadtrip.service';
import type { User } from '../../../src/types';
import { HttpException } from '@nestjs/common';
import { GUARDS_METADATA } from '@nestjs/common/constants';
import { describe, it, expect, vi } from 'vitest';
const VIA = { id: 5, day_id: 4, after_order_index: 0, sequence: 0, lat: 53, lng: 10 };
const USER = { id: 3, role: 'user' } as User;
/** The writer restWriter hands back, as the use cases receive it. */
const WRITER = { userId: 3, role: 'user', surface: 'rest' } as TripWriter;
function svc(o: Partial<RoadtripService> = {}): RoadtripService {
return {
@@ -33,13 +42,12 @@ function svc(o: Partial<RoadtripService> = {}): RoadtripService {
listForDay: vi.fn().mockReturnValue([VIA]),
listForTrip: vi.fn().mockReturnValue([VIA]),
tracksForTrip: vi.fn().mockReturnValue([]),
create: vi.fn().mockReturnValue(VIA),
createMany: vi.fn().mockReturnValue([VIA]),
reanchor: vi.fn().mockReturnValue([VIA]),
move: vi.fn().mockReturnValue(VIA),
remove: vi.fn().mockReturnValue(true),
// Every write announces the day's new shape to the trip's other clients.
broadcast: vi.fn(),
restWriter: vi.fn().mockReturnValue(WRITER),
addVia: vi.fn().mockReturnValue(VIA),
addVias: vi.fn().mockReturnValue([VIA]),
reanchorVias: vi.fn().mockReturnValue([VIA]),
moveVia: vi.fn().mockReturnValue(VIA),
removeVia: vi.fn().mockReturnValue(true),
...o,
} as unknown as RoadtripService;
}
@@ -101,9 +109,9 @@ describe('RoadtripController — writes', () => {
it('ROADTRIP-CTL-005: creating one checks the day first', async () => {
const s = svc();
expect(await new RoadtripController(s).create('7', '4', body)).toEqual({ via: VIA });
expect(await new RoadtripController(s).create(USER, '7', '4', body)).toEqual({ via: VIA });
expect(s.dayExists).toHaveBeenCalledWith('4', '7');
expect(s.create).toHaveBeenCalledWith('4', body);
expect(s.addVia).toHaveBeenCalledWith('4', body, WRITER);
});
it('ROADTRIP-CTL-006: every write refuses a day from another trip', async () => {
@@ -111,25 +119,25 @@ describe('RoadtripController — writes', () => {
const c = new RoadtripController(s);
const notFound = { status: 404, body: { error: 'Day not found' } };
expect(await thrown(() => c.create('7', '999', body))).toEqual(notFound);
expect(await thrown(() => c.createMany('7', '999', { vias: [body] }))).toEqual(notFound);
expect(await thrown(() => c.reanchor('7', '999', { vias: [] }))).toEqual(notFound);
expect(await thrown(() => c.update('7', '999', '5', { lat: 1, lng: 2 }))).toEqual(notFound);
expect(await thrown(() => c.remove('7', '999', '5'))).toEqual(notFound);
expect(await thrown(() => c.create(USER, '7', '999', body))).toEqual(notFound);
expect(await thrown(() => c.createMany(USER, '7', '999', { vias: [body] }))).toEqual(notFound);
expect(await thrown(() => c.reanchor(USER, '7', '999', { vias: [] }))).toEqual(notFound);
expect(await thrown(() => c.update(USER, '7', '999', '5', { lat: 1, lng: 2 }))).toEqual(notFound);
expect(await thrown(() => c.remove(USER, '7', '999', '5'))).toEqual(notFound);
// None of them reached the service.
expect(s.create).not.toHaveBeenCalled();
expect(s.createMany).not.toHaveBeenCalled();
expect(s.reanchor).not.toHaveBeenCalled();
expect(s.move).not.toHaveBeenCalled();
expect(s.remove).not.toHaveBeenCalled();
expect(s.addVia).not.toHaveBeenCalled();
expect(s.addVias).not.toHaveBeenCalled();
expect(s.reanchorVias).not.toHaveBeenCalled();
expect(s.moveVia).not.toHaveBeenCalled();
expect(s.removeVia).not.toHaveBeenCalled();
});
it('ROADTRIP-CTL-007: a batch lays the whole chain in one call', async () => {
const s = svc();
const batch = { vias: [body, body], replace_legs: [0] };
expect(await new RoadtripController(s).createMany('7', '4', batch)).toEqual({ vias: [VIA] });
expect(s.createMany).toHaveBeenCalledWith('4', batch);
expect(await new RoadtripController(s).createMany(USER, '7', '4', batch)).toEqual({ vias: [VIA] });
expect(s.addVias).toHaveBeenCalledWith('7', '4', batch, WRITER);
});
it('ROADTRIP-CTL-008: a track from another trip cannot become this day label', async () => {
@@ -138,14 +146,16 @@ describe('RoadtripController — writes', () => {
// would become a label that can never be drawn.
const s = svc({ trackExists: vi.fn().mockReturnValue(false) });
expect(
await thrown(() => new RoadtripController(s).createMany('7', '4', { vias: [body], track: { place_id: 3 } })),
await thrown(() =>
new RoadtripController(s).createMany(USER, '7', '4', { vias: [body], track: { place_id: 3 } }),
),
).toEqual({ status: 404, body: { error: 'Track not found' } });
expect(s.createMany).not.toHaveBeenCalled();
expect(s.addVias).not.toHaveBeenCalled();
});
it('ROADTRIP-CTL-009: the track check runs against the trip, not the day', async () => {
const s = svc();
await new RoadtripController(s).createMany('7', '4', { vias: [body], track: { place_id: 3 } });
await new RoadtripController(s).createMany(USER, '7', '4', { vias: [body], track: { place_id: 3 } });
expect(s.trackExists).toHaveBeenCalledWith(3, '7');
});
@@ -154,7 +164,7 @@ describe('RoadtripController — writes', () => {
// check the existence of. Treating it as one would refuse the only way to
// detach a track.
const s = svc({ trackExists: vi.fn().mockReturnValue(false) });
expect(await new RoadtripController(s).createMany('7', '4', { vias: [body], track: null })).toEqual({
expect(await new RoadtripController(s).createMany(USER, '7', '4', { vias: [body], track: null })).toEqual({
vias: [VIA],
});
expect(s.trackExists).not.toHaveBeenCalled();
@@ -163,27 +173,27 @@ describe('RoadtripController — writes', () => {
it('ROADTRIP-CTL-011: re-anchoring goes through as one batch', async () => {
const s = svc();
const plan = { vias: [{ id: 5, after_order_index: 1 }], remove: [6] };
expect(await new RoadtripController(s).reanchor('7', '4', plan)).toEqual({ vias: [VIA] });
expect(s.reanchor).toHaveBeenCalledWith('4', plan);
expect(await new RoadtripController(s).reanchor(USER, '7', '4', plan)).toEqual({ vias: [VIA] });
expect(s.reanchorVias).toHaveBeenCalledWith('4', plan, WRITER);
});
it('ROADTRIP-CTL-012: moving a via reports where it went', async () => {
const s = svc();
expect(await new RoadtripController(s).update('7', '4', '5', { lat: 54, lng: 11 })).toEqual({ via: VIA });
// Undefined anchor, explicitly: a drag that stayed between the same two stops sends
// no new one, and the service must leave the existing pin alone rather than clear it.
expect(s.move).toHaveBeenCalledWith('5', '4', 54, 11, undefined);
expect(await new RoadtripController(s).update(USER, '7', '4', '5', { lat: 54, lng: 11 })).toEqual({ via: VIA });
// No anchor, explicitly: a drag that stayed between the same two stops sends no new
// one, and the service must leave the existing pin alone rather than clear it.
expect(s.moveVia).toHaveBeenCalledWith('4', '5', { lat: 54, lng: 11 }, WRITER);
});
it('ROADTRIP-CTL-015: a new anchor is passed on, so a via dragged past a stop is re-pinned', async () => {
const s = svc();
await new RoadtripController(s).update('7', '4', '5', { lat: 54, lng: 11, after_order_index: 2 });
expect(s.move).toHaveBeenCalledWith('5', '4', 54, 11, 2);
await new RoadtripController(s).update(USER, '7', '4', '5', { lat: 54, lng: 11, after_order_index: 2 });
expect(s.moveVia).toHaveBeenCalledWith('4', '5', { lat: 54, lng: 11, after_order_index: 2 }, WRITER);
});
it('ROADTRIP-CTL-013: a via that is not on this day is a 404, not a silent no-op', async () => {
const s = svc({ move: vi.fn().mockReturnValue(null) });
expect(await thrown(() => new RoadtripController(s).update('7', '4', '999', { lat: 1, lng: 2 }))).toEqual({
const s = svc({ moveVia: vi.fn().mockReturnValue(null) });
expect(await thrown(() => new RoadtripController(s).update(USER, '7', '4', '999', { lat: 1, lng: 2 }))).toEqual({
status: 404,
body: { error: 'Via not found' },
});
@@ -191,13 +201,13 @@ describe('RoadtripController — writes', () => {
it('ROADTRIP-CTL-014: removing one reports success', async () => {
const s = svc();
expect(await new RoadtripController(s).remove('7', '4', '5')).toEqual({ success: true });
expect(s.remove).toHaveBeenCalledWith('5', '4');
expect(await new RoadtripController(s).remove(USER, '7', '4', '5')).toEqual({ success: true });
expect(s.removeVia).toHaveBeenCalledWith('4', '5', WRITER);
});
it('ROADTRIP-CTL-015: removing a via that is not there is a 404', async () => {
const s = svc({ remove: vi.fn().mockReturnValue(false) });
expect(await thrown(() => new RoadtripController(s).remove('7', '4', '999'))).toEqual({
it('ROADTRIP-CTL-016: removing a via that is not there is a 404', async () => {
const s = svc({ removeVia: vi.fn().mockReturnValue(false) });
expect(await thrown(() => new RoadtripController(s).remove(USER, '7', '4', '999'))).toEqual({
status: 404,
body: { error: 'Via not found' },
});
@@ -206,43 +216,28 @@ describe('RoadtripController — writes', () => {
/**
* Two people planning one road trip look at the same line on the same map, and a
* reshaped drive moves every arrival time after it. These routes used to write in
* silence, which left the other side reading a route nobody could see change.
* silence, which left the other side reading a route nobody could see change. What
* each write sends is the service's; the route decides who sends it.
*/
describe('telling the trip', () => {
it('SRV-ROADTRIP-020: every write announces the new shape of the day', async () => {
const cases: [string, (c: RoadtripController) => unknown][] = [
['create', (c) => c.create('7', '4', { after_order_index: 0, lat: 1, lng: 2 }, 'sock')],
['createMany', (c) => c.createMany('7', '4', { vias: [{ after_order_index: 0, lat: 1, lng: 2 }] }, 'sock')],
['reanchor', (c) => c.reanchor('7', '4', { vias: [] }, 'sock')],
['update', (c) => c.update('7', '4', '9', { lat: 1, lng: 2 }, 'sock')],
['remove', (c) => c.remove('7', '4', '9', 'sock')],
it('SRV-ROADTRIP-020: every write hands its use case the writer of this trip, user and socket', async () => {
const cases: [string, keyof RoadtripService, (c: RoadtripController) => unknown][] = [
['create', 'addVia', (c) => c.create(USER, '7', '4', { after_order_index: 0, lat: 1, lng: 2 }, 'sock')],
[
'createMany',
'addVias',
(c) => c.createMany(USER, '7', '4', { vias: [{ after_order_index: 0, lat: 1, lng: 2 }] }, 'sock'),
],
['reanchor', 'reanchorVias', (c) => c.reanchor(USER, '7', '4', { vias: [] }, 'sock')],
['update', 'moveVia', (c) => c.update(USER, '7', '4', '9', { lat: 1, lng: 2 }, 'sock')],
['remove', 'removeVia', (c) => c.remove(USER, '7', '4', '9', 'sock')],
];
for (const [name, run] of cases) {
for (const [name, useCase, run] of cases) {
const s = svc();
await run(new RoadtripController(s));
expect(s.broadcast, name).toHaveBeenCalledWith('7', 'roadtripVia:changed', { dayId: '4', vias: [VIA] }, 'sock');
expect(s.restWriter, name).toHaveBeenCalledWith('7', USER, 'sock');
expect(vi.mocked(s[useCase]).mock.lastCall?.at(-1), name).toBe(WRITER);
}
});
it('SRV-ROADTRIP-021: the day list is read back, not assembled from the write', async () => {
// A reanchor rewrites the whole set and a batch may clear legs before filling them,
// so what the write returned is not what the day now holds.
const s = svc({ reanchor: vi.fn().mockReturnValue([]) } as Partial<RoadtripService>);
await new RoadtripController(s).reanchor('7', '4', { vias: [] }, 'sock');
expect(s.broadcast).toHaveBeenCalledWith('7', 'roadtripVia:changed', { dayId: '4', vias: [VIA] }, 'sock');
});
it('SRV-ROADTRIP-022: laying a track down says which track the day now follows', async () => {
const track = { day_id: 4, place_id: 3, name: 'B96' };
const s = svc({ tracksForTrip: vi.fn().mockReturnValue([track]) } as Partial<RoadtripService>);
await new RoadtripController(s).createMany('7', '4', { vias: [], track: { place_id: 3 } }, 'sock');
expect(s.broadcast).toHaveBeenCalledWith('7', 'roadtripTrack:changed', { dayId: '4', track }, 'sock');
});
it('SRV-ROADTRIP-023: a refused write announces nothing', async () => {
const s = svc({ remove: vi.fn().mockReturnValue(false) } as Partial<RoadtripService>);
await expect(new RoadtripController(s).remove('7', '4', '9', 'sock')).rejects.toThrow();
expect(s.broadcast).not.toHaveBeenCalled();
});
});
});
+127 -3
View File
@@ -2,15 +2,18 @@ import { Days } from '../../../src/db/entities/Days.entity';
import { Places } from '../../../src/db/entities/Places.entity';
import { RoadtripDayTracks } from '../../../src/db/entities/RoadtripDayTracks.entity';
import { RoadtripVias } from '../../../src/db/entities/RoadtripVias.entity';
import type { TripWriter } from '../../../src/nest/common/trip-writer';
import { RoadtripService } from '../../../src/nest/roadtrip/roadtrip.service';
import type { User } from '../../../src/types';
import { createSnapshotTestDb } from '../../helpers/db-mock';
import { createDay, createPlace, createTrip, createUser } from '../../helpers/factories';
import { updateRows } from '../../helpers/factories/rows';
import { resetTestDb } from '../../helpers/test-db';
import { createTestOrm, type TestOrm } from '../../helpers/test-orm';
import { createTestUnitOfWork } from '../../helpers/test-uow';
import type { RoadtripVia } from '@trek/shared';
import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest';
import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest';
/**
* Via points and tracks, against real rows through
@@ -32,9 +35,9 @@ afterAll(async () => {
testDb.close();
});
async function makeService() {
async function makeService(realtime: { broadcast: (...args: unknown[]) => void } = { broadcast: () => {} }) {
return new RoadtripService(
{ broadcast: () => {} } as never,
realtime as never,
await createTestUnitOfWork(testDb),
t.repo(Days),
t.repo(Places),
@@ -374,3 +377,124 @@ describe('RoadtripService', () => {
expect(rows.map((r) => r.sequence).sort((a, b) => a - b)).toEqual([0, 1]);
});
});
interface Sent {
event: string;
payload: { dayId?: unknown; vias?: RoadtripVia[]; track?: unknown };
all: boolean;
}
/** A writer that keeps what a use case sends, in order, as a TripEventSink receives it. */
function recordingWriter(): { sent: Sent[]; writer: TripWriter } {
const sent: Sent[] = [];
const keep = (event: string, payload: unknown, all: boolean) => {
sent.push({ event, payload: payload as Sent['payload'], all });
};
const writer: TripWriter = {
userId: 1,
role: 'user',
surface: 'mcp',
events: {
emit: (event, payload) => keep(event, payload, false),
emitAll: (event, payload) => keep(event, payload, true),
},
};
return { sent, writer };
}
/**
* Two people planning one road trip look at the same line on the same map, and a reshaped
* drive moves every arrival time after it. Each via write is one use case for the routes
* and the tools alike, and tells the trip what the day runs through now once it is written.
*/
describe('RoadtripService: telling the trip', () => {
let service: RoadtripService;
let f: Awaited<ReturnType<typeof fixture>>;
beforeEach(async () => {
service = await makeService();
f = await fixture();
});
it('SRV-ROADTRIP-021: every via write sends the day read back after it, with the day id as text', async () => {
const { sent, writer } = recordingWriter();
const first = await service.addVia(f.dayA1.id, { after_order_index: 0, lat: 53, lng: 10 }, writer);
const second = await service.addVia(String(f.dayA1.id), { after_order_index: 1, lat: 54, lng: 11 }, writer);
const moved = await service.moveVia(f.dayA1.id, first.id, { lat: 53.5, lng: 10.5 }, writer);
// The reanchor answers nothing it was asked to move: what goes out is the day as it is now.
await service.reanchorVias(f.dayA1.id, { vias: [], remove: [second.id] }, writer);
await service.removeVia(f.dayA1.id, first.id, writer);
expect(sent.map((s) => s.event)).toEqual(Array(5).fill('roadtripVia:changed'));
expect(sent.some((s) => s.all)).toBe(false);
// G8: the routes always sent the day id as the path spelled it; a tool sends it the same way.
expect(sent.map((s) => s.payload.dayId)).toEqual(Array(5).fill(String(f.dayA1.id)));
expect(sent.map((s) => s.payload.vias?.map((v) => v.id))).toEqual([
[first.id],
[first.id, second.id],
[first.id, second.id],
[first.id],
[],
]);
expect(sent[2]?.payload.vias?.[0]).toEqual(moved);
});
it('SRV-ROADTRIP-022: a chain says which track the day follows now, after its vias', async () => {
const { sent, writer } = recordingWriter();
const chain = { vias: [{ after_order_index: 0, lat: 53, lng: 10 }], track: { place_id: f.track.id, stray_km: 2 } };
const made = await service.addVias(f.tripA.id, f.dayA1.id, chain, writer);
// Left out, the track stays; the event still says which one the day follows.
await service.addVias(f.tripA.id, f.dayA1.id, { vias: [] }, writer);
await service.addVias(f.tripA.id, f.dayA1.id, { vias: [], track: null }, writer);
const followed = { day_id: f.dayA1.id, place_id: f.track.id, stray_km: 2 };
expect(sent.map((s) => [s.event, s.payload])).toEqual([
['roadtripVia:changed', { dayId: String(f.dayA1.id), vias: made }],
['roadtripTrack:changed', { dayId: String(f.dayA1.id), track: followed }],
['roadtripVia:changed', { dayId: String(f.dayA1.id), vias: made }],
['roadtripTrack:changed', { dayId: String(f.dayA1.id), track: followed }],
['roadtripVia:changed', { dayId: String(f.dayA1.id), vias: made }],
['roadtripTrack:changed', { dayId: String(f.dayA1.id), track: null }],
]);
});
it('SRV-ROADTRIP-023: a via that is not on the day changes nothing and tells nobody', async () => {
const { sent, writer } = recordingWriter();
const other = await service.create(f.dayA2.id, { after_order_index: 0, lat: 53, lng: 10 });
expect(await service.moveVia(f.dayA1.id, other.id, { lat: 1, lng: 2 }, writer)).toBeNull();
expect(await service.removeVia(f.dayA1.id, other.id, writer)).toBe(false);
expect(sent).toEqual([]);
expect(await service.listForDay(f.dayA2.id)).toEqual([other]);
});
it('SRV-ROADTRIP-024: the REST writer sends to the trip and leaves out the socket that made the change', async () => {
const calls: unknown[][] = [];
const rest = await makeService({ broadcast: (...args) => calls.push(args) });
const user = { id: 1, role: 'user' } as User;
const tripId = String(f.tripA.id);
const dayId = String(f.dayA1.id);
const via = await rest.addVia(
dayId,
{ after_order_index: 0, lat: 53, lng: 10 },
rest.restWriter(tripId, user, 'sock'),
);
expect(calls).toEqual([[tripId, 'roadtripVia:changed', { dayId, vias: [via] }, 'sock']]);
});
it('SRV-ROADTRIP-025: a read that fails after the commit is logged, and the write still answers', async () => {
const { sent, writer } = recordingWriter();
vi.spyOn(service, 'listForDay').mockRejectedValueOnce(new Error('read failed'));
const via = await service.addVia(f.dayA1.id, { after_order_index: 0, lat: 53, lng: 10 }, writer);
expect(sent).toEqual([]);
// The chain's track read fails on its own: the via event has gone, the track event is skipped.
vi.spyOn(service, 'tracksForTrip').mockRejectedValueOnce(new Error('read failed'));
const vias = await service.addVias(f.tripA.id, f.dayA1.id, { vias: [] }, writer);
expect(sent.map((s) => s.event)).toEqual(['roadtripVia:changed']);
expect(vias).toEqual([via]);
expect(await service.listForDay(f.dayA1.id)).toEqual([via]);
});
});
+6 -1
View File
@@ -110,6 +110,10 @@ export const TREK_WS_EVENTS = {
// rather than a change to the itinerary everybody reads. That does not hold: two people
// planning a road trip see the same line on the same map, and a reshaped drive changes
// every arrival time after it.
//
// The via writes send dayId as text, the way the routes always did, on every surface.
// Assignments, accommodations and hike plans still send it as a number; the client
// reads it through Number(), so both forms stay valid here.
'roadtripVia:changed': {
scope: 'trip',
payload: z.object({ dayId: id, vias: z.array(entity) }),
@@ -198,7 +202,8 @@ export const TREK_WS_EVENTS = {
payload: z.union([z.object({ reservation: entity }), empty]),
},
'reservation:deleted': { scope: 'trip', payload: z.object({ reservationId: id }) },
// DRIFT: REST sends { positions, day_id }; the MCP tool sends { positions, dayId }.
// REST sends { positions, day_id }; the MCP tool sends { positions, day_id, dayId },
// keeping the dayId key it always sent next to REST's.
'reservation:positions': {
scope: 'trip',
payload: z.union([