mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
feat(roadtrip): show every change to a road trip live to everyone on it
Two people shaping one drive were each editing their own copy: a via dragged on one screen reached the other only on the next reload, and the route it changed looked settled until then. Every write in the domain now announces itself over the websocket the rest of the trip already uses — the five REST routes and the matching MCP tools alike, so an assistant moving a stop is as visible as a person doing it. Two new trip-scoped events carry it: roadtripVia:changed with the day's whole via list, and roadtripTrack:changed. The whole list rather than a delta, because a via is identified by its position in a chain and a delta protocol would need an ordering guarantee to be worth anything. The originating socket is excluded through the X-Socket-Id header the other domains already forward, so nobody watches their own change arrive twice.
This commit is contained in:
@@ -30,6 +30,11 @@ export const HANDLED_OUTSIDE_TRIP_STORE = [
|
||||
'collab:message:created',
|
||||
'collab:message:reacted',
|
||||
'collab:message:deleted',
|
||||
// Road trip shaping — components/Roadtrip/useRoadtripVias. The points a drive is
|
||||
// routed through live in that hook and nowhere else; a store slice for them would be a
|
||||
// second copy of the same list to keep in step.
|
||||
'roadtripVia:changed',
|
||||
'roadtripTrack:changed',
|
||||
// In-app notifications — hooks/useInAppNotificationListener
|
||||
'notification:new',
|
||||
'notification:updated',
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { Body, Controller, Delete, Get, HttpCode, HttpException, Param, Post, Put, UseGuards } from '@nestjs/common';
|
||||
import { Body, Controller, Delete, Get, Headers, HttpCode, HttpException, Param, Post, Put, UseGuards } from '@nestjs/common';
|
||||
import type { RoadtripDayTrack, RoadtripVia } from '@trek/shared';
|
||||
import { RoadtripService } from './roadtrip.service';
|
||||
import { RoadtripViaBatchDto, RoadtripViaCreateDto, RoadtripViaReanchorDto, RoadtripViaUpdateDto } from './roadtrip.dto';
|
||||
@@ -14,11 +14,12 @@ import { ADDON_IDS } from '../../addons';
|
||||
* Gated on the road trip addon: an instance with it switched off gets 404s here, the same
|
||||
* as every other addon surface, and nothing about the plain day plan changes.
|
||||
*
|
||||
* No WebSocket broadcast, unlike the assignment routes. A via is a detail of how one
|
||||
* person's route is drawn rather than a change to the itinerary everybody reads, and
|
||||
* adding an event family for it would mean extending the typed trip-event catalogue for
|
||||
* something a collaborator sees on their next load anyway. Worth revisiting if vias ever
|
||||
* become shared editing rather than route shaping.
|
||||
* Every write here broadcasts, like the assignment routes. This used to be silent, on
|
||||
* the reasoning that a via is how one person draws their route rather than a change to
|
||||
* the itinerary everybody reads. That does not survive contact with two people planning
|
||||
* 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.
|
||||
*/
|
||||
@Controller('api/trips/:tripId/roadtrip')
|
||||
// AddonGuard FIRST: @RequireAddon is metadata and does nothing on its own, and the guard
|
||||
@@ -48,9 +49,12 @@ export class RoadtripController {
|
||||
@Param('tripId') tripId: string,
|
||||
@Param('dayId') dayId: string,
|
||||
@Body() body: RoadtripViaCreateDto,
|
||||
@Headers('x-socket-id') socketId?: string,
|
||||
): { via: RoadtripVia } {
|
||||
this.requireDay(dayId, tripId);
|
||||
return { via: this.roadtrip.create(dayId, body) };
|
||||
const via = this.roadtrip.create(dayId, body);
|
||||
this.announce(tripId, dayId, socketId);
|
||||
return { via };
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -68,6 +72,7 @@ export class RoadtripController {
|
||||
@Param('tripId') tripId: string,
|
||||
@Param('dayId') dayId: string,
|
||||
@Body() body: RoadtripViaBatchDto,
|
||||
@Headers('x-socket-id') socketId?: string,
|
||||
): { vias: RoadtripVia[] } {
|
||||
this.requireDay(dayId, tripId);
|
||||
// Permission is not enough on its own: a place id from somebody else's trip would
|
||||
@@ -76,7 +81,15 @@ export class RoadtripController {
|
||||
if (body.track && !this.roadtrip.trackExists(body.track.place_id, tripId)) {
|
||||
throw new HttpException({ error: 'Track not found' }, 404);
|
||||
}
|
||||
return { vias: this.roadtrip.createMany(dayId, body) };
|
||||
const vias = this.roadtrip.createMany(dayId, body);
|
||||
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.
|
||||
this.roadtrip.broadcast(tripId, 'roadtripTrack:changed', {
|
||||
dayId,
|
||||
track: this.roadtrip.tracksForTrip(tripId).find(t => String(t.day_id) === String(dayId)) ?? null,
|
||||
}, socketId);
|
||||
return { vias };
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -93,9 +106,12 @@ export class RoadtripController {
|
||||
@Param('tripId') tripId: string,
|
||||
@Param('dayId') dayId: string,
|
||||
@Body() body: RoadtripViaReanchorDto,
|
||||
@Headers('x-socket-id') socketId?: string,
|
||||
): { vias: RoadtripVia[] } {
|
||||
this.requireDay(dayId, tripId);
|
||||
return { vias: this.roadtrip.reanchor(dayId, body) };
|
||||
const vias = this.roadtrip.reanchor(dayId, body);
|
||||
this.announce(tripId, dayId, socketId);
|
||||
return { vias };
|
||||
}
|
||||
|
||||
@RequirePermission('day_edit')
|
||||
@@ -106,10 +122,12 @@ export class RoadtripController {
|
||||
@Param('dayId') dayId: string,
|
||||
@Param('id') id: string,
|
||||
@Body() body: RoadtripViaUpdateDto,
|
||||
@Headers('x-socket-id') socketId?: string,
|
||||
): { via: RoadtripVia } {
|
||||
this.requireDay(dayId, tripId);
|
||||
const via = this.roadtrip.move(id, dayId, body.lat, body.lng, body.after_order_index);
|
||||
if (!via) throw new HttpException({ error: 'Via not found' }, 404);
|
||||
this.announce(tripId, dayId, socketId);
|
||||
return { via };
|
||||
}
|
||||
|
||||
@@ -119,14 +137,31 @@ export class RoadtripController {
|
||||
@Param('tripId') tripId: string,
|
||||
@Param('dayId') dayId: string,
|
||||
@Param('id') id: string,
|
||||
@Headers('x-socket-id') socketId?: string,
|
||||
): { success: true } {
|
||||
this.requireDay(dayId, tripId);
|
||||
if (!this.roadtrip.remove(id, dayId)) {
|
||||
throw new HttpException({ error: 'Via not found' }, 404);
|
||||
}
|
||||
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 announce(tripId: string, dayId: string, socketId: string | undefined): void {
|
||||
this.roadtrip.broadcast(tripId, 'roadtripVia:changed', {
|
||||
dayId,
|
||||
vias: 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
|
||||
|
||||
@@ -77,7 +77,9 @@ export class RoadtripMcp {
|
||||
if (!this.db.canAccessTrip(tripId, ctx.userId)) return noAccess();
|
||||
if (!this.guards.hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied();
|
||||
if (!this.roadtrip.dayExists(dayId, tripId)) return noAccess();
|
||||
return ok({ via: this.roadtrip.create(dayId, { after_order_index, lat, lng }) });
|
||||
const via = this.roadtrip.create(dayId, { after_order_index, lat, lng });
|
||||
this.announce(tripId, dayId);
|
||||
return ok({ via });
|
||||
}
|
||||
|
||||
@Tool({
|
||||
@@ -121,7 +123,13 @@ 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 && !this.roadtrip.trackExists(track.place_id, tripId)) return noAccess();
|
||||
return ok({ vias: this.roadtrip.createMany(dayId, { vias, replace_legs, track }) });
|
||||
const made = this.roadtrip.createMany(dayId, { vias, replace_legs, track });
|
||||
this.announce(tripId, dayId);
|
||||
this.roadtrip.broadcast(String(tripId), 'roadtripTrack:changed', {
|
||||
dayId,
|
||||
track: this.roadtrip.tracksForTrip(String(tripId)).find(t => String(t.day_id) === String(dayId)) ?? null,
|
||||
}, undefined);
|
||||
return ok({ vias: made });
|
||||
}
|
||||
|
||||
@Tool({
|
||||
@@ -149,7 +157,9 @@ export class RoadtripMcp {
|
||||
if (!this.db.canAccessTrip(tripId, ctx.userId)) return noAccess();
|
||||
if (!this.guards.hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied();
|
||||
if (!this.roadtrip.dayExists(dayId, tripId)) return noAccess();
|
||||
return ok({ vias: this.roadtrip.reanchor(dayId, { vias, remove }) });
|
||||
const next = this.roadtrip.reanchor(dayId, { vias, remove });
|
||||
this.announce(tripId, dayId);
|
||||
return ok({ vias: next });
|
||||
}
|
||||
|
||||
@Tool({
|
||||
@@ -170,6 +180,22 @@ export class RoadtripMcp {
|
||||
if (!this.guards.hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied();
|
||||
if (!this.roadtrip.dayExists(dayId, tripId)) return noAccess();
|
||||
if (!this.roadtrip.remove(viaId, dayId)) return noAccess();
|
||||
this.announce(tripId, dayId);
|
||||
return ok({ success: true });
|
||||
}
|
||||
|
||||
/**
|
||||
* 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 announce(tripId: number, dayId: number): void {
|
||||
this.roadtrip.broadcast(String(tripId), 'roadtripVia:changed', {
|
||||
dayId,
|
||||
vias: this.roadtrip.listForDay(String(dayId)),
|
||||
}, undefined);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -5,13 +5,14 @@ import { RoadtripMcp } from './roadtrip.mcp';
|
||||
import { McpSharedModule } from '../mcp-shared/mcp-shared.module';
|
||||
import { PermissionsModule } from '../permissions/permissions.module';
|
||||
import { AuthModule } from '../auth/auth.module';
|
||||
import { RealtimeModule } from '../realtime/realtime.module';
|
||||
import { AddonsModule } from '../addons/addons.module';
|
||||
|
||||
/** Road trip domain (#1797): the points a drive is routed through. Registered in AppModule. */
|
||||
@Module({
|
||||
// McpShared brings the tool guards, Auth the demo check, Permissions the trip guard,
|
||||
// Addons the enabled-check the MCP tools gate on (the controller has @RequireAddon).
|
||||
imports: [McpSharedModule, PermissionsModule, AuthModule, AddonsModule],
|
||||
imports: [McpSharedModule, PermissionsModule, AuthModule, AddonsModule, RealtimeModule],
|
||||
controllers: [RoadtripController],
|
||||
providers: [RoadtripService, RoadtripMcp],
|
||||
exports: [RoadtripService],
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { Injectable } from '@nestjs/common';
|
||||
import type { RoadtripDayTrack, RoadtripVia } from '@trek/shared';
|
||||
import type { RoadtripDayTrack, RoadtripVia, TrekWsPayload, TrekWsTripEventName } from '@trek/shared';
|
||||
import { DatabaseService } from '../database/database.service';
|
||||
import { RealtimeService } from '../realtime/realtime.service';
|
||||
|
||||
/**
|
||||
* Via points: the places a day's drive is made to pass through without stopping.
|
||||
@@ -12,7 +13,25 @@ import { DatabaseService } from '../database/database.service';
|
||||
*/
|
||||
@Injectable()
|
||||
export class RoadtripService {
|
||||
constructor(private readonly db: DatabaseService) {}
|
||||
constructor(
|
||||
private readonly db: DatabaseService,
|
||||
private readonly realtime: RealtimeService,
|
||||
) {}
|
||||
|
||||
/**
|
||||
* Tells the trip's other clients what the drive looks like 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
|
||||
* 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.
|
||||
*/
|
||||
broadcast<E extends TrekWsTripEventName>(tripId: string, event: E, payload: TrekWsPayload<E>, socketId: string | undefined): void {
|
||||
this.realtime.broadcast(tripId, event, payload, socketId);
|
||||
}
|
||||
|
||||
/** The day exists and belongs to this trip. 404 material, checked before every write. */
|
||||
dayExists(dayId: string | number, tripId: string | number): boolean {
|
||||
|
||||
@@ -37,6 +37,8 @@ function svc(o: Partial<RoadtripService> = {}): RoadtripService {
|
||||
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(),
|
||||
...o,
|
||||
} as unknown as RoadtripService;
|
||||
}
|
||||
@@ -188,4 +190,52 @@ describe('RoadtripController — writes', () => {
|
||||
expect(thrown(() => new RoadtripController(s).remove('7', '4', '999')))
|
||||
.toEqual({ status: 404, body: { error: 'Via not found' } });
|
||||
});
|
||||
|
||||
/**
|
||||
* 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.
|
||||
*/
|
||||
describe('telling the trip', () => {
|
||||
it('SRV-ROADTRIP-020: every write announces the new shape of the day', () => {
|
||||
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')],
|
||||
];
|
||||
for (const [name, run] of cases) {
|
||||
const s = svc();
|
||||
run(new RoadtripController(s));
|
||||
expect(s.broadcast, name).toHaveBeenCalledWith(
|
||||
'7',
|
||||
'roadtripVia:changed',
|
||||
{ dayId: '4', vias: [VIA] },
|
||||
'sock',
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
it('SRV-ROADTRIP-021: the day list is read back, not assembled from the write', () => {
|
||||
// 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>);
|
||||
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', () => {
|
||||
const track = { day_id: 4, place_id: 3, name: 'B96' };
|
||||
const s = svc({ tracksForTrip: vi.fn().mockReturnValue([track]) } as Partial<RoadtripService>);
|
||||
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', () => {
|
||||
const s = svc({ remove: vi.fn().mockReturnValue(false) } as Partial<RoadtripService>);
|
||||
expect(() => new RoadtripController(s).remove('7', '4', '9', 'sock')).toThrow();
|
||||
expect(s.broadcast).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -43,7 +43,9 @@ function makeService() {
|
||||
run: (sql: string, ...params: unknown[]) => raw.prepare(sql).run(...params as never[]),
|
||||
transaction: <T>(fn: (conn: unknown) => T) => raw.transaction(() => fn(raw))(),
|
||||
};
|
||||
return new RoadtripService(db as never);
|
||||
// The realtime side is exercised through the controller and the MCP layer; here the
|
||||
// service is under test for what it writes, so a broadcast that goes nowhere is right.
|
||||
return new RoadtripService(db as never, { broadcast: () => {} } as never);
|
||||
}
|
||||
|
||||
describe('RoadtripService', () => {
|
||||
|
||||
@@ -28,6 +28,8 @@ const FIXTURES: Record<TrekWsEventName, Record<string, unknown>> = {
|
||||
'day:updated': { day: { id: 2, date: '2026-06-11' } },
|
||||
'day:deleted': { dayId: 2 },
|
||||
'day:reordered': { orderedIds: [2, 1] },
|
||||
'roadtripVia:changed': { dayId: 2, vias: [{ id: 9, lat: 53.5, lng: 9.9 }] },
|
||||
'roadtripTrack:changed': { dayId: 2, track: null },
|
||||
'dayNote:created': { dayId: 2, note: { id: 5 } },
|
||||
'dayNote:updated': { dayId: 2, note: { id: 5 } },
|
||||
'dayNote:deleted': { noteId: 5, dayId: 2 },
|
||||
@@ -138,11 +140,12 @@ const DRIFT_VARIANTS: Partial<Record<TrekWsEventName, Record<string, unknown>[]>
|
||||
};
|
||||
|
||||
describe('@trek/shared realtime event registry', () => {
|
||||
it('WSEVT-REG-001: pins the authoritative inventory counts (69 trip + 32 user = 101)', () => {
|
||||
it('WSEVT-REG-001: pins the authoritative inventory counts (71 trip + 32 user = 103)', () => {
|
||||
// 67th to 69th trip event: the three collab:link:* a shared link emits.
|
||||
expect(TREK_WS_TRIP_EVENT_NAMES).toHaveLength(69);
|
||||
// 70th and 71st: the road trip's vias and tracks, which used to be written silently.
|
||||
expect(TREK_WS_TRIP_EVENT_NAMES).toHaveLength(71);
|
||||
expect(TREK_WS_USER_EVENT_NAMES).toHaveLength(32);
|
||||
expect(TREK_WS_EVENT_NAMES).toHaveLength(101);
|
||||
expect(TREK_WS_EVENT_NAMES).toHaveLength(103);
|
||||
});
|
||||
|
||||
it('WSEVT-REG-002: every name is domain:action shaped and outside the reserved plugin: namespace', () => {
|
||||
|
||||
@@ -92,6 +92,25 @@ export const TREK_WS_EVENTS = {
|
||||
payload: z.union([z.object({ orderedIds: idList }), z.object({ day: entity })]),
|
||||
},
|
||||
|
||||
// ── Road trip ────────────────────────────────────────────────────────────
|
||||
// The points a drive is routed through, and the tracks a day follows. Broadcast in one
|
||||
// event carrying the day's whole list rather than one event per point: a via has no
|
||||
// identity anyone reads, the client keeps them per day anyway, and a drag is a rapid
|
||||
// burst of writes whose only interesting state is the one that lands last.
|
||||
//
|
||||
// These used to be silent, on the reasoning that a via is how one person draws a route
|
||||
// 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.
|
||||
'roadtripVia:changed': {
|
||||
scope: 'trip',
|
||||
payload: z.object({ dayId: id, vias: z.array(entity) }),
|
||||
},
|
||||
'roadtripTrack:changed': {
|
||||
scope: 'trip',
|
||||
payload: z.object({ dayId: id, track: entity.nullable() }),
|
||||
},
|
||||
|
||||
// ── Day notes ────────────────────────────────────────────────────────────
|
||||
'dayNote:created': { scope: 'trip', payload: z.object({ dayId: id, note: entity }) },
|
||||
'dayNote:updated': { scope: 'trip', payload: z.object({ dayId: id, note: entity }) },
|
||||
|
||||
Reference in New Issue
Block a user