mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
feat(planner): surface and edit day-assignment notes across desktop, mobile and pdf (#2163)
This commit is contained in:
@@ -17,6 +17,7 @@ import {
|
||||
AssignmentReorderDto,
|
||||
AssignmentMoveDto,
|
||||
AssignmentTimeDto,
|
||||
AssignmentNotesDto,
|
||||
AssignmentTransportDto,
|
||||
AssignmentParticipantsDto,
|
||||
} from './assignments.dto';
|
||||
@@ -171,6 +172,26 @@ export class AssignmentOpsController {
|
||||
return { assignment };
|
||||
}
|
||||
|
||||
// #2163: the per-assignment note was write-once (create bodies, MCP, plugin
|
||||
// RPC) with no edit path anywhere. Same guard shape and 404 body as its
|
||||
// neighbours; no reconcile — the note doesn't touch the journey skeleton.
|
||||
@RequirePermission('day_edit')
|
||||
@Put(':id/notes')
|
||||
notes(
|
||||
@CurrentUser() user: User,
|
||||
@Param('tripId') tripId: string,
|
||||
@Param('id') id: string,
|
||||
@Body() body: AssignmentNotesDto,
|
||||
@Headers('x-socket-id') socketId?: string,
|
||||
) {
|
||||
if (!this.assignments.getAssignmentForTrip(id, tripId)) {
|
||||
throw new HttpException({ error: 'Assignment not found' }, 404);
|
||||
}
|
||||
const assignment = this.assignments.updateNotes(id, body.notes);
|
||||
this.assignments.broadcast(tripId, 'assignment:updated', { assignment }, socketId);
|
||||
return { assignment };
|
||||
}
|
||||
|
||||
@RequirePermission('day_edit')
|
||||
@Put(':id/transport')
|
||||
transport(
|
||||
|
||||
@@ -4,6 +4,7 @@ import {
|
||||
assignmentReorderRequestSchema,
|
||||
assignmentMoveRequestSchema,
|
||||
assignmentTimeRequestSchema,
|
||||
assignmentNotesRequestSchema,
|
||||
assignmentTransportRequestSchema,
|
||||
assignmentParticipantsRequestSchema,
|
||||
} from '@trek/shared';
|
||||
@@ -19,5 +20,6 @@ export class AssignmentCreateDto extends createZodDto(assignmentCreateRequestSch
|
||||
export class AssignmentReorderDto extends createZodDto(assignmentReorderRequestSchema) {}
|
||||
export class AssignmentMoveDto extends createZodDto(assignmentMoveRequestSchema) {}
|
||||
export class AssignmentTimeDto extends createZodDto(assignmentTimeRequestSchema) {}
|
||||
export class AssignmentNotesDto extends createZodDto(assignmentNotesRequestSchema) {}
|
||||
export class AssignmentTransportDto extends createZodDto(assignmentTransportRequestSchema) {}
|
||||
export class AssignmentParticipantsDto extends createZodDto(assignmentParticipantsRequestSchema) {}
|
||||
|
||||
@@ -115,6 +115,33 @@ export class AssignmentsMcp {
|
||||
return ok({ assignment });
|
||||
}
|
||||
|
||||
@Tool({
|
||||
name: 'update_assignment_notes',
|
||||
description: 'Set or clear the day-specific note on a place assignment (the note assign_place_to_day and create_and_assign_place accept at creation). Pass null or an empty string to clear it.',
|
||||
inputSchema: {
|
||||
tripId: z.number().int().positive(),
|
||||
assignmentId: z.number().int().positive(),
|
||||
// Mirrors the REST contract (assignmentNotesRequestSchema): uncapped on
|
||||
// purpose, unlike the create tools' max(500) — a longer note written
|
||||
// through REST or the plugin RPC must stay editable here (#2163).
|
||||
notes: z.string().nullable().describe('The note text, or null to clear'),
|
||||
},
|
||||
annotations: TOOL_ANNOTATIONS_WRITE,
|
||||
access: { group: 'places', mode: 'write' },
|
||||
})
|
||||
async updateAssignmentNotes(
|
||||
{ tripId, assignmentId, notes }: { tripId: number; assignmentId: number; notes: string | null },
|
||||
ctx: McpContext,
|
||||
) {
|
||||
if (this.auth.isDemoUser(ctx.userId)) return demoDenied();
|
||||
if (!this.assignments.verifyTripAccess(tripId, ctx.userId)) return noAccess();
|
||||
if (!this.guards.hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied();
|
||||
if (!this.assignments.getAssignmentForTrip(assignmentId, tripId)) return errorResult('Assignment not found.');
|
||||
const assignment = this.assignments.updateNotes(assignmentId, notes);
|
||||
this.guards.safeBroadcast(tripId, 'assignment:updated', { assignment });
|
||||
return ok({ assignment });
|
||||
}
|
||||
|
||||
@Tool({
|
||||
name: 'set_leg_transport_mode',
|
||||
description: 'Set the travel mode of a route leg for a place assignment. Use direction "outgoing" (default) for the common case: the leg leaving this stop toward the next. Use direction "incoming" ONLY when this stop\'s arriving leg originates from something that is not itself a place assignment (e.g. a flight/train booking arrival, or a morning hotel departure) – setting "incoming" on an ordinary place-to-place leg is stored but has no effect on route rendering, because it targets a column that is only read for non-place origins. transport_mode is a route profile key: "driving", "walking", "cycling", or a plugin profile written as "plugin:<pluginId>/<profileId>". Any other value is stored but drawn as a driving route. null clears it so the leg inherits the day default.',
|
||||
|
||||
@@ -261,6 +261,19 @@ export class AssignmentsService {
|
||||
return this.getAssignmentWithPlace(Number(id));
|
||||
}
|
||||
|
||||
/**
|
||||
* Edit the per-assignment note after creation (#2163) — until now the note
|
||||
* was write-once via the create paths (REST body, MCP tools, plugin RPC) and
|
||||
* invisible in the app. Falsy notes ('' or null) clear the column, the same
|
||||
* `notes || null` normalisation createAssignment applies. No auto-sort and no
|
||||
* journey reconcile: the note affects neither the day order nor the skeleton
|
||||
* mirror (same as the transport-mode writes).
|
||||
*/
|
||||
updateNotes(id: string | number, notes: string | null | undefined) {
|
||||
this.dbs.run('UPDATE day_assignments SET notes = ? WHERE id = ?', notes || null, id);
|
||||
return this.getAssignmentWithPlace(Number(id));
|
||||
}
|
||||
|
||||
/**
|
||||
* Set the travel mode of the leg leaving this stop (#1281). null clears the
|
||||
* override so the leg falls back to the day's default_transport_mode. This is
|
||||
|
||||
@@ -155,6 +155,44 @@ describe('Assignments e2e (real auth guard + temp SQLite)', () => {
|
||||
expect(reconcileTripSkeletons).toHaveBeenCalledWith(5, undefined);
|
||||
});
|
||||
|
||||
it('200 notes roundtrip: create with note, PUT edits it, GET list shows the new value (#2163)', async () => {
|
||||
const create = await request(server).post('/api/trips/5/days/3/assignments').set('Cookie', sessionCookie(1))
|
||||
.send({ place_id: 2, notes: 'Book the 10:00 timed entry' });
|
||||
expect(create.status).toBe(201);
|
||||
expect(create.body.assignment.notes).toBe('Book the 10:00 timed entry');
|
||||
const id = create.body.assignment.id;
|
||||
|
||||
const put = await request(server)
|
||||
.put(`/api/trips/5/assignments/${id}/notes`)
|
||||
.set('Cookie', sessionCookie(1))
|
||||
.send({ notes: 'Arrive 15 minutes early' });
|
||||
expect(put.status).toBe(200);
|
||||
expect(put.body.assignment).toMatchObject({ id, notes: 'Arrive 15 minutes early' });
|
||||
expect(db.prepare('SELECT notes FROM day_assignments WHERE id = ?').get(id)).toEqual({ notes: 'Arrive 15 minutes early' });
|
||||
|
||||
const list = await request(server).get('/api/trips/5/days/3/assignments').set('Cookie', sessionCookie(1));
|
||||
expect(list.status).toBe(200);
|
||||
expect(list.body.assignments.find((a: { id: number }) => a.id === id).notes).toBe('Arrive 15 minutes early');
|
||||
});
|
||||
|
||||
it('200 notes clear: null and empty string both null the column (#2163)', async () => {
|
||||
const id = seedAssignment();
|
||||
db.prepare('UPDATE day_assignments SET notes = ? WHERE id = ?').run('old note', id);
|
||||
const cleared = await request(server).put(`/api/trips/5/assignments/${id}/notes`).set('Cookie', sessionCookie(1)).send({ notes: null });
|
||||
expect(cleared.status).toBe(200);
|
||||
expect(cleared.body.assignment.notes).toBeNull();
|
||||
db.prepare('UPDATE day_assignments SET notes = ? WHERE id = ?').run('old note', id);
|
||||
const emptied = await request(server).put(`/api/trips/5/assignments/${id}/notes`).set('Cookie', sessionCookie(1)).send({ notes: '' });
|
||||
expect(emptied.status).toBe(200);
|
||||
expect(db.prepare('SELECT notes FROM day_assignments WHERE id = ?').get(id)).toEqual({ notes: null });
|
||||
});
|
||||
|
||||
it('400 notes body without the notes key is rejected by the Zod pipe (#2163)', async () => {
|
||||
const id = seedAssignment();
|
||||
const res = await request(server).put(`/api/trips/5/assignments/${id}/notes`).set('Cookie', sessionCookie(1)).send({});
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
|
||||
it('200 update time reconciles journey skeletons', async () => {
|
||||
reconcileTripSkeletons.mockClear();
|
||||
const id = seedAssignment();
|
||||
@@ -275,6 +313,16 @@ describe('Assignments e2e (real auth guard + temp SQLite)', () => {
|
||||
expect(db.prepare('SELECT COUNT(*) AS n FROM assignment_participants WHERE assignment_id = ?').get(id)).toEqual({ n: 0 });
|
||||
});
|
||||
|
||||
it('404s a notes change on a foreign assignment instead of editing it (#2163)', async () => {
|
||||
const id = seedForeignAssignment();
|
||||
const res = await request(server)
|
||||
.put(`/api/trips/${FOREIGN_TRIP}/assignments/${id}/notes`)
|
||||
.set('Cookie', sessionCookie(1))
|
||||
.send({ notes: 'hijacked' });
|
||||
expect(res.status).toBe(404);
|
||||
expect(db.prepare('SELECT notes FROM day_assignments WHERE id = ?').get(id)).toEqual({ notes: null });
|
||||
});
|
||||
|
||||
it('403s when the caller is on the trip but lacks day_edit', async () => {
|
||||
const id = seedAssignment();
|
||||
checkPermission.mockReturnValue(false);
|
||||
@@ -284,5 +332,16 @@ describe('Assignments e2e (real auth guard + temp SQLite)', () => {
|
||||
.send({ place_time: '09:00', end_time: null });
|
||||
expect(res.status).toBe(403);
|
||||
});
|
||||
|
||||
it('403s a notes change without day_edit (#2163)', async () => {
|
||||
const id = seedAssignment();
|
||||
checkPermission.mockReturnValue(false);
|
||||
const res = await request(server)
|
||||
.put(`/api/trips/5/assignments/${id}/notes`)
|
||||
.set('Cookie', sessionCookie(1))
|
||||
.send({ notes: 'nope' });
|
||||
expect(res.status).toBe(403);
|
||||
expect(db.prepare('SELECT notes FROM day_assignments WHERE id = ?').get(id)).toEqual({ notes: null });
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
/**
|
||||
* Unit tests for MCP assignment tools: assign_place_to_day, unassign_place,
|
||||
* reorder_day_assignments, update_assignment_time.
|
||||
* reorder_day_assignments, update_assignment_time, update_assignment_notes.
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest';
|
||||
|
||||
@@ -408,3 +408,85 @@ describe('Tool: update_assignment_time', () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// update_assignment_notes (#2163)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('Tool: update_assignment_notes', () => {
|
||||
it('sets the day-specific note on an assignment', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const day = createDay(testDb, trip.id);
|
||||
const place = createPlace(testDb, trip.id);
|
||||
const assignment = createDayAssignment(testDb, day.id, place.id);
|
||||
|
||||
await withHarness(user.id, async (h) => {
|
||||
const result = await h.client.callTool({
|
||||
name: 'update_assignment_notes',
|
||||
arguments: { tripId: trip.id, assignmentId: assignment.id, notes: 'Book the 10:00 timed entry and arrive 15 minutes early' },
|
||||
});
|
||||
const data = parseToolResult(result) as any;
|
||||
expect(data.assignment.notes).toBe('Book the 10:00 timed entry and arrive 15 minutes early');
|
||||
expect(testDb.prepare('SELECT notes FROM day_assignments WHERE id = ?').get(assignment.id)).toEqual({ notes: 'Book the 10:00 timed entry and arrive 15 minutes early' });
|
||||
});
|
||||
});
|
||||
|
||||
it('clears the note with null and with an empty string', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const day = createDay(testDb, trip.id);
|
||||
const place = createPlace(testDb, trip.id);
|
||||
const assignment = createDayAssignment(testDb, day.id, place.id);
|
||||
testDb.prepare('UPDATE day_assignments SET notes = ? WHERE id = ?').run('old', assignment.id);
|
||||
|
||||
await withHarness(user.id, async (h) => {
|
||||
const cleared = parseToolResult(await h.client.callTool({
|
||||
name: 'update_assignment_notes',
|
||||
arguments: { tripId: trip.id, assignmentId: assignment.id, notes: null },
|
||||
})) as any;
|
||||
expect(cleared.assignment.notes).toBeNull();
|
||||
testDb.prepare('UPDATE day_assignments SET notes = ? WHERE id = ?').run('old', assignment.id);
|
||||
const emptied = parseToolResult(await h.client.callTool({
|
||||
name: 'update_assignment_notes',
|
||||
arguments: { tripId: trip.id, assignmentId: assignment.id, notes: '' },
|
||||
})) as any;
|
||||
expect(emptied.assignment.notes).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
it('broadcasts assignment:updated event', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const day = createDay(testDb, trip.id);
|
||||
const place = createPlace(testDb, trip.id);
|
||||
const assignment = createDayAssignment(testDb, day.id, place.id);
|
||||
await withHarness(user.id, async (h) => {
|
||||
await h.client.callTool({ name: 'update_assignment_notes', arguments: { tripId: trip.id, assignmentId: assignment.id, notes: 'n' } });
|
||||
expect(broadcastMock).toHaveBeenCalledWith(trip.id, 'assignment:updated', expect.any(Object));
|
||||
});
|
||||
});
|
||||
|
||||
it('returns error when assignment not found', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
await withHarness(user.id, async (h) => {
|
||||
const result = await h.client.callTool({ name: 'update_assignment_notes', arguments: { tripId: trip.id, assignmentId: 99999, notes: 'n' } });
|
||||
expect(result.isError).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
it('returns access denied for non-member', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const { user: other } = createUser(testDb);
|
||||
const trip = createTrip(testDb, other.id);
|
||||
const day = createDay(testDb, trip.id);
|
||||
const place = createPlace(testDb, trip.id);
|
||||
const assignment = createDayAssignment(testDb, day.id, place.id);
|
||||
await withHarness(user.id, async (h) => {
|
||||
const result = await h.client.callTool({ name: 'update_assignment_notes', arguments: { tripId: trip.id, assignmentId: assignment.id, notes: 'n' } });
|
||||
expect(result.isError).toBe(true);
|
||||
expect(testDb.prepare('SELECT notes FROM day_assignments WHERE id = ?').get(assignment.id)).toEqual({ notes: null });
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -94,6 +94,17 @@ describe('AssignmentOpsController (parity with the per-assignment op routes)', (
|
||||
expect(reconcile).toHaveBeenCalledWith('5', 'sock');
|
||||
});
|
||||
|
||||
it('PUT /:id/notes 404 missing, else updates + broadcasts (#2163)', () => {
|
||||
expect(thrown(() => new AssignmentOpsController(svc({ getAssignmentForTrip: vi.fn().mockReturnValue(undefined) } as Partial<AssignmentsService>)).notes(user, '5', '9', { notes: 'n' }))).toEqual({ status: 404, body: { error: 'Assignment not found' } });
|
||||
const updateNotes = vi.fn().mockReturnValue({ id: 9, notes: 'Book the 10:00 entry' }); const broadcast = vi.fn(); const reconcile = vi.fn();
|
||||
const s = svc({ getAssignmentForTrip: vi.fn().mockReturnValue({ id: 9 }), updateNotes, broadcast, reconcile } as Partial<AssignmentsService>);
|
||||
expect(new AssignmentOpsController(s).notes(user, '5', '9', { notes: 'Book the 10:00 entry' }, 'sock')).toEqual({ assignment: { id: 9, notes: 'Book the 10:00 entry' } });
|
||||
expect(updateNotes).toHaveBeenCalledWith('9', 'Book the 10:00 entry');
|
||||
expect(broadcast).toHaveBeenCalledWith('5', 'assignment:updated', { assignment: { id: 9, notes: 'Book the 10:00 entry' } }, 'sock');
|
||||
// The note touches neither day order nor the journey skeleton — no reconcile.
|
||||
expect(reconcile).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('PUT /:id/participants 404 on a foreign assignment, else sets + broadcasts (non-array bodies are the Zod pipe\'s 400, covered in e2e)', () => {
|
||||
const setParticipants = vi.fn().mockReturnValue([{ user_id: 2 }]); const broadcast = vi.fn();
|
||||
expect(thrown(() => new AssignmentOpsController(svc({ getAssignmentForTrip: vi.fn().mockReturnValue(undefined), setParticipants } as Partial<AssignmentsService>)).setParticipants(user, '5', '9', { user_ids: [2] })))
|
||||
|
||||
@@ -407,6 +407,35 @@ describe('updateTime', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('updateNotes (#2163)', () => {
|
||||
it('ASG-SVC-030: persists the note and re-selects the nested shape', () => {
|
||||
const { day, place } = fixture();
|
||||
const a = createDayAssignment(testDb, day.id, place.id);
|
||||
const updated = svc.updateNotes(a.id, 'Book the 10:00 timed entry');
|
||||
expect(updated).toMatchObject({ id: a.id, notes: 'Book the 10:00 timed entry' });
|
||||
expect(testDb.prepare('SELECT notes FROM day_assignments WHERE id = ?').get(a.id)).toEqual({ notes: 'Book the 10:00 timed entry' });
|
||||
});
|
||||
|
||||
it('ASG-SVC-031: empty string and null both clear the column (same normalisation as createAssignment)', () => {
|
||||
const { day, place } = fixture();
|
||||
const a = createDayAssignment(testDb, day.id, place.id);
|
||||
svc.updateNotes(a.id, 'gone soon');
|
||||
expect(svc.updateNotes(a.id, '')!.notes).toBeNull();
|
||||
svc.updateNotes(a.id, 'gone again');
|
||||
expect(svc.updateNotes(a.id, null)!.notes).toBeNull();
|
||||
expect(testDb.prepare('SELECT notes FROM day_assignments WHERE id = ?').get(a.id)).toEqual({ notes: null });
|
||||
});
|
||||
|
||||
it('ASG-SVC-032: leaves order and times untouched', () => {
|
||||
const { day, place } = fixture();
|
||||
const a = createDayAssignment(testDb, day.id, place.id, { order_index: 3 });
|
||||
svc.updateTime(a.id, '09:00', '10:00');
|
||||
const updated = svc.updateNotes(a.id, 'note');
|
||||
expect(updated).toMatchObject({ assignment_time: '09:00', assignment_end_time: '10:00' });
|
||||
expect((testDb.prepare('SELECT order_index FROM day_assignments WHERE id = ?').get(a.id) as { order_index: number }).order_index).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('setLegTransportMode', () => {
|
||||
it('ASG-SVC-025: sets and clears the leg override (sticky per #1281)', () => {
|
||||
const { day, place } = fixture();
|
||||
|
||||
Reference in New Issue
Block a user