diff --git a/server/src/nest/collab/collab.service.ts b/server/src/nest/collab/collab.service.ts index 12de88b3e..02286bc7b 100644 --- a/server/src/nest/collab/collab.service.ts +++ b/server/src/nest/collab/collab.service.ts @@ -254,8 +254,8 @@ export class CollabService { } /** The writer as a notification names them: by id, and by the address the route read off its session. */ - private async actorOf(writer: TripWriter): Promise> { - return { id: writer.userId, email: await this.notifications.actorEmail(writer.userId) }; + private async actorOf(writer: TripWriter): Promise> { + return { id: writer.userId, username: await this.notifications.actorName(writer.userId) }; } /* ------------------------------------------------------------------ */ @@ -907,7 +907,7 @@ export class CollabService { } /** Fire-and-forget collab notification (mirrors the legacy route's dynamic import). */ - async notifyCollab(tripId: string, actor: Pick, preview?: string): Promise { + async notifyCollab(tripId: string, actor: Pick, preview?: string): Promise { // Injected, not a lazy import of the old notifications bridge. The laziness bought // nothing the module graph does not already give — NotificationsModule // reaches nothing in this direction — and it hid the edge while handing the diff --git a/server/src/nest/notifications/notifications.service.ts b/server/src/nest/notifications/notifications.service.ts index 1da3483c5..4edd66f3c 100644 --- a/server/src/nest/notifications/notifications.service.ts +++ b/server/src/nest/notifications/notifications.service.ts @@ -662,9 +662,9 @@ export class NotificationsService { return true; } - /** The address a notification names its actor by, as the routes read it off their session user. */ - async actorEmail(userId: number): Promise { - return (await this.mailer.getUserEmail(userId)) ?? ''; + /** The name a notification gives its actor, as the routes read it off their session user. */ + async actorName(userId: number): Promise { + return (await this.notificationsRepo.findUserBasic(userId))?.username ?? ''; } async deleteOne(notificationId: number, userId: number): Promise { diff --git a/server/src/nest/packing/packing-writes.service.ts b/server/src/nest/packing/packing-writes.service.ts index 0bb8019b9..65a7bcd68 100644 --- a/server/src/nest/packing/packing-writes.service.ts +++ b/server/src/nest/packing/packing-writes.service.ts @@ -2,7 +2,7 @@ import type { User } from '../../types'; import { isUpdateConflict } from '../common/conflictResult'; import { DomainError } from '../common/domain-error'; import { requireTripWrite, type TripWriter } from '../common/trip-writer'; -import { MailerService } from '../notifications/mailer/mailer.service'; +import { NotificationsService } from '../notifications/notifications.service'; import { PermissionsService } from '../permissions/permissions.service'; import { PackingService, isInvalidBagRef, type PackingVisibility } from './packing.service'; import { Injectable } from '@nestjs/common'; @@ -56,7 +56,7 @@ export class PackingWritesService { constructor( private readonly packing: PackingService, private readonly permissions: PermissionsService, - private readonly mailer: MailerService, + private readonly notifications: NotificationsService, ) {} /** @@ -209,10 +209,10 @@ export class PackingWritesService { const assignees = await this.packing.updateCategoryAssignees(tripId, category, userIds); writer.events.emit('packing:assignees', { category, assignees }); if (userIds.length > 0) { - // The notification names its actor by email. A writer carries only the id, - // so the address is read here; a guest never signs in, so never writes. - const email = await this.mailer.getUserEmail(writer.userId); - if (email) await this.packing.notifyTagged(String(tripId), { id: writer.userId, email }, category, userIds); + // The notification names its actor by name. A writer carries only the id, + // so the name is read here; an account that is gone notifies nobody. + const username = await this.notifications.actorName(writer.userId); + if (username) await this.packing.notifyTagged(String(tripId), { id: writer.userId, username }, category, userIds); } return assignees; } diff --git a/server/src/nest/packing/packing.service.ts b/server/src/nest/packing/packing.service.ts index 1889be4a1..232a6af5a 100644 --- a/server/src/nest/packing/packing.service.ts +++ b/server/src/nest/packing/packing.service.ts @@ -891,7 +891,7 @@ export class PackingService { } /** Fire-and-forget tag notification, mirroring the legacy dynamic import. */ - async notifyTagged(tripId: string, actor: Pick, category: string, userIds: unknown): Promise { + async notifyTagged(tripId: string, actor: Pick, category: string, userIds: unknown): Promise { if (!Array.isArray(userIds) || userIds.length === 0) return; // Injected, not a lazy import of the old notifications bridge. The laziness bought // nothing the module graph does not already give — NotificationsModule diff --git a/server/src/nest/trip-members/trip-member-writes.service.ts b/server/src/nest/trip-members/trip-member-writes.service.ts index 6222f36ce..54cc1321d 100644 --- a/server/src/nest/trip-members/trip-member-writes.service.ts +++ b/server/src/nest/trip-members/trip-member-writes.service.ts @@ -38,8 +38,8 @@ export class TripMemberWritesService { async addMember(tripId: string | number, identifier: string, tripOwnerId: number, writer: TripWriter) { const result = await this.roster.addMember(tripId, identifier, tripOwnerId, writer.userId); writer.events.emit('member:added', { member: result.member }); - // The invitation names who sent it by address, as the route always did with its session user. - const actor = { id: writer.userId, email: (await this.users.getEmail(writer.userId)) ?? '' }; + // The invitation names who sent it by name, as the route does with its session user. + const actor = { id: writer.userId, username: (await this.users.findById(writer.userId))?.username ?? '' }; this.roster.notifyInvite(String(tripId), actor, result.targetUserId, result.tripTitle, result.member.email); return result; } diff --git a/server/src/nest/trip-members/trip-members.service.ts b/server/src/nest/trip-members/trip-members.service.ts index 717f78241..4c5939717 100644 --- a/server/src/nest/trip-members/trip-members.service.ts +++ b/server/src/nest/trip-members/trip-members.service.ts @@ -120,7 +120,7 @@ export class TripMembersService { /** Fire-and-forget trip-invite notification (mirrors the route's dynamic import). */ notifyInvite( tripId: string, - actor: Pick, + actor: Pick, targetUserId: number, tripTitle: string, inviteeEmail: string, diff --git a/server/tests/unit/mcp/tools-collab-polls-chat.test.ts b/server/tests/unit/mcp/tools-collab-polls-chat.test.ts index ca0d715e8..0d4b33b56 100644 --- a/server/tests/unit/mcp/tools-collab-polls-chat.test.ts +++ b/server/tests/unit/mcp/tools-collab-polls-chat.test.ts @@ -553,7 +553,7 @@ describe('send_collab_message, as the chat posts it', () => { event: 'collab_message', actorId: user.id, targetId: trip.id, - params: expect.objectContaining({ actor: user.email, preview: 'Dinner at 8?' }), + params: expect.objectContaining({ actor: user.username, preview: 'Dinner at 8?' }), }), ); } finally { diff --git a/server/tests/unit/mcp/tools-notes.test.ts b/server/tests/unit/mcp/tools-notes.test.ts index aeb7f2c49..2b8541291 100644 --- a/server/tests/unit/mcp/tools-notes.test.ts +++ b/server/tests/unit/mcp/tools-notes.test.ts @@ -817,7 +817,7 @@ describe('create_collab_note, as the Collab tab posts it', () => { event: 'collab_message', actorId: user.id, targetId: trip.id, - params: expect.objectContaining({ actor: user.email }), + params: expect.objectContaining({ actor: user.username }), }), ); } finally { diff --git a/server/tests/unit/mcp/tools-packing-advanced.test.ts b/server/tests/unit/mcp/tools-packing-advanced.test.ts index 96b2c9625..207cb7ad3 100644 --- a/server/tests/unit/mcp/tools-packing-advanced.test.ts +++ b/server/tests/unit/mcp/tools-packing-advanced.test.ts @@ -406,7 +406,7 @@ describe('Tool: set_packing_category_assignees', () => { await new Promise((resolve) => setImmediate(resolve)); const sent = await findRow(orm, Notifications, { recipient: member.id }); expect(sent?.text_key).toBe('notif.packing_tagged.text'); - expect(JSON.parse(String(sent?.text_params))).toMatchObject({ actor: user.email, category: 'Clothing' }); + expect(JSON.parse(String(sent?.text_params))).toMatchObject({ actor: user.username, category: 'Clothing' }); }); }); diff --git a/server/tests/unit/mcp/tools-trip-members.test.ts b/server/tests/unit/mcp/tools-trip-members.test.ts index 8abd91bea..0bc6a6b96 100644 --- a/server/tests/unit/mcp/tools-trip-members.test.ts +++ b/server/tests/unit/mcp/tools-trip-members.test.ts @@ -987,7 +987,7 @@ describe('Guest tools stay owner-only', () => { // --------------------------------------------------------------------------- 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 () => { + it('MCP-MEMBERS-001: add_trip_member sends the invitation the planner sends, naming the inviter by name', async () => { const { user: owner } = createUser(testDb); const { user: invitee } = createUser(testDb); const trip = createTrip(testDb, owner.id); @@ -1004,7 +1004,7 @@ describe('Roster writes, as REST makes them', () => { actorId: owner.id, scope: 'user', targetId: invitee.id, - params: expect.objectContaining({ actor: owner.email, invitee: invitee.email, tripId: String(trip.id) }), + params: expect.objectContaining({ actor: owner.username, invitee: invitee.email, tripId: String(trip.id) }), }), ); } finally { diff --git a/server/tests/unit/nest/collab.service.test.ts b/server/tests/unit/nest/collab.service.test.ts index 0a8e36da0..baa896f83 100644 --- a/server/tests/unit/nest/collab.service.test.ts +++ b/server/tests/unit/nest/collab.service.test.ts @@ -413,7 +413,7 @@ describe('postNote / postMessage', () => { const send = vi.fn(async (_payload: { params: Record }) => ({ attempted: 0, delivered: 0 })); const notifications = { send, - actorEmail: vi.fn(async () => 'owner@collab.test'), + actorName: vi.fn(async () => 'owner'), } as unknown as NotificationsService; const service = await buildCollabService(collabFx.storage, rateLimit, notifications, realtime); return { service, realtime, send }; @@ -440,7 +440,7 @@ describe('postNote / postMessage', () => { actorId: user1.id, scope: 'trip', targetId: trip.id, - params: expect.objectContaining({ actor: 'owner@collab.test', tripId: String(trip.id) }), + params: expect.objectContaining({ actor: 'owner', tripId: String(trip.id) }), }), ); }); diff --git a/server/tests/unit/nest/packing-writes.service.test.ts b/server/tests/unit/nest/packing-writes.service.test.ts index 38319eda1..13dee98d3 100644 --- a/server/tests/unit/nest/packing-writes.service.test.ts +++ b/server/tests/unit/nest/packing-writes.service.test.ts @@ -1,6 +1,6 @@ import { DomainError } from '../../../src/nest/common/domain-error'; import { McpToolGuardsService } from '../../../src/nest/mcp-shared/mcp-tool-guards.service'; -import type { MailerService } from '../../../src/nest/notifications/mailer/mailer.service'; +import type { NotificationsService } from '../../../src/nest/notifications/notifications.service'; import { PackingWritesService } from '../../../src/nest/packing/packing-writes.service'; import type { PackingService } from '../../../src/nest/packing/packing.service'; import type { PermissionsService } from '../../../src/nest/permissions/permissions.service'; @@ -64,8 +64,8 @@ function setup(overrides: Record = {}, opts: { allowed?: boolea const permissions = { checkPermission: vi.fn().mockResolvedValue(opts.allowed ?? true), } as unknown as PermissionsService; - const mailer = { getUserEmail: vi.fn().mockResolvedValue(user.email) } as unknown as MailerService; - const writes = new PackingWritesService(packing, permissions, mailer); + const notifications = { actorName: vi.fn().mockResolvedValue('u') } as unknown as NotificationsService; + const writes = new PackingWritesService(packing, permissions, notifications); const guards = new McpToolGuardsService( { getOwnerId: vi.fn().mockResolvedValue(1) } as never, { getRole: vi.fn().mockResolvedValue('user') } as never, @@ -82,7 +82,7 @@ function setup(overrides: Record = {}, opts: { allowed?: boolea await fn(writer); return { names: names(), calls: [...realtime.broadcast.mock.calls] }; }; - return { writes, packing, permissions, mailer, realtime, run }; + return { writes, packing, permissions, notifications, realtime, run }; } describe('packing item events: REST == MCP', () => { @@ -274,19 +274,19 @@ describe('packing list use cases: REST == MCP', () => { const mcp = await s.run('mcp', (w) => s.writes.setCategoryAssignees(5, 'Clothes', [2], w)); expect(mcp.names).toEqual(rest.names); expect(rest.calls).toEqual([['5', 'packing:assignees', { category: 'Clothes', assignees }, 'sock']]); - // The MCP tool used to tell none of them; both name the actor by its email. - const actor = { id: 1, email: user.email }; + // The MCP tool used to tell none of them; both name the actor by name. + const actor = { id: 1, username: 'u' }; expect(s.packing.notifyTagged).toHaveBeenNthCalledWith(1, '5', actor, 'Clothes', [2]); expect(s.packing.notifyTagged).toHaveBeenNthCalledWith(2, '5', actor, 'Clothes', [2]); - expect(s.mailer.getUserEmail).toHaveBeenCalledWith(1); + expect(s.notifications.actorName).toHaveBeenCalledWith(1); }); - it('PACK-LIST-006: clearing a category notifies nobody, and an actor without an address is not named', async () => { + it('PACK-LIST-006: clearing a category notifies nobody, and an actor without a name is not named', async () => { const s = setup({ updateCategoryAssignees: vi.fn().mockResolvedValue([]), notifyTagged: vi.fn() }); await s.run('mcp', (w) => s.writes.setCategoryAssignees(5, 'Clothes', [], w)); - expect(s.mailer.getUserEmail).not.toHaveBeenCalled(); + expect(s.notifications.actorName).not.toHaveBeenCalled(); - vi.mocked(s.mailer.getUserEmail).mockResolvedValueOnce(null); + vi.mocked(s.notifications.actorName).mockResolvedValueOnce(''); await s.run('mcp', (w) => s.writes.setCategoryAssignees(5, 'Clothes', [2], w)); expect(s.packing.notifyTagged).not.toHaveBeenCalled(); }); diff --git a/server/tests/unit/nest/packing.controller.test.ts b/server/tests/unit/nest/packing.controller.test.ts index c987ca115..c576f86bc 100644 --- a/server/tests/unit/nest/packing.controller.test.ts +++ b/server/tests/unit/nest/packing.controller.test.ts @@ -1,4 +1,4 @@ -import type { MailerService } from '../../../src/nest/notifications/mailer/mailer.service'; +import type { NotificationsService } from '../../../src/nest/notifications/notifications.service'; import { PackingWritesService } from '../../../src/nest/packing/packing-writes.service'; import { PackingController as RoutePackingController } from '../../../src/nest/packing/packing.controller'; import type { PackingService } from '../../../src/nest/packing/packing.service'; @@ -63,13 +63,13 @@ function makeService(overrides: Partial = {}): PackingService { * The route over the real item use cases (PackingWritesService), with the * permission granted: the 403 lives in trip-access.guard.test.ts, and these * cases pin what the route answers and broadcasts through the service mock. - * The mailer answers the actor's email the assignee notification names. + * The notifications answer the actor's name the assignee notification gives. */ const allowAll = { checkPermission: vi.fn().mockResolvedValue(true) } as unknown as PermissionsService; -const mailer = { getUserEmail: vi.fn().mockResolvedValue(user.email) } as unknown as MailerService; +const notifications = { actorName: vi.fn().mockResolvedValue('u') } as unknown as NotificationsService; class PackingController extends RoutePackingController { constructor(svc: PackingService) { - super(svc, new PackingWritesService(svc, allowAll, mailer)); + super(svc, new PackingWritesService(svc, allowAll, notifications)); } } @@ -797,8 +797,8 @@ describe('PackingController (parity with the legacy /api/trips/:tripId/packing r { category: 'Clothes', assignees: [{ user_id: 2 }] }, 'sock', ); - // The actor is named by the email on its account row, as the MCP tool names it. - expect(notifyTagged).toHaveBeenCalledWith('5', { id: user.id, email: user.email }, 'Clothes', [2]); + // The actor is named by the name on its account row, as the MCP tool names it. + expect(notifyTagged).toHaveBeenCalledWith('5', { id: user.id, username: 'u' }, 'Clothes', [2]); }); it('notifies nobody when the category is cleared', async () => { diff --git a/server/tests/unit/nest/trip-member-writes.service.test.ts b/server/tests/unit/nest/trip-member-writes.service.test.ts index a95775947..8c091679f 100644 --- a/server/tests/unit/nest/trip-member-writes.service.test.ts +++ b/server/tests/unit/nest/trip-member-writes.service.test.ts @@ -57,7 +57,7 @@ afterAll(async () => { }); describe('TripMemberWritesService', () => { - it('MEMBER-WRITES-001: an inviter whose address cannot be read still sends the invitation, with an empty address', async () => { + it('MEMBER-WRITES-001: an inviter whose name cannot be read still sends the invitation, with an empty name', async () => { const member = { id: 21, email: 'invitee@example.test' }; roster.addMember.mockResolvedValue({ member, targetUserId: 21, tripTitle: 'Lisbon' }); const writer = writerFor(987654); @@ -69,7 +69,7 @@ describe('TripMemberWritesService', () => { expect(writer.events.emit).toHaveBeenCalledWith('member:added', { member }); expect(roster.notifyInvite).toHaveBeenCalledWith( '7', - { id: 987654, email: '' }, + { id: 987654, username: '' }, 21, 'Lisbon', 'invitee@example.test',