From fd0aa1cd59c73c8f9c12f0567775226300a4a7fd Mon Sep 17 00:00:00 2001 From: Mariano Fuentes Date: Fri, 24 Apr 2026 09:46:18 -0700 Subject: [PATCH] fix(api): scope task status-change emails to assignee, not whole org (#2669) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(api): scope task status-change emails to assignee, not whole org notifyStatusChange and notifyBulkStatusChange were emailing every non-platform-admin member of the org on any task status change, then leaning on isUserUnsubscribed to filter. That filter had gaps (multi-role users, custom roles, unsaved matrix state), so employees with "Task Assignments" unchecked still received status-change emails. Now: - Single status change: emails only the task's assignee. If the task has no assignee, falls back to owners + admins. Actor always excluded, isUserUnsubscribed still honored. - Bulk status change: groups tasks by assignee and sends each one a bulk email with the count of THEIR tasks. Unassigned tasks are routed to owners/admins with the unassigned count. Co-Authored-By: Claude Opus 4.7 (1M context) * feat(roles): add permission-based member filter helper RolesService.filterMembersWithPermission(orgId, members, resource, action) returns the subset of members whose combined (built-in + custom) role permissions grant the requested resource:action. One batched organizationRole.findMany query regardless of member count. Matches better-auth's hasPermissionFn semantics: comma-separated role strings treated as a union; unknown role names skipped silently. Uses BUILT_IN_ROLE_PERMISSIONS (derived from the same role.statements that better-auth initializes with) so answers stay in lockstep with the runtime permission guard. Enables upcoming migration of notifier recipient selection from hardcoded role-string matching (role.includes('admin')) to permission-based filtering — so custom roles like "Compliance Manager" with task:update automatically qualify. Co-Authored-By: Claude Opus 4.7 (1M context) --------- Co-authored-by: Claude Opus 4.7 (1M context) --- apps/api/src/roles/roles.service.spec.ts | 193 +++++++++++ apps/api/src/roles/roles.service.ts | 65 ++++ .../src/tasks/task-notifier.service.spec.ts | 315 ++++++++++++++++++ apps/api/src/tasks/task-notifier.service.ts | 287 ++++++++-------- 4 files changed, 721 insertions(+), 139 deletions(-) create mode 100644 apps/api/src/tasks/task-notifier.service.spec.ts diff --git a/apps/api/src/roles/roles.service.spec.ts b/apps/api/src/roles/roles.service.spec.ts index 3aeba0fd09..b5677628a2 100644 --- a/apps/api/src/roles/roles.service.spec.ts +++ b/apps/api/src/roles/roles.service.spec.ts @@ -520,4 +520,197 @@ describe('RolesService', () => { ); }); }); + + describe('filterMembersWithPermission', () => { + const organizationId = 'org_1'; + + it('returns empty array when members list is empty', async () => { + const result = await service.filterMembersWithPermission( + organizationId, + [], + 'task', + 'update', + ); + expect(result).toEqual([]); + expect(mockDb.organizationRole.findMany).not.toHaveBeenCalled(); + }); + + it('keeps built-in roles that grant the permission (owner has task:update)', async () => { + const members = [ + { id: 'm1', role: 'owner' }, + { id: 'm2', role: 'admin' }, + ]; + const result = await service.filterMembersWithPermission( + organizationId, + members, + 'task', + 'update', + ); + expect(result.map((m) => m.id).sort()).toEqual(['m1', 'm2']); + }); + + it('excludes built-in roles that lack the permission (employee has no task perms)', async () => { + const members = [ + { id: 'm1', role: 'employee' }, + { id: 'm2', role: 'contractor' }, + { id: 'm3', role: 'owner' }, + ]; + const result = await service.filterMembersWithPermission( + organizationId, + members, + 'task', + 'update', + ); + expect(result.map((m) => m.id)).toEqual(['m3']); + }); + + it('excludes auditor for task:update but keeps them for task:read', async () => { + const members = [{ id: 'm1', role: 'auditor' }]; + + const forUpdate = await service.filterMembersWithPermission( + organizationId, + members, + 'task', + 'update', + ); + expect(forUpdate).toEqual([]); + + const forRead = await service.filterMembersWithPermission( + organizationId, + members, + 'task', + 'read', + ); + expect(forRead.map((m) => m.id)).toEqual(['m1']); + }); + + it('treats comma-separated roles as a union (employee,admin gets included)', async () => { + const members = [{ id: 'm1', role: 'employee,admin' }]; + const result = await service.filterMembersWithPermission( + organizationId, + members, + 'task', + 'update', + ); + expect(result.map((m) => m.id)).toEqual(['m1']); + }); + + it('includes a member whose custom role grants the permission', async () => { + (mockDb.organizationRole.findMany as jest.Mock).mockResolvedValue([ + { + name: 'compliance-lead', + permissions: JSON.stringify({ + task: ['read', 'update'], + app: ['read'], + }), + }, + ]); + const members = [{ id: 'm1', role: 'compliance-lead' }]; + const result = await service.filterMembersWithPermission( + organizationId, + members, + 'task', + 'update', + ); + expect(result.map((m) => m.id)).toEqual(['m1']); + expect(mockDb.organizationRole.findMany).toHaveBeenCalledTimes(1); + }); + + it('excludes a member whose custom role lacks the permission', async () => { + (mockDb.organizationRole.findMany as jest.Mock).mockResolvedValue([ + { + name: 'readonly', + permissions: JSON.stringify({ task: ['read'] }), + }, + ]); + const members = [{ id: 'm1', role: 'readonly' }]; + const result = await service.filterMembersWithPermission( + organizationId, + members, + 'task', + 'update', + ); + expect(result).toEqual([]); + }); + + it('excludes members with null, empty, or unknown roles', async () => { + (mockDb.organizationRole.findMany as jest.Mock).mockResolvedValue([]); + const members = [ + { id: 'm1', role: null }, + { id: 'm2', role: '' }, + { id: 'm3', role: 'nonexistent-role' }, + ]; + const result = await service.filterMembersWithPermission( + organizationId, + members, + 'task', + 'update', + ); + expect(result).toEqual([]); + }); + + it('makes exactly one DB query regardless of member count', async () => { + (mockDb.organizationRole.findMany as jest.Mock).mockResolvedValue([ + { + name: 'custom-a', + permissions: JSON.stringify({ task: ['update'] }), + }, + ]); + const members = Array.from({ length: 25 }, (_, i) => ({ + id: `m${i}`, + role: i % 2 === 0 ? 'custom-a' : 'employee', + })); + const result = await service.filterMembersWithPermission( + organizationId, + members, + 'task', + 'update', + ); + expect(result.length).toBe(13); // 0,2,4,...,24 → 13 members + expect(mockDb.organizationRole.findMany).toHaveBeenCalledTimes(1); + }); + + it('skips the DB query when all roles are built-in', async () => { + const members = [ + { id: 'm1', role: 'owner' }, + { id: 'm2', role: 'admin,auditor' }, + { id: 'm3', role: 'employee' }, + ]; + await service.filterMembersWithPermission( + organizationId, + members, + 'app', + 'read', + ); + expect(mockDb.organizationRole.findMany).not.toHaveBeenCalled(); + }); + + it('parses permissions that are already objects (not strings)', async () => { + (mockDb.organizationRole.findMany as jest.Mock).mockResolvedValue([ + { + name: 'object-role', + permissions: { task: ['update'] }, + }, + ]); + const members = [{ id: 'm1', role: 'object-role' }]; + const result = await service.filterMembersWithPermission( + organizationId, + members, + 'task', + 'update', + ); + expect(result.map((m) => m.id)).toEqual(['m1']); + }); + + it('trims whitespace around comma-separated role names', async () => { + const members = [{ id: 'm1', role: 'employee , admin' }]; + const result = await service.filterMembersWithPermission( + organizationId, + members, + 'task', + 'update', + ); + expect(result.map((m) => m.id)).toEqual(['m1']); + }); + }); }); diff --git a/apps/api/src/roles/roles.service.ts b/apps/api/src/roles/roles.service.ts index 3d77ce81b1..42cdccf5ff 100644 --- a/apps/api/src/roles/roles.service.ts +++ b/apps/api/src/roles/roles.service.ts @@ -467,6 +467,71 @@ export class RolesService { return combined; } + /** + * Filter a list of members down to those whose combined role permissions + * grant the given `resource:action`. Built-in role definitions come from + * `BUILT_IN_ROLE_PERMISSIONS`; custom roles are fetched in a single batched + * `organizationRole.findMany` keyed by the distinct role names present in + * the input. + * + * Matches better-auth's `hasPermissionFn` semantics: comma-separated roles + * in `member.role` are treated as a union (ANY role granting the permission + * is sufficient). Unknown role names are skipped silently. + */ + async filterMembersWithPermission( + organizationId: string, + members: M[], + resource: string, + action: string, + ): Promise { + if (members.length === 0) return []; + + const distinctRoles = new Set(); + for (const m of members) { + if (!m.role) continue; + for (const r of m.role.split(',').map((r) => r.trim()).filter(Boolean)) { + distinctRoles.add(r); + } + } + if (distinctRoles.size === 0) return []; + + const customRoleNames = [...distinctRoles].filter( + (r) => !BUILT_IN_ROLES.includes(r), + ); + + const customRoles = + customRoleNames.length > 0 + ? await db.organizationRole.findMany({ + where: { organizationId, name: { in: customRoleNames } }, + select: { name: true, permissions: true }, + }) + : []; + + const permsByRole = new Map>(); + for (const name of distinctRoles) { + if (BUILT_IN_ROLES.includes(name)) { + permsByRole.set(name, BUILT_IN_ROLE_PERMISSIONS[name] ?? {}); + } else { + const custom = customRoles.find((c) => c.name === name); + if (!custom) continue; + const perms = + typeof custom.permissions === 'string' + ? (JSON.parse(custom.permissions) as Record) + : (custom.permissions as Record); + permsByRole.set(name, perms); + } + } + + return members.filter((m) => { + if (!m.role) return false; + const roles = m.role + .split(',') + .map((r) => r.trim()) + .filter(Boolean); + return roles.some((r) => permsByRole.get(r)?.[resource]?.includes(action)); + }); + } + /** * Get merged obligations for a list of custom role names. * Used by the frontend to resolve effective obligations for custom roles. diff --git a/apps/api/src/tasks/task-notifier.service.spec.ts b/apps/api/src/tasks/task-notifier.service.spec.ts new file mode 100644 index 0000000000..87effb9d58 --- /dev/null +++ b/apps/api/src/tasks/task-notifier.service.spec.ts @@ -0,0 +1,315 @@ +const mockDb = { + organization: { findUnique: jest.fn() }, + user: { findUnique: jest.fn() }, + task: { findUnique: jest.fn(), findMany: jest.fn() }, + member: { findMany: jest.fn() }, +}; + +jest.mock('@db', () => ({ + db: mockDb, + TaskStatus: { + todo: 'todo', + in_progress: 'in_progress', + done: 'done', + not_relevant: 'not_relevant', + }, +})); + +const isUserUnsubscribedMock = jest.fn().mockResolvedValue(false); +jest.mock('@trycompai/email', () => ({ + isUserUnsubscribed: (...args: unknown[]) => isUserUnsubscribedMock(...args), +})); + +const triggerEmailMock = jest.fn().mockResolvedValue({ id: 'email_1' }); +jest.mock('../email/trigger-email', () => ({ + triggerEmail: (...args: unknown[]) => triggerEmailMock(...args), +})); + +jest.mock('../email/templates/task-status-changed', () => ({ + TaskStatusChangedEmail: () => null, +})); +jest.mock('../email/templates/task-bulk-status-changed', () => ({ + TaskBulkStatusChangedEmail: () => null, +})); +jest.mock('../email/templates/task-assignee-changed', () => ({ + TaskAssigneeChangedEmail: () => null, +})); +jest.mock('../email/templates/task-bulk-assignee-changed', () => ({ + TaskBulkAssigneeChangedEmail: () => null, +})); +jest.mock('../email/templates/evidence-review-requested', () => ({ + EvidenceReviewRequestedEmail: () => null, +})); +jest.mock('../email/templates/evidence-bulk-review-requested', () => ({ + EvidenceBulkReviewRequestedEmail: () => null, +})); +jest.mock('../email/templates/automation-failures', () => ({ + AutomationFailuresEmail: () => null, +})); +jest.mock('../email/templates/automation-bulk-failures', () => ({ + AutomationBulkFailuresEmail: () => null, +})); + +import { TaskNotifierService } from './task-notifier.service'; + +interface UserFixture { + id: string; + name: string | null; + email: string; +} + +function makeUser(id: string, email: string, name: string | null = null): UserFixture { + return { id, name, email }; +} + +function recipientEmails(): string[] { + return triggerEmailMock.mock.calls.map((call) => call[0].to as string); +} + +describe('TaskNotifierService', () => { + const novu = { trigger: jest.fn().mockResolvedValue(undefined) }; + let service: TaskNotifierService; + + beforeEach(() => { + jest.clearAllMocks(); + isUserUnsubscribedMock.mockResolvedValue(false); + service = new TaskNotifierService(novu as never); + }); + + describe('notifyStatusChange', () => { + it('sends email only to the task assignee when the task has an assignee', async () => { + const actor = makeUser('usr_actor', 'actor@acme.com', 'Actor'); + const assignee = makeUser('usr_assignee', 'assignee@acme.com', 'Assignee'); + + mockDb.organization.findUnique.mockResolvedValue({ name: 'Acme' }); + mockDb.user.findUnique.mockResolvedValue({ + name: actor.name, + email: actor.email, + }); + mockDb.task.findUnique.mockResolvedValue({ + assignee: { user: assignee }, + }); + + await service.notifyStatusChange({ + organizationId: 'org_1', + taskId: 'tsk_1', + taskTitle: '2FA', + oldStatus: 'done' as never, + newStatus: 'todo' as never, + changedByUserId: actor.id, + }); + + expect(triggerEmailMock).toHaveBeenCalledTimes(1); + expect(recipientEmails()).toEqual(['assignee@acme.com']); + expect(mockDb.member.findMany).not.toHaveBeenCalled(); + }); + + it('does not send email when the actor is the assignee', async () => { + const actor = makeUser('usr_actor', 'actor@acme.com', 'Actor'); + + mockDb.organization.findUnique.mockResolvedValue({ name: 'Acme' }); + mockDb.user.findUnique.mockResolvedValue({ + name: actor.name, + email: actor.email, + }); + mockDb.task.findUnique.mockResolvedValue({ + assignee: { user: actor }, + }); + + await service.notifyStatusChange({ + organizationId: 'org_1', + taskId: 'tsk_1', + taskTitle: '2FA', + oldStatus: 'done' as never, + newStatus: 'todo' as never, + changedByUserId: actor.id, + }); + + expect(triggerEmailMock).not.toHaveBeenCalled(); + }); + + it('falls back to owners and admins when the task has no assignee', async () => { + const actor = makeUser('usr_actor', 'actor@acme.com', 'Actor'); + const owner = makeUser('usr_owner', 'owner@acme.com', 'Owner'); + const admin = makeUser('usr_admin', 'admin@acme.com', 'Admin'); + const employee = makeUser('usr_emp', 'emp@acme.com', 'Emp'); + const auditor = makeUser('usr_aud', 'aud@acme.com', 'Aud'); + + mockDb.organization.findUnique.mockResolvedValue({ name: 'Acme' }); + mockDb.user.findUnique.mockResolvedValue({ + name: actor.name, + email: actor.email, + }); + mockDb.task.findUnique.mockResolvedValue({ assignee: null }); + mockDb.member.findMany.mockResolvedValue([ + { role: 'owner', user: owner }, + { role: 'admin', user: admin }, + { role: 'employee', user: employee }, + { role: 'auditor', user: auditor }, + ]); + + await service.notifyStatusChange({ + organizationId: 'org_1', + taskId: 'tsk_1', + taskTitle: '2FA', + oldStatus: 'done' as never, + newStatus: 'todo' as never, + changedByUserId: actor.id, + }); + + expect(recipientEmails().sort()).toEqual( + ['admin@acme.com', 'owner@acme.com'].sort(), + ); + }); + + it('excludes the actor from the owner/admin fallback', async () => { + const actor = makeUser('usr_admin', 'admin@acme.com', 'Admin'); + const owner = makeUser('usr_owner', 'owner@acme.com', 'Owner'); + + mockDb.organization.findUnique.mockResolvedValue({ name: 'Acme' }); + mockDb.user.findUnique.mockResolvedValue({ + name: actor.name, + email: actor.email, + }); + mockDb.task.findUnique.mockResolvedValue({ assignee: null }); + mockDb.member.findMany.mockResolvedValue([ + { role: 'admin', user: actor }, + { role: 'owner', user: owner }, + ]); + + await service.notifyStatusChange({ + organizationId: 'org_1', + taskId: 'tsk_1', + taskTitle: '2FA', + oldStatus: 'done' as never, + newStatus: 'todo' as never, + changedByUserId: actor.id, + }); + + expect(recipientEmails()).toEqual(['owner@acme.com']); + }); + + it('honors isUserUnsubscribed for the assignee', async () => { + const actor = makeUser('usr_actor', 'actor@acme.com', 'Actor'); + const assignee = makeUser('usr_assignee', 'assignee@acme.com', 'Assignee'); + + mockDb.organization.findUnique.mockResolvedValue({ name: 'Acme' }); + mockDb.user.findUnique.mockResolvedValue({ + name: actor.name, + email: actor.email, + }); + mockDb.task.findUnique.mockResolvedValue({ + assignee: { user: assignee }, + }); + isUserUnsubscribedMock.mockResolvedValue(true); + + await service.notifyStatusChange({ + organizationId: 'org_1', + taskId: 'tsk_1', + taskTitle: '2FA', + oldStatus: 'done' as never, + newStatus: 'todo' as never, + changedByUserId: actor.id, + }); + + expect(triggerEmailMock).not.toHaveBeenCalled(); + }); + }); + + describe('notifyBulkStatusChange', () => { + it('sends each assignee a bulk email covering only their own tasks', async () => { + const actor = makeUser('usr_actor', 'actor@acme.com', 'Actor'); + const assigneeA = makeUser('usr_a', 'a@acme.com', 'A'); + const assigneeB = makeUser('usr_b', 'b@acme.com', 'B'); + + mockDb.organization.findUnique.mockResolvedValue({ name: 'Acme' }); + mockDb.user.findUnique.mockResolvedValue({ + name: actor.name, + email: actor.email, + }); + mockDb.task.findMany.mockResolvedValue([ + { id: 'tsk_1', title: 't1', assignee: { user: assigneeA } }, + { id: 'tsk_2', title: 't2', assignee: { user: assigneeA } }, + { id: 'tsk_3', title: 't3', assignee: { user: assigneeB } }, + ]); + + await service.notifyBulkStatusChange({ + organizationId: 'org_1', + taskIds: ['tsk_1', 'tsk_2', 'tsk_3'], + newStatus: 'done' as never, + changedByUserId: actor.id, + }); + + const calls = triggerEmailMock.mock.calls.map((c) => ({ + to: c[0].to, + subject: c[0].subject, + })); + expect(calls.length).toBe(2); + + const aCall = calls.find((c) => c.to === 'a@acme.com'); + const bCall = calls.find((c) => c.to === 'b@acme.com'); + expect(aCall?.subject).toContain('2 tasks'); + expect(bCall?.subject).toContain('1 task'); + }); + + it('routes unassigned tasks to owners and admins', async () => { + const actor = makeUser('usr_actor', 'actor@acme.com', 'Actor'); + const owner = makeUser('usr_owner', 'owner@acme.com', 'Owner'); + const admin = makeUser('usr_admin', 'admin@acme.com', 'Admin'); + const employee = makeUser('usr_emp', 'emp@acme.com', 'Emp'); + + mockDb.organization.findUnique.mockResolvedValue({ name: 'Acme' }); + mockDb.user.findUnique.mockResolvedValue({ + name: actor.name, + email: actor.email, + }); + mockDb.task.findMany.mockResolvedValue([ + { id: 'tsk_1', title: 't1', assignee: null }, + { id: 'tsk_2', title: 't2', assignee: null }, + ]); + mockDb.member.findMany.mockResolvedValue([ + { role: 'owner', user: owner }, + { role: 'admin', user: admin }, + { role: 'employee', user: employee }, + ]); + + await service.notifyBulkStatusChange({ + organizationId: 'org_1', + taskIds: ['tsk_1', 'tsk_2'], + newStatus: 'done' as never, + changedByUserId: actor.id, + }); + + expect(recipientEmails().sort()).toEqual( + ['admin@acme.com', 'owner@acme.com'].sort(), + ); + const aCall = triggerEmailMock.mock.calls.find( + (c) => c[0].to === 'owner@acme.com', + ); + expect(aCall?.[0].subject).toContain('2 tasks'); + }); + + it('does not send email when actor is the only assignee across tasks', async () => { + const actor = makeUser('usr_actor', 'actor@acme.com', 'Actor'); + + mockDb.organization.findUnique.mockResolvedValue({ name: 'Acme' }); + mockDb.user.findUnique.mockResolvedValue({ + name: actor.name, + email: actor.email, + }); + mockDb.task.findMany.mockResolvedValue([ + { id: 'tsk_1', title: 't1', assignee: { user: actor } }, + { id: 'tsk_2', title: 't2', assignee: { user: actor } }, + ]); + + await service.notifyBulkStatusChange({ + organizationId: 'org_1', + taskIds: ['tsk_1', 'tsk_2'], + newStatus: 'done' as never, + changedByUserId: actor.id, + }); + + expect(triggerEmailMock).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/apps/api/src/tasks/task-notifier.service.ts b/apps/api/src/tasks/task-notifier.service.ts index 91dbf22d11..2ace0f781c 100644 --- a/apps/api/src/tasks/task-notifier.service.ts +++ b/apps/api/src/tasks/task-notifier.service.ts @@ -16,12 +16,54 @@ import { NovuService } from '../notifications/novu.service'; const BULK_TASK_WORKFLOW_ID = 'evidence-bulk-updated'; const TASK_WORKFLOW_ID = 'evidence-updated'; +type StatusRecipient = { id: string; name: string; email: string }; + +function toStatusRecipient(user: { + id: string; + name: string | null; + email: string; +}): StatusRecipient { + return { + id: user.id, + name: user.name?.trim() || user.email?.trim() || 'User', + email: user.email, + }; +} + @Injectable() export class TaskNotifierService { private readonly logger = new Logger(TaskNotifierService.name); constructor(private readonly novuService: NovuService) {} + /** + * Members with 'owner' or 'admin' in their comma-separated role string, + * excluding the actor. Used as the fallback recipient pool when a task + * has no assignee. + */ + private async getOwnerAdminRecipients( + organizationId: string, + actorUserId: string | undefined, + ): Promise { + const members = await db.member.findMany({ + where: { organizationId, deactivated: false }, + select: { + role: true, + user: { select: { id: true, name: true, email: true } }, + }, + }); + + const recipients: StatusRecipient[] = []; + for (const member of members) { + if (!member.user?.id || !member.user.email) continue; + if (member.user.id === actorUserId) continue; + const roles = (member.role ?? '').split(',').map((r) => r.trim()); + if (!roles.includes('owner') && !roles.includes('admin')) continue; + recipients.push(toStatusRecipient(member.user)); + } + return recipients; + } + async notifyBulkStatusChange(params: { organizationId: string; taskIds: string[]; @@ -31,64 +73,37 @@ export class TaskNotifierService { const { organizationId, taskIds, newStatus, changedByUserId } = params; try { - const [organization, changedByUser, tasks, allMembers] = - await Promise.all([ - db.organization.findUnique({ - where: { id: organizationId }, - select: { name: true }, - }), - db.user.findUnique({ - where: { id: changedByUserId }, - select: { name: true, email: true }, - }), - db.task.findMany({ - where: { - id: { in: taskIds }, - organizationId, - }, - select: { - id: true, - title: true, - assigneeId: true, - assignee: { - select: { - id: true, - user: { - select: { - id: true, - name: true, - email: true, - }, + const [organization, changedByUser, tasks] = await Promise.all([ + db.organization.findUnique({ + where: { id: organizationId }, + select: { name: true }, + }), + db.user.findUnique({ + where: { id: changedByUserId }, + select: { name: true, email: true }, + }), + db.task.findMany({ + where: { + id: { in: taskIds }, + organizationId, + }, + select: { + id: true, + title: true, + assignee: { + select: { + user: { + select: { + id: true, + name: true, + email: true, }, }, }, }, - }), - db.member.findMany({ - where: { - organizationId, - deactivated: false, - OR: [ - { user: { role: { not: 'admin' } } }, - { role: { contains: 'owner' } }, - ], - }, - select: { - id: true, - user: { - select: { - id: true, - name: true, - email: true, - }, - }, - }, - }), - ]); - - this.logger.debug( - `[notifyBulkStatusChange] Found ${allMembers.length} total members for organization ${organizationId}`, - ); + }, + }), + ]); const organizationName = organization?.name ?? 'your organization'; const changedByName = @@ -96,29 +111,51 @@ export class TaskNotifierService { changedByUser?.email?.trim() || 'Someone'; - // Build recipient list: all members excluding actor. - // The isUserUnsubscribed check handles role-based filtering via the notification matrix. - const recipientMap = new Map< + // Recipients: each assignee gets a bulk email scoped to the tasks they + // own. Unassigned tasks are routed to owners/admins so someone can act. + // The actor is always excluded. + const recipientBuckets = new Map< string, - { id: string; name: string; email: string } + { recipient: StatusRecipient; taskCount: number } >(); + let unassignedTaskCount = 0; + for (const task of tasks) { + const user = task.assignee?.user; + if (user?.id && user.email) { + if (user.id === changedByUserId) continue; + const existing = recipientBuckets.get(user.id); + if (existing) { + existing.taskCount += 1; + } else { + recipientBuckets.set(user.id, { + recipient: toStatusRecipient(user), + taskCount: 1, + }); + } + } else { + unassignedTaskCount += 1; + } + } - for (const member of allMembers) { - if (member.user?.id && member.user.email) { - const userId = member.user.id; - if (userId !== changedByUserId) { - recipientMap.set(userId, { - id: userId, - name: - member.user.name?.trim() || member.user.email?.trim() || 'User', - email: member.user.email, + if (unassignedTaskCount > 0) { + const ownerAdmins = await this.getOwnerAdminRecipients( + organizationId, + changedByUserId, + ); + for (const r of ownerAdmins) { + const existing = recipientBuckets.get(r.id); + if (existing) { + existing.taskCount += unassignedTaskCount; + } else { + recipientBuckets.set(r.id, { + recipient: r, + taskCount: unassignedTaskCount, }); } } } - const recipients = Array.from(recipientMap.values()); - const taskCount = tasks.length; + const recipients = Array.from(recipientBuckets.values()); const statusLabel = newStatus.replace('_', ' '); const appUrl = @@ -128,12 +165,12 @@ export class TaskNotifierService { const tasksUrl = `${appUrl}/${organizationId}/tasks`; this.logger.log( - `Sending bulk status change notifications to ${recipients.length} recipients for ${taskCount} task(s)`, + `Sending bulk status change notifications to ${recipients.length} recipients for ${tasks.length} task(s)`, ); // Send notifications to each recipient await Promise.allSettled( - recipients.map(async (recipient) => { + recipients.map(async ({ recipient, taskCount }) => { const isUnsubscribed = await isUserUnsubscribed( db, recipient.email, @@ -390,60 +427,34 @@ export class TaskNotifierService { } = params; try { - const [organization, changedByUser, task, allMembers] = await Promise.all( - [ - db.organization.findUnique({ - where: { id: organizationId }, - select: { name: true }, - }), - changedByUserId - ? db.user.findUnique({ - where: { id: changedByUserId }, - select: { name: true, email: true }, - }) - : Promise.resolve(null), - db.task.findUnique({ - where: { id: taskId }, - select: { - assignee: { - select: { - user: { - select: { - id: true, - name: true, - email: true, - }, + const [organization, changedByUser, task] = await Promise.all([ + db.organization.findUnique({ + where: { id: organizationId }, + select: { name: true }, + }), + changedByUserId + ? db.user.findUnique({ + where: { id: changedByUserId }, + select: { name: true, email: true }, + }) + : Promise.resolve(null), + db.task.findUnique({ + where: { id: taskId }, + select: { + assignee: { + select: { + user: { + select: { + id: true, + name: true, + email: true, }, }, }, }, - }), - db.member.findMany({ - where: { - organizationId, - deactivated: false, - OR: [ - { user: { role: { not: 'admin' } } }, - { role: { contains: 'owner' } }, - ], - }, - select: { - id: true, - user: { - select: { - id: true, - name: true, - email: true, - }, - }, - }, - }), - ], - ); - - this.logger.debug( - `[notifyStatusChange] Found ${allMembers.length} total members for organization ${organizationId}`, - ); + }, + }), + ]); const organizationName = organization?.name ?? 'your organization'; const changedByName = @@ -453,24 +464,22 @@ export class TaskNotifierService { const oldStatusLabel = oldStatus.replace('_', ' '); const newStatusLabel = newStatus.replace('_', ' '); - // Build recipient list: all members excluding actor. - // The isUserUnsubscribed check handles role-based filtering via the notification matrix. - const recipientMap = new Map< - string, - { id: string; name: string; email: string } - >(); - - for (const member of allMembers) { - if (member.user?.id && member.user.email) { - const userId = member.user.id; - if (userId !== changedByUserId) { - recipientMap.set(userId, { - id: userId, - name: - member.user.name?.trim() || member.user.email?.trim() || 'User', - email: member.user.email, - }); - } + // Recipients: the task's assignee (if any). If the task is unassigned, + // fall back to owners/admins so someone who can act on it is notified. + // The actor is always excluded. + const recipientMap = new Map(); + const assigneeUser = task?.assignee?.user; + if (assigneeUser?.id && assigneeUser.email) { + if (assigneeUser.id !== changedByUserId) { + recipientMap.set(assigneeUser.id, toStatusRecipient(assigneeUser)); + } + } else { + const ownerAdmins = await this.getOwnerAdminRecipients( + organizationId, + changedByUserId, + ); + for (const r of ownerAdmins) { + recipientMap.set(r.id, r); } }