k1nq/TREK
1
0
Fork 0
mirror of https://github.com/mauriceboe/TREK.git synced 2026-10-23 15:36:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

Move places, days and itinerary onto the decorators

Browse Source 
Eight more methods, so the registry owns 38 of 113 and rpc-host.ts is under 1000
lines for the first time.

itinerary.* lives in assignments/, not in days/, because AssignmentsModule already
imports DaysModule and putting the handler the other way round would have closed a
cycle. It keeps riding on day_edit, which is what the app gates it on.

The delete path in places carries an ordering constraint that is silent when
broken, so it has two tests of its own: the journey hook has to run BEFORE the row
goes, because journey_entries.source_place_id is ON DELETE SET NULL and afterwards
there is nothing left to detach, and the place id has to be scoped to the trip
before the hook runs at all, because the hook keys on the place alone and would
otherwise detach another trip's entries even though the delete itself is refused.
This commit is contained in:
Maurice
2026-08-09 13:05:52 +02:00
committed by Maurice
parent d58f7aa399
commit c326d39a07
14 changed files with 540 additions and 281 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/tests/unit/plugins/plugin-host-deps.factory.test.ts
+2 -79
View File
@@ -441,93 +441,16 @@ describe('host-deps factory — planner write + metadata deps', () => {
beforeEach(() => { checkPermission.mockReset(); checkPermission.mockReturnValue(true); });
afterAll(() => closePluginDataDb('writer'));
it('places.create/update/delete delegate + broadcast; a missing place is RESOURCE_FORBIDDEN', async () => {
const h = host('db:write:places');
expect((await call(h, 'places.create', { tripId: 1, input: { name: 'P' } })).ok).toBe(true);
// Write deps re-emit the SAME core event the controllers do (not the plugin: namespace).
expect(broadcast).toHaveBeenCalledWith(1, 'place:created', expect.anything());
expect((await call(h, 'places.update', { tripId: 1, placeId: 5, input: { name: 'Q' } })).ok).toBe(true);
expect((await call(h, 'places.update', { tripId: 1, placeId: 99, input: {} })).error.code).toBe('RESOURCE_FORBIDDEN');
expect((await call(h, 'places.delete', { tripId: 1, placeId: 5 })).ok).toBe(true);
expect((await call(h, 'places.delete', { tripId: 1, placeId: 99 })).error.code).toBe('RESOURCE_FORBIDDEN');
});
// places, days and itinerary left this factory with the decorator migration; the
// cases now run in tests/unit/places/ and tests/unit/days/.
// #1705: a place write from a plugin has to update a linked journey the same way
// the REST route does, otherwise the journey shows the old title (or an entry for
// a place that no longer exists) until it is reloaded.
it('places writes fire the journey hooks, delete ahead of the row, and skip a foreign id', async () => {
const h = host('db:write:places');
const created = vi.mocked(onPlaceCreated);
const updated = vi.mocked(onPlaceUpdated);
const deleted = vi.mocked(onPlaceDeleted);
const removePlace = removePlaceStub;
[created, updated, deleted, removePlace].forEach((m) => m.mockClear());
expect((await call(h, 'places.create', { tripId: 1, input: { name: 'P' } })).ok).toBe(true);
expect(created).toHaveBeenCalledWith(1, 10);
expect((await call(h, 'places.update', { tripId: 1, placeId: 5, input: { name: 'Q' } })).ok).toBe(true);
expect(updated).toHaveBeenCalledWith(5);
expect((await call(h, 'places.delete', { tripId: 1, placeId: 5 })).ok).toBe(true);
expect(deleted).toHaveBeenCalledWith(5);
// source_place_id is ON DELETE SET NULL — after the row is gone the hook has
// nothing left to detach, so the order is part of the fix.
expect(deleted.mock.invocationCallOrder[0]).toBeLessThan(removePlace.mock.invocationCallOrder[0]);
// A place on another trip: refused before the hook can touch that trip's journeys.
deleted.mockClear();
updated.mockClear();
expect((await call(h, 'places.delete', { tripId: 1, placeId: 99 })).error.code).toBe('RESOURCE_FORBIDDEN');
expect((await call(h, 'places.update', { tripId: 1, placeId: 99, input: {} })).error.code).toBe('RESOURCE_FORBIDDEN');
expect(deleted).not.toHaveBeenCalled();
expect(updated).not.toHaveBeenCalled();
});
// A journey link that blows up must not turn a successful place write into an
// RPC error — every other caller of these hooks swallows them too.
it('places writes survive a throwing journey hook', async () => {
const h = host('db:write:places');
const updated = vi.mocked(onPlaceUpdated);
updated.mockImplementationOnce(() => { throw new Error('journey db locked'); });
expect((await call(h, 'places.update', { tripId: 1, placeId: 5, input: { name: 'Q' } })).ok).toBe(true);
});
it('days + itinerary delegate; a day/place/assignment outside the trip is refused', async () => {
const h = host('db:write:days', 'db:write:itinerary');
expect((await call(h, 'days.create', { tripId: 1, input: { notes: 'n' } })).ok).toBe(true);
expect((await call(h, 'days.update', { tripId: 1, dayId: 3, input: { notes: 'x' } })).ok).toBe(true);
expect((await call(h, 'days.delete', { tripId: 1, dayId: 3 })).ok).toBe(true);
expect((await call(h, 'days.update', { tripId: 1, dayId: 99, input: {} })).error.code).toBe('RESOURCE_FORBIDDEN');
expect((await call(h, 'itinerary.assign', { tripId: 1, dayId: 3, placeId: 7 })).ok).toBe(true);
expect((await call(h, 'itinerary.assign', { tripId: 1, dayId: 99, placeId: 7 })).error.code).toBe('RESOURCE_FORBIDDEN');
expect((await call(h, 'itinerary.assign', { tripId: 1, dayId: 3, placeId: 99 })).error.code).toBe('RESOURCE_FORBIDDEN');
expect((await call(h, 'itinerary.unassign', { tripId: 1, assignmentId: 30 })).ok).toBe(true);
expect((await call(h, 'itinerary.unassign', { tripId: 1, assignmentId: 99 })).error.code).toBe('RESOURCE_FORBIDDEN');
});
// #1705: a plugin itinerary write has to reach open sessions exactly like the REST
// route. The delete payload needs the dayId the client reducer evicts by, and both
// directions run the same journey-skeleton reconcile the controller/MCP tool run.
it('itinerary writes carry the dayId the client evicts by and re-mirror linked journeys', async () => {
const h = host('db:write:itinerary');
const reconcile = vi.mocked(assignmentsStub.reconcile);
reconcile.mockClear();
expect((await call(h, 'itinerary.assign', { tripId: 1, dayId: 3, placeId: 7 })).ok).toBe(true);
expect(reconcile).toHaveBeenCalledWith(1);
reconcile.mockClear();
expect((await call(h, 'itinerary.unassign', { tripId: 1, assignmentId: 30 })).ok).toBe(true);
expect(broadcast).toHaveBeenCalledWith(1, 'assignment:deleted', { assignmentId: 30, dayId: 3 });
expect(reconcile).toHaveBeenCalledWith(1);
// A refused unassign deletes nothing, so it must not touch the journeys either.
reconcile.mockClear();
expect((await call(h, 'itinerary.unassign', { tripId: 1, assignmentId: 99 })).error.code).toBe('RESOURCE_FORBIDDEN');
expect(reconcile).not.toHaveBeenCalled();
});
it('trips.update: archive/cover need their own permission; service errors map to RPC codes', async () => {
const h = host('db:write:trips');
expect((await call(h, 'trips.update', { tripId: 1, input: { title: 'T' } })).ok).toBe(true);
server/tests/unit/plugins/rpc-coverage.test.ts
+10 -2
View File
@@ -54,8 +54,8 @@ describe('plugin RPC coverage ledger', () => {
// Update these two numbers in every rollout PR. They make the diff state the size
// of the move out loud, which is the number a reviewer wants to see.
const total = KNOWN_METHODS.length + UNCONDITIONAL_METHODS.length;
expect(fromRegistry.size).toBe(30);
expect(legacy.size).toBe(total - 30);
expect(fromRegistry.size).toBe(38);
expect(legacy.size).toBe(total - 38);
});
it('RPCLEDGER-006 the migrated methods are exactly the ones this PR claims', () => {
@@ -65,12 +65,17 @@ describe('plugin RPC coverage ledger', () => {
'daynotes.delete',
'daynotes.list',
'daynotes.update',
'days.create',
'days.delete',
'days.update',
'files.create',
'files.createLink',
'files.getContent',
'files.list',
'files.softDelete',
'files.update',
'itinerary.assign',
'itinerary.unassign',
'packing.create',
'packing.createBag',
'packing.delete',
@@ -80,6 +85,9 @@ describe('plugin RPC coverage ledger', () => {
'packing.setBagMembers',
'packing.update',
'packing.updateBag',
'places.create',
'places.delete',
'places.update',
'rates.get',
'tags.create',
'tags.delete',
server/tests/unit/plugins/rpc-host.test.ts
+1 -41
View File
@@ -662,53 +662,13 @@ describe('PluginRpcHost — capability enforcement', () => {
});
// --- Planner writes (#1429) ---
it('db:write:places creates a place on a trip the acting user may edit', async () => {
const host = new PluginRpcHost('p', new Set(['db:write:places']), deps);
const res = await host.dispatch(req('places.create', { tripId: 1, input: { name: 'Fushimi Inari' } }), 42);
expect(ok(res)).toBe(true);
expect(deps.createPlace).toHaveBeenCalledWith(1, expect.objectContaining({ name: 'Fushimi Inari' }));
});
it('places.create rejects an over-long string field (mirrors the REST STRING_LIMITS)', async () => {
const host = new PluginRpcHost('p', new Set(['db:write:places']), deps);
const res = await host.dispatch(req('places.create', { tripId: 1, input: { name: 'A'.repeat(201) } }), 42);
expect((res as RpcError).error.code).toBe('BAD_PARAMS');
expect(deps.createPlace).not.toHaveBeenCalled();
});
// The places, days and itinerary cases moved to their own domain suites.
it('places.create is PERMISSION_DENIED without db:write:places', async () => {
const host = new PluginRpcHost('p', new Set(['db:read:trips']), deps);
const res = await host.dispatch(req('places.create', { tripId: 1, input: { name: 'X' } }), 42);
expect((res as RpcError).error.code).toBe('PERMISSION_DENIED');
});
it('places.create is RESOURCE_FORBIDDEN on a trip the acting user cannot edit', async () => {
const host = new PluginRpcHost('p', new Set(['db:write:places']), deps);
const res = await host.dispatch(req('places.create', { tripId: 2, input: { name: 'X' } }), 42);
expect((res as RpcError).error.code).toBe('RESOURCE_FORBIDDEN');
expect(deps.createPlace).not.toHaveBeenCalled();
});
it('places.create with no bound acting user is RESOURCE_FORBIDDEN (jobs / forged calls)', async () => {
const host = new PluginRpcHost('p', new Set(['db:write:places']), deps);
const res = await host.dispatch(req('places.create', { tripId: 1, input: { name: 'X' } }), undefined);
expect((res as RpcError).error.code).toBe('RESOURCE_FORBIDDEN');
});
it('places.create with an invalid payload (no name) is BAD_PARAMS', async () => {
const host = new PluginRpcHost('p', new Set(['db:write:places']), deps);
const res = await host.dispatch(req('places.create', { tripId: 1, input: {} }), 42);
expect((res as RpcError).error.code).toBe('BAD_PARAMS');
expect(deps.createPlace).not.toHaveBeenCalled();
});
it('db:write:itinerary assigns a place to a day (day_edit gated)', async () => {
const host = new PluginRpcHost('p', new Set(['db:write:itinerary']), deps);
const res = await host.dispatch(req('itinerary.assign', { tripId: 1, dayId: 3, placeId: 10 }), 42);
expect(ok(res)).toBe(true);
expect(deps.assignPlaceToDay).toHaveBeenCalledWith(1, 3, 10, null);
});
it('db:write:trips updates a trip the acting user may edit', async () => {
const host = new PluginRpcHost('p', new Set(['db:write:trips']), deps);
const res = await host.dispatch(req('trips.update', { tripId: 1, input: { title: 'Renamed' } }), 42);