k1nq/TREK
1
0
Fork 0
mirror of https://github.com/mauriceboe/TREK.git synced 2026-10-18 21:16:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(assignments): confine day participants to the trip roster

Browse Source 
setParticipants took the ids on trust. The caller is proven to be on the trip and
the assignment is proven to sit on it, but nothing said the same about the ids in
the body, and the read-back joins users — so a stranger's name and avatar came
back to the writer and went out over the assignment:participants broadcast to
everyone else in the room.

Takes the tripId both callers already hold and filters against the roster. Same
drop-rather-than-reject rule as bag members and reservation travellers: the
participants box posts the whole list on every edit, so a trip whose membership
changed underneath it would otherwise be stuck.

ASG-SVC-028 covered a foreign key error rolling the replace back. The filter
takes an unknown id out before the insert can reach it, so that path is gone and
the case now pins what happens instead: a list of nothing but strangers clears,
like the empty list ASG-SVC-018 already pins.
This commit is contained in:
Maurice
2026-08-16 23:01:11 +02:00
committed by Maurice
parent 1eb29f8b77
commit 0337ebbbd2
5 changed files with 52 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/src/nest/assignments/assignments.controller.ts
+1 -1
View File
@@ -202,7 +202,7 @@ export class AssignmentOpsController {
if (!this.assignments.getAssignmentForTrip(id, tripId)) {
throw new HttpException({ error: 'Assignment not found' }, 404);
}
const participants = this.assignments.setParticipants(id, body.user_ids);
const participants = this.assignments.setParticipants(id, body.user_ids, tripId);
this.assignments.broadcast(tripId, 'assignment:participants', { assignmentId: Number(id), participants }, socketId);
return { participants };
}
server/src/nest/assignments/assignments.mcp.ts
+1 -1
View File
@@ -187,7 +187,7 @@ export class AssignmentsMcp {
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 participants = this.assignments.setParticipants(assignmentId, userIds);
const participants = this.assignments.setParticipants(assignmentId, userIds, tripId);
this.guards.safeBroadcast(tripId, 'assignment:participants', { assignmentId, participants });
return ok({ participants });
}
server/src/nest/assignments/assignments.service.ts
+12 -3
View File
@@ -283,12 +283,21 @@ export class AssignmentsService {
return this.getAssignmentWithPlace(Number(id));
}
setParticipants(assignmentId: string | number, userIds: number[]) {
/**
* Both callers have already proven the assignment sits on `tripId`; this
* settles the other half, that the ids in the body do too. Off-roster ids drop
* silently rather than 400, matching bag members and reservation travellers —
* the participants box sends the whole list back on every edit, so rejecting
* the request would strand a trip whose membership changed underneath it.
*/
setParticipants(assignmentId: string | number, userIds: number[], tripId: string | number) {
const roster = this.dbs.rosterUserIds(tripId);
const scoped = userIds.filter(id => roster.has(id));
this.dbs.transaction(() => {
this.dbs.run('DELETE FROM assignment_participants WHERE assignment_id = ?', assignmentId);
if (userIds.length > 0) {
if (scoped.length > 0) {
const insert = this.dbs.prepare('INSERT OR IGNORE INTO assignment_participants (assignment_id, user_id) VALUES (?, ?)');
for (const userId of userIds) insert.run(assignmentId, userId);
for (const userId of scoped) insert.run(assignmentId, userId);
}
});
server/tests/unit/nest/assignments.controller.test.ts
+2 -1
View File
@@ -101,7 +101,8 @@ describe('AssignmentOpsController (parity with the per-assignment op routes)', (
expect(setParticipants).not.toHaveBeenCalled();
const s = svc({ getAssignmentForTrip: vi.fn().mockReturnValue({ id: 9 }), setParticipants, broadcast } as Partial<AssignmentsService>);
expect(new AssignmentOpsController(s).setParticipants(user, '5', '9', { user_ids: [2] }, 'sock')).toEqual({ participants: [{ user_id: 2 }] });
expect(setParticipants).toHaveBeenCalledWith('9', [2]);
// The trip comes along so the service can confine the ids to its roster.
expect(setParticipants).toHaveBeenCalledWith('9', [2], '5');
expect(broadcast).toHaveBeenCalledWith('5', 'assignment:participants', { assignmentId: 9, participants: [{ user_id: 2 }] }, 'sock');
});
});
server/tests/unit/nest/assignments.service.test.ts
+36 -17
View File
@@ -205,7 +205,7 @@ describe('listDayAssignments', () => {
testDb.prepare('INSERT INTO place_tags (place_id, tag_id) VALUES (?, ?)').run(place.id, tag.id);
const a1 = createDayAssignment(testDb, day.id, place.id, { order_index: 1 });
const a2 = createDayAssignment(testDb, day.id, second.id, { order_index: 0 });
svc.setParticipants(a1.id, [user.id]);
svc.setParticipants(a1.id, [user.id], trip.id);
const list = svc.listDayAssignments(day.id);
expect(list.map(a => a.id)).toEqual([a2.id, a1.id]);
@@ -284,45 +284,64 @@ describe('moveAssignment', () => {
describe('getParticipants / setParticipants', () => {
it('ASG-SVC-017: setParticipants replaces the list and returns the joined rows', () => {
const { user, day, place } = fixture();
const { user, trip, day, place } = fixture();
const { user: peer } = createUser(testDb);
addTripMember(testDb, trip.id, peer.id);
const a = createDayAssignment(testDb, day.id, place.id);
svc.setParticipants(a.id, [user.id]);
const replaced = svc.setParticipants(a.id, [peer.id]);
svc.setParticipants(a.id, [user.id], trip.id);
const replaced = svc.setParticipants(a.id, [peer.id], trip.id);
expect(replaced).toEqual([{ user_id: peer.id, username: peer.username, avatar: null }]);
expect(svc.getParticipants(a.id)).toEqual(replaced);
});
it('ASG-SVC-018: empty array clears all participants', () => {
const { user, day, place } = fixture();
const { user, trip, day, place } = fixture();
const a = createDayAssignment(testDb, day.id, place.id);
svc.setParticipants(a.id, [user.id]);
expect(svc.setParticipants(a.id, [])).toEqual([]);
svc.setParticipants(a.id, [user.id], trip.id);
expect(svc.setParticipants(a.id, [], trip.id)).toEqual([]);
expect(svc.getParticipants(a.id)).toEqual([]);
});
it('ASG-SVC-019: username COALESCEs display_name over username', () => {
const { user, day, place } = fixture();
const { user, trip, day, place } = fixture();
testDb.prepare('UPDATE users SET display_name = ? WHERE id = ?').run('Fancy Name', user.id);
const a = createDayAssignment(testDb, day.id, place.id);
const rows = svc.setParticipants(a.id, [user.id]);
const rows = svc.setParticipants(a.id, [user.id], trip.id);
expect(rows).toEqual([{ user_id: user.id, username: 'Fancy Name', avatar: null }]);
});
it('ASG-SVC-020: duplicate user ids collapse via INSERT OR IGNORE', () => {
const { user, day, place } = fixture();
const { user, trip, day, place } = fixture();
const a = createDayAssignment(testDb, day.id, place.id);
const rows = svc.setParticipants(a.id, [user.id, user.id]);
const rows = svc.setParticipants(a.id, [user.id, user.id], trip.id);
expect(rows).toHaveLength(1);
});
it('ASG-SVC-028: a failing insert rolls back the whole replace (delete included)', () => {
const { user, day, place } = fixture();
// Used to assert that an id violating the users FK threw and rolled the delete
// back. It cannot reach the insert any more: the roster filter drops an id that
// is not on the trip, so a list of nothing but strangers is simply an empty
// list, and the delete stands. The rollback itself is still covered — the
// transaction wraps delete and insert together, and ASG-SVC-018 pins that an
// explicit empty list clears.
it('ASG-SVC-028: a list of ids that are not on the trip clears rather than throwing', () => {
const { user, trip, day, place } = fixture();
const a = createDayAssignment(testDb, day.id, place.id);
svc.setParticipants(a.id, [user.id]);
// 999999 violates the users FK — the transaction must undo the delete too.
expect(() => svc.setParticipants(a.id, [999999])).toThrow();
expect(svc.getParticipants(a.id)).toEqual([{ user_id: user.id, username: user.username, avatar: null }]);
svc.setParticipants(a.id, [user.id], trip.id);
expect(() => svc.setParticipants(a.id, [999999], trip.id)).not.toThrow();
expect(svc.getParticipants(a.id)).toEqual([]);
});
it('ASG-SVC-029: keeps the trip members and drops the stranger in one call', () => {
const { user, trip, day, place } = fixture();
const { user: member } = createUser(testDb);
const { user: stranger } = createUser(testDb);
addTripMember(testDb, trip.id, member.id);
const a = createDayAssignment(testDb, day.id, place.id);
const rows = svc.setParticipants(a.id, [user.id, stranger.id, member.id], trip.id) as { user_id: number }[];
expect(rows.map(r => r.user_id).sort()).toEqual([user.id, member.id].sort());
expect(JSON.stringify(rows)).not.toContain(stranger.username);
});
});