Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 51 additions & 1 deletion src/app/api/notification/notification.service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -248,6 +249,7 @@ describe('NotificationService grouped-email interception', () => {
['cu_a', 'cu_b', 'cu_c'],
{
email: true,
isRecipientIu: false,
},
)

Expand All @@ -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)
})
Expand All @@ -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()
})
})
})

Expand Down Expand Up @@ -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)
Expand All @@ -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()
Expand Down
18 changes: 7 additions & 11 deletions src/app/api/notification/notification.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
8 changes: 5 additions & 3 deletions src/app/api/tasks/task-notifications.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
}
Expand Down Expand Up @@ -424,6 +425,7 @@ export class TaskNotificationsService extends BaseService {
await this.notificationService.createBulkNotification(NotificationTaskActions.SharedToCompany, task, recipientIds, {
email: true,
disableInProduct: true,
isRecipientIu: false,
})
}

Expand All @@ -448,7 +450,7 @@ export class TaskNotificationsService extends BaseService {
NotificationTaskActions.CompletedToSharedCompany,
task,
recipientIds,
{ email: true, disableInProduct: true },
{ email: true, disableInProduct: true, isRecipientIu: false },
)
}

Expand Down Expand Up @@ -516,7 +518,7 @@ export class TaskNotificationsService extends BaseService {
isReassigned ? NotificationTaskActions.ReassignedToCompany : NotificationTaskActions.AssignedToCompany,
task,
recipientIds,
{ email: true, emailOverride },
{ email: true, emailOverride, isRecipientIu: false },
)
}

Expand Down
5 changes: 4 additions & 1 deletion src/jobs/notifications/send-comment-create-notifications.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -40,6 +41,7 @@ export const sendCommentCreateNotifications = task({
email: true,
disableInProduct: true,
commentId: comment.id,
isRecipientIu: false,
})

const { recipientIds: iuRecipientIds, senderCompanyId } = await commentNotificationService.getNotificationParties(
Expand All @@ -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,
})
},
})
6 changes: 5 additions & 1 deletion src/jobs/notifications/send-reply-create-notifications.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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 = {
Expand Down
Loading