fix(realtime): trip, member, chat, file, settings, tag and notification writes in parity

This commit is contained in:
Maurice
2026-10-22 12:14:27 +02:00
parent 94b8c3194c
commit ce7b5bf453
51 changed files with 2055 additions and 382 deletions
@@ -11,11 +11,13 @@ import { db as testDb } from '../../../src/db/database';
import { CollabPollVotes } from '../../../src/db/entities/CollabPollVotes.entity';
import { CollabPolls } from '../../../src/db/entities/CollabPolls.entity';
import { TripMembers } from '../../../src/db/entities/TripMembers.entity';
import { NotificationsService } from '../../../src/nest/notifications/notifications.service';
import { createUser, createTrip } from '../../helpers/factories';
import { makeCollabMessage } from '../../helpers/factories/collab';
import { countRows, findRow, insertRow } from '../../helpers/factories/rows';
import { FakeRealtimeService } from '../../helpers/fake-realtime';
import { createMcpHarness, parseToolResult, parseResourceResult, type McpHarness } from '../../helpers/mcp-harness';
import { bootTestApp } from '../../helpers/test-app';
import { resetTestDb } from '../../helpers/test-db';
import { setAddonEnabled } from '../../helpers/test-db';
import { createTestOrm, type TestOrm } from '../../helpers/test-orm';
@@ -408,7 +410,12 @@ describe('Tool: delete_collab_message', () => {
});
const data = parseToolResult(result) as any;
expect(data.success).toBe(true);
expect(broadcastMock).toHaveBeenCalledWith(trip.id, 'collab:message:deleted', expect.any(Object));
// Named like REST names it: the author, who is the only one allowed to delete.
expect(broadcastMock).toHaveBeenCalledWith(
trip.id,
'collab:message:deleted',
expect.objectContaining({ messageId: Number(msgId), username: user.username }),
);
});
});
@@ -528,3 +535,29 @@ describe('Resource: trek://trips/{tripId}/collab/messages', () => {
});
});
});
describe('send_collab_message, as the chat posts it', () => {
it('MCP-COLLAB-001: notifies the fellow travellers with the preview a typed message gets, after the room heard it', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
const send = vi
.spyOn((await bootTestApp(realtime)).get(NotificationsService), 'send')
.mockResolvedValue({ attempted: 0, delivered: 0 });
try {
await withHarness(user.id, async (h) => {
await h.client.callTool({ name: 'send_collab_message', arguments: { tripId: trip.id, text: 'Dinner at 8?' } });
});
expect(broadcastMock).toHaveBeenCalledWith(trip.id, 'collab:message:created', expect.any(Object));
expect(send).toHaveBeenCalledWith(
expect.objectContaining({
event: 'collab_message',
actorId: user.id,
targetId: trip.id,
params: expect.objectContaining({ actor: user.email, preview: 'Dinner at 8?' }),
}),
);
} finally {
send.mockRestore();
}
});
});
+52
View File
@@ -829,6 +829,8 @@ describe('Tool: link_trip_file', () => {
});
expect(result.isError).toBe(true);
expect(JSON.stringify(result.content)).toContain('File not found.');
// Nothing was linked, so no tab is told about the other trip's file.
expect(broadcastMock).not.toHaveBeenCalledWith(expect.anything(), 'file:updated', expect.anything());
});
});
@@ -992,6 +994,56 @@ describe('Tool: unlink_trip_file', () => {
});
});
describe('Link changes reach every tab', () => {
/** The one file:updated a link change sent, and the file it carried. */
function sentFile() {
expect(broadcastMock.mock.calls.map((call) => call[1])).toEqual(['file:updated']);
return broadcastMock.mock.calls[0]![2].file as {
id: number;
linked_place_ids: number[];
linked_reservation_ids: number[];
};
}
it('MCP-FILES-001: link_trip_file sends the file again with every link it now carries', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
const reservation = createReservation(testDb, trip.id);
const place = createPlace(testDb, trip.id);
const file = await insertFile(trip.id);
await linkFile(orm, file.id, { reservation: reservation.id });
await withHarness(user.id, async (h) => {
await h.client.callTool({
name: 'link_trip_file',
arguments: { tripId: trip.id, fileId: file.id, place_id: place.id },
});
});
expect(sentFile()).toMatchObject({
id: file.id,
linked_reservation_ids: [reservation.id],
linked_place_ids: [place.id],
});
});
it('MCP-FILES-002: unlink_trip_file sends the file with the links it has left', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
const reservation = createReservation(testDb, trip.id);
const place = createPlace(testDb, trip.id);
const file = await insertFile(trip.id);
await linkFile(orm, file.id, { reservation: reservation.id });
await linkFile(orm, file.id, { place: place.id });
const doomed = (await linkRows(file.id))[0]!.id;
await withHarness(user.id, async (h) => {
await h.client.callTool({
name: 'unlink_trip_file',
arguments: { tripId: trip.id, fileId: file.id, linkId: doomed },
});
});
expect(sentFile()).toMatchObject({ id: file.id, linked_reservation_ids: [], linked_place_ids: [place.id] });
});
});
// ---------------------------------------------------------------------------
// list_trip_file_links
// ---------------------------------------------------------------------------
+28
View File
@@ -9,11 +9,13 @@ import { db as testDb } from '../../../src/db/database';
import { CollabNotes } from '../../../src/db/entities/CollabNotes.entity';
import { DayNotes } from '../../../src/db/entities/DayNotes.entity';
import { TripFiles } from '../../../src/db/entities/TripFiles.entity';
import { NotificationsService } from '../../../src/nest/notifications/notifications.service';
import { createUser, createTrip, createDay, createDayNote, createCollabNote } from '../../helpers/factories';
import { makeTripFile } from '../../helpers/factories/files';
import { countRows, findRow, updateRows } from '../../helpers/factories/rows';
import { FakeRealtimeService } from '../../helpers/fake-realtime';
import { createMcpHarness, parseToolResult, parseResourceResult, type McpHarness } from '../../helpers/mcp-harness';
import { bootTestApp } from '../../helpers/test-app';
import { resetTestDb } from '../../helpers/test-db';
import { createTestOrm, type TestOrm } from '../../helpers/test-orm';
@@ -797,3 +799,29 @@ describe('Resource: trek://trips/{tripId}/collab-notes', () => {
});
});
});
describe('create_collab_note, as the Collab tab posts it', () => {
it('MCP-COLLAB-002: notifies the fellow travellers like the route does, after the room heard of the note', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
const send = vi
.spyOn((await bootTestApp(realtime)).get(NotificationsService), 'send')
.mockResolvedValue({ attempted: 0, delivered: 0 });
try {
await withHarness(user.id, async (h) => {
await h.client.callTool({ name: 'create_collab_note', arguments: { tripId: trip.id, title: 'Ideas' } });
});
expect(broadcastMock).toHaveBeenCalledWith(trip.id, 'collab:note:created', expect.any(Object));
expect(send).toHaveBeenCalledWith(
expect.objectContaining({
event: 'collab_message',
actorId: user.id,
targetId: trip.id,
params: expect.objectContaining({ actor: user.email }),
}),
);
} finally {
send.mockRestore();
}
});
});
@@ -11,10 +11,13 @@
*/
import { db as testDb } from '../../../src/db/database';
import { Notifications } from '../../../src/db/entities/Notifications.entity';
import { RealtimeService } from '../../../src/nest/realtime/realtime.service';
import { createUser } from '../../helpers/factories';
import { makeNotification } from '../../helpers/factories/notifications';
import { countRows, findRow, updateRows } from '../../helpers/factories/rows';
import type { FakeRealtimeService } from '../../helpers/fake-realtime';
import { createMcpHarness, parseToolResult, parseResourceResult, type McpHarness } from '../../helpers/mcp-harness';
import { bootTestApp } from '../../helpers/test-app';
import { resetTestDb } from '../../helpers/test-db';
import { createTestOrm, type TestOrm } from '../../helpers/test-orm';
@@ -276,3 +279,60 @@ describe('Resource: trek://notifications/in-app', () => {
});
});
});
describe('Read state reaches every tab of the user', () => {
/** The fake the harness container was booted with, emptied for the case. */
async function userMessages() {
const realtime = (await bootTestApp()).get(RealtimeService) as FakeRealtimeService;
realtime.broadcastToUserMock.mockClear();
return realtime.broadcastToUserMock;
}
it('MCP-NOTIF-001: mark_notification_read and _unread send the row as it now reads, to every tab', async () => {
const { user } = createUser(testDb);
const notif = await createNotification(user.id);
const sent = await userMessages();
await withHarness(user.id, async (h) => {
await h.client.callTool({ name: 'mark_notification_read', arguments: { notificationId: notif.id } });
await h.client.callTool({ name: 'mark_notification_unread', arguments: { notificationId: notif.id } });
});
expect(sent.mock.calls.map(([to, message, skip]) => [to, message.type, skip])).toEqual([
[user.id, 'notification:updated', undefined],
[user.id, 'notification:updated', undefined],
]);
const states = sent.mock.calls.map(([, message]) => message.notification as { id: number; is_read: number });
expect(states).toEqual([
expect.objectContaining({ id: notif.id, is_read: 1 }),
expect.objectContaining({ id: notif.id, is_read: 0 }),
]);
});
it('MCP-NOTIF-002: mark_all_notifications_read pings once when something changed, and not for nothing', async () => {
const { user } = createUser(testDb);
await createNotification(user.id);
const sent = await userMessages();
await withHarness(user.id, async (h) => {
await h.client.callTool({ name: 'mark_all_notifications_read', arguments: {} });
await h.client.callTool({ name: 'mark_all_notifications_read', arguments: {} });
});
expect(sent.mock.calls).toEqual([[user.id, { type: 'notification:read-all' }, undefined]]);
});
it("MCP-NOTIF-003: marking somebody else's notification is refused and sends nothing to either user", async () => {
const { user: owner } = createUser(testDb);
const { user: stranger } = createUser(testDb);
const notif = await createNotification(owner.id);
const sent = await userMessages();
await withHarness(stranger.id, async (h) => {
const read = await h.client.callTool({ name: 'mark_notification_read', arguments: { notificationId: notif.id } });
const unread = await h.client.callTool({
name: 'mark_notification_unread',
arguments: { notificationId: notif.id },
});
expect(read.isError).toBe(true);
expect(unread.isError).toBe(true);
});
expect(sent).not.toHaveBeenCalled();
expect((await findRow(orm, Notifications, { id: notif.id }))!.is_read).toBe(0);
});
});
+5 -1
View File
@@ -133,6 +133,9 @@ beforeAll(async () => {
readModelStub,
addonsStub,
promptGuards,
// The trip and roster writes: no prompt reaches a write.
undefined as never,
undefined as never,
);
promptPackingService = new PackingService(
new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)),
@@ -164,7 +167,8 @@ beforeAll(async () => {
new TripMembershipService(await createTestTripsRepo(testDb), await createTestTripMembersRepo(testDb)),
addonsStub,
promptGuards,
await createTestUnitOfWork(testDb),
// BudgetWritesService: a prompt writes nothing.
{} as never,
await createTestPlacesRepo(testDb),
await createTestTripsRepo(testDb),
);
@@ -14,6 +14,7 @@ import { AppSettings } from '../../../src/db/entities/AppSettings.entity';
import { Settings } from '../../../src/db/entities/Settings.entity';
import { encrypt_api_key } from '../../../src/nest/common/crypto/apiKeyCrypto';
import { MANAGED_LOCKED_SETTING_KEYS } from '../../../src/nest/common/managed';
import { RealtimeService } from '../../../src/nest/realtime/realtime.service';
import { DISPLAY_PREFERENCE_KEYS } from '../../../src/nest/settings/settings.mcp';
import {
isAdminOnlyEndpointSetting,
@@ -23,7 +24,9 @@ import {
import { createUser } from '../../helpers/factories';
import { countRows, findRow, insertRow } from '../../helpers/factories/rows';
import { setUserSetting } from '../../helpers/factories/settings';
import type { FakeRealtimeService } from '../../helpers/fake-realtime';
import { createMcpHarness, parseToolResult, type McpHarness } from '../../helpers/mcp-harness';
import { bootTestApp } from '../../helpers/test-app';
import { resetTestDb } from '../../helpers/test-db';
import { createTestOrm, type TestOrm } from '../../helpers/test-orm';
import { MASKED_SETTING_VALUE } from '@trek/shared';
@@ -574,3 +577,28 @@ describe('Settings tools, scope gating', () => {
expect(names).not.toContain('update_display_settings');
});
});
describe('update_display_settings reaches every tab', () => {
it('MCP-SETTINGS-001: tells every tab of the user which keys changed, never their values', async () => {
const { user } = createUser(testDb);
// The fake the harness container was booted with, which records every user message.
const realtime = (await bootTestApp()).get(RealtimeService) as FakeRealtimeService;
realtime.broadcastToUserMock.mockClear();
await withHarness(user.id, async (h) => {
await update(h, { temperature_unit: 'fahrenheit', week_start: 'sunday' });
});
expect(realtime.broadcastToUserMock.mock.calls).toEqual([
[user.id, { type: 'settings:updated', keys: ['temperature_unit', 'week_start'] }, undefined],
]);
});
it('MCP-SETTINGS-002: a refused change sends nothing', async () => {
const { user } = createUser(testDb);
const realtime = (await bootTestApp()).get(RealtimeService) as FakeRealtimeService;
realtime.broadcastToUserMock.mockClear();
await withHarness(user.id, async (h) => {
expect((await update(h, { temperature_unit: 'kelvin' })).isError).toBe(true);
});
expect(realtime.broadcastToUserMock).not.toHaveBeenCalled();
});
});
@@ -273,6 +273,33 @@ describe('Tool: delete_tag', () => {
});
});
describe('Tag writes reach every tab of the user', () => {
it('MCP-TAGS-001: create_tag, update_tag and delete_tag each ping the tag pickers of every tab', async () => {
const { user } = createUser(testDb);
realtime.broadcastToUserMock.mockClear();
await withHarness(user.id, async (h) => {
const created = parseToolResult(
await h.client.callTool({ name: 'create_tag', arguments: { name: 'Beach' } }),
) as { tag: { id: number } };
await h.client.callTool({ name: 'update_tag', arguments: { tagId: created.tag.id, name: 'Coast' } });
await h.client.callTool({ name: 'delete_tag', arguments: { tagId: created.tag.id } });
});
const ping = [user.id, { type: 'tags:changed' }, undefined];
expect(realtime.broadcastToUserMock.mock.calls).toEqual([ping, ping, ping]);
});
it('MCP-TAGS-002: a tag of another user is refused without a ping', async () => {
const { user } = createUser(testDb);
const { user: other } = createUser(testDb);
const foreign = await makeTag(orm, other.id);
realtime.broadcastToUserMock.mockClear();
await withHarness(user.id, async (h) => {
expect((await h.client.callTool({ name: 'delete_tag', arguments: { tagId: foreign.id } })).isError).toBe(true);
});
expect(realtime.broadcastToUserMock).not.toHaveBeenCalled();
});
});
// ---------------------------------------------------------------------------
// tags scope gating (places:read / places:write, registration-time)
// ---------------------------------------------------------------------------
@@ -5,19 +5,22 @@
* copy_trip, export_trip_ics, get_share_link, create_share_link, delete_share_link.
*/
import { db as testDb } from '../../../src/db/database';
import { BudgetItems } from '../../../src/db/entities/BudgetItems.entity';
import { ShareTokens } from '../../../src/db/entities/ShareTokens.entity';
import { TripMembers } from '../../../src/db/entities/TripMembers.entity';
import { Trips } from '../../../src/db/entities/Trips.entity';
import { Users } from '../../../src/db/entities/Users.entity';
import { GUEST_HAS_PAYMENTS } from '../../../src/nest/budget/budget-departure.helpers';
import { NotificationsService } from '../../../src/nest/notifications/notifications.service';
import { invalidatePermissionsCache } from '../../../src/nest/permissions/permissions-cache';
import { createUser, createAdmin, createTrip, addTripMember } from '../../helpers/factories';
import { addBudgetItemPayer, makeBudgetItem } from '../../helpers/factories/budget';
import { countRows, findRow } from '../../helpers/factories/rows';
import { addBudgetItemMember, addBudgetItemPayer, makeBudgetItem } from '../../helpers/factories/budget';
import { countRows, findRow, updateRows } from '../../helpers/factories/rows';
import { setAppSetting } from '../../helpers/factories/settings';
import { makeShareToken } from '../../helpers/factories/trips';
import { FakeRealtimeService } from '../../helpers/fake-realtime';
import { createMcpHarness, parseToolResult, type McpHarness } from '../../helpers/mcp-harness';
import { bootTestApp } from '../../helpers/test-app';
import { resetTestDb } from '../../helpers/test-db';
import { createTestOrm, type TestOrm } from '../../helpers/test-orm';
@@ -770,6 +773,7 @@ describe('Tool: rename_trip_guest', () => {
const otherTrip = createTrip(testDb, owner.id);
await withHarness(owner.id, async (h) => {
const guestId = await makeGuest(h, otherTrip.id, 'Anna');
broadcastMock.mockClear();
const result = await h.client.callTool({
name: 'rename_trip_guest',
arguments: { tripId: ownTrip.id, guestId, name: 'Renamed' },
@@ -777,6 +781,8 @@ describe('Tool: rename_trip_guest', () => {
expect(result.isError).toBe(true);
const row = await userRow(guestId);
expect(row.display_name).toBe('Anna');
// A refused rename announces nothing, on either trip.
expect(broadcastMock).not.toHaveBeenCalledWith(expect.anything(), 'member:updated', expect.anything());
});
});
@@ -856,6 +862,7 @@ describe('Tool: delete_trip_guest', () => {
expect(JSON.stringify(result.content)).toContain(GUEST_HAS_PAYMENTS);
expect(await findRow(orm, Users, { id: guestId })).not.toBeNull();
expect(broadcastMock).not.toHaveBeenCalledWith(trip.id, 'member:removed', expect.anything());
expect(broadcastMock).not.toHaveBeenCalledWith(trip.id, 'budget:updated', expect.anything());
});
});
@@ -973,3 +980,86 @@ describe('Guest tools stay owner-only', () => {
expect(await countRows(orm, Users, { is_guest: 1 })).toBe(1);
});
});
// ---------------------------------------------------------------------------
// The roster use cases REST shares (TripMemberWritesService): what each tool
// sends besides its member event.
// ---------------------------------------------------------------------------
describe('Roster writes, as REST makes them', () => {
it('MCP-MEMBERS-001: add_trip_member sends the invitation the planner sends, naming the inviter by address', async () => {
const { user: owner } = createUser(testDb);
const { user: invitee } = createUser(testDb);
const trip = createTrip(testDb, owner.id);
const send = vi
.spyOn((await bootTestApp(realtime)).get(NotificationsService), 'send')
.mockResolvedValue({ attempted: 0, delivered: 0 });
try {
await withHarness(owner.id, async (h) => {
await h.client.callTool({ name: 'add_trip_member', arguments: { tripId: trip.id, identifier: invitee.email } });
});
expect(send).toHaveBeenCalledWith(
expect.objectContaining({
event: 'trip_invite',
actorId: owner.id,
scope: 'user',
targetId: invitee.id,
params: expect.objectContaining({ actor: owner.email, invitee: invitee.email, tripId: String(trip.id) }),
}),
);
} finally {
send.mockRestore();
}
});
it('MCP-MEMBERS-002: rename_trip_guest tells the room the new name, and nothing else of the row', async () => {
const { user: owner } = createUser(testDb);
const trip = createTrip(testDb, owner.id);
await withHarness(owner.id, async (h) => {
const guestId = await makeGuest(h, trip.id, 'Anna');
broadcastMock.mockClear();
await h.client.callTool({
name: 'rename_trip_guest',
arguments: { tripId: trip.id, guestId, name: ' Anna B. ' },
});
expect(broadcastMock.mock.calls).toEqual([
[trip.id, 'member:updated', { member: { id: guestId, username: 'Anna B.' }, _source: 'mcp' }],
]);
});
});
it('MCP-MEMBERS-003: delete_trip_guest sends each expense the guest shared again, split among the rest, after the member', async () => {
const { user: owner } = createUser(testDb);
const trip = createTrip(testDb, owner.id);
await withHarness(owner.id, async (h) => {
const guestId = await makeGuest(h, trip.id, 'Anna');
const shared = await makeBudgetItem(orm, trip.id, { total_price: 60 });
await addBudgetItemMember(orm, shared.id, owner.id);
await addBudgetItemMember(orm, shared.id, guestId);
await makeBudgetItem(orm, trip.id, { total_price: 10 });
broadcastMock.mockClear();
await h.client.callTool({ name: 'delete_trip_guest', arguments: { tripId: trip.id, guestId } });
expect(broadcastMock.mock.calls.map((call) => call[1])).toEqual(['member:removed', 'budget:updated']);
const [, , payload] = broadcastMock.mock.calls[1]!;
const item = payload.item as { id: number; members: { user_id: number }[] };
expect(item.id).toBe(shared.id);
expect(item.members.map((m) => m.user_id)).toEqual([owner.id]);
});
});
it('MCP-MEMBERS-004: delete_trip_guest also sends an expense the guest was only the legacy payer of, cleared', async () => {
const { user: owner } = createUser(testDb);
const trip = createTrip(testDb, owner.id);
await withHarness(owner.id, async (h) => {
const guestId = await makeGuest(h, trip.id, 'Anna');
const legacy = await makeBudgetItem(orm, trip.id, { total_price: 40 });
await updateRows(orm, BudgetItems, { id: legacy.id }, { paidByUser: guestId });
broadcastMock.mockClear();
await h.client.callTool({ name: 'delete_trip_guest', arguments: { tripId: trip.id, guestId } });
expect(broadcastMock.mock.calls.map((call) => call[1])).toEqual(['member:removed', 'budget:updated']);
const item = broadcastMock.mock.calls[1]![2].item as { id: number; paid_by_user_id: number | null };
expect(item.id).toBe(legacy.id);
expect(item.paid_by_user_id).toBeNull();
});
});
});
+91 -1
View File
@@ -13,6 +13,7 @@ import { Trips } from '../../../src/db/entities/Trips.entity';
import { VacayEntries } from '../../../src/db/entities/VacayEntries.entity';
import { VacayUserYears } from '../../../src/db/entities/VacayUserYears.entity';
import { VacayYears } from '../../../src/db/entities/VacayYears.entity';
import { invalidatePermissionsCache } from '../../../src/nest/permissions/permissions-cache';
import {
createUser,
createTrip,
@@ -29,7 +30,7 @@ import {
} from '../../helpers/factories';
import { makePackingItem } from '../../helpers/factories/packing';
import { countRows, findRow, findRows, insertRow, insertRows, updateRows } from '../../helpers/factories/rows';
import { setUserSetting } from '../../helpers/factories/settings';
import { setAppSetting, setUserSetting } from '../../helpers/factories/settings';
import { makeVacayEntry, makeVacayPlan, addVacayPlanMember } from '../../helpers/factories/vacay';
import { FakeRealtimeService } from '../../helpers/fake-realtime';
import { createMcpHarness, parseToolResult, parseResourceResult, type McpHarness } from '../../helpers/mcp-harness';
@@ -1090,3 +1091,92 @@ describe('scope gating', () => {
for (const t of [...WRITE_TOOLS, ...READ_TOOLS, 'delete_trip', ...SHARE_TOOLS]) expect(names).not.toContain(t);
});
});
// ---------------------------------------------------------------------------
// The trip writes REST shares (TripWritesService) and the rights PUT
// /api/trips/:id checks field by field.
// ---------------------------------------------------------------------------
describe('Trip writes, as REST makes them', () => {
/** Lower or raise a configurable action the way the admin permission panel does. */
async function setPermission(action: string, level: string): Promise<void> {
await setAppSetting(orm, `perm_${action}`, level);
await invalidatePermissionsCache();
}
beforeEach(() => {
realtime.broadcastToUserMock.mockClear();
});
afterEach(async () => {
await invalidatePermissionsCache();
});
it('MCP-TRIPS-001: create_trip and copy_trip tell every tab of the owner that the trip list changed', async () => {
const { user } = createUser(testDb);
await withHarness(user.id, async (h) => {
const created = parseToolResult(
await h.client.callTool({ name: 'create_trip', arguments: { title: 'Lisbon' } }),
) as { trip: { id: number } };
await h.client.callTool({ name: 'copy_trip', arguments: { tripId: created.trip.id } });
});
expect(realtime.broadcastToUserMock.mock.calls).toEqual([
[user.id, { type: 'trips:changed' }, undefined],
[user.id, { type: 'trips:changed' }, undefined],
]);
});
it('MCP-TRIPS-002: create_trip and copy_trip are refused without trip_create, as the routes refuse them', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
await setPermission('trip_create', 'admin');
await withHarness(user.id, async (h) => {
const created = await h.client.callTool({ name: 'create_trip', arguments: { title: 'Lisbon' } });
const copied = await h.client.callTool({ name: 'copy_trip', arguments: { tripId: trip.id } });
for (const result of [created, copied]) {
expect(result.isError).toBe(true);
expect(JSON.stringify(result.content)).toContain('You do not have permission to create trips.');
}
});
expect(await countRows(orm, Trips, { user: user.id })).toBe(1);
expect(realtime.broadcastToUserMock).not.toHaveBeenCalled();
});
it('MCP-TRIPS-003: delete_trip tells the room the trip is gone, as the route does', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
await withHarness(user.id, async (h) => {
await h.client.callTool({ name: 'delete_trip', arguments: { tripId: trip.id } });
});
expect(broadcastMock.mock.calls).toEqual([[trip.id, 'trip:deleted', { id: trip.id, _source: 'mcp' }]]);
});
it('MCP-TRIPS-004: update_trip asks trip_archive and trip_cover_upload for those fields, not trip_edit alone', async () => {
const { user: owner } = createUser(testDb);
const { user: member } = createUser(testDb);
const trip = createTrip(testDb, owner.id, { title: 'Before' });
addTripMember(testDb, trip.id, member.id);
await setPermission('trip_edit', 'trip_member');
await withHarness(member.id, async (h) => {
const archive = await h.client.callTool({
name: 'update_trip',
arguments: { tripId: trip.id, is_archived: true },
});
const cover = await h.client.callTool({ name: 'update_trip', arguments: { tripId: trip.id, cover_image: null } });
expect(archive.isError).toBe(true);
expect(cover.isError).toBe(true);
expect((await tripRow(trip.id)).is_archived).toBe(0);
const rename = await h.client.callTool({ name: 'update_trip', arguments: { tripId: trip.id, title: 'After' } });
expect(rename.isError).toBeFalsy();
});
await setPermission('trip_archive', 'trip_member');
await withHarness(member.id, async (h) => {
const archive = await h.client.callTool({
name: 'update_trip',
arguments: { tripId: trip.id, is_archived: true },
});
expect(archive.isError).toBeFalsy();
});
expect(await tripRow(trip.id)).toMatchObject({ title: 'After', is_archived: 1 });
});
});
@@ -2,8 +2,9 @@
* Sharing an assistant answer in the trip chat, over the real thread and
* message repositories on a migrated test db: the chat must be on, the thread
* the user's on this trip, the message an answer in it; then it is posted as
* the user's own message with the assistant's origin, broadcast and notified
* like a typed one.
* the user's own message with the assistant's origin, through the chat's own
* posting use case, which broadcasts and notifies like a typed message
* (collab.service.test.ts holds that, the preview included).
*
* Replaced: the addon switches and the collab chat itself.
*/
@@ -33,11 +34,14 @@ const addons = {
getCollabFeatures: vi.fn(async () => ({ chat: true, notes: true, polls: true })),
};
const collab = {
createMessage: vi.fn(async (_trip: number, userId: number, text: string) => ({
message: { id: 90, user_id: userId, text: text.trim(), origin: 'assistant' },
restWriter: vi.fn((tripId: string, user: User, socketId: string | undefined) => ({
tripId,
userId: user.id,
socketId,
})),
postMessage: vi.fn(async (_trip: number, input: { text: string; origin?: string | null }, w: { userId: number }) => ({
message: { id: 90, user_id: w.userId, text: input.text.trim(), origin: input.origin },
})),
broadcast: vi.fn(),
notifyCollab: vi.fn(async () => undefined),
};
beforeAll(async () => {
@@ -81,25 +85,24 @@ function shareAs(user: User, tripId: number, threadId: number, messageId: string
}
describe('AssistantShareService', () => {
it('ASTSHR-001: posts the answer as the user, marked as from the assistant, broadcast and notified', async () => {
it('ASTSHR-001: posts the answer as the user, marked as from the assistant, through the chat posting use case', async () => {
const c = await conversation();
const service = t.get(AssistantShareService);
const shared = await service.share(c.user, c.tripId, c.threadId, `msg-${c.answer.id}`, 'x'.repeat(100), 'sock-1');
expect(collab.createMessage).toHaveBeenCalledWith(c.tripId, c.user.id, 'x'.repeat(100), null, [], 'assistant');
expect(shared).toEqual({ message: expect.objectContaining({ id: 90, origin: 'assistant' }) });
expect(collab.broadcast).toHaveBeenCalledWith(
String(c.tripId),
'collab:message:created',
{ message: shared.message },
'sock-1',
const shared = await service.share(c.user, c.tripId, c.threadId, `msg-${c.answer.id}`, 'x'.repeat(100), undefined);
expect(collab.postMessage).toHaveBeenCalledWith(
c.tripId,
{ text: 'x'.repeat(100), origin: 'assistant' },
{ tripId: String(c.tripId), userId: c.user.id, socketId: undefined },
);
expect(collab.notifyCollab).toHaveBeenCalledWith(String(c.tripId), c.user, `${'x'.repeat(80)}...`);
expect(shared).toEqual({ message: expect.objectContaining({ id: 90, origin: 'assistant' }) });
});
it('ASTSHR-002: a short answer is previewed whole', async () => {
it('ASTSHR-002: the asking tab hears its shared message too: the socket the route names is not left out', async () => {
const c = await conversation();
await shareAs(c.user, c.tripId, c.threadId, `msg-${c.answer.id}`);
expect(collab.notifyCollab).toHaveBeenCalledWith(String(c.tripId), c.user, 'Hi');
await t.get(AssistantShareService).share(c.user, c.tripId, c.threadId, `msg-${c.answer.id}`, 'Hi', 'sock-1');
// It never put the message into its own chat, so a writer skipping it would leave the chat without it.
expect(collab.restWriter).toHaveBeenCalledWith(String(c.tripId), c.user, undefined);
expect(collab.postMessage.mock.calls[0]?.[2]).toMatchObject({ socketId: undefined });
});
it('ASTSHR-003: refused while the collab addon or its chat is off', async () => {
@@ -109,7 +112,7 @@ describe('AssistantShareService', () => {
await expect(share()).rejects.toMatchObject({ status: 403, message: 'Collab chat is off' });
addons.getCollabFeatures.mockResolvedValueOnce({ chat: false, notes: true, polls: true });
await expect(share()).rejects.toMatchObject({ status: 403, code: 'forbidden' });
expect(collab.createMessage).not.toHaveBeenCalled();
expect(collab.postMessage).not.toHaveBeenCalled();
});
it('ASTSHR-004: a thread of another user or trip, a question, an answer elsewhere or no id is not found', async () => {
@@ -121,14 +124,14 @@ describe('AssistantShareService', () => {
await expect(shareAs(c.user, c.tripId, c.threadId, `msg-${c.question.id}`)).rejects.toMatchObject(notFound);
await expect(shareAs(c.user, c.tripId, c.threadId, `msg-${other.answer.id}`)).rejects.toMatchObject(notFound);
await expect(shareAs(c.user, c.tripId, c.threadId, 'msg-x')).rejects.toBeInstanceOf(DomainError);
expect(collab.createMessage).not.toHaveBeenCalled();
expect(collab.postMessage).not.toHaveBeenCalled();
});
it('ASTSHR-005: a message the chat could not post is a refusal, never a silent success', async () => {
const c = await conversation();
collab.createMessage.mockResolvedValueOnce({ message: undefined } as never);
collab.postMessage.mockResolvedValueOnce({ message: undefined } as never);
await expect(shareAs(c.user, c.tripId, c.threadId, `msg-${c.answer.id}`)).rejects.toMatchObject({ status: 400 });
expect(collab.broadcast).not.toHaveBeenCalled();
expect(collab.postMessage).toHaveBeenCalledTimes(1);
});
it('ASTSHR-006: reads a wire message id as its row', () => {
@@ -4,7 +4,8 @@ import {
collabChatImageFilter,
collabNoteFileFilter,
} from '../../../src/nest/collab/collab.controller';
import type { CollabService } from '../../../src/nest/collab/collab.service';
import type { ChatMessageInput, CollabService } from '../../../src/nest/collab/collab.service';
import type { TripWriter } from '../../../src/nest/common/trip-writer';
import { TripAccessGuard, TRIP_PERMISSION_KEY } from '../../../src/nest/permissions/trip-access.guard';
import type { StorageService } from '../../../src/nest/storage/storage.service';
import type { User } from '../../../src/types';
@@ -17,15 +18,30 @@ import { describe, it, expect, vi, beforeEach } from 'vitest';
const user = { id: 1, username: 'u', role: 'user', email: 'u@example.test' } as User;
/** The writer a route builds for its request, as the stub hands it back. */
const restWriter = (socketId?: string) => ({ userId: 1, role: 'user', socketId });
function svc(o: Partial<CollabService> = {}): CollabService {
return {
const s = {
verifyTripAccess: vi.fn().mockResolvedValue({ user_id: 1 }),
canEdit: vi.fn().mockResolvedValue(true),
canUploadFiles: vi.fn().mockResolvedValue(true),
broadcast: vi.fn(),
notifyCollab: vi.fn(),
restWriter: vi.fn((_tripId: string, _user: User, socketId: string | undefined) => restWriter(socketId)),
...o,
} as unknown as CollabService;
// The two posting use cases over the stubbed building blocks, so a case can stub
// createNote or createMessage as before. What the real ones send and notify is
// held by collab.service.test.ts and the broadcast parity suite.
const post = s as unknown as Record<'postNote' | 'postMessage', unknown>;
post.postNote ??= vi.fn((tripId: string, data: Parameters<CollabService['createNote']>[2], w: TripWriter) =>
s.createNote(tripId, w.userId, data),
);
post.postMessage ??= vi.fn((tripId: string, input: ChatMessageInput, w: TripWriter) =>
s.createMessage(tripId, w.userId, input.text, input.replyTo ?? null, input.files ?? [], input.origin ?? null),
);
return s;
}
const storageStub = {
@@ -53,23 +69,18 @@ describe('CollabController (parity with the legacy /api/trips/:tripId/collab rou
expect(await new CollabController(s, storageStub).listNotes(user, '5')).toEqual({ notes: [{ id: 1 }] });
});
it('POST creates + broadcasts + notifies (empty title now 400s in the Zod pipe)', async () => {
it('POST creates through the posting use case, which broadcasts and notifies (empty title now 400s in the Zod pipe)', async () => {
const createNote = vi.fn().mockReturnValue({ id: 9 });
const broadcast = vi.fn();
const notifyCollab = vi.fn();
const s = svc({ createNote, broadcast, notifyCollab } as Partial<CollabService>);
const s = svc({ createNote } as Partial<CollabService>);
expect(
await new CollabController(s, storageStub).createNote(user, '5', { title: 'T', content: 'c' }, 'sock'),
).toEqual({ note: { id: 9 } });
expect(createNote).toHaveBeenCalledWith('5', 1, {
title: 'T',
content: 'c',
category: undefined,
color: undefined,
website: undefined,
});
expect(broadcast).toHaveBeenCalledWith('5', 'collab:note:created', { note: { id: 9 } }, 'sock');
expect(notifyCollab).toHaveBeenCalledWith('5', user);
const fields = { title: 'T', content: 'c', category: undefined, color: undefined, website: undefined };
expect(createNote).toHaveBeenCalledWith('5', 1, fields);
expect(s.restWriter).toHaveBeenCalledWith('5', user, 'sock');
expect(s.postNote).toHaveBeenCalledWith('5', fields, restWriter('sock'));
// Nothing goes out from the route itself, or it would go out twice.
expect(s.broadcast).not.toHaveBeenCalled();
});
it('PUT 404 when missing, else updates + broadcasts', async () => {
@@ -292,18 +303,13 @@ describe('CollabController (parity with the legacy /api/trips/:tripId/collab rou
).createMessage(user, '5', { text: 'hi', reply_to: 99 }, undefined),
),
).toEqual({ status: 400, body: { error: 'Reply target message not found' } });
const broadcast = vi.fn();
const notifyCollab = vi.fn();
const s = svc({
createMessage: vi.fn().mockResolvedValue({ message: { id: 3 } }),
broadcast,
notifyCollab,
} as Partial<CollabService>);
const s = svc({ createMessage: vi.fn().mockResolvedValue({ message: { id: 3 } }) } as Partial<CollabService>);
expect(
await new CollabController(s, storageStub).createMessage(user, '5', { text: 'hello' }, undefined, 'sock'),
).toEqual({ message: { id: 3 } });
expect(broadcast).toHaveBeenCalledWith('5', 'collab:message:created', { message: { id: 3 } }, 'sock');
expect(notifyCollab).toHaveBeenCalledWith('5', user, 'hello');
// The posting use case sends and notifies; the route hands it the trimmed text and its writer.
expect(s.postMessage).toHaveBeenCalledWith('5', { text: 'hello', replyTo: null, files: [] }, restWriter('sock'));
expect(s.broadcast).not.toHaveBeenCalled();
});
it('refuses a caller who cannot reach the trip or cannot write, from inside the handler', async () => {
@@ -380,12 +386,12 @@ describe('CollabController (parity with the legacy /api/trips/:tripId/collab rou
expect(broadcast).toHaveBeenCalledWith('5', 'collab:message:deleted', { messageId: 3, username: 'bob' }, 'sock');
});
it("U5: deleteMessage broadcast falls back to the caller's own username when the service's username is undefined (the real, pre-existing shape)", async () => {
it("U5: deleteMessage broadcast falls back to the caller's own username when the service's username is undefined", async () => {
const broadcast = vi.fn();
// `CollabService.deleteMessage` really does return `{ username: undefined }`
// (findInTrip never joins users) — the controller's `result.username ||
// user.username` fallback is what keeps the broadcast correct, and only
// works because the route already requires caller === message owner.
// `CollabService.deleteMessage` returned `{ username: undefined }` for
// years (findInTrip never joined users). It now reads the author's name,
// but the controller keeps its `result.username || user.username`
// fallback, which is correct because only the owner may delete.
const s = svc({
deleteMessage: vi.fn().mockResolvedValue({ username: undefined }),
broadcast,
+100 -8
View File
@@ -25,6 +25,8 @@ import { dbNow } from '../../../src/db/types/db-timestamp.type';
import { CollabService } from '../../../src/nest/collab/collab.service';
import { avatarUrl } from '../../../src/nest/common/avatarUrl';
import { RateLimitService } from '../../../src/nest/common/rate-limit.service';
import type { TripWriter } from '../../../src/nest/common/trip-writer';
import type { NotificationsService } from '../../../src/nest/notifications/notifications.service';
import { PermissionsService } from '../../../src/nest/permissions/permissions.service';
import { RealtimeService } from '../../../src/nest/realtime/realtime.service';
import {
@@ -37,6 +39,7 @@ import {
} from '../../helpers/collab-repos';
import { createUser, createTrip } from '../../helpers/factories';
import { countRows, findRow, findRows, insertRow, updateRows } from '../../helpers/factories/rows';
import { FakeRealtimeService } from '../../helpers/fake-realtime';
import { notificationsStub } from '../../helpers/notifications';
import { makeStorageFixture } from '../../helpers/storage-fixture';
import { resetTestDb } from '../../helpers/test-db';
@@ -111,13 +114,18 @@ let pollVotesRepo: CollabPollVotesRepository;
let linksRepo: CollabLinksRepository;
let messagesRepo: CollabMessagesRepository;
async function buildCollabService(storage = collabFx.storage, rl = rateLimit): Promise<CollabService> {
async function buildCollabService(
storage = collabFx.storage,
rl = rateLimit,
notifications = notificationsStub(),
realtime: RealtimeService = new RealtimeService(),
): Promise<CollabService> {
// Plan 4 Task 2/4 — CollabService's own DatabaseService param is gone:
// canAccessTrip now reads through the TripsRepository passed at the end.
return new CollabService(
new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)),
new RealtimeService(),
notificationsStub(),
realtime,
notifications,
storage,
rl,
await createTestUnitOfWork(testDb),
@@ -396,6 +404,92 @@ describe('createMessage', () => {
});
});
// ── posting (every surface) ───────────────────────────────────────────────────
describe('postNote / postMessage', () => {
/** A CollabService whose broadcasts and notifications a case reads. */
async function postingSvc() {
const realtime = new FakeRealtimeService();
const send = vi.fn(async (_payload: { params: Record<string, string> }) => ({ attempted: 0, delivered: 0 }));
const notifications = {
send,
actorEmail: vi.fn(async () => 'owner@collab.test'),
} as unknown as NotificationsService;
const service = await buildCollabService(collabFx.storage, rateLimit, notifications, realtime);
return { service, realtime, send };
}
/** The MCP shape of a writer: every socket, no socket left out. */
const mcpWriter = (service: CollabService, tripId: number, userId: number): TripWriter => ({
...service.restWriter(tripId, { id: userId, role: 'user' }, undefined),
surface: 'mcp',
});
it('COLLAB-SVC-050: a posted note goes to the room minus the asking tab, then notifies the trip as its writer', async () => {
const { user1, trip } = setup();
const { service, realtime, send } = await postingSvc();
const note = await service.postNote(
trip.id,
{ title: 'Ideas' },
service.restWriter(String(trip.id), { id: user1.id, role: 'user' }, 'sock-1'),
);
expect(realtime.broadcastMock.mock.calls).toEqual([[String(trip.id), 'collab:note:created', { note }, 'sock-1']]);
expect(send).toHaveBeenCalledWith(
expect.objectContaining({
event: 'collab_message',
actorId: user1.id,
scope: 'trip',
targetId: trip.id,
params: expect.objectContaining({ actor: 'owner@collab.test', tripId: String(trip.id) }),
}),
);
});
it('COLLAB-SVC-051: a posted message is sent and notified with an 80 character preview, from any surface', async () => {
const { user1, trip } = setup();
const { service, realtime, send } = await postingSvc();
const long = 'x'.repeat(100);
const result = await service.postMessage(trip.id, { text: ` ${long} ` }, mcpWriter(service, trip.id, user1.id));
expect(result.message!.text).toBe(long);
expect(realtime.broadcastMock).toHaveBeenCalledWith(
trip.id,
'collab:message:created',
{ message: result.message },
undefined,
);
expect(send).toHaveBeenCalledWith(
expect.objectContaining({ params: expect.objectContaining({ preview: `${'x'.repeat(80)}...` }) }),
);
});
it('COLLAB-SVC-052: a short message is previewed whole, and an image without text says so', async () => {
const { user1, trip } = setup();
const { service, send } = await postingSvc();
const writer = mcpWriter(service, trip.id, user1.id);
await service.postMessage(trip.id, { text: 'Hi' }, writer);
await service.postMessage(
trip.id,
{ text: '', files: [{ filename: 'a.jpg', originalname: 'a.jpg', size: 3, mimetype: 'image/jpeg' }] },
writer,
);
const previews = send.mock.calls.map(([payload]) => payload.params.preview);
expect(previews).toEqual(['Hi', 'sent an image']);
});
it('COLLAB-SVC-053: a reply to a message that is gone sends and notifies nothing', async () => {
const { user1, trip } = setup();
const { service, realtime, send } = await postingSvc();
const result = await service.postMessage(
trip.id,
{ text: 'Hi', replyTo: 9999 },
mcpWriter(service, trip.id, user1.id),
);
expect(result).toEqual({ error: 'reply_not_found' });
expect(realtime.broadcastMock).not.toHaveBeenCalled();
expect(send).not.toHaveBeenCalled();
});
});
// ── deleteMessage ownership check ─────────────────────────────────────────────
describe('deleteMessage', () => {
@@ -419,11 +513,9 @@ describe('deleteMessage', () => {
const result = await svc.deleteMessage(trip.id, r.message!.id, user1.id);
expect(result.error).toBeUndefined();
// U5 (pre-existing, pinned not fixed): findInTrip never joins users, so
// this has always been undefined at runtime — the controller falls back
// to the caller's own username (CollabController.deleteMessage, correct
// because only the owner may delete their own message).
expect(result.username).toBeUndefined();
// The author's name, read before the delete: MCP has no session user to
// fall back on, and REST's fallback names the same person anyway.
expect(result.username).toBe(user1.username);
const row = await findRow(orm, CollabMessages, { id: r.message!.id });
expect(row?.deleted).toBe(1);
@@ -336,22 +336,40 @@ describe('FilesController (parity with the legacy /api/trips/:tripId/files route
),
),
).toEqual({ status: 404, body: { error: 'File not found' } });
const createFileLink = vi.fn().mockReturnValue([{ id: 1 }]);
const s = fsvc({ getFileById: vi.fn().mockReturnValue({ id: 9 }), createFileLink } as Partial<FilesService>);
expect(await fc(s).link(user, trip, '5', '9', { reservation_id: 2 })).toEqual({
// Through the link use case, which sends the file with its links to the room minus the asking tab.
const writer = { socketId: 'sock' };
const restWriter = vi.fn().mockReturnValue(writer);
const linkFile = vi.fn().mockReturnValue([{ id: 1 }]);
const s = fsvc({
getFileById: vi.fn().mockReturnValue({ id: 9 }),
linkFile,
restWriter,
} as Partial<FilesService>);
expect(await fc(s).link(user, trip, '5', '9', { reservation_id: 2 }, 'sock')).toEqual({
success: true,
links: [{ id: 1 }],
});
expect(restWriter).toHaveBeenCalledWith('5', user, 'sock');
expect(linkFile).toHaveBeenCalledWith(
'9',
{ reservation_id: 2, assignment_id: undefined, place_id: undefined, budget_item_id: undefined },
writer,
);
});
it('DELETE /:id/link/:linkId removes the link; GET /:id/links lists', async () => {
const deleteFileLink = vi.fn();
const writer = { socketId: 'sock' };
const unlinkFile = vi.fn();
expect(
await fc(
fsvc({ getFileById: vi.fn().mockReturnValue({ id: 9 }), deleteFileLink } as Partial<FilesService>),
).unlink(user, trip, '5', '9', '3'),
fsvc({
getFileById: vi.fn().mockReturnValue({ id: 9 }),
unlinkFile,
restWriter: vi.fn().mockReturnValue(writer),
} as Partial<FilesService>),
).unlink(user, trip, '5', '9', '3', 'sock'),
).toEqual({ success: true });
expect(deleteFileLink).toHaveBeenCalledWith('3', '9');
expect(unlinkFile).toHaveBeenCalledWith('9', '3', writer);
const s = fsvc({
getFileById: vi.fn().mockReturnValue({ id: 9 }),
getFileLinks: vi.fn().mockReturnValue([{ id: 1 }]),
@@ -363,7 +381,7 @@ describe('FilesController (parity with the legacy /api/trips/:tripId/files route
const foreign = () =>
fsvc({
getFileById: vi.fn().mockReturnValue(undefined),
deleteFileLink: vi.fn(),
unlinkFile: vi.fn(),
getFileLinks: vi.fn(),
} as Partial<FilesService>);
const unlinkSvc = foreign();
@@ -371,7 +389,7 @@ describe('FilesController (parity with the legacy /api/trips/:tripId/files route
status: 404,
body: { error: 'File not found' },
});
expect(unlinkSvc.deleteFileLink).not.toHaveBeenCalled();
expect(unlinkSvc.unlinkFile).not.toHaveBeenCalled();
const listSvc = foreign();
expect(await rejected(fc(listSvc).links(user, trip, '5', '9'))).toEqual({
status: 404,
@@ -199,7 +199,9 @@ describe('NotificationsController (parity with the legacy /api/notifications rou
describe('bulk + single mutations', () => {
it('read-all returns success + count', async () => {
const markAllRead = vi.fn().mockReturnValue(3);
expect(await makeController({ markAllRead }).readAll(user)).toEqual({ success: true, count: 3 });
expect(await makeController({ markAllRead }).readAll(user, 'sock')).toEqual({ success: true, count: 3 });
// The service tells the user's other tabs; the one that asked is left out.
expect(markAllRead).toHaveBeenCalledWith(4, 'sock');
});
it('delete-all returns success + count', async () => {
@@ -224,8 +226,18 @@ describe('NotificationsController (parity with the legacy /api/notifications rou
it('mark-read success', async () => {
const markRead = vi.fn().mockReturnValue(true);
expect(await makeController({ markRead }).markRead(user, '5')).toEqual({ success: true });
expect(markRead).toHaveBeenCalledWith(5, 4);
expect(await makeController({ markRead }).markRead(user, '5', 'sock')).toEqual({ success: true });
expect(markRead).toHaveBeenCalledWith(5, 4, 'sock');
});
it('mark-unread hands the socket on, and 404s when nothing changed', async () => {
const markUnread = vi.fn().mockReturnValueOnce(true).mockReturnValueOnce(false);
expect(await makeController({ markUnread }).markUnread(user, '5', 'sock')).toEqual({ success: true });
expect(markUnread).toHaveBeenCalledWith(5, 4, 'sock');
expect(await thrown(() => makeController({ markUnread }).markUnread(user, '6'))).toEqual({
status: 404,
body: { error: 'Not found' },
});
});
it('delete single success', async () => {
@@ -1,4 +1,5 @@
import type { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env.service';
import type { SettingsWritesService } from '../../../src/nest/settings/settings-writes.service';
import { SettingsController } from '../../../src/nest/settings/settings.controller';
import type { SettingsService } from '../../../src/nest/settings/settings.service';
import type { User } from '../../../src/types';
@@ -30,12 +31,34 @@ async function thrown(fn: () => unknown): Promise<{ status: number; body: unknow
beforeEach(() => vi.clearAllMocks());
/**
* SettingsWritesService over the stubbed service: the writes go through it, so
* it hands each on and records the socket the route read. The settings:updated
* it sends is held by tools-settings.test.ts and the broadcast parity suite.
*/
function writesFor(s: SettingsService) {
return {
saveOne: vi.fn((userId: number, key: string, value: unknown, _socketId: string | undefined) =>
s.upsertSetting(userId, key, value),
),
saveMany: vi.fn((userId: number, values: Record<string, unknown>, _socketId: string | undefined) =>
s.bulkUpsertSettings(userId, values),
),
};
}
let writes: ReturnType<typeof writesFor>;
function controller(s: SettingsService) {
writes = writesFor(s);
const env = { isManaged: () => false } as unknown as RuntimeEnvService;
return new SettingsController(s, env, writes as unknown as SettingsWritesService);
}
describe('SettingsController', () => {
it('GET / returns the settings', async () => {
expect(
await new SettingsController(
await controller(
svc({ getUserSettings: vi.fn().mockReturnValue({ theme: 'dark' }) } as Partial<SettingsService>),
{ isManaged: () => false } as unknown as RuntimeEnvService,
).list(user),
).toEqual({ settings: { theme: 'dark' } });
});
@@ -45,28 +68,27 @@ describe('SettingsController', () => {
it('PUT / no-ops on the masked sentinel without writing', async () => {
const upsertSetting = vi.fn();
const c = new SettingsController(svc({ upsertSetting } as Partial<SettingsService>), {
isManaged: () => false,
} as unknown as RuntimeEnvService);
const c = controller(svc({ upsertSetting } as Partial<SettingsService>));
expect(await c.upsert(user, { key: 'immich_api_key', value: '••••••••' })).toEqual({
success: true,
key: 'immich_api_key',
unchanged: true,
});
expect(upsertSetting).not.toHaveBeenCalled();
expect(writes.saveOne).not.toHaveBeenCalled();
});
it('PUT / writes a real value', async () => {
const upsertSetting = vi.fn();
const c = new SettingsController(svc({ upsertSetting } as Partial<SettingsService>), {
isManaged: () => false,
} as unknown as RuntimeEnvService);
expect(await c.upsert(user, { key: 'theme', value: 'dark' })).toEqual({
const c = controller(svc({ upsertSetting } as Partial<SettingsService>));
expect(await c.upsert(user, { key: 'theme', value: 'dark' }, 'sock')).toEqual({
success: true,
key: 'theme',
value: 'dark',
});
expect(upsertSetting).toHaveBeenCalledWith(1, 'theme', 'dark');
// Through the writes, which tell the user's other tabs and leave out this one.
expect(writes.saveOne).toHaveBeenCalledWith(1, 'theme', 'dark', 'sock');
});
it('POST /bulk 500 on a write error, else returns the count', async () => {
@@ -75,21 +97,22 @@ describe('SettingsController', () => {
vi.spyOn(console, 'error').mockImplementation(() => {});
expect(
await thrown(() =>
new SettingsController(
controller(
svc({
bulkUpsertSettings: vi.fn(() => {
throw new Error('db');
}),
} as Partial<SettingsService>),
{ isManaged: () => false } as unknown as RuntimeEnvService,
).bulk(user, { settings: { a: 1 } }),
),
).toEqual({ status: 500, body: { error: 'Error saving settings' } });
expect(
await new SettingsController(
svc({ bulkUpsertSettings: vi.fn().mockReturnValue(3) } as Partial<SettingsService>),
{ isManaged: () => false } as unknown as RuntimeEnvService,
).bulk(user, { settings: { a: 1, b: 2, c: 3 } }),
await controller(svc({ bulkUpsertSettings: vi.fn().mockReturnValue(3) } as Partial<SettingsService>)).bulk(
user,
{ settings: { a: 1, b: 2, c: 3 } },
'sock',
),
).toEqual({ success: true, updated: 3 });
expect(writes.saveMany).toHaveBeenCalledWith(1, { a: 1, b: 2, c: 3 }, 'sock');
});
});
@@ -0,0 +1,88 @@
/**
* SettingsWritesService: the save goes through SettingsService, and the user's
* other tabs hear which keys changed. Keys only, never a value, and a masked
* secret echoed back by a settings form is named in no event, because nothing
* about it changed.
*
* Replaced: SettingsService (its writes have their own suite) and the realtime
* transport, which the test module fakes.
*/
import { SettingsWritesService } from '../../../../src/nest/settings/settings-writes.service';
import { SettingsService } from '../../../../src/nest/settings/settings.service';
import { createSnapshotTestDb } from '../../../helpers/db-mock';
import { createTestModule, type TestModule } from '../../../helpers/test-module';
import { MASKED_SETTING_VALUE } from '@trek/shared';
import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest';
const db = createSnapshotTestDb();
let t: TestModule;
let writes: SettingsWritesService;
const settings = {
upsertSetting: vi.fn(async (_userId: number, _key: string, _value: unknown) => undefined),
bulkUpsertSettings: vi.fn(async (_userId: number, values: Record<string, unknown>) => {
return Object.values(values).filter((value) => value !== MASKED_SETTING_VALUE).length;
}),
};
beforeAll(async () => {
t = await createTestModule({
db,
imports: [],
providers: [SettingsWritesService, { provide: SettingsService, useValue: settings }],
});
writes = t.get(SettingsWritesService);
});
beforeEach(() => {
vi.clearAllMocks();
});
afterAll(async () => {
await t.close();
});
describe('SettingsWritesService', () => {
it('SET-WR-001: saveOne writes the key and names it to the other tabs, leaving the asking one out', async () => {
await writes.saveOne(7, 'temperature_unit', 'celsius', 'sock-1');
expect(settings.upsertSetting).toHaveBeenCalledWith(7, 'temperature_unit', 'celsius');
expect(t.realtime.broadcastToUserMock).toHaveBeenCalledTimes(1);
expect(t.realtime.broadcastToUserMock).toHaveBeenCalledWith(
7,
{ type: 'settings:updated', keys: ['temperature_unit'] },
'sock-1',
);
});
it('SET-WR-002: saveMany leaves a masked secret out of the keys and never sends a value', async () => {
const written = await writes.saveMany(7, { maps_api_key: MASKED_SETTING_VALUE, week_start: 'monday' }, 'sock-2');
expect(written).toBe(1);
expect(t.realtime.broadcastToUserMock).toHaveBeenCalledTimes(1);
expect(t.realtime.broadcastToUserMock).toHaveBeenCalledWith(
7,
{ type: 'settings:updated', keys: ['week_start'] },
'sock-2',
);
});
it('SET-WR-003: saveMany with nothing but masked secrets sends no event at all', async () => {
const written = await writes.saveMany(7, { maps_api_key: MASKED_SETTING_VALUE }, undefined);
expect(written).toBe(0);
expect(settings.bulkUpsertSettings).toHaveBeenCalledTimes(1);
expect(t.realtime.broadcastToUserMock).not.toHaveBeenCalled();
});
it('SET-WR-004: without a socket (the MCP path) every tab of the user hears it', async () => {
await writes.saveMany(7, { week_start: 'sunday' }, undefined);
expect(t.realtime.broadcastToUserMock).toHaveBeenCalledWith(
7,
{ type: 'settings:updated', keys: ['week_start'] },
undefined,
);
});
});
+33 -8
View File
@@ -1,3 +1,4 @@
import type { TagWritesService } from '../../../src/nest/tags/tag-writes.service';
import { TagsController } from '../../../src/nest/tags/tags.controller';
import type { TagsService } from '../../../src/nest/tags/tags.service';
import type { User } from '../../../src/types';
@@ -9,8 +10,28 @@ import { describe, it, expect, vi } from 'vitest';
const user = { id: 5 } as User;
/**
* TagWritesService over the stubbed service: the writes go through it, so it
* hands each on and records the socket the route read. The tags:changed it
* sends is held by tools-tags-maps-weather.test.ts and the broadcast parity suite.
*/
function writesFor(svc: Partial<TagsService>) {
return {
create: vi.fn((userId: number, name: string, color: string | undefined, _socketId: string | undefined) =>
svc.create!(userId, name, color),
),
update: vi.fn(
(_userId: number, id: string, changes: { name?: string; color?: string }, _socketId: string | undefined) =>
svc.update!(id, changes.name, changes.color),
),
remove: vi.fn((_userId: number, id: string, _socketId: string | undefined) => svc.remove!(id)),
};
}
let writes: ReturnType<typeof writesFor>;
function makeController(svc: Partial<TagsService>) {
return new TagsController(svc as TagsService);
writes = writesFor(svc);
return new TagsController(svc as TagsService, writes as unknown as TagWritesService);
}
const tag: Tag = { id: 1, user_id: 5, name: 'Beach', color: '#10b981' };
@@ -45,10 +66,12 @@ describe('TagsController (parity with the legacy /api/tags route)', () => {
it('creates a tag for the caller', async () => {
const create = vi.fn().mockReturnValue(tag);
expect(await makeController({ create }).create(user, anyBody({ name: 'Beach', color: '#10b981' }))).toEqual({
tag,
});
expect(
await makeController({ create }).create(user, anyBody({ name: 'Beach', color: '#10b981' }), 'sock'),
).toEqual({ tag });
expect(create).toHaveBeenCalledWith(5, 'Beach', '#10b981');
// Through the writes, which tell the caller's other tabs and leave out this one.
expect(writes.create).toHaveBeenCalledWith(5, 'Beach', '#10b981', 'sock');
});
});
@@ -69,10 +92,11 @@ describe('TagsController (parity with the legacy /api/tags route)', () => {
it('updates an owned tag', async () => {
const getByIdAndUser = vi.fn().mockReturnValue(tag);
const update = vi.fn().mockReturnValue({ ...tag, name: 'Hike' });
expect(await makeController({ getByIdAndUser, update }).update(user, '1', anyBody({ name: 'Hike' }))).toEqual({
tag: { ...tag, name: 'Hike' },
});
expect(
await makeController({ getByIdAndUser, update }).update(user, '1', anyBody({ name: 'Hike' }), 'sock'),
).toEqual({ tag: { ...tag, name: 'Hike' } });
expect(update).toHaveBeenCalledWith('1', 'Hike', undefined);
expect(writes.update).toHaveBeenCalledWith(5, '1', { name: 'Hike', color: undefined }, 'sock');
});
});
@@ -90,8 +114,9 @@ describe('TagsController (parity with the legacy /api/tags route)', () => {
it('deletes an owned tag', async () => {
const getByIdAndUser = vi.fn().mockReturnValue(tag);
const remove = vi.fn();
expect(await makeController({ getByIdAndUser, remove }).remove(user, '1')).toEqual({ success: true });
expect(await makeController({ getByIdAndUser, remove }).remove(user, '1', 'sock')).toEqual({ success: true });
expect(remove).toHaveBeenCalledWith('1');
expect(writes.remove).toHaveBeenCalledWith(5, '1', 'sock');
});
});
});
@@ -2,6 +2,8 @@ import type { AuditService } from '../../../src/nest/audit/audit.service';
import { GUEST_HAS_PAYMENTS } from '../../../src/nest/budget/budget-departure.helpers';
import { DomainError } from '../../../src/nest/common/domain-error';
import { NotFoundError, ValidationError } from '../../../src/nest/common/domain-errors';
import type { TripWriter } from '../../../src/nest/common/trip-writer';
import type { TripMemberWritesService } from '../../../src/nest/trip-members/trip-member-writes.service';
import { TripMembersController } from '../../../src/nest/trip-members/trip-members.controller';
import { TripMembersModule } from '../../../src/nest/trip-members/trip-members.module';
import type { TripMembersService } from '../../../src/nest/trip-members/trip-members.service';
@@ -32,7 +34,38 @@ const req = { headers: {} } as Request;
const writeAudit = vi.fn();
const audit = { writeAudit } as unknown as AuditService;
const tc = (s: TripMembersService) => new TripMembersController(s, audit);
/**
* TripMemberWritesService over the stubbed roster: the roster routes hand their
* writes to it, so it passes each on to the stub and records the writer the
* route built. What the real one sends (member:added/removed/updated, the
* invitation, the re-split expenses) is held by tools-trip-members.test.ts and
* the broadcast parity suite.
*/
function writesFor(s: TripMembersService) {
return {
restWriter: vi.fn(
(_tripId: string | number, u: User, socketId: string | undefined) =>
({ userId: u.id, role: u.role, socketId }) as TripWriter,
),
addMember: vi.fn((tripId: string, identifier: string, ownerId: number, w: TripWriter) =>
s.addMember(tripId, identifier, ownerId, w.userId),
),
removeMember: vi.fn((tripId: number, userId: number, _w: TripWriter) => s.removeMember(tripId, userId)),
createGuest: vi.fn((tripId: string, name: string, w: TripWriter) => s.createGuest(tripId, name, w.userId)),
renameGuest: vi.fn((tripId: string, guestId: number, name: string, _w: TripWriter) =>
s.renameGuest(tripId, guestId, name),
),
deleteGuest: vi.fn((tripId: string, guestId: number, _w: TripWriter) => s.deleteGuest(tripId, guestId)),
};
}
let writes: ReturnType<typeof writesFor>;
const tc = (s: TripMembersService) => {
writes = writesFor(s);
return new TripMembersController(s, audit, writes as unknown as TripMemberWritesService);
};
/** The writer a route builds for the user of these cases. */
const restWriter = (socketId?: string) => ({ userId: 1, role: 'user', socketId });
function svc(o: Partial<TripMembersService> = {}): TripMembersService {
return {
@@ -70,24 +103,24 @@ describe('TripMembersController', () => {
expect(await tc(s).members(user, '9')).toEqual({ owner: { id: 1 }, members: [], current_user_id: 1 });
});
it('POST 403 without member_manage, else adds + notifies', async () => {
it('POST 403 without member_manage, else adds through the roster writes', async () => {
expect(
await thrown(() =>
tc(svc({ can: vi.fn().mockReturnValue(false) })).addMember(user, '9', { identifier: 'bob@x.y' }),
),
).toEqual({ status: 403, body: { error: 'No permission to manage members' } });
expect(writes.addMember).not.toHaveBeenCalled();
const addMember = vi
.fn()
.mockReturnValue({ member: { id: 2, email: 'bob@x.y' }, targetUserId: 2, tripTitle: 'T' });
const notifyInvite = vi.fn();
const s = svc({ addMember, notifyInvite } as Partial<TripMembersService>);
expect(await tc(s).addMember(user, '9', { identifier: 'bob@x.y' })).toEqual({
const s = svc({ addMember } as Partial<TripMembersService>);
expect(await tc(s).addMember(user, '9', { identifier: 'bob@x.y' }, 'sock')).toEqual({
member: { id: 2, email: 'bob@x.y' },
});
expect(notifyInvite).toHaveBeenCalledWith('9', user, 2, 'T', 'bob@x.y');
// Task 6 review, L1: the pre-existing REST↔MCP asymmetry (§13b) — these five
// controller mutations broadcast NOTHING while the MCP twins all emit
// member:added/member:removed. Pinned here rather than "fixed".
// The use case announces member:added and sends the invitation, as the MCP
// tool does through the same method; the route only builds its writer.
expect(writes.restWriter).toHaveBeenCalledWith('9', user, 'sock');
expect(writes.addMember).toHaveBeenCalledWith('9', 'bob@x.y', 1, restWriter('sock'));
expect(s.broadcast).not.toHaveBeenCalled();
});
@@ -138,13 +171,15 @@ describe('TripMembersController', () => {
const removeMember = vi.fn();
const s = svc({ can: vi.fn().mockReturnValue(false), removeMember } as Partial<TripMembersService>);
// self-removal (targetId === user.id) bypasses the permission check
expect(await tc(s).removeMember(user, '9', '1')).toEqual({ success: true });
expect(await tc(s).removeMember(user, '9', '1', 'sock')).toEqual({ success: true });
// The parsed trip id, as the removal itself takes it; member:removed goes out from the use case.
expect(writes.removeMember).toHaveBeenCalledWith(9, 1, restWriter('sock'));
expect(removeMember).toHaveBeenCalledWith(9, 1);
expect(await thrown(() => tc(s).removeMember(user, '9', '2'))).toEqual({
status: 403,
body: { error: 'No permission to remove members' },
});
// Task 6 review, L1: see the addMember case above.
expect(s.broadcast).not.toHaveBeenCalled();
expect(writes.removeMember).not.toHaveBeenCalled();
});
});
@@ -211,12 +246,12 @@ describe('TripMembersController', () => {
});
const createGuest = vi.fn().mockReturnValue({ member: { id: 7, username: 'Anna', is_guest: true } });
const s = svc({ createGuest } as Partial<TripMembersService>);
expect(await tc(s).createGuest(user, '9', { name: 'Anna' })).toEqual({
expect(await tc(s).createGuest(user, '9', { name: 'Anna' }, 'sock')).toEqual({
member: { id: 7, username: 'Anna', is_guest: true },
});
expect(createGuest).toHaveBeenCalledWith('9', 'Anna', user.id);
// Task 6 review, L1: see the addMember case above.
expect(s.broadcast).not.toHaveBeenCalled();
// member:added goes out from the use case, to the room minus the asking tab.
expect(writes.createGuest).toHaveBeenCalledWith('9', 'Anna', restWriter('sock'));
});
it('rename: 404 when the guest is missing, else success', async () => {
@@ -226,9 +261,8 @@ describe('TripMembersController', () => {
body: { error: 'Guest not found' },
});
const ok = svc({ renameGuest: vi.fn().mockReturnValue(true) } as Partial<TripMembersService>);
expect(await tc(ok).renameGuest(user, '9', '7', { name: 'Bob' })).toEqual({ success: true });
// Task 6 review, L1: see the addMember case above.
expect(ok.broadcast).not.toHaveBeenCalled();
expect(await tc(ok).renameGuest(user, '9', '7', { name: 'Bob' }, 'sock')).toEqual({ success: true });
expect(writes.renameGuest).toHaveBeenCalledWith('9', 7, 'Bob', restWriter('sock'));
});
it('delete: 404 when the guest is missing, else success', async () => {
@@ -238,9 +272,8 @@ describe('TripMembersController', () => {
body: { error: 'Guest not found' },
});
const ok = svc({ deleteGuest: vi.fn().mockResolvedValue(true) } as Partial<TripMembersService>);
expect(await tc(ok).deleteGuest(user, '9', '7')).toEqual({ success: true });
// Task 6 review, L1: see the addMember case above.
expect(ok.broadcast).not.toHaveBeenCalled();
expect(await tc(ok).deleteGuest(user, '9', '7', 'sock')).toEqual({ success: true });
expect(writes.deleteGuest).toHaveBeenCalledWith('9', 7, restWriter('sock'));
});
it('passes the 409 of a guest who moved money on as it stands, the text the MCP tool answers too (#2604)', async () => {
@@ -432,6 +432,21 @@ describe('listMembers shaping', () => {
expect(byId.get(bare.id)!.is_guest).toBe(false);
expect(ownerRow.is_guest).toBe(false);
});
it('MEMBERS-SVC-020: findMember returns one roster row by id, and nothing for the owner, a stranger or a missing trip', async () => {
const { user: owner } = createUser(testDb);
const { user: member } = createUser(testDb);
const { user: stranger } = createUser(testDb);
const trip = createTrip(testDb, owner.id);
addTripMember(testDb, trip.id, member.id);
const row = await roster.findMember(trip.id, member.id);
expect(row).toMatchObject({ id: member.id, role: 'member', is_guest: false, avatar_url: null });
// The owner is not a trip_members row, so the plugin path never announces them.
expect(await roster.findMember(trip.id, owner.id)).toBeUndefined();
expect(await roster.findMember(trip.id, stranger.id)).toBeUndefined();
expect(await roster.findMember(999999, member.id)).toBeUndefined();
});
});
// ── Task 6 review items (M1, L2) ─────────────────────────────────────────────
+61 -11
View File
@@ -2,7 +2,9 @@ import { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env.serv
import type { AuditService } from '../../../src/nest/audit/audit.service';
import type { CalendarService } from '../../../src/nest/calendar/calendar.service';
import { DomainError } from '../../../src/nest/common/domain-error';
import type { TripWriter } from '../../../src/nest/common/trip-writer';
import type { TripReadModelService } from '../../../src/nest/trip-read-model/trip-read-model.service';
import type { TripListWriter, TripWritesService } from '../../../src/nest/trips/trip-writes.service';
import { TripsController, MAX_COVER_SIZE, TRIP_COVER_FILE_FILTER } from '../../../src/nest/trips/trips.controller';
import type { TripsService } from '../../../src/nest/trips/trips.service';
import { NotFoundError, ValidationError } from '../../../src/nest/trips/trips.service';
@@ -52,7 +54,30 @@ const audit = { writeAudit } as unknown as AuditService;
const storageStub = {
put: vi.fn().mockResolvedValue(undefined),
} as unknown as import('../../../src/nest/storage/storage.service').StorageService;
const tc = (s: TripsService, calendar?: Partial<CalendarService>, readModel?: Partial<TripReadModelService>) =>
/**
* TripWritesService over the stubbed TripsService: create, copy and delete go
* through it, so it hands each on to the stub and records the writer the route
* built. What the real one sends is held by tools-trips.test.ts and the
* broadcast parity suite, on the booted app.
*/
function writesFor(s: TripsService) {
return {
restWriter: vi.fn(
(_tripId: string, u: User, socketId: string | undefined) =>
({ userId: u.id, role: u.role, socketId }) as TripWriter,
),
create: vi.fn((input: Parameters<TripsService['create']>[1], w: TripListWriter) => s.create(w.userId, input)),
copy: vi.fn((id: string, title: string | undefined, w: TripListWriter) => s.copy(id, w.userId, title)),
remove: vi.fn((id: string, w: TripWriter) => s.remove(id, w.userId, w.role)),
};
}
const tc = (
s: TripsService,
calendar?: Partial<CalendarService>,
readModel?: Partial<TripReadModelService>,
writes = writesFor(s),
) =>
new TripsController(
s,
audit,
@@ -61,6 +86,7 @@ const tc = (s: TripsService, calendar?: Partial<CalendarService>, readModel?: Pa
(calendar ?? {}) as CalendarService,
(readModel ?? {}) as TripReadModelService,
storageStub,
writes as unknown as TripWritesService,
);
function svc(o: Partial<TripsService> = {}): TripsService {
@@ -290,6 +316,19 @@ describe('TripsController (parity with the legacy /api/trips route)', async () =
trip: { id: 9 },
});
});
it('creates through the trip writes as the session user, naming the tab that asked', async () => {
const s = svc({
create: vi.fn().mockReturnValue({ trip: { id: 9 }, tripId: 9, reminderDays: 0 }),
} as Partial<TripsService>);
const writes = writesFor(s);
await tc(s, undefined, undefined, writes).create(user, { title: 'T' }, req, 'sock');
expect(writes.create).toHaveBeenCalledWith(expect.objectContaining({ title: 'T' }), {
userId: 1,
role: 'user',
socketId: 'sock',
});
});
});
it('GET /:id 404 when missing', async () => {
@@ -440,7 +479,11 @@ describe('TripsController (parity with the legacy /api/trips route)', async () =
copy: vi.fn().mockReturnValue(42),
getCopiedTrip: vi.fn().mockReturnValue({ id: 42 }),
} as Partial<TripsService>);
expect(await tc(s).copy(user, '9', { title: 'Copy' }, req)).toEqual({ trip: { id: 42 } });
const writes = writesFor(s);
expect(await tc(s, undefined, undefined, writes).copy(user, '9', { title: 'Copy' }, req, 'sock')).toEqual({
trip: { id: 42 },
});
expect(writes.copy).toHaveBeenCalledWith('9', 'Copy', { userId: 1, role: 'user', socketId: 'sock' });
});
});
@@ -487,20 +530,27 @@ describe('TripsController (parity with the legacy /api/trips route)', async () =
expect(remove).toHaveBeenCalledWith('9', 1, 'admin');
});
it('deletes, audits and broadcasts', async () => {
it('deletes through the trip writes, which announce it to the room minus the asking tab, and audits', async () => {
const remove = vi.fn().mockReturnValue({ tripId: 9, title: 'T', isAdminDelete: false });
const broadcast = vi.fn();
const s = svc({ getOwner: vi.fn().mockReturnValue({ user_id: 1 }), remove, broadcast } as Partial<TripsService>);
expect(await tc(s).remove(user, '9', req, 'sock')).toEqual({ success: true });
expect(broadcast).toHaveBeenCalledWith('9', 'trip:deleted', { id: 9 }, 'sock');
const s = svc({ getOwner: vi.fn().mockReturnValue({ user_id: 1 }), remove } as Partial<TripsService>);
const writes = writesFor(s);
expect(await tc(s, undefined, undefined, writes).remove(user, '9', req, 'sock')).toEqual({ success: true });
expect(writes.restWriter).toHaveBeenCalledWith('9', user, 'sock');
expect(writes.remove).toHaveBeenCalledWith('9', { userId: 1, role: 'user', socketId: 'sock' });
expect(writeAudit).toHaveBeenCalledWith(expect.objectContaining({ action: 'trip.delete' }));
// The broadcast left the controller for the use case: nothing goes out from here twice.
expect(s.broadcast).not.toHaveBeenCalled();
});
it('admin delete logs the owner', async () => {
const remove = vi.fn().mockReturnValue({ tripId: 9, title: 'T', isAdminDelete: true, ownerEmail: 'owner@x.y' });
const broadcast = vi.fn();
const s = svc({ getOwner: vi.fn().mockReturnValue({ user_id: 2 }), remove, broadcast } as Partial<TripsService>);
expect(await tc(s).remove(user, '9', req)).toEqual({ success: true });
expect(broadcast).toHaveBeenCalledWith('9', 'trip:deleted', { id: 9 }, undefined);
const s = svc({ getOwner: vi.fn().mockReturnValue({ user_id: 2 }), remove } as Partial<TripsService>);
const writes = writesFor(s);
expect(await tc(s, undefined, undefined, writes).remove(user, '9', req)).toEqual({ success: true });
expect(writes.restWriter).toHaveBeenCalledWith('9', user, undefined);
expect(writeAudit).toHaveBeenCalledWith(
expect.objectContaining({ details: expect.objectContaining({ owner: 'owner@x.y' }) }),
);
});
});
+15
View File
@@ -71,6 +71,7 @@ export function build(opts: { allow?: (action: string) => boolean; updateThrows?
// Membership left TripsService for its own domain.
const roster = {
removeMember: vi.fn(),
findMember: vi.fn(async (_tripId: number, userId: number) => ({ id: userId, username: 'bob', role: 'member' })),
} as unknown as TripMembersService & Record<string, ReturnType<typeof vi.fn>>;
const membership = { joinTripAsMember: vi.fn(() => ({ joined: true })) } as unknown as TripMembershipService &
Record<string, ReturnType<typeof vi.fn>>;
@@ -328,6 +329,18 @@ describe('TripsRpc membership', () => {
const f = build();
expect((await f.host().dispatch(req('trips.addMember', { tripId: 1, userId: 7 }), 42)).ok).toBe(true);
expect(f.membership.joinTripAsMember).toHaveBeenCalledWith(1, 7, 42);
// The whole room hears it: a plugin has no socket to leave out.
expect(f.realtime.broadcast).toHaveBeenCalledWith(1, 'member:added', {
member: { id: 7, username: 'bob', role: 'member' },
});
});
it('TRIPS-RPC-021 a join that changes nothing announces nothing', async () => {
const f = build();
f.membership.joinTripAsMember.mockResolvedValueOnce({ joined: false, tripId: 1 });
expect((await f.host().dispatch(req('trips.addMember', { tripId: 1, userId: 7 }), 42)).ok).toBe(true);
expect(f.roster.findMember).not.toHaveBeenCalled();
expect(f.realtime.broadcast).not.toHaveBeenCalled();
});
it('TRIPS-RPC-014 member management is its own grant, separate from every other write', async () => {
@@ -352,12 +365,14 @@ describe('TripsRpc membership', () => {
const res = (await f.host().dispatch(req('trips.removeMember', { tripId: 1, userId: 42 }), 42)) as RpcError;
expect(res.error.message).toBe('cannot remove the trip owner');
expect(f.roster.removeMember).not.toHaveBeenCalled();
expect(f.realtime.broadcast).not.toHaveBeenCalled();
});
it('TRIPS-RPC-017 a non-owner member is removed', async () => {
const f = build();
expect((await f.host().dispatch(req('trips.removeMember', { tripId: 1, userId: 7 }), 42)).ok).toBe(true);
expect(f.roster.removeMember).toHaveBeenCalledWith(1, 7);
expect(f.realtime.broadcast).toHaveBeenCalledWith(1, 'member:removed', { userId: 7 });
});
it('TRIPS-RPC-018 without member_manage nothing changes', async () => {