diff --git a/src/app/api/notification/notification.service.test.ts b/src/app/api/notification/notification.service.test.ts index c9450b50d..da2a2e775 100644 --- a/src/app/api/notification/notification.service.test.ts +++ b/src/app/api/notification/notification.service.test.ts @@ -228,6 +228,7 @@ describe('NotificationService grouped-email interception', () => { email: true, disableInProduct: true, commentId: '44444444-4444-4444-4444-444444444444', + isRecipientIu: false, }) expect(mockGroupedCreateMany).toHaveBeenCalledTimes(2) @@ -248,6 +249,7 @@ describe('NotificationService grouped-email interception', () => { ['cu_a', 'cu_b', 'cu_c'], { email: true, + isRecipientIu: false, }, ) @@ -263,7 +265,10 @@ describe('NotificationService grouped-email interception', () => { associations: [{ companyId: assocCompany }] as unknown as Task['associations'], }) - await buildService().createBulkNotification(NotificationTaskActions.SharedToCompany, task, ['cu_a'], { email: true }) + await buildService().createBulkNotification(NotificationTaskActions.SharedToCompany, task, ['cu_a'], { + email: true, + isRecipientIu: false, + }) expect(mockGroupedCreateMany.mock.calls[0][0].data[0].recipientCompanyId).toBe(assocCompany) }) @@ -273,11 +278,54 @@ describe('NotificationService grouped-email interception', () => { email: false, disableInProduct: false, commentId: '44444444-4444-4444-4444-444444444444', + isRecipientIu: false, }) expect(mockGroupedCreateMany).not.toHaveBeenCalled() expect(mockCreateNotification).toHaveBeenCalledTimes(1) }) + + it('buffers a Commented IU email as an IU row and dispatches the in-product notification to the IU', async () => { + await buildService().createBulkNotification(NotificationTaskActions.Commented, makeTask(), ['iu_a', 'iu_b'], { + email: true, + disableInProduct: false, + commentId: '44444444-4444-4444-4444-444444444444', + isRecipientIu: true, + }) + + expect(mockGroupedCreateMany).toHaveBeenCalledTimes(2) + const rows = mockGroupedCreateMany.mock.calls.map((c) => c[0].data[0]) + expect(rows.map((r) => r.recipientIuId)).toEqual(['iu_a', 'iu_b']) + for (const row of rows) { + expect(row.eventType).toBe(GroupedEmailEventType.COMMENT) + expect(row.recipientClientId).toBeNull() + expect(row.individualEmail.recipientInternalUserId).toBeDefined() + expect(row.individualEmail.recipientClientId).toBeUndefined() + } + + // in-product still fires immediately, routed to the IU with the email stripped + const sent = mockCreateNotification.mock.calls.map((c) => c[0]) + expect(sent.map((s) => s.recipientInternalUserId)).toEqual(['iu_a', 'iu_b']) + for (const s of sent) { + expect(s.recipientClientId).toBeUndefined() + expect(s.deliveryTargets.email).toBeUndefined() + } + }) + + it('routes an email-enabled Commented email to the client when isRecipientIu is false (no email-absence inference)', async () => { + await buildService().createBulkNotification(NotificationTaskActions.Commented, makeTask(), ['cu_a'], { + email: true, + disableInProduct: true, + commentId: '44444444-4444-4444-4444-444444444444', + isRecipientIu: false, + }) + + const row = mockGroupedCreateMany.mock.calls[0][0].data[0] + expect(row.recipientClientId).toBe('cu_a') + expect(row.recipientIuId).toBeNull() + expect(row.individualEmail.recipientClientId).toBe('cu_a') + expect(row.individualEmail.recipientInternalUserId).toBeUndefined() + }) }) }) @@ -430,6 +478,7 @@ describe('guard: IU completion emails', () => { it('bulk Completed buffers one COMPLETED IU row per recipient and strips the email from dispatch', async () => { await buildService().createBulkNotification(NotificationTaskActions.Completed, makeTask(), ['iu_a', 'iu_b'], { email: true, + isRecipientIu: true, }) expect(mockGroupedCreateMany).toHaveBeenCalledTimes(2) @@ -453,6 +502,7 @@ describe('guard: IU completion emails', () => { it('bulk CompletedByCompanyMember neither buffers nor emails when the flag is off (email opt falsy)', async () => { await buildService().createBulkNotification(NotificationTaskActions.CompletedByCompanyMember, makeTask(), ['iu_a'], { email: false, + isRecipientIu: true, }) expect(mockGroupedCreateMany).not.toHaveBeenCalled() diff --git a/src/app/api/notification/notification.service.ts b/src/app/api/notification/notification.service.ts index c547bf67b..cde79f88b 100644 --- a/src/app/api/notification/notification.service.ts +++ b/src/app/api/notification/notification.service.ts @@ -144,7 +144,10 @@ export class NotificationService extends BaseService { action: NotificationTaskActions, task: Task, recipientIds: string[], - opts?: { + // isRecipientIu is required: the same action (e.g. Commented) fans out to both CU and IU + // recipient lists, so routing must be declared by the caller, never inferred. + opts: { + isRecipientIu: boolean email?: boolean disableInProduct?: boolean commentId?: string @@ -191,14 +194,9 @@ export class NotificationService extends BaseService { const iuNotifications = [] const association = AssociationsSchema.parse(task.associations)?.[0] - // Non-null only when these CU emails should be diverted into the grouped buffer. + // Non-null only when these emails should be diverted into the grouped buffer. const groupedType = email ? this.groupedEventTypeFor(action) : null - // Completion recipients are IUs; undefined (not false) elsewhere so paths like the - // Commented-to-IU job keep the absence-of-email inference in buildNotificationDetails - const isRecipientIu = - action === NotificationTaskActions.Completed || action === NotificationTaskActions.CompletedByCompanyMember - ? true - : undefined + const isRecipientIu = opts.isRecipientIu // NOTE: The reason we are skipping using NotificationService#create and implementing notification dispatch + save manually is because // we can just do one `createMany` DB call instead of one per notification, saving a ton of DB calls @@ -728,9 +726,7 @@ export class NotificationService extends BaseService { recipientCompanyId: task.companyId ?? association?.companyId ?? undefined, deliveryTargets: deliveryTargets || {}, } - // Fall back to inferring IU from absence of email for paths not yet updated (e.g. CommentToIU). - const isIU = isRecipientIu ?? !notificationDetails.deliveryTargets?.email - if (isIU) { + if (isRecipientIu) { delete notificationDetails.recipientCompanyId delete notificationDetails.recipientClientId notificationDetails.recipientInternalUserId = recipientId diff --git a/src/app/api/tasks/task-notifications.service.ts b/src/app/api/tasks/task-notifications.service.ts index 3adfff09a..ffbf1dc4d 100644 --- a/src/app/api/tasks/task-notifications.service.ts +++ b/src/app/api/tasks/task-notifications.service.ts @@ -372,7 +372,7 @@ export class TaskNotificationsService extends BaseService { NotificationTaskActions.CompletedByCompanyMember, updatedTask, recipientIds, - { senderCompanyId, email: isIuEmailEnabled() }, + { senderCompanyId, email: isIuEmailEnabled(), isRecipientIu: true }, ) await this.notificationService.markAsReadForAllRecipients(updatedTask) } else { @@ -384,6 +384,7 @@ export class TaskNotificationsService extends BaseService { await this.notificationService.createBulkNotification(NotificationTaskActions.Completed, updatedTask, recipientIds, { senderCompanyId, email: isIuEmailEnabled(), + isRecipientIu: true, }) await this.notificationService.markClientNotificationAsRead(updatedTask) } @@ -424,6 +425,7 @@ export class TaskNotificationsService extends BaseService { await this.notificationService.createBulkNotification(NotificationTaskActions.SharedToCompany, task, recipientIds, { email: true, disableInProduct: true, + isRecipientIu: false, }) } @@ -448,7 +450,7 @@ export class TaskNotificationsService extends BaseService { NotificationTaskActions.CompletedToSharedCompany, task, recipientIds, - { email: true, disableInProduct: true }, + { email: true, disableInProduct: true, isRecipientIu: false }, ) } @@ -516,7 +518,7 @@ export class TaskNotificationsService extends BaseService { isReassigned ? NotificationTaskActions.ReassignedToCompany : NotificationTaskActions.AssignedToCompany, task, recipientIds, - { email: true, emailOverride }, + { email: true, emailOverride, isRecipientIu: false }, ) } diff --git a/src/jobs/notifications/send-comment-create-notifications.ts b/src/jobs/notifications/send-comment-create-notifications.ts index 4efffa96e..43ca2e1ca 100644 --- a/src/jobs/notifications/send-comment-create-notifications.ts +++ b/src/jobs/notifications/send-comment-create-notifications.ts @@ -1,6 +1,7 @@ import User from '@/app/api/core/models/User.model' import { NotificationTaskActions } from '@/app/api/core/types/tasks' import { UserRole } from '@/app/api/core/types/user' +import { isIuEmailEnabled } from '@/app/api/notification/isIuEmailEnabled' import { NotificationService } from '@/app/api/notification/notification.service' import { CopilotAPI } from '@/utils/CopilotAPI' import { Comment, Task } from '@prisma/client' @@ -40,6 +41,7 @@ export const sendCommentCreateNotifications = task({ email: true, disableInProduct: true, commentId: comment.id, + isRecipientIu: false, }) const { recipientIds: iuRecipientIds, senderCompanyId } = await commentNotificationService.getNotificationParties( @@ -50,10 +52,11 @@ export const sendCommentCreateNotifications = task({ const filteredIUIds = iuRecipientIds.filter((id: string) => id !== comment.initiatorId) console.info('creating notifications for IUs', filteredIUIds) await commentNotificationService.createBulkNotification(NotificationTaskActions.Commented, task, filteredIUIds, { - email: false, + email: isIuEmailEnabled(), disableInProduct: false, commentId: comment.id, senderCompanyId, + isRecipientIu: true, }) }, }) diff --git a/src/jobs/notifications/send-reply-create-notifications.ts b/src/jobs/notifications/send-reply-create-notifications.ts index 630a0487e..0bc4a69a5 100644 --- a/src/jobs/notifications/send-reply-create-notifications.ts +++ b/src/jobs/notifications/send-reply-create-notifications.ts @@ -5,6 +5,7 @@ import { CopilotAPI } from '@/utils/CopilotAPI' import { isMessagableError } from '@/utils/copilotError' import { CommentRepository } from '@/app/api/comments/comment.repository' import { CommentService } from '@/app/api/comments/comment.service' +import { isIuEmailEnabled } from '@/app/api/notification/isIuEmailEnabled' import User from '@api/core/models/User.model' import { TasksService } from '@api/tasks/tasks.service' import { Comment, CommentInitiator, Task } from '@prisma/client' @@ -166,7 +167,10 @@ const getInitiatorNotificationPromises = ( body = { ...base, recipientInternalUserId: initiator.initiatorId, - deliveryTargets: { inProduct: deliveryTargets.inProduct }, + deliveryTargets: { + inProduct: deliveryTargets.inProduct, + ...(isIuEmailEnabled() && { email: deliveryTargets.email }), + }, } } else if (initiator.initiatorType === CommentInitiator.client || assume === CommentInitiator.client) { body = {