From db865e3663cc122c9b3ae5d6ba7c0731cae379f6 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 3 Jun 2026 08:51:37 +0000 Subject: [PATCH 01/20] Short-circuit detail HEAD requests Co-authored-by: Neil Raina --- src/proxy.test.ts | 18 ++++++++++++++++++ src/proxy.ts | 18 ++++++++++++++++++ 2 files changed, 36 insertions(+) create mode 100644 src/proxy.test.ts create mode 100644 src/proxy.ts diff --git a/src/proxy.test.ts b/src/proxy.test.ts new file mode 100644 index 000000000..c22cebc24 --- /dev/null +++ b/src/proxy.test.ts @@ -0,0 +1,18 @@ +import { proxy } from './proxy' + +const buildRequest = (method: string) => ({ method }) as Parameters[0] + +describe('proxy', () => { + it('short-circuits HEAD detail requests without caching', () => { + const response = proxy(buildRequest('HEAD')) + + expect(response.status).toBe(200) + expect(response.headers.get('Cache-Control')).toBe('no-store') + }) + + it('continues non-HEAD detail requests to the route handler', () => { + const response = proxy(buildRequest('GET')) + + expect(response.headers.get('x-middleware-next')).toBe('1') + }) +}) diff --git a/src/proxy.ts b/src/proxy.ts new file mode 100644 index 000000000..a55f42d4c --- /dev/null +++ b/src/proxy.ts @@ -0,0 +1,18 @@ +import { NextResponse, type NextRequest } from 'next/server' + +export function proxy(request: NextRequest) { + if (request.method === 'HEAD') { + return new NextResponse(null, { + status: 200, + headers: { + 'Cache-Control': 'no-store', + }, + }) + } + + return NextResponse.next() +} + +export const config = { + matcher: ['/detail/:task_id/:user_type'], +} From 1539f5a7034f422aa487dccd7d32c79e3d6b037c Mon Sep 17 00:00:00 2001 From: priosshrsth Date: Mon, 15 Jun 2026 03:41:58 +0000 Subject: [PATCH 02/20] remove proxy test --- src/proxy.test.ts | 18 ------------------ 1 file changed, 18 deletions(-) delete mode 100644 src/proxy.test.ts diff --git a/src/proxy.test.ts b/src/proxy.test.ts deleted file mode 100644 index c22cebc24..000000000 --- a/src/proxy.test.ts +++ /dev/null @@ -1,18 +0,0 @@ -import { proxy } from './proxy' - -const buildRequest = (method: string) => ({ method }) as Parameters[0] - -describe('proxy', () => { - it('short-circuits HEAD detail requests without caching', () => { - const response = proxy(buildRequest('HEAD')) - - expect(response.status).toBe(200) - expect(response.headers.get('Cache-Control')).toBe('no-store') - }) - - it('continues non-HEAD detail requests to the route handler', () => { - const response = proxy(buildRequest('GET')) - - expect(response.headers.get('x-middleware-next')).toBe('1') - }) -}) From 77a098a0fc7896c78a88d12148ba36f308862abf Mon Sep 17 00:00:00 2001 From: arpandhakal-lgtm Date: Tue, 30 Jun 2026 14:25:39 +0545 Subject: [PATCH 03/20] feat(notifications): extend grouped email system to support IU recipients - Wire recipientIuId into bufferGroupedEmailEvent with a separate window query and IU-specific window key (avoids collisions with CU keys) - Teach flushGroupedEmail to group and flush IU recipient rows via recipientInternalUserId in the Copilot notification payload - Add isIuEmailEnabled() stub backed by IU_EMAIL_ALWAYS_ENABLED env var so IU emails can be tested before OUT-3929 (platform preference flag) ships - Remove disableEmail hard-block for IU assignees; gate on isIuEmailEnabled() - Fix buildNotificationDetails to accept explicit isIuRecipient flag so the payload sets recipientInternalUserId correctly when IU has email Co-Authored-By: Claude Sonnet 4.6 --- src/app/api/notification/isIuEmailEnabled.ts | 3 + .../notification/notification.service.test.ts | 2 +- .../api/notification/notification.service.ts | 81 ++++++++----- .../api/tasks/task-notifications.service.ts | 3 +- src/config/index.ts | 3 + src/jobs/notifications/flush-grouped-email.ts | 108 ++++++++++++++---- src/jobs/notifications/send-grouped-email.ts | 9 +- 7 files changed, 153 insertions(+), 56 deletions(-) create mode 100644 src/app/api/notification/isIuEmailEnabled.ts diff --git a/src/app/api/notification/isIuEmailEnabled.ts b/src/app/api/notification/isIuEmailEnabled.ts new file mode 100644 index 000000000..b144a8e67 --- /dev/null +++ b/src/app/api/notification/isIuEmailEnabled.ts @@ -0,0 +1,3 @@ +import { iuEmailAlwaysEnabled } from '@/config' + +export const isIuEmailEnabled = (): boolean => iuEmailAlwaysEnabled diff --git a/src/app/api/notification/notification.service.test.ts b/src/app/api/notification/notification.service.test.ts index ce7c59d75..0e46daa38 100644 --- a/src/app/api/notification/notification.service.test.ts +++ b/src/app/api/notification/notification.service.test.ts @@ -318,6 +318,6 @@ describe('guard: CU wiring boundaries', () => { it('never writes recipientIuId in a CU grouped event row', async () => { await buildService().create(NotificationTaskActions.Assigned, makeTask()) const row = mockGroupedCreateMany.mock.calls[0][0].data[0] - expect(row.recipientIuId).toBeUndefined() + expect(row.recipientIuId).toBeNull() }) }) diff --git a/src/app/api/notification/notification.service.ts b/src/app/api/notification/notification.service.ts index e37a311e7..c27127f87 100644 --- a/src/app/api/notification/notification.service.ts +++ b/src/app/api/notification/notification.service.ts @@ -55,6 +55,10 @@ export class NotificationService extends BaseService { action, ) + const isIuRecipient = + task.assigneeType === AssigneeType.internalUser && + (action === NotificationTaskActions.Assigned || action === NotificationTaskActions.ReassignedToIU) + const inProduct = opts.disableInProduct ? undefined : getInProductNotificationDetails(workspace, actionUser, task, { companyName, commentId: opts?.commentId })[action] @@ -63,21 +67,28 @@ export class NotificationService extends BaseService { : getEmailDetails(workspace, actionUser, task, { commentId: opts?.commentId })[action] const email = baseEmail ? mergeEmailOverride({ base: baseEmail, override: opts.emailOverride }) : baseEmail - // Non-null only when this CU email should be diverted into the grouped buffer. const groupedType = email && recipientId ? this.groupedEventTypeFor(action) : null if (groupedType) { const association = AssociationsSchema.parse(task.associations)?.[0] await this.bufferGroupedEmailEvent({ task, - recipientClientId: recipientId, - recipientCompanyId: task.companyId ?? association?.companyId ?? null, + recipientClientId: isIuRecipient ? null : recipientId, + recipientCompanyId: isIuRecipient ? null : (task.companyId ?? association?.companyId ?? null), + recipientIuId: isIuRecipient ? recipientId : null, eventType: groupedType, commentId: opts.commentId, - individualEmail: this.buildNotificationDetails(task, senderId, recipientId, { email }, senderCompanyId), + individualEmail: this.buildNotificationDetails( + task, + senderId, + recipientId, + { email }, + senderCompanyId, + isIuRecipient, + ), }) } - // Build with the email so the recipient is routed as a client, then drop the email target + // Build with the email so the recipient is routed correctly, then drop the email target // once it has been diverted to the buffer (the in-product notification still fires now). const notificationDetails = this.buildNotificationDetails( task, @@ -85,6 +96,7 @@ export class NotificationService extends BaseService { recipientId, { inProduct, email }, senderCompanyId, + isIuRecipient, ) if (groupedType) notificationDetails.deliveryTargets = { inProduct } if (!inProduct && !notificationDetails.deliveryTargets?.email) return @@ -106,10 +118,7 @@ export class NotificationService extends BaseService { // NOTE: There are cases where task.assigneeType does not account for IU notification! // E.g. When receiving notifications from others completing task that IU created. // For now we don't have to store these so this hasn't been accounted for - const shouldSendIUNotification = - task.assigneeType === AssigneeType.internalUser && - (action === NotificationTaskActions.Assigned || action === NotificationTaskActions.ReassignedToIU) - if (shouldSendIUNotification) { + if (isIuRecipient) { // Notification recipient is IU in this case await this.db.internalUserNotification.create({ data: { @@ -601,35 +610,49 @@ export class NotificationService extends BaseService { private async bufferGroupedEmailEvent(args: { task: Task - recipientClientId: string - recipientCompanyId: string | null + recipientClientId?: string | null + recipientCompanyId?: string | null + recipientIuId?: string | null eventType: GroupedEmailEventType commentId?: string individualEmail: NotificationRequestBody }): Promise { - const { task, recipientClientId, recipientCompanyId, eventType, commentId, individualEmail } = args - - const activeWindow = await this.db.$queryRaw<{ windowKey: string }[]>` - SELECT "windowKey" FROM "GroupedEmailEvents" - WHERE "workspaceId" = ${task.workspaceId} - AND "recipientClientId" = ${recipientClientId}::uuid - AND "recipientCompanyId" IS NOT DISTINCT FROM ${recipientCompanyId}::uuid - AND "sentAt" IS NULL - AND "createdAt" > now() - interval '5 minutes' - ORDER BY "createdAt" DESC - LIMIT 1` + const { task, recipientClientId, recipientCompanyId, recipientIuId, eventType, commentId, individualEmail } = args + const isIuRecipient = !!recipientIuId + + const activeWindow = isIuRecipient + ? await this.db.$queryRaw<{ windowKey: string }[]>` + SELECT "windowKey" FROM "GroupedEmailEvents" + WHERE "workspaceId" = ${task.workspaceId} + AND "recipientIuId" = ${recipientIuId}::uuid + AND "sentAt" IS NULL + AND "createdAt" > now() - interval '5 minutes' + ORDER BY "createdAt" DESC + LIMIT 1` + : await this.db.$queryRaw<{ windowKey: string }[]>` + SELECT "windowKey" FROM "GroupedEmailEvents" + WHERE "workspaceId" = ${task.workspaceId} + AND "recipientClientId" = ${recipientClientId}::uuid + AND "recipientCompanyId" IS NOT DISTINCT FROM ${recipientCompanyId}::uuid + AND "sentAt" IS NULL + AND "createdAt" > now() - interval '5 minutes' + ORDER BY "createdAt" DESC + LIMIT 1` const isNewWindow = activeWindow.length === 0 const windowKey = isNewWindow - ? `${recipientClientId}:${recipientCompanyId ?? 'none'}:${randomUUID()}` + ? isIuRecipient + ? `${recipientIuId}:iu:${randomUUID()}` + : `${recipientClientId}:${recipientCompanyId ?? 'none'}:${randomUUID()}` : activeWindow[0].windowKey await this.db.groupedEmailEvent.createMany({ data: [ { workspaceId: task.workspaceId, - recipientClientId, - recipientCompanyId, + recipientClientId: recipientClientId ?? null, + recipientCompanyId: recipientCompanyId ?? null, + recipientIuId: recipientIuId ?? null, eventType, taskId: task.id, taskTitleSnapshot: task.title, @@ -670,8 +693,8 @@ export class NotificationService extends BaseService { recipientId: string, deliveryTargets: NotificationRequestBody['deliveryTargets'], senderCompanyId?: string, + isIuRecipient?: boolean, ): NotificationRequestBody { - // Assume client notification then change details body if IU const associations = AssociationsSchema.parse(task.associations) const association = associations?.[0] const notificationDetails: NotificationRequestBody = { @@ -680,12 +703,10 @@ export class NotificationService extends BaseService { senderType: this.user.role, recipientClientId: recipientId ?? undefined, recipientCompanyId: task.companyId ?? association?.companyId ?? undefined, - // If any of the given action is not present in details obj, that type of notification is not sent deliveryTargets: deliveryTargets || {}, } - //! Since IU's NEVER get email notifications, we send recipientCompanyId only if email is present - const isIU = !notificationDetails.deliveryTargets?.email - // In case this logic ever changes, good luck + // Fall back to inferring IU from absence of email for paths not yet updated (e.g. CommentToIU). + const isIU = isIuRecipient ?? !notificationDetails.deliveryTargets?.email if (isIU) { delete notificationDetails.recipientCompanyId delete notificationDetails.recipientClientId diff --git a/src/app/api/tasks/task-notifications.service.ts b/src/app/api/tasks/task-notifications.service.ts index e91aaffe2..0d74533e2 100644 --- a/src/app/api/tasks/task-notifications.service.ts +++ b/src/app/api/tasks/task-notifications.service.ts @@ -6,6 +6,7 @@ import { CopilotAPI } from '@/utils/CopilotAPI' import User from '@api/core/models/User.model' import { BaseService } from '@api/core/services/base.service' import { NotificationTaskActions } from '@api/core/types/tasks' +import { isIuEmailEnabled } from '@api/notification/isIuEmailEnabled' import { NotificationService } from '@api/notification/notification.service' import { AssigneeType, StateType, Task, WorkflowState } from '@prisma/client' import { z } from 'zod' @@ -484,7 +485,7 @@ export class TaskNotificationsService extends BaseService { // In future when reassignment is supported, change this logic to support reassigned to client as well notificationType, task, - { disableEmail: task.assigneeType === AssigneeType.internalUser, emailOverride }, + { disableEmail: task.assigneeType === AssigneeType.internalUser && !isIuEmailEnabled(), emailOverride }, ) // Create a new entry in ClientNotifications table so we can mark as read on // behalf of client later diff --git a/src/config/index.ts b/src/config/index.ts index 3813ba9f0..d75521224 100644 --- a/src/config/index.ts +++ b/src/config/index.ts @@ -53,6 +53,9 @@ export const assemblyApiDomain = z.string().url().parse(process.env.NEXT_PUBLIC_ // Substring stripped from the task title when building the reminder email subject for // subject-override workspaces, and the value it's replaced with. Configured via env so the // workspace-specific phrasing isn't hardcoded (OUT-3919). +// Bypasses the platform preference check while OUT-3929 is pending. Set true in dev/staging. +export const iuEmailAlwaysEnabled = process.env.IU_EMAIL_ALWAYS_ENABLED === 'true' + export const reminderSubjectSearch = process.env.REMINDER_SUBJECT_SEARCH || '' export const reminderSubjectReplacement = process.env.REMINDER_SUBJECT_REPLACEMENT || '' diff --git a/src/jobs/notifications/flush-grouped-email.ts b/src/jobs/notifications/flush-grouped-email.ts index 47ab2a51c..f5f2c7560 100644 --- a/src/jobs/notifications/flush-grouped-email.ts +++ b/src/jobs/notifications/flush-grouped-email.ts @@ -24,26 +24,32 @@ type WindowEvent = GroupedEmailEventInput & { individualEmail: NotificationReque type BufferedRow = WindowEvent & { recipientClientId: string | null recipientCompanyId: string | null + recipientIuId: string | null } -type RecipientGroup = { +type CuRecipientGroup = { recipientClientId: string recipientCompanyId: string | null events: WindowEvent[] } +type IuRecipientGroup = { + recipientIuId: string + events: WindowEvent[] +} + const TASK_ID = 'flush-grouped-email' const readUnsentWindowEvents = (db: ReturnType, windowKey: string) => db.$queryRaw` - SELECT "eventType", "taskId", "taskTitleSnapshot", "createdAt", "recipientClientId", "recipientCompanyId", "individualEmail" + SELECT "eventType", "taskId", "taskTitleSnapshot", "createdAt", "recipientClientId", "recipientCompanyId", "recipientIuId", "individualEmail" FROM "GroupedEmailEvents" WHERE "windowKey" = ${windowKey} AND "sentAt" IS NULL` const deleteWindowRows = (db: ReturnType, windowKey: string) => db.$executeRaw`DELETE FROM "GroupedEmailEvents" WHERE "windowKey" = ${windowKey} AND "sentAt" IS NOT NULL` -const markRecipientSent = ( +const markCuRecipientSent = ( db: ReturnType, windowKey: string, recipientClientId: string, @@ -53,6 +59,16 @@ const markRecipientSent = ( UPDATE "GroupedEmailEvents" SET "sentAt" = now(), "batchId" = ${batchId}::uuid WHERE "windowKey" = ${windowKey} AND "recipientClientId" = ${recipientClientId}::uuid AND "sentAt" IS NULL` +const markIuRecipientSent = ( + db: ReturnType, + windowKey: string, + recipientIuId: string, + batchId: string, +) => + db.$executeRaw` + UPDATE "GroupedEmailEvents" SET "sentAt" = now(), "batchId" = ${batchId}::uuid + WHERE "windowKey" = ${windowKey} AND "recipientIuId" = ${recipientIuId}::uuid AND "sentAt" IS NULL` + const resolveSenderId = async (copilot: CopilotAPI): Promise => { const { data } = await copilot.getInternalUsers({ limit: 1 }) const senderId = data[0]?.id @@ -81,29 +97,41 @@ const getLiveTaskIds = async (db: ReturnType, taskI return new Set(live.map((t) => t.id)) } -const groupByRecipient = (rows: BufferedRow[]): RecipientGroup[] => { - const groups = new Map() +const toWindowEvent = (row: BufferedRow): WindowEvent => ({ + eventType: row.eventType, + taskId: row.taskId, + taskTitleSnapshot: row.taskTitleSnapshot, + createdAt: row.createdAt, + individualEmail: row.individualEmail, +}) + +const groupCuRecipients = (rows: BufferedRow[]): CuRecipientGroup[] => { + const groups = new Map() for (const row of rows) { if (!row.recipientClientId) continue const group = groups.get(row.recipientClientId) - const event: WindowEvent = { - eventType: row.eventType, - taskId: row.taskId, - taskTitleSnapshot: row.taskTitleSnapshot, - createdAt: row.createdAt, - individualEmail: row.individualEmail, - } - if (group) group.events.push(event) + if (group) group.events.push(toWindowEvent(row)) else groups.set(row.recipientClientId, { recipientClientId: row.recipientClientId, recipientCompanyId: row.recipientCompanyId, - events: [event], + events: [toWindowEvent(row)], }) } return [...groups.values()] } +const groupIuRecipients = (rows: BufferedRow[]): IuRecipientGroup[] => { + const groups = new Map() + for (const row of rows) { + if (!row.recipientIuId) continue + const group = groups.get(row.recipientIuId) + if (group) group.events.push(toWindowEvent(row)) + else groups.set(row.recipientIuId, { recipientIuId: row.recipientIuId, events: [toWindowEvent(row)] }) + } + return [...groups.values()] +} + export const flushGroupedEmailRun = async (payload: FlushGroupedEmailPayload) => { const { workspaceId, windowKey } = payload const db = DBClient.getInstance() @@ -120,20 +148,22 @@ export const flushGroupedEmailRun = async (payload: FlushGroupedEmailPayload) => const liveTaskIds = await getLiveTaskIds(db, [...new Set(rows.map((r) => r.taskId))]) const skippedDeletedTasks = rows.length - rows.filter((r) => liveTaskIds.has(r.taskId)).length - const groups = groupByRecipient(rows) + const cuGroups = groupCuRecipients(rows) + const iuGroups = groupIuRecipients(rows) logger.log('flush-grouped-email: starting', { workspaceId, windowKey, bufferedEvents: rows.length, skippedDeletedTasks, - recipients: groups.length, + recipients: cuGroups.length + iuGroups.length, }) let sent = 0 let sentGrouped = 0 let sentIndividual = 0 let senderId: string | undefined // resolved lazily — only the grouped branch needs a workspace IU - for (const group of groups) { + + for (const group of cuGroups) { const liveEvents = group.events.filter((e) => liveTaskIds.has(e.taskId)) // A single event reads awkwardly as a "summary" — replay the original individual email verbatim. // Pre-migration rows have no snapshot; fall back to the grouped summary rather than crash. @@ -141,7 +171,7 @@ export const flushGroupedEmailRun = async (payload: FlushGroupedEmailPayload) => Sentry.addBreadcrumb({ category: 'flush-grouped-email', - message: `recipient ${group.recipientClientId}`, + message: `cu recipient ${group.recipientClientId}`, data: { workspaceId, windowKey, recipientClientId: group.recipientClientId, liveEvents: liveEvents.length }, }) @@ -162,7 +192,7 @@ export const flushGroupedEmailRun = async (payload: FlushGroupedEmailPayload) => sentGrouped += 1 } - await markRecipientSent(db, windowKey, group.recipientClientId, batchId) + await markCuRecipientSent(db, windowKey, group.recipientClientId, batchId) logger.log('flush-grouped-email: recipient processed', { workspaceId, windowKey, @@ -172,6 +202,42 @@ export const flushGroupedEmailRun = async (payload: FlushGroupedEmailPayload) => }) } + for (const group of iuGroups) { + const liveEvents = group.events.filter((e) => liveTaskIds.has(e.taskId)) + const singleEmail = liveEvents.length === 1 ? liveEvents[0].individualEmail : null + + Sentry.addBreadcrumb({ + category: 'flush-grouped-email', + message: `iu recipient ${group.recipientIuId}`, + data: { workspaceId, windowKey, recipientIuId: group.recipientIuId, liveEvents: liveEvents.length }, + }) + + if (singleEmail) { + await sendIndividualEmail(copilot, singleEmail) + sent += 1 + sentIndividual += 1 + } else if (liveEvents.length >= 1) { + senderId ??= await resolveSenderId(copilot) + await sendGroupedEmail({ + content: composeGroupedEmail(liveEvents), + senderId, + recipientInternalUserId: group.recipientIuId, + copilot, + }) + sent += 1 + sentGrouped += 1 + } + + await markIuRecipientSent(db, windowKey, group.recipientIuId, batchId) + logger.log('flush-grouped-email: recipient processed', { + workspaceId, + windowKey, + recipientIuId: group.recipientIuId, + liveEvents: liveEvents.length, + outcome: singleEmail ? ('individual' as const) : liveEvents.length >= 1 ? ('grouped' as const) : ('skipped' as const), + }) + } + try { await deleteWindowRows(db, windowKey) } catch (err) { @@ -185,14 +251,14 @@ export const flushGroupedEmailRun = async (payload: FlushGroupedEmailPayload) => logger.log('flush-grouped-email: run summary', { workspaceId, windowKey, - recipients: groups.length, + recipients: cuGroups.length + iuGroups.length, sent, sentGrouped, sentIndividual, bufferedEvents: rows.length, skippedDeletedTasks, }) - return { windowKey, recipients: groups.length, sent, sentGrouped, sentIndividual } + return { windowKey, recipients: cuGroups.length + iuGroups.length, sent, sentGrouped, sentIndividual } } export const flushGroupedEmailOnFailure = async ({ payload, error }: { payload: unknown; error: unknown }) => { diff --git a/src/jobs/notifications/send-grouped-email.ts b/src/jobs/notifications/send-grouped-email.ts index 14d0f9bcf..7a4a29813 100644 --- a/src/jobs/notifications/send-grouped-email.ts +++ b/src/jobs/notifications/send-grouped-email.ts @@ -8,8 +8,9 @@ import { CopilotAPI } from '@/utils/CopilotAPI' export type SendGroupedEmailArgs = { content: GroupedEmailContent senderId: string - recipientClientId: string - recipientCompanyId: string | null + recipientClientId?: string | null + recipientCompanyId?: string | null + recipientInternalUserId?: string | null copilot: CopilotAPI } @@ -18,6 +19,7 @@ export const sendGroupedEmail = async ({ senderId, recipientClientId, recipientCompanyId, + recipientInternalUserId, copilot, }: SendGroupedEmailArgs): Promise => { const email = renderGroupedEmail(content) @@ -25,8 +27,9 @@ export const sendGroupedEmail = async ({ const payload: NotificationRequestBody = { senderId, senderType: 'internalUser', - recipientClientId, + recipientClientId: recipientClientId ?? undefined, recipientCompanyId: recipientCompanyId ?? undefined, + recipientInternalUserId: recipientInternalUserId ?? undefined, deliveryTargets: { email: { subject: email.subject, From c50428e98a6b09d607c2acdb9ea6c475c52ff3ba Mon Sep 17 00:00:00 2001 From: arpandhakal-lgtm Date: Tue, 30 Jun 2026 14:45:19 +0545 Subject: [PATCH 04/20] test(notifications): add IU grouped email buffer coverage Co-Authored-By: Claude Sonnet 4.6 --- .../notification/notification.service.test.ts | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/src/app/api/notification/notification.service.test.ts b/src/app/api/notification/notification.service.test.ts index 0e46daa38..c8a6d56a2 100644 --- a/src/app/api/notification/notification.service.test.ts +++ b/src/app/api/notification/notification.service.test.ts @@ -8,6 +8,7 @@ const mockFindFirst = jest.fn() const mockFindMany = jest.fn() const mockClientNotifCreate = jest.fn() const mockClientNotifCreateMany = jest.fn() +const mockInternalUserNotifCreate = jest.fn() const mockQueryRaw = jest.fn() const mockGroupedCreateMany = jest.fn() @@ -30,6 +31,7 @@ jest.mock('@/lib/db', () => ({ createMany: (...args: unknown[]) => mockClientNotifCreateMany(...args), }, groupedEmailEvent: { createMany: (...args: unknown[]) => mockGroupedCreateMany(...args) }, + internalUserNotification: { create: (...args: unknown[]) => mockInternalUserNotifCreate(...args) }, $queryRaw: (...args: unknown[]) => mockQueryRaw(...args), }), }, @@ -90,6 +92,7 @@ beforeEach(() => { mockGroupedCreateMany.mockResolvedValue({ count: 1 }) mockClientNotifCreate.mockResolvedValue({}) mockClientNotifCreateMany.mockResolvedValue({ count: 1 }) + mockInternalUserNotifCreate.mockResolvedValue({}) mockCreateNotification.mockResolvedValue({ id: 'notif_1', createdAt: '2026-06-15T10:00:00.000Z', @@ -321,3 +324,47 @@ describe('guard: CU wiring boundaries', () => { expect(row.recipientIuId).toBeNull() }) }) + +describe('guard: IU wiring boundaries', () => { + it('buffers an Assigned IU email with recipientIuId set and all client fields null', async () => { + const task = makeTask({ assigneeType: AssigneeType.internalUser, clientId: null }) + await buildService().create(NotificationTaskActions.Assigned, task, { disableEmail: false }) + + expect(mockGroupedCreateMany).toHaveBeenCalledTimes(1) + const row = mockGroupedCreateMany.mock.calls[0][0].data[0] + expect(row).toMatchObject({ + workspaceId: 'ws_1', + recipientIuId: task.assigneeId, + recipientClientId: null, + recipientCompanyId: null, + eventType: GroupedEmailEventType.ASSIGNED, + taskId: task.id, + }) + expect(row.windowKey).toMatch(/^33333333-3333-3333-3333-333333333333:iu:/) + expect(mockEnqueueFlush).toHaveBeenCalledWith({ workspaceId: 'ws_1', windowKey: row.windowKey }) + + // individual email snapshot routes to the IU, not a client + expect(row.individualEmail.recipientInternalUserId).toBe(task.assigneeId) + expect(row.individualEmail.recipientClientId).toBeUndefined() + }) + + it('sends the in-product notification to recipientInternalUserId and strips the email target', async () => { + const task = makeTask({ assigneeType: AssigneeType.internalUser, clientId: null }) + await buildService().create(NotificationTaskActions.Assigned, task, { disableEmail: false }) + + expect(mockCreateNotification).toHaveBeenCalledTimes(1) + const sent = mockCreateNotification.mock.calls[0][0] + expect(sent.recipientInternalUserId).toBe(task.assigneeId) + expect(sent.recipientClientId).toBeUndefined() + expect(deliveryTargetsOf(0).inProduct).toBeDefined() + expect(deliveryTargetsOf(0).email).toBeUndefined() + }) + + it('does not buffer when disableEmail is true for an IU task', async () => { + const task = makeTask({ assigneeType: AssigneeType.internalUser, clientId: null }) + await buildService().create(NotificationTaskActions.Assigned, task, { disableEmail: true }) + + expect(mockGroupedCreateMany).not.toHaveBeenCalled() + expect(mockEnqueueFlush).not.toHaveBeenCalled() + }) +}) From 02b33c82be6b551908a59f499d6433506a37ce8b Mon Sep 17 00:00:00 2001 From: arpandhakal-lgtm Date: Tue, 30 Jun 2026 14:48:33 +0545 Subject: [PATCH 05/20] fix(notifications): buffer ReassignedToIU emails via grouped queue Co-Authored-By: Claude Sonnet 4.6 --- src/app/api/notification/notification.service.test.ts | 4 +++- src/app/api/notification/notification.service.ts | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/app/api/notification/notification.service.test.ts b/src/app/api/notification/notification.service.test.ts index c8a6d56a2..6e695149e 100644 --- a/src/app/api/notification/notification.service.test.ts +++ b/src/app/api/notification/notification.service.test.ts @@ -280,12 +280,13 @@ describe('NotificationService grouped-email interception', () => { }) describe('guard: CU wiring boundaries', () => { - it('maps every CU-targeted action to the correct GroupedEmailEventType', () => { + it('maps every buffered action to the correct GroupedEmailEventType', () => { const svc = buildService() as unknown as { groupedEventTypeFor: (a: NotificationTaskActions) => GroupedEmailEventType | null } expect(svc.groupedEventTypeFor(NotificationTaskActions.Assigned)).toBe(GroupedEmailEventType.ASSIGNED) expect(svc.groupedEventTypeFor(NotificationTaskActions.AssignedToCompany)).toBe(GroupedEmailEventType.ASSIGNED) + expect(svc.groupedEventTypeFor(NotificationTaskActions.ReassignedToIU)).toBe(GroupedEmailEventType.ASSIGNED) expect(svc.groupedEventTypeFor(NotificationTaskActions.Shared)).toBe(GroupedEmailEventType.SHARED) expect(svc.groupedEventTypeFor(NotificationTaskActions.SharedToCompany)).toBe(GroupedEmailEventType.SHARED) expect(svc.groupedEventTypeFor(NotificationTaskActions.Commented)).toBe(GroupedEmailEventType.COMMENT) @@ -298,6 +299,7 @@ describe('guard: CU wiring boundaries', () => { const mapped = [ NotificationTaskActions.Assigned, NotificationTaskActions.AssignedToCompany, + NotificationTaskActions.ReassignedToIU, NotificationTaskActions.Shared, NotificationTaskActions.SharedToCompany, NotificationTaskActions.Commented, diff --git a/src/app/api/notification/notification.service.ts b/src/app/api/notification/notification.service.ts index c27127f87..b06d6088b 100644 --- a/src/app/api/notification/notification.service.ts +++ b/src/app/api/notification/notification.service.ts @@ -597,6 +597,7 @@ export class NotificationService extends BaseService { switch (action) { case NotificationTaskActions.Assigned: case NotificationTaskActions.AssignedToCompany: + case NotificationTaskActions.ReassignedToIU: return GroupedEmailEventType.ASSIGNED case NotificationTaskActions.Shared: case NotificationTaskActions.SharedToCompany: From 636b366d777d42176177d41c39b34e316ec374c8 Mon Sep 17 00:00:00 2001 From: arpandhakal-lgtm Date: Thu, 2 Jul 2026 13:27:14 +0545 Subject: [PATCH 06/20] refactor(notifications): use conditional spread for grouped email recipients Address PR review: pass recipient ids via conditional spread instead of explicit null, and drop the null type from bufferGroupedEmailEvent params. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/app/api/notification/notification.service.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/app/api/notification/notification.service.ts b/src/app/api/notification/notification.service.ts index b06d6088b..b73ae7f58 100644 --- a/src/app/api/notification/notification.service.ts +++ b/src/app/api/notification/notification.service.ts @@ -72,9 +72,9 @@ export class NotificationService extends BaseService { const association = AssociationsSchema.parse(task.associations)?.[0] await this.bufferGroupedEmailEvent({ task, - recipientClientId: isIuRecipient ? null : recipientId, - recipientCompanyId: isIuRecipient ? null : (task.companyId ?? association?.companyId ?? null), - recipientIuId: isIuRecipient ? recipientId : null, + ...(!isIuRecipient && { recipientClientId: recipientId }), + ...(!isIuRecipient && { recipientCompanyId: task.companyId ?? association?.companyId ?? undefined }), + ...(isIuRecipient && { recipientIuId: recipientId }), eventType: groupedType, commentId: opts.commentId, individualEmail: this.buildNotificationDetails( @@ -207,7 +207,7 @@ export class NotificationService extends BaseService { await this.bufferGroupedEmailEvent({ task, recipientClientId: recipientId, - recipientCompanyId: task.companyId ?? association?.companyId ?? null, + recipientCompanyId: task.companyId ?? association?.companyId ?? undefined, eventType: groupedType, commentId: opts?.commentId, individualEmail: this.buildNotificationDetails(task, senderId, recipientId, { email }, opts?.senderCompanyId), @@ -611,9 +611,9 @@ export class NotificationService extends BaseService { private async bufferGroupedEmailEvent(args: { task: Task - recipientClientId?: string | null - recipientCompanyId?: string | null - recipientIuId?: string | null + recipientClientId?: string + recipientCompanyId?: string + recipientIuId?: string eventType: GroupedEmailEventType commentId?: string individualEmail: NotificationRequestBody From 5f29eaf279b4b2d8e678c787a49d3be4081fec17 Mon Sep 17 00:00:00 2001 From: arpandhakal-lgtm Date: Thu, 2 Jul 2026 18:54:19 +0545 Subject: [PATCH 07/20] fix(notifications): send grouped emails from the real actor, not an arbitrary IU The flush job attributed grouped emails to resolveSenderId (an arbitrary workspace internal user). Copilot creates the notification but does not deliver the email to an IU recipient from an unrelated sender, so IU grouped emails (event count > 1) silently failed while single-event and client grouped emails worked. Use the sender captured on the buffered individualEmail and fall back to resolveSenderId only for pre-migration rows. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../notifications/flush-grouped-email.test.ts | 32 +++++++++++++++++-- src/jobs/notifications/flush-grouped-email.ts | 13 +++++--- 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/src/jobs/notifications/flush-grouped-email.test.ts b/src/jobs/notifications/flush-grouped-email.test.ts index f59f13f2d..3f7565a7a 100644 --- a/src/jobs/notifications/flush-grouped-email.test.ts +++ b/src/jobs/notifications/flush-grouped-email.test.ts @@ -100,13 +100,38 @@ describe('flushGroupedEmailRun', () => { expect(mockSendGroupedEmail).toHaveBeenCalledTimes(1) const args = mockSendGroupedEmail.mock.calls[0][0] - expect(args).toMatchObject({ senderId: 'iu_1', recipientClientId: 'client_1', recipientCompanyId: 'company_1' }) + // Sender is the real actor from the buffered event, not an arbitrary workspace IU. + expect(args).toMatchObject({ senderId: 'actor_1', recipientClientId: 'client_1', recipientCompanyId: 'company_1' }) + expect(mockGetInternalUsers).not.toHaveBeenCalled() expect(args.content.totalEventCount).toBe(2) expect(mockCreateNotification).not.toHaveBeenCalled() expect(mockExecuteRaw).toHaveBeenCalledTimes(2) // markRecipientSent + deleteWindowRows expect(result).toMatchObject({ recipients: 1, sent: 1, sentGrouped: 1, sentIndividual: 0 }) }) + it('sends an IU grouped summary from the real actor, not an arbitrary workspace IU', async () => { + const iuRow = () => + row({ + recipientClientId: null, + recipientCompanyId: null, + recipientIuId: 'iu_recipient', + individualEmail: { + senderId: 'actor_1', + recipientInternalUserId: 'iu_recipient', + deliveryTargets: { email: { subject: 'A task' } }, + }, + }) + mockQueryRaw.mockResolvedValue([iuRow(), iuRow()]) + + const result = await flushGroupedEmailRun(payload) + + expect(mockSendGroupedEmail).toHaveBeenCalledTimes(1) + const args = mockSendGroupedEmail.mock.calls[0][0] + expect(args).toMatchObject({ senderId: 'actor_1', recipientInternalUserId: 'iu_recipient' }) + expect(mockGetInternalUsers).not.toHaveBeenCalled() + expect(result).toMatchObject({ recipients: 1, sent: 1, sentGrouped: 1 }) + }) + it('replays the original individual email when the window has a single live event', async () => { mockQueryRaw.mockResolvedValue([row()]) @@ -202,8 +227,9 @@ describe('flushGroupedEmailRun', () => { expect(mockExecuteRaw).not.toHaveBeenCalled() }) - it('throws when a grouped window has no internal user to send as', async () => { - mockQueryRaw.mockResolvedValue([row(), row()]) + it('throws when a grouped window has no buffered sender and no workspace internal user', async () => { + // Pre-migration rows carry no individualEmail, so the sender falls back to a workspace IU. + mockQueryRaw.mockResolvedValue([row({ individualEmail: null }), row({ individualEmail: null })]) mockGetInternalUsers.mockResolvedValue({ data: [] }) await expect(flushGroupedEmailRun(payload)).rejects.toThrow('no internal user') diff --git a/src/jobs/notifications/flush-grouped-email.ts b/src/jobs/notifications/flush-grouped-email.ts index f5f2c7560..6981ae7e3 100644 --- a/src/jobs/notifications/flush-grouped-email.ts +++ b/src/jobs/notifications/flush-grouped-email.ts @@ -76,6 +76,11 @@ const resolveSenderId = async (copilot: CopilotAPI): Promise => { return senderId } +// Copilot only emails an IU recipient when the sender is a real participant, so attribute the +// grouped summary to the actual actor from the buffered events, not an arbitrary workspace IU. +const senderFromEvents = (events: WindowEvent[]): string | undefined => + events.map((e) => e.individualEmail?.senderId).find((id): id is string => Boolean(id)) + const sendIndividualEmail = async (copilot: CopilotAPI, payload: NotificationRequestBody): Promise => { try { await copilot.createNotification(payload) @@ -180,10 +185,10 @@ export const flushGroupedEmailRun = async (payload: FlushGroupedEmailPayload) => sent += 1 sentIndividual += 1 } else if (liveEvents.length >= 1) { - senderId ??= await resolveSenderId(copilot) + const groupSenderId = senderFromEvents(liveEvents) ?? (senderId ??= await resolveSenderId(copilot)) await sendGroupedEmail({ content: composeGroupedEmail(liveEvents), - senderId, + senderId: groupSenderId, recipientClientId: group.recipientClientId, recipientCompanyId: group.recipientCompanyId, copilot, @@ -217,10 +222,10 @@ export const flushGroupedEmailRun = async (payload: FlushGroupedEmailPayload) => sent += 1 sentIndividual += 1 } else if (liveEvents.length >= 1) { - senderId ??= await resolveSenderId(copilot) + const groupSenderId = senderFromEvents(liveEvents) ?? (senderId ??= await resolveSenderId(copilot)) await sendGroupedEmail({ content: composeGroupedEmail(liveEvents), - senderId, + senderId: groupSenderId, recipientInternalUserId: group.recipientIuId, copilot, }) From 196c38d6b209fda640f2928183669111d60c2179 Mon Sep 17 00:00:00 2001 From: arpandhakal-lgtm Date: Thu, 2 Jul 2026 19:11:15 +0545 Subject: [PATCH 08/20] fix(notifications): add missing ReassignedToIU email template MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit getEmailDetails had no ReassignedToIU entry, so reassignment to an IU produced no email and was never buffered into the grouped queue — only the in-product notification fired. Add the template (same copy as ReassignedToClient) and a regression test asserting IU-recipient actions have email templates. Co-Authored-By: Claude Fable 5 --- .../notification/notification.helpers.test.ts | 17 ++++++++++++++++- .../api/notification/notification.helpers.ts | 7 +++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/app/api/notification/notification.helpers.test.ts b/src/app/api/notification/notification.helpers.test.ts index 5874c181e..7e5802700 100644 --- a/src/app/api/notification/notification.helpers.test.ts +++ b/src/app/api/notification/notification.helpers.test.ts @@ -1,5 +1,6 @@ +import { NotificationTaskActions } from '@api/core/types/tasks' import { WorkspaceResponse } from '@/types/common' -import { getReminderEmailDetails } from './notification.helpers' +import { getEmailDetails, getReminderEmailDetails } from './notification.helpers' import { TaskReminderType } from '@prisma/client' const workspace: WorkspaceResponse = { @@ -53,3 +54,17 @@ describe('getReminderEmailDetails', () => { } }) }) + +describe('getEmailDetails', () => { + // Actions that email an IU recipient must have a template here, or the grouped + // buffer silently skips them (in-product fires but no email is ever flushed). + it.each([NotificationTaskActions.Assigned, NotificationTaskActions.ReassignedToIU])( + 'defines an email template for IU-recipient action %s', + (action) => { + const details = getEmailDetails(workspace, 'Arpan Two')[action] + expect(details).toBeDefined() + expect(details?.subject).toBeTruthy() + expect(details?.body).toBeTruthy() + }, + ) +}) diff --git a/src/app/api/notification/notification.helpers.ts b/src/app/api/notification/notification.helpers.ts index 17ea30dd3..80013c9e2 100644 --- a/src/app/api/notification/notification.helpers.ts +++ b/src/app/api/notification/notification.helpers.ts @@ -202,6 +202,13 @@ export const getEmailDetails = ( title: 'View task', ctaParams, }, + [NotificationTaskActions.ReassignedToIU]: { + subject: 'A task was reassigned to you', + header: 'A task was reassigned to you', + title: 'View task', + body: `The task ‘${task?.title}’ was reassigned to you by ${actionUser}. To see details about the task open it below.`, + ctaParams, + }, [NotificationTaskActions.ReassignedToClient]: { subject: 'A task was reassigned to you', header: 'A task was reassigned to you', From ab585da33dde2f8f4cb07eacbe7940748c19a9b4 Mon Sep 17 00:00:00 2001 From: arpandhakal-lgtm Date: Fri, 3 Jul 2026 15:56:10 +0545 Subject: [PATCH 09/20] refactor(notifications): simplify grouped email buffer API per review - rename isIuRecipient to isAssignedToIu / isRecipientIu for clarity - bufferGroupedEmailEvent now takes recipientId + companyId + isRecipientIu and routes to the right column internally - collapse the duplicated window queries into one with a recipient filter fragment Co-Authored-By: Claude Fable 5 --- .../notification/notification.service.test.ts | 6 +- .../api/notification/notification.service.ts | 74 +++++++++---------- 2 files changed, 37 insertions(+), 43 deletions(-) diff --git a/src/app/api/notification/notification.service.test.ts b/src/app/api/notification/notification.service.test.ts index 6e695149e..340855f24 100644 --- a/src/app/api/notification/notification.service.test.ts +++ b/src/app/api/notification/notification.service.test.ts @@ -101,6 +101,8 @@ beforeEach(() => { }) const deliveryTargetsOf = (call: number) => mockCreateNotification.mock.calls[call][0].deliveryTargets +const queryRawParamsOf = (call: number) => + mockQueryRaw.mock.calls[call].flatMap((arg: { values?: unknown[] }) => arg?.values ?? arg) describe('NotificationService grouped-email interception', () => { describe('create()', () => { @@ -121,7 +123,7 @@ describe('NotificationService grouped-email interception', () => { }) // window is scoped to the (clientId, companyId) pair, not the client alone expect(row.windowKey).toMatch(new RegExp(`^${task.assigneeId}:${task.companyId}:`)) - expect(mockQueryRaw.mock.calls[0]).toContain(task.companyId) + expect(queryRawParamsOf(0)).toContain(task.companyId) expect(mockEnqueueFlush).toHaveBeenCalledWith({ workspaceId: 'ws_1', windowKey: row.windowKey }) // the row snapshots the exact individual email to replay for a single-event window @@ -156,7 +158,7 @@ describe('NotificationService grouped-email interception', () => { const row = mockGroupedCreateMany.mock.calls[0][0].data[0] expect(row.recipientCompanyId).toBe(companyB) expect(row.windowKey).toMatch(new RegExp(`^33333333-3333-3333-3333-333333333333:${companyB}:`)) - expect(mockQueryRaw.mock.calls[0]).toContain(companyB) + expect(queryRawParamsOf(0)).toContain(companyB) }) it('does not buffer or strip email for a non-target action (byte-for-byte)', async () => { diff --git a/src/app/api/notification/notification.service.ts b/src/app/api/notification/notification.service.ts index b73ae7f58..1b1bdf5f6 100644 --- a/src/app/api/notification/notification.service.ts +++ b/src/app/api/notification/notification.service.ts @@ -55,7 +55,7 @@ export class NotificationService extends BaseService { action, ) - const isIuRecipient = + const isAssignedToIu = task.assigneeType === AssigneeType.internalUser && (action === NotificationTaskActions.Assigned || action === NotificationTaskActions.ReassignedToIU) @@ -72,9 +72,9 @@ export class NotificationService extends BaseService { const association = AssociationsSchema.parse(task.associations)?.[0] await this.bufferGroupedEmailEvent({ task, - ...(!isIuRecipient && { recipientClientId: recipientId }), - ...(!isIuRecipient && { recipientCompanyId: task.companyId ?? association?.companyId ?? undefined }), - ...(isIuRecipient && { recipientIuId: recipientId }), + recipientId, + companyId: task.companyId ?? association?.companyId ?? undefined, + isRecipientIu: isAssignedToIu, eventType: groupedType, commentId: opts.commentId, individualEmail: this.buildNotificationDetails( @@ -83,7 +83,7 @@ export class NotificationService extends BaseService { recipientId, { email }, senderCompanyId, - isIuRecipient, + isAssignedToIu, ), }) } @@ -96,7 +96,7 @@ export class NotificationService extends BaseService { recipientId, { inProduct, email }, senderCompanyId, - isIuRecipient, + isAssignedToIu, ) if (groupedType) notificationDetails.deliveryTargets = { inProduct } if (!inProduct && !notificationDetails.deliveryTargets?.email) return @@ -118,7 +118,7 @@ export class NotificationService extends BaseService { // NOTE: There are cases where task.assigneeType does not account for IU notification! // E.g. When receiving notifications from others completing task that IU created. // For now we don't have to store these so this hasn't been accounted for - if (isIuRecipient) { + if (isAssignedToIu) { // Notification recipient is IU in this case await this.db.internalUserNotification.create({ data: { @@ -206,8 +206,8 @@ export class NotificationService extends BaseService { if (groupedType) { await this.bufferGroupedEmailEvent({ task, - recipientClientId: recipientId, - recipientCompanyId: task.companyId ?? association?.companyId ?? undefined, + recipientId, + companyId: task.companyId ?? association?.companyId ?? undefined, eventType: groupedType, commentId: opts?.commentId, individualEmail: this.buildNotificationDetails(task, senderId, recipientId, { email }, opts?.senderCompanyId), @@ -611,49 +611,41 @@ export class NotificationService extends BaseService { private async bufferGroupedEmailEvent(args: { task: Task - recipientClientId?: string - recipientCompanyId?: string - recipientIuId?: string + recipientId: string + companyId?: string + isRecipientIu?: boolean eventType: GroupedEmailEventType commentId?: string individualEmail: NotificationRequestBody }): Promise { - const { task, recipientClientId, recipientCompanyId, recipientIuId, eventType, commentId, individualEmail } = args - const isIuRecipient = !!recipientIuId - - const activeWindow = isIuRecipient - ? await this.db.$queryRaw<{ windowKey: string }[]>` - SELECT "windowKey" FROM "GroupedEmailEvents" - WHERE "workspaceId" = ${task.workspaceId} - AND "recipientIuId" = ${recipientIuId}::uuid - AND "sentAt" IS NULL - AND "createdAt" > now() - interval '5 minutes' - ORDER BY "createdAt" DESC - LIMIT 1` - : await this.db.$queryRaw<{ windowKey: string }[]>` - SELECT "windowKey" FROM "GroupedEmailEvents" - WHERE "workspaceId" = ${task.workspaceId} - AND "recipientClientId" = ${recipientClientId}::uuid - AND "recipientCompanyId" IS NOT DISTINCT FROM ${recipientCompanyId}::uuid - AND "sentAt" IS NULL - AND "createdAt" > now() - interval '5 minutes' - ORDER BY "createdAt" DESC - LIMIT 1` + const { task, recipientId, companyId, isRecipientIu, eventType, commentId, individualEmail } = args + + const recipientFilter = isRecipientIu + ? Prisma.sql`"recipientIuId" = ${recipientId}::uuid` + : Prisma.sql`"recipientClientId" = ${recipientId}::uuid AND "recipientCompanyId" IS NOT DISTINCT FROM ${companyId ?? null}::uuid` + const activeWindow = await this.db.$queryRaw<{ windowKey: string }[]>` + SELECT "windowKey" FROM "GroupedEmailEvents" + WHERE "workspaceId" = ${task.workspaceId} + AND ${recipientFilter} + AND "sentAt" IS NULL + AND "createdAt" > now() - interval '5 minutes' + ORDER BY "createdAt" DESC + LIMIT 1` const isNewWindow = activeWindow.length === 0 const windowKey = isNewWindow - ? isIuRecipient - ? `${recipientIuId}:iu:${randomUUID()}` - : `${recipientClientId}:${recipientCompanyId ?? 'none'}:${randomUUID()}` + ? isRecipientIu + ? `${recipientId}:iu:${randomUUID()}` + : `${recipientId}:${companyId ?? 'none'}:${randomUUID()}` : activeWindow[0].windowKey await this.db.groupedEmailEvent.createMany({ data: [ { workspaceId: task.workspaceId, - recipientClientId: recipientClientId ?? null, - recipientCompanyId: recipientCompanyId ?? null, - recipientIuId: recipientIuId ?? null, + recipientClientId: isRecipientIu ? null : recipientId, + recipientCompanyId: isRecipientIu ? null : (companyId ?? null), + recipientIuId: isRecipientIu ? recipientId : null, eventType, taskId: task.id, taskTitleSnapshot: task.title, @@ -694,7 +686,7 @@ export class NotificationService extends BaseService { recipientId: string, deliveryTargets: NotificationRequestBody['deliveryTargets'], senderCompanyId?: string, - isIuRecipient?: boolean, + isRecipientIu?: boolean, ): NotificationRequestBody { const associations = AssociationsSchema.parse(task.associations) const association = associations?.[0] @@ -707,7 +699,7 @@ export class NotificationService extends BaseService { deliveryTargets: deliveryTargets || {}, } // Fall back to inferring IU from absence of email for paths not yet updated (e.g. CommentToIU). - const isIU = isIuRecipient ?? !notificationDetails.deliveryTargets?.email + const isIU = isRecipientIu ?? !notificationDetails.deliveryTargets?.email if (isIU) { delete notificationDetails.recipientCompanyId delete notificationDetails.recipientClientId From f1d6966b77a89b88b4fd108d77a45258e5d7df9b Mon Sep 17 00:00:00 2001 From: arpandhakal-lgtm Date: Fri, 3 Jul 2026 17:53:39 +0545 Subject: [PATCH 10/20] feat(notifications): send task-marked-as-done emails to IUs Adds the IU-only 'task marked as done' email (OUT-3928), routed through the existing 5-minute grouped buffer per the PRD grouping rule. - add completion email templates (Completed, CompletedByIU, CompletedByCompanyMember, CompletedForCompanyByIU) and map them to GroupedEmailEventType.COMPLETED - gate completion emails on isIuEmailEnabled() - carry the real actor's senderType/company through the grouped flush so a client-actor completion isn't sent as senderId(client)/senderType(IU), which Copilot rejects and which wipes the whole window on retry failure - skip the client-notification dedup guard for IU recipients so CompletedByIU on a client-assigned task is no longer dropped by the client's leftover row Co-Authored-By: Claude Fable 5 --- .../notification/notification.helpers.test.ts | 37 ++++-- .../api/notification/notification.helpers.ts | 19 ++-- .../notification/notification.service.test.ts | 107 ++++++++++++++++-- .../api/notification/notification.service.ts | 58 +++++++--- .../api/tasks/task-notifications.service.ts | 13 ++- src/jobs/notifications/flush-grouped-email.ts | 22 +++- .../notifications/send-grouped-email.test.ts | 40 +++++++ src/jobs/notifications/send-grouped-email.ts | 25 +++- 8 files changed, 264 insertions(+), 57 deletions(-) diff --git a/src/app/api/notification/notification.helpers.test.ts b/src/app/api/notification/notification.helpers.test.ts index 7e5802700..e5bb5bd53 100644 --- a/src/app/api/notification/notification.helpers.test.ts +++ b/src/app/api/notification/notification.helpers.test.ts @@ -1,7 +1,7 @@ import { NotificationTaskActions } from '@api/core/types/tasks' import { WorkspaceResponse } from '@/types/common' import { getEmailDetails, getReminderEmailDetails } from './notification.helpers' -import { TaskReminderType } from '@prisma/client' +import { Task, TaskReminderType } from '@prisma/client' const workspace: WorkspaceResponse = { id: 'ws_1', @@ -58,13 +58,30 @@ describe('getReminderEmailDetails', () => { describe('getEmailDetails', () => { // Actions that email an IU recipient must have a template here, or the grouped // buffer silently skips them (in-product fires but no email is ever flushed). - it.each([NotificationTaskActions.Assigned, NotificationTaskActions.ReassignedToIU])( - 'defines an email template for IU-recipient action %s', - (action) => { - const details = getEmailDetails(workspace, 'Arpan Two')[action] - expect(details).toBeDefined() - expect(details?.subject).toBeTruthy() - expect(details?.body).toBeTruthy() - }, - ) + it.each([ + NotificationTaskActions.Assigned, + NotificationTaskActions.ReassignedToIU, + NotificationTaskActions.Completed, + NotificationTaskActions.CompletedByIU, + NotificationTaskActions.CompletedByCompanyMember, + NotificationTaskActions.CompletedForCompanyByIU, + ])('defines an email template for IU-recipient action %s', (action) => { + const details = getEmailDetails(workspace, 'Arpan Two')[action] + expect(details).toBeDefined() + expect(details?.subject).toBeTruthy() + expect(details?.body).toBeTruthy() + }) + + it.each([ + NotificationTaskActions.Completed, + NotificationTaskActions.CompletedByIU, + NotificationTaskActions.CompletedByCompanyMember, + NotificationTaskActions.CompletedForCompanyByIU, + ])('uses the task-marked-as-done copy for completion action %s', (action) => { + const details = getEmailDetails(workspace, 'Casey Client', task as unknown as Task)[action] + expect(details?.subject).toBe('Task marked as done') + expect(details?.header).toBe('A task has been completed') + expect(details?.body).toContain('has been marked as done by Casey Client') + expect(details?.title).toBe('View task') + }) }) diff --git a/src/app/api/notification/notification.helpers.ts b/src/app/api/notification/notification.helpers.ts index 80013c9e2..73bc5712d 100644 --- a/src/app/api/notification/notification.helpers.ts +++ b/src/app/api/notification/notification.helpers.ts @@ -166,6 +166,14 @@ export const getEmailDetails = ( } : undefined + const completedDetail = { + subject: 'Task marked as done', + header: 'A task has been completed', + title: 'View task', + body: `The task ‘${task?.title}’ has been marked as done by ${actionUser}.\n\nTo see details about the task, open it below.`, + ctaParams, + } + return { [NotificationTaskActions.Assigned]: { subject: 'A task was assigned to you', @@ -181,13 +189,10 @@ export const getEmailDetails = ( title: 'View task', ctaParams, }, - //! Currently disable all IU email notifications - // [NotificationTaskActions.Completed]: { - // title: 'A client completed a task', - // subject: 'A client completed a task', - // header: 'A client completed a task', - // body: `A new task was completed by ${actionUser}. You are receiving this notification because you have access to the client.`, - // }, + [NotificationTaskActions.Completed]: completedDetail, + [NotificationTaskActions.CompletedByIU]: completedDetail, + [NotificationTaskActions.CompletedByCompanyMember]: completedDetail, + [NotificationTaskActions.CompletedForCompanyByIU]: completedDetail, [NotificationTaskActions.Commented]: { subject: 'Comment was added', header: 'Comment was added', diff --git a/src/app/api/notification/notification.service.test.ts b/src/app/api/notification/notification.service.test.ts index 340855f24..fdf457cce 100644 --- a/src/app/api/notification/notification.service.test.ts +++ b/src/app/api/notification/notification.service.test.ts @@ -292,9 +292,13 @@ describe('guard: CU wiring boundaries', () => { expect(svc.groupedEventTypeFor(NotificationTaskActions.Shared)).toBe(GroupedEmailEventType.SHARED) expect(svc.groupedEventTypeFor(NotificationTaskActions.SharedToCompany)).toBe(GroupedEmailEventType.SHARED) expect(svc.groupedEventTypeFor(NotificationTaskActions.Commented)).toBe(GroupedEmailEventType.COMMENT) + expect(svc.groupedEventTypeFor(NotificationTaskActions.Completed)).toBe(GroupedEmailEventType.COMPLETED) + expect(svc.groupedEventTypeFor(NotificationTaskActions.CompletedByIU)).toBe(GroupedEmailEventType.COMPLETED) + expect(svc.groupedEventTypeFor(NotificationTaskActions.CompletedByCompanyMember)).toBe(GroupedEmailEventType.COMPLETED) + expect(svc.groupedEventTypeFor(NotificationTaskActions.CompletedForCompanyByIU)).toBe(GroupedEmailEventType.COMPLETED) }) - it('returns null for every action that must not be buffered', () => { + it('returns null for every action that must not be buffered (incl. shared-CU completion emails)', () => { const svc = buildService() as unknown as { groupedEventTypeFor: (a: NotificationTaskActions) => GroupedEmailEventType | null } @@ -305,6 +309,10 @@ describe('guard: CU wiring boundaries', () => { NotificationTaskActions.Shared, NotificationTaskActions.SharedToCompany, NotificationTaskActions.Commented, + NotificationTaskActions.Completed, + NotificationTaskActions.CompletedByIU, + NotificationTaskActions.CompletedByCompanyMember, + NotificationTaskActions.CompletedForCompanyByIU, ] const unmapped = Object.values(NotificationTaskActions).filter((a) => !mapped.includes(a)) for (const action of unmapped) { @@ -312,16 +320,6 @@ describe('guard: CU wiring boundaries', () => { } }) - it('never returns COMPLETED — that type is reserved for the deferred IU milestone', () => { - const svc = buildService() as unknown as { - groupedEventTypeFor: (a: NotificationTaskActions) => GroupedEmailEventType | null - } - const allActions = Object.values(NotificationTaskActions) - for (const action of allActions) { - expect(svc.groupedEventTypeFor(action)).not.toBe(GroupedEmailEventType.COMPLETED) - } - }) - it('never writes recipientIuId in a CU grouped event row', async () => { await buildService().create(NotificationTaskActions.Assigned, makeTask()) const row = mockGroupedCreateMany.mock.calls[0][0].data[0] @@ -372,3 +370,90 @@ describe('guard: IU wiring boundaries', () => { expect(mockEnqueueFlush).not.toHaveBeenCalled() }) }) + +describe('guard: IU completion emails', () => { + it.each([NotificationTaskActions.CompletedByIU, NotificationTaskActions.CompletedForCompanyByIU])( + 'buffers a %s email as a COMPLETED IU event and keeps the in-product notification immediate', + async (action) => { + await buildService().create(action, makeTask({ assigneeType: AssigneeType.internalUser, clientId: null })) + + expect(mockGroupedCreateMany).toHaveBeenCalledTimes(1) + const row = mockGroupedCreateMany.mock.calls[0][0].data[0] + expect(row).toMatchObject({ + recipientIuId: '33333333-3333-3333-3333-333333333333', + recipientClientId: null, + recipientCompanyId: null, + eventType: GroupedEmailEventType.COMPLETED, + }) + expect(row.individualEmail.recipientInternalUserId).toBe('33333333-3333-3333-3333-333333333333') + + expect(mockCreateNotification).toHaveBeenCalledTimes(1) + const sent = mockCreateNotification.mock.calls[0][0] + expect(sent.recipientInternalUserId).toBe('33333333-3333-3333-3333-333333333333') + expect(sent.recipientClientId).toBeUndefined() + expect(deliveryTargetsOf(0).inProduct).toBeDefined() + expect(deliveryTargetsOf(0).email).toBeUndefined() + }, + ) + + it('does not buffer and still routes the in-product CompletedByIU notification to the IU when email is disabled', async () => { + const task = makeTask({ assigneeType: AssigneeType.internalUser, clientId: null }) + await buildService().create(NotificationTaskActions.CompletedByIU, task, { disableEmail: true }) + + expect(mockGroupedCreateMany).not.toHaveBeenCalled() + const sent = mockCreateNotification.mock.calls[0][0] + expect(sent.recipientInternalUserId).toBe('33333333-3333-3333-3333-333333333333') + expect(sent.recipientClientId).toBeUndefined() + expect(deliveryTargetsOf(0).email).toBeUndefined() + }) + + it('is not blocked by the client-notification dedup guard on a client-assigned task', async () => { + // A client-assigned task still has an unread ClientNotification from its assignment when the + // IU completes it; the CompletedByIU recipient is the creator IU, so the guard must not fire. + mockFindFirst.mockResolvedValue({ id: 'existing-client-notif' }) + const task = makeTask({ assigneeType: AssigneeType.client, clientId: '33333333-3333-3333-3333-333333333333' }) + + await buildService().create(NotificationTaskActions.CompletedByIU, task, { disableEmail: false }) + + // guard skipped: the completion email buffers and the IU notification dispatches + expect(mockFindFirst).not.toHaveBeenCalled() + expect(mockGroupedCreateMany).toHaveBeenCalledTimes(1) + expect(mockGroupedCreateMany.mock.calls[0][0].data[0].recipientIuId).toBe('33333333-3333-3333-3333-333333333333') + expect(mockCreateNotification).toHaveBeenCalledTimes(1) + expect(mockCreateNotification.mock.calls[0][0].recipientInternalUserId).toBe('33333333-3333-3333-3333-333333333333') + }) + + 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, + }) + + 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.COMPLETED) + expect(row.recipientClientId).toBeNull() + expect(row.individualEmail.recipientInternalUserId).toBeDefined() + } + + expect(mockCreateNotification).toHaveBeenCalledTimes(2) + 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('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, + }) + + expect(mockGroupedCreateMany).not.toHaveBeenCalled() + expect(mockCreateNotification).toHaveBeenCalledTimes(1) + expect(deliveryTargetsOf(0).email).toBeUndefined() + expect(mockCreateNotification.mock.calls[0][0].recipientInternalUserId).toBe('iu_a') + }) +}) diff --git a/src/app/api/notification/notification.service.ts b/src/app/api/notification/notification.service.ts index 1b1bdf5f6..76306d180 100644 --- a/src/app/api/notification/notification.service.ts +++ b/src/app/api/notification/notification.service.ts @@ -36,13 +36,25 @@ export class NotificationService extends BaseService { } = { disableEmail: false }, ) { try { - // 1.Check for existing notification. Skip if duplicate - const existingNotification = task.clientId - ? await this.db.clientNotification.findFirst({ - where: { taskId: task.id, clientId: task.clientId, companyId: task.companyId }, - }) - : null - if (task.clientId && existingNotification && !opts.commentId) { + const isAssignedToIu = + task.assigneeType === AssigneeType.internalUser && + (action === NotificationTaskActions.Assigned || action === NotificationTaskActions.ReassignedToIU) + // Completion notifications go to the task creator, an IU + const isRecipientIu = + isAssignedToIu || + action === NotificationTaskActions.CompletedByIU || + action === NotificationTaskActions.CompletedForCompanyByIU + + // 1. Check for existing notification. Skip if duplicate. This dedup is keyed on the client + // assignee, so it must not gate IU-recipient notifications (e.g. CompletedByIU on a + // client-assigned task, whose recipient is the creator IU, not the client). + const existingNotification = + task.clientId && !isRecipientIu + ? await this.db.clientNotification.findFirst({ + where: { taskId: task.id, clientId: task.clientId, companyId: task.companyId }, + }) + : null + if (existingNotification && !opts.commentId) { console.error(`NotificationService#create | Found existing notification for ${task.clientId}`, existingNotification) return } @@ -55,10 +67,6 @@ export class NotificationService extends BaseService { action, ) - const isAssignedToIu = - task.assigneeType === AssigneeType.internalUser && - (action === NotificationTaskActions.Assigned || action === NotificationTaskActions.ReassignedToIU) - const inProduct = opts.disableInProduct ? undefined : getInProductNotificationDetails(workspace, actionUser, task, { companyName, commentId: opts?.commentId })[action] @@ -74,7 +82,7 @@ export class NotificationService extends BaseService { task, recipientId, companyId: task.companyId ?? association?.companyId ?? undefined, - isRecipientIu: isAssignedToIu, + isRecipientIu, eventType: groupedType, commentId: opts.commentId, individualEmail: this.buildNotificationDetails( @@ -83,7 +91,7 @@ export class NotificationService extends BaseService { recipientId, { email }, senderCompanyId, - isAssignedToIu, + isRecipientIu, ), }) } @@ -96,7 +104,7 @@ export class NotificationService extends BaseService { recipientId, { inProduct, email }, senderCompanyId, - isAssignedToIu, + isRecipientIu, ) if (groupedType) notificationDetails.deliveryTargets = { inProduct } if (!inProduct && !notificationDetails.deliveryTargets?.email) return @@ -188,6 +196,12 @@ export class NotificationService extends BaseService { const association = AssociationsSchema.parse(task.associations)?.[0] // Non-null only when these CU 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 // 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 @@ -208,9 +222,17 @@ export class NotificationService extends BaseService { task, recipientId, companyId: task.companyId ?? association?.companyId ?? undefined, + isRecipientIu, eventType: groupedType, commentId: opts?.commentId, - individualEmail: this.buildNotificationDetails(task, senderId, recipientId, { email }, opts?.senderCompanyId), + individualEmail: this.buildNotificationDetails( + task, + senderId, + recipientId, + { email }, + opts?.senderCompanyId, + isRecipientIu, + ), }) if (!inProduct) continue } @@ -223,6 +245,7 @@ export class NotificationService extends BaseService { recipientId, { inProduct, email }, opts?.senderCompanyId, + isRecipientIu, ) if (groupedType) notificationDetails.deliveryTargets = { inProduct } @@ -604,6 +627,11 @@ export class NotificationService extends BaseService { return GroupedEmailEventType.SHARED case NotificationTaskActions.Commented: return GroupedEmailEventType.COMMENT + case NotificationTaskActions.Completed: + case NotificationTaskActions.CompletedByIU: + case NotificationTaskActions.CompletedByCompanyMember: + case NotificationTaskActions.CompletedForCompanyByIU: + return GroupedEmailEventType.COMPLETED default: return null } diff --git a/src/app/api/tasks/task-notifications.service.ts b/src/app/api/tasks/task-notifications.service.ts index 0d74533e2..3adfff09a 100644 --- a/src/app/api/tasks/task-notifications.service.ts +++ b/src/app/api/tasks/task-notifications.service.ts @@ -289,12 +289,14 @@ export class TaskNotificationsService extends BaseService { // Don't do this in parallel since this can cause rate-limits, each of them has their own bottlenecks for avoiding ratelimits shouldCreateNotification && (await this.notificationService.create(NotificationTaskActions.CompletedForCompanyByIU, updatedTask, { - disableEmail: true, + disableEmail: !isIuEmailEnabled(), })) await this.notificationService.markAsReadForAllRecipients(updatedTask) } else if (updatedTask.assigneeType === AssigneeType.client) { shouldCreateNotification && - (await this.notificationService.create(NotificationTaskActions.CompletedByIU, updatedTask, { disableEmail: true })) + (await this.notificationService.create(NotificationTaskActions.CompletedByIU, updatedTask, { + disableEmail: !isIuEmailEnabled(), + })) try { await this.notificationService.markClientNotificationAsRead(updatedTask) return @@ -303,7 +305,9 @@ export class TaskNotificationsService extends BaseService { } } else if (updatedTask.assigneeType === AssigneeType.internalUser) { shouldCreateNotification && - (await this.notificationService.create(NotificationTaskActions.CompletedByIU, updatedTask, { disableEmail: true })) + (await this.notificationService.create(NotificationTaskActions.CompletedByIU, updatedTask, { + disableEmail: !isIuEmailEnabled(), + })) } } @@ -368,7 +372,7 @@ export class TaskNotificationsService extends BaseService { NotificationTaskActions.CompletedByCompanyMember, updatedTask, recipientIds, - { senderCompanyId }, + { senderCompanyId, email: isIuEmailEnabled() }, ) await this.notificationService.markAsReadForAllRecipients(updatedTask) } else { @@ -379,6 +383,7 @@ export class TaskNotificationsService extends BaseService { ) await this.notificationService.createBulkNotification(NotificationTaskActions.Completed, updatedTask, recipientIds, { senderCompanyId, + email: isIuEmailEnabled(), }) await this.notificationService.markClientNotificationAsRead(updatedTask) } diff --git a/src/jobs/notifications/flush-grouped-email.ts b/src/jobs/notifications/flush-grouped-email.ts index 6981ae7e3..c776d4c37 100644 --- a/src/jobs/notifications/flush-grouped-email.ts +++ b/src/jobs/notifications/flush-grouped-email.ts @@ -78,8 +78,14 @@ const resolveSenderId = async (copilot: CopilotAPI): Promise => { // Copilot only emails an IU recipient when the sender is a real participant, so attribute the // grouped summary to the actual actor from the buffered events, not an arbitrary workspace IU. -const senderFromEvents = (events: WindowEvent[]): string | undefined => - events.map((e) => e.individualEmail?.senderId).find((id): id is string => Boolean(id)) +// The full sender context (type + company) must ride along, or a client actor produces a +// senderId/senderType mismatch that Copilot rejects (or silently drops). +type EventSender = Pick +const senderFromEvents = (events: WindowEvent[]): EventSender | undefined => { + const email = events.map((e) => e.individualEmail).find((e): e is NotificationRequestBody => Boolean(e?.senderId)) + if (!email) return undefined + return { senderId: email.senderId, senderType: email.senderType, senderCompanyId: email.senderCompanyId } +} const sendIndividualEmail = async (copilot: CopilotAPI, payload: NotificationRequestBody): Promise => { try { @@ -185,10 +191,12 @@ export const flushGroupedEmailRun = async (payload: FlushGroupedEmailPayload) => sent += 1 sentIndividual += 1 } else if (liveEvents.length >= 1) { - const groupSenderId = senderFromEvents(liveEvents) ?? (senderId ??= await resolveSenderId(copilot)) + const sender = senderFromEvents(liveEvents) await sendGroupedEmail({ content: composeGroupedEmail(liveEvents), - senderId: groupSenderId, + senderId: sender?.senderId ?? (senderId ??= await resolveSenderId(copilot)), + senderType: sender?.senderType, + senderCompanyId: sender?.senderCompanyId, recipientClientId: group.recipientClientId, recipientCompanyId: group.recipientCompanyId, copilot, @@ -222,10 +230,12 @@ export const flushGroupedEmailRun = async (payload: FlushGroupedEmailPayload) => sent += 1 sentIndividual += 1 } else if (liveEvents.length >= 1) { - const groupSenderId = senderFromEvents(liveEvents) ?? (senderId ??= await resolveSenderId(copilot)) + const sender = senderFromEvents(liveEvents) await sendGroupedEmail({ content: composeGroupedEmail(liveEvents), - senderId: groupSenderId, + senderId: sender?.senderId ?? (senderId ??= await resolveSenderId(copilot)), + senderType: sender?.senderType, + senderCompanyId: sender?.senderCompanyId, recipientInternalUserId: group.recipientIuId, copilot, }) diff --git a/src/jobs/notifications/send-grouped-email.test.ts b/src/jobs/notifications/send-grouped-email.test.ts index c41215706..f02e8927a 100644 --- a/src/jobs/notifications/send-grouped-email.test.ts +++ b/src/jobs/notifications/send-grouped-email.test.ts @@ -87,4 +87,44 @@ describe('sendGroupedEmail', () => { }), ).rejects.toThrow('copilot 5xx') }) + + it('honors an explicit client sender type and company (client actor completing their own task)', async () => { + const createNotification = jest.fn().mockResolvedValue({ id: 'notif_3', createdAt: '2026-06-09T00:00:00Z' }) + + await sendGroupedEmail({ + content, + senderId: 'client_actor', + senderType: 'client', + senderCompanyId: 'company_1', + recipientInternalUserId: 'iu_1', + copilot: buildCopilotMock(createNotification), + }) + + expect(createNotification.mock.calls[0][0]).toMatchObject({ + senderId: 'client_actor', + senderType: 'client', + senderCompanyId: 'company_1', + recipientInternalUserId: 'iu_1', + }) + }) + + it('retries without senderCompanyId when Copilot rejects it (single-company workspace)', async () => { + const createNotification = jest + .fn() + .mockRejectedValueOnce({ message: 'invalid', body: { message: 'sender company ID is invalid based on sender' } }) + .mockResolvedValueOnce({ id: 'notif_4', createdAt: '2026-06-09T00:00:00Z' }) + + const id = await sendGroupedEmail({ + content, + senderId: 'client_actor', + senderType: 'client', + senderCompanyId: 'company_1', + recipientInternalUserId: 'iu_1', + copilot: buildCopilotMock(createNotification), + }) + + expect(id).toBe('notif_4') + expect(createNotification).toHaveBeenCalledTimes(2) + expect(createNotification.mock.calls[1][0].senderCompanyId).toBeUndefined() + }) }) diff --git a/src/jobs/notifications/send-grouped-email.ts b/src/jobs/notifications/send-grouped-email.ts index 7a4a29813..93588d6c9 100644 --- a/src/jobs/notifications/send-grouped-email.ts +++ b/src/jobs/notifications/send-grouped-email.ts @@ -2,12 +2,17 @@ import 'server-only' import { GroupedEmailContent } from '@/app/api/notification/groupedEmail.composer' import { renderGroupedEmail } from '@/app/api/notification/groupedEmail.renderer' -import { NotificationRequestBody } from '@/types/common' +import { NotificationRequestBody, NotificationSender } from '@/types/common' +import { isMessagableError } from '@/utils/copilotError' import { CopilotAPI } from '@/utils/CopilotAPI' export type SendGroupedEmailArgs = { content: GroupedEmailContent senderId: string + // Copilot rejects an IU-recipient email whose sender identity is inconsistent, so the summary must + // carry the real actor's type/company (a client can be the actor, e.g. completing their own task). + senderType?: NotificationSender + senderCompanyId?: string recipientClientId?: string | null recipientCompanyId?: string | null recipientInternalUserId?: string | null @@ -17,6 +22,8 @@ export type SendGroupedEmailArgs = { export const sendGroupedEmail = async ({ content, senderId, + senderType, + senderCompanyId, recipientClientId, recipientCompanyId, recipientInternalUserId, @@ -26,7 +33,8 @@ export const sendGroupedEmail = async ({ const payload: NotificationRequestBody = { senderId, - senderType: 'internalUser', + senderType: senderType ?? 'internalUser', + senderCompanyId, recipientClientId: recipientClientId ?? undefined, recipientCompanyId: recipientCompanyId ?? undefined, recipientInternalUserId: recipientInternalUserId ?? undefined, @@ -40,6 +48,15 @@ export const sendGroupedEmail = async ({ }, } - const notification = await copilot.createNotification(payload) - return notification.id + try { + const notification = await copilot.createNotification(payload) + return notification.id + } catch (e: unknown) { + // Account for workspaces without multi-companies, which reject senderCompanyId (mirrors NotificationService). + if (isMessagableError(e) && e.body?.message === 'sender company ID is invalid based on sender') { + const notification = await copilot.createNotification({ ...payload, senderCompanyId: undefined }) + return notification.id + } + throw e + } } From 8403c940bec7653180092e36ff174662b1616a94 Mon Sep 17 00:00:00 2001 From: arpandhakal-lgtm Date: Mon, 6 Jul 2026 15:48:40 +0545 Subject: [PATCH 11/20] refactor(notifications): make create() route all completion actions to IU Greptile flagged that groupedEventTypeFor maps all four Completed* actions to COMPLETED, but create()'s isRecipientIu only covered CompletedByIU and CompletedForCompanyByIU. Latent today (Completed/CompletedByCompanyMember only reach createBulkNotification), but a future create(Completed) call would buffer the IU recipient as a CU row and mis-deliver. Extract a single isCompletionAction predicate used by both create() and groupedEventTypeFor so the two can't drift, and extend the completion test to cover all four actions. Co-Authored-By: Claude Fable 5 --- .../notification/notification.service.test.ts | 46 ++++++++++--------- .../api/notification/notification.service.ts | 22 +++++---- 2 files changed, 37 insertions(+), 31 deletions(-) diff --git a/src/app/api/notification/notification.service.test.ts b/src/app/api/notification/notification.service.test.ts index fdf457cce..c9450b50d 100644 --- a/src/app/api/notification/notification.service.test.ts +++ b/src/app/api/notification/notification.service.test.ts @@ -372,29 +372,33 @@ describe('guard: IU wiring boundaries', () => { }) describe('guard: IU completion emails', () => { - it.each([NotificationTaskActions.CompletedByIU, NotificationTaskActions.CompletedForCompanyByIU])( - 'buffers a %s email as a COMPLETED IU event and keeps the in-product notification immediate', - async (action) => { - await buildService().create(action, makeTask({ assigneeType: AssigneeType.internalUser, clientId: null })) + // Every completion action routes to an IU; create() must buffer them as IU rows regardless of + // which one is passed, so the guard stays consistent with groupedEventTypeFor. + it.each([ + NotificationTaskActions.CompletedByIU, + NotificationTaskActions.CompletedForCompanyByIU, + NotificationTaskActions.Completed, + NotificationTaskActions.CompletedByCompanyMember, + ])('buffers a %s email as a COMPLETED IU event and keeps the in-product notification immediate', async (action) => { + await buildService().create(action, makeTask({ assigneeType: AssigneeType.internalUser, clientId: null })) - expect(mockGroupedCreateMany).toHaveBeenCalledTimes(1) - const row = mockGroupedCreateMany.mock.calls[0][0].data[0] - expect(row).toMatchObject({ - recipientIuId: '33333333-3333-3333-3333-333333333333', - recipientClientId: null, - recipientCompanyId: null, - eventType: GroupedEmailEventType.COMPLETED, - }) - expect(row.individualEmail.recipientInternalUserId).toBe('33333333-3333-3333-3333-333333333333') + expect(mockGroupedCreateMany).toHaveBeenCalledTimes(1) + const row = mockGroupedCreateMany.mock.calls[0][0].data[0] + expect(row).toMatchObject({ + recipientIuId: '33333333-3333-3333-3333-333333333333', + recipientClientId: null, + recipientCompanyId: null, + eventType: GroupedEmailEventType.COMPLETED, + }) + expect(row.individualEmail.recipientInternalUserId).toBe('33333333-3333-3333-3333-333333333333') - expect(mockCreateNotification).toHaveBeenCalledTimes(1) - const sent = mockCreateNotification.mock.calls[0][0] - expect(sent.recipientInternalUserId).toBe('33333333-3333-3333-3333-333333333333') - expect(sent.recipientClientId).toBeUndefined() - expect(deliveryTargetsOf(0).inProduct).toBeDefined() - expect(deliveryTargetsOf(0).email).toBeUndefined() - }, - ) + expect(mockCreateNotification).toHaveBeenCalledTimes(1) + const sent = mockCreateNotification.mock.calls[0][0] + expect(sent.recipientInternalUserId).toBe('33333333-3333-3333-3333-333333333333') + expect(sent.recipientClientId).toBeUndefined() + expect(deliveryTargetsOf(0).inProduct).toBeDefined() + expect(deliveryTargetsOf(0).email).toBeUndefined() + }) it('does not buffer and still routes the in-product CompletedByIU notification to the IU when email is disabled', async () => { const task = makeTask({ assigneeType: AssigneeType.internalUser, clientId: null }) diff --git a/src/app/api/notification/notification.service.ts b/src/app/api/notification/notification.service.ts index 76306d180..c547bf67b 100644 --- a/src/app/api/notification/notification.service.ts +++ b/src/app/api/notification/notification.service.ts @@ -39,11 +39,8 @@ export class NotificationService extends BaseService { const isAssignedToIu = task.assigneeType === AssigneeType.internalUser && (action === NotificationTaskActions.Assigned || action === NotificationTaskActions.ReassignedToIU) - // Completion notifications go to the task creator, an IU - const isRecipientIu = - isAssignedToIu || - action === NotificationTaskActions.CompletedByIU || - action === NotificationTaskActions.CompletedForCompanyByIU + // Completion notifications always go to IUs (task creator, or IUs with access) + const isRecipientIu = isAssignedToIu || this.isCompletionAction(action) // 1. Check for existing notification. Skip if duplicate. This dedup is keyed on the client // assignee, so it must not gate IU-recipient notifications (e.g. CompletedByIU on a @@ -616,7 +613,17 @@ export class NotificationService extends BaseService { }) } + private isCompletionAction(action: NotificationTaskActions): boolean { + return ( + action === NotificationTaskActions.Completed || + action === NotificationTaskActions.CompletedByIU || + action === NotificationTaskActions.CompletedByCompanyMember || + action === NotificationTaskActions.CompletedForCompanyByIU + ) + } + private groupedEventTypeFor(action: NotificationTaskActions): GroupedEmailEventType | null { + if (this.isCompletionAction(action)) return GroupedEmailEventType.COMPLETED switch (action) { case NotificationTaskActions.Assigned: case NotificationTaskActions.AssignedToCompany: @@ -627,11 +634,6 @@ export class NotificationService extends BaseService { return GroupedEmailEventType.SHARED case NotificationTaskActions.Commented: return GroupedEmailEventType.COMMENT - case NotificationTaskActions.Completed: - case NotificationTaskActions.CompletedByIU: - case NotificationTaskActions.CompletedByCompanyMember: - case NotificationTaskActions.CompletedForCompanyByIU: - return GroupedEmailEventType.COMPLETED default: return null } From 4868e8b135dbf567852f437f9cb5a0917b0cce2c Mon Sep 17 00:00:00 2001 From: arpandhakal-lgtm Date: Mon, 6 Jul 2026 17:25:12 +0545 Subject: [PATCH 12/20] feat(notifications): send comment & reply email notifications to IUs Implements OUT-3927. IUs now receive email (not just in-product) for new comments and thread replies, reusing the existing CU comment templates. - comment job: IU recipients now get email (gated on isIuEmailEnabled()) via the grouped buffer, consistent with CU comment emails - reply job: IU initiator branch now includes the email delivery target alongside inProduct, gated on isIuEmailEnabled() Removes the deferred routing landmine (was tracked for this ticket): the `?? !email` inference in buildNotificationDetails assumed "has email => client", which inverts once IUs get emails. Flipping the comment IU call to email:true would have routed IU comment emails to recipientClientId. Now: - createBulkNotification takes an explicit isRecipientIu opt; recipient type is declared by the caller (the same Commented action fans out to both CU and IU lists, so it can't be inferred from the action) - buildNotificationDetails uses isRecipientIu directly, no email-absence fallback - completion callers pass isRecipientIu: true explicitly Co-Authored-By: Claude Fable 5 --- .../notification/notification.service.test.ts | 43 +++++++++++++++++++ .../api/notification/notification.service.ts | 16 +++---- .../api/tasks/task-notifications.service.ts | 3 +- .../send-comment-create-notifications.ts | 4 +- .../send-reply-create-notifications.ts | 6 ++- 5 files changed, 59 insertions(+), 13 deletions(-) diff --git a/src/app/api/notification/notification.service.test.ts b/src/app/api/notification/notification.service.test.ts index c9450b50d..612c45a43 100644 --- a/src/app/api/notification/notification.service.test.ts +++ b/src/app/api/notification/notification.service.test.ts @@ -278,6 +278,47 @@ describe('NotificationService grouped-email interception', () => { 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 a Commented email with email enabled to the client when isRecipientIu is not set (no email-absence inference)', async () => { + await buildService().createBulkNotification(NotificationTaskActions.Commented, makeTask(), ['cu_a'], { + email: true, + disableInProduct: true, + commentId: '44444444-4444-4444-4444-444444444444', + }) + + 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 +471,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 +495,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..90df7f044 100644 --- a/src/app/api/notification/notification.service.ts +++ b/src/app/api/notification/notification.service.ts @@ -150,6 +150,7 @@ export class NotificationService extends BaseService { commentId?: string senderCompanyId?: string emailOverride?: EmailNotificationDetails + isRecipientIu?: boolean }, ) { try { @@ -191,14 +192,11 @@ 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 + // Recipient type is declared by the caller — the same action (e.g. Commented) fans out to + // both CU and IU recipient lists, so it can't be inferred from the action alone. + const isRecipientIu = opts?.isRecipientIu ?? false // 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..afc092f12 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) } diff --git a/src/jobs/notifications/send-comment-create-notifications.ts b/src/jobs/notifications/send-comment-create-notifications.ts index 4efffa96e..ebaee11fe 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' @@ -50,10 +51,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 = { From a3893dc6e4324d13ba98f6a8a4b04c0d992647f8 Mon Sep 17 00:00:00 2001 From: arpandhakal-lgtm Date: Mon, 6 Jul 2026 17:59:10 +0545 Subject: [PATCH 13/20] refactor(notifications): make createBulkNotification isRecipientIu required MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses Greptile review: defaulting isRecipientIu to false is a silent misrouting footgun — a future IU-recipient bulk caller that omits it would route to recipientClientId with no compile-time or runtime signal. Make isRecipientIu a required field on the opts object so every caller must declare recipient type. All existing CU callers now pass isRecipientIu: false explicitly; IU callers already pass true. Co-Authored-By: Claude Fable 5 --- src/app/api/notification/notification.service.test.ts | 11 +++++++++-- src/app/api/notification/notification.service.ts | 10 +++++----- src/app/api/tasks/task-notifications.service.ts | 5 +++-- .../send-comment-create-notifications.ts | 1 + 4 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/app/api/notification/notification.service.test.ts b/src/app/api/notification/notification.service.test.ts index 612c45a43..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,6 +278,7 @@ describe('NotificationService grouped-email interception', () => { email: false, disableInProduct: false, commentId: '44444444-4444-4444-4444-444444444444', + isRecipientIu: false, }) expect(mockGroupedCreateMany).not.toHaveBeenCalled() @@ -306,11 +312,12 @@ describe('NotificationService grouped-email interception', () => { } }) - it('routes a Commented email with email enabled to the client when isRecipientIu is not set (no email-absence inference)', async () => { + 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] diff --git a/src/app/api/notification/notification.service.ts b/src/app/api/notification/notification.service.ts index 90df7f044..cde79f88b 100644 --- a/src/app/api/notification/notification.service.ts +++ b/src/app/api/notification/notification.service.ts @@ -144,13 +144,15 @@ 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 senderCompanyId?: string emailOverride?: EmailNotificationDetails - isRecipientIu?: boolean }, ) { try { @@ -194,9 +196,7 @@ export class NotificationService extends BaseService { const association = AssociationsSchema.parse(task.associations)?.[0] // Non-null only when these emails should be diverted into the grouped buffer. const groupedType = email ? this.groupedEventTypeFor(action) : null - // Recipient type is declared by the caller — the same action (e.g. Commented) fans out to - // both CU and IU recipient lists, so it can't be inferred from the action alone. - const isRecipientIu = opts?.isRecipientIu ?? false + 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 diff --git a/src/app/api/tasks/task-notifications.service.ts b/src/app/api/tasks/task-notifications.service.ts index afc092f12..ffbf1dc4d 100644 --- a/src/app/api/tasks/task-notifications.service.ts +++ b/src/app/api/tasks/task-notifications.service.ts @@ -425,6 +425,7 @@ export class TaskNotificationsService extends BaseService { await this.notificationService.createBulkNotification(NotificationTaskActions.SharedToCompany, task, recipientIds, { email: true, disableInProduct: true, + isRecipientIu: false, }) } @@ -449,7 +450,7 @@ export class TaskNotificationsService extends BaseService { NotificationTaskActions.CompletedToSharedCompany, task, recipientIds, - { email: true, disableInProduct: true }, + { email: true, disableInProduct: true, isRecipientIu: false }, ) } @@ -517,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 ebaee11fe..43ca2e1ca 100644 --- a/src/jobs/notifications/send-comment-create-notifications.ts +++ b/src/jobs/notifications/send-comment-create-notifications.ts @@ -41,6 +41,7 @@ export const sendCommentCreateNotifications = task({ email: true, disableInProduct: true, commentId: comment.id, + isRecipientIu: false, }) const { recipientIds: iuRecipientIds, senderCompanyId } = await commentNotificationService.getNotificationParties( From cc6a6d26749655de0aae5a84d2a4423ee5815b72 Mon Sep 17 00:00:00 2001 From: arpandhakal-lgtm Date: Wed, 8 Jul 2026 15:35:19 +0545 Subject: [PATCH 14/20] feat(notifications): gate IU notifications via platform notification settings Implements OUT-3929. IUs can now toggle Product/Email per notification type on the Assembly /settings/notifications page; the platform enforces those prefs at send time. The app declares one setting per category (assigned/comment/completed), fetches their stable ids, and wires them into IU sends. - add notificationSettingId to the notification body + NotificationSetting(s) schemas; CopilotAPI.getNotificationSettings() resolves the install by appId and fetches installs/{id}/notification-settings - resolveIuNotificationSetting({category}) maps a category -> declared setting by label, returning { id, emailEnabled }; per-workspace cache (5m TTL, config only, never IU prefs). Fails closed on error and when a category isn't declared yet: no id, email withheld - IU sends attach the category id to the in-product dispatch so the platform gates in-product per IU. The email surface is gated app-side: a grouped summary is cross-category and can't carry a per-category id, so an IU email is only buffered when the category's declared setting enables the email surface. Grouped windows stay cross-category. Reply job dispatches directly, so it just passes the id. - remove the IU_EMAIL_ALWAYS_ENABLED env kill switch: IU sends always request both surfaces and the platform is the sole gate Co-Authored-By: Claude Opus 4.8 (1M context) --- src/app/api/notification/isIuEmailEnabled.ts | 3 - .../notification/notification.service.test.ts | 63 ++++++++++++ .../api/notification/notification.service.ts | 40 +++++++- .../resolveNotificationSettingId.test.ts | 98 +++++++++++++++++++ .../resolveNotificationSettingId.ts | 69 +++++++++++++ .../api/tasks/task-notifications.service.ts | 13 ++- src/config/index.ts | 3 - .../send-comment-create-notifications.ts | 3 +- .../send-reply-create-notifications.ts | 98 +++++++++++-------- src/types/common.ts | 17 ++++ src/utils/CopilotAPI.ts | 18 ++++ 11 files changed, 365 insertions(+), 60 deletions(-) delete mode 100644 src/app/api/notification/isIuEmailEnabled.ts create mode 100644 src/app/api/notification/resolveNotificationSettingId.test.ts create mode 100644 src/app/api/notification/resolveNotificationSettingId.ts diff --git a/src/app/api/notification/isIuEmailEnabled.ts b/src/app/api/notification/isIuEmailEnabled.ts deleted file mode 100644 index b144a8e67..000000000 --- a/src/app/api/notification/isIuEmailEnabled.ts +++ /dev/null @@ -1,3 +0,0 @@ -import { iuEmailAlwaysEnabled } from '@/config' - -export const isIuEmailEnabled = (): boolean => iuEmailAlwaysEnabled diff --git a/src/app/api/notification/notification.service.test.ts b/src/app/api/notification/notification.service.test.ts index da2a2e775..6cb1acab2 100644 --- a/src/app/api/notification/notification.service.test.ts +++ b/src/app/api/notification/notification.service.test.ts @@ -15,6 +15,7 @@ const mockGroupedCreateMany = jest.fn() const mockGetWorkspace = jest.fn() const mockMe = jest.fn() const mockCreateNotification = jest.fn() +const mockGetNotificationSettings = jest.fn() jest.mock('@/jobs/notifications/flush-grouped-email', () => ({ enqueueGroupedEmailFlush: (...args: unknown[]) => mockEnqueueFlush(...args), @@ -42,10 +43,12 @@ jest.mock('@/utils/CopilotAPI', () => ({ getWorkspace: (...args: unknown[]) => mockGetWorkspace(...args), me: (...args: unknown[]) => mockMe(...args), createNotification: (...args: unknown[]) => mockCreateNotification(...args), + getNotificationSettings: (...args: unknown[]) => mockGetNotificationSettings(...args), })), })) import { NotificationService } from './notification.service' +import { __clearNotificationSettingCache } from './resolveNotificationSettingId' const user = { token: 'tok', @@ -84,6 +87,16 @@ const buildService = () => { beforeEach(() => { jest.clearAllMocks() + __clearNotificationSettingCache() + // Default: all IU categories declared with the email surface enabled, so IU emails buffer and + // carry the resolved setting id. Individual cases override to exercise the email-surface gate. + mockGetNotificationSettings.mockResolvedValue({ + notifications: [ + { id: 'setting_assigned', label: 'New task assigned', surfaces: ['product', 'email'] }, + { id: 'setting_comment', label: 'New comment on a task', surfaces: ['product', 'email'] }, + { id: 'setting_completed', label: 'Task completed', surfaces: ['product', 'email'] }, + ], + }) mockGetWorkspace.mockResolvedValue({ labels: {} }) mockMe.mockResolvedValue({ id: 'creator_1', givenName: 'Jane', familyName: 'IU' }) mockFindFirst.mockResolvedValue(null) @@ -419,6 +432,56 @@ describe('guard: IU wiring boundaries', () => { }) }) +describe('guard: IU notification setting gating', () => { + it('resolves the id by action category and attaches it to both the buffered email and the in-product dispatch', async () => { + const task = makeTask({ assigneeType: AssigneeType.internalUser, clientId: null }) + await buildService().create(NotificationTaskActions.Assigned, task, { disableEmail: false }) + + expect(mockGroupedCreateMany.mock.calls[0][0].data[0].individualEmail.notificationSettingId).toBe('setting_assigned') + expect(mockCreateNotification.mock.calls[0][0].notificationSettingId).toBe('setting_assigned') + }) + + it('resolves a different id per category (COMPLETED vs ASSIGNED)', async () => { + const task = makeTask({ assigneeType: AssigneeType.internalUser, clientId: null }) + await buildService().create(NotificationTaskActions.CompletedByIU, task, { disableEmail: false }) + + expect(mockCreateNotification.mock.calls[0][0].notificationSettingId).toBe('setting_completed') + }) + + it('keeps IU grouped windows cross-category (window key is not scoped by event type)', async () => { + const task = makeTask({ assigneeType: AssigneeType.internalUser, clientId: null }) + await buildService().create(NotificationTaskActions.Assigned, task, { disableEmail: false }) + + expect(mockGroupedCreateMany.mock.calls[0][0].data[0].windowKey).toMatch(new RegExp(`^${task.assigneeId}:iu:[^:]+$`)) + }) + + it('does NOT buffer the IU email when the category setting omits the email surface, but still fires in-product with the id', async () => { + mockGetNotificationSettings.mockResolvedValue({ + notifications: [{ id: 'setting_assigned', label: 'New task assigned', surfaces: ['product'] }], + }) + const task = makeTask({ assigneeType: AssigneeType.internalUser, clientId: null }) + await buildService().create(NotificationTaskActions.Assigned, task, { disableEmail: false }) + + expect(mockGroupedCreateMany).not.toHaveBeenCalled() + expect(mockEnqueueFlush).not.toHaveBeenCalled() + const sent = mockCreateNotification.mock.calls[0][0] + expect(sent.recipientInternalUserId).toBe(task.assigneeId) + expect(sent.notificationSettingId).toBe('setting_assigned') + expect(deliveryTargetsOf(0).inProduct).toBeDefined() + expect(deliveryTargetsOf(0).email).toBeUndefined() + }) + + it('does NOT buffer the IU email when the category has no declared setting (email off until declared)', async () => { + mockGetNotificationSettings.mockResolvedValue({ notifications: [] }) + const task = makeTask({ assigneeType: AssigneeType.internalUser, clientId: null }) + await buildService().create(NotificationTaskActions.Assigned, task, { disableEmail: false }) + + expect(mockGroupedCreateMany).not.toHaveBeenCalled() + expect(mockCreateNotification.mock.calls[0][0].notificationSettingId).toBeUndefined() + expect(deliveryTargetsOf(0).inProduct).toBeDefined() + }) +}) + describe('guard: IU completion emails', () => { // Every completion action routes to an IU; create() must buffer them as IU rows regardless of // which one is passed, so the guard stays consistent with groupedEventTypeFor. diff --git a/src/app/api/notification/notification.service.ts b/src/app/api/notification/notification.service.ts index cde79f88b..0668270e3 100644 --- a/src/app/api/notification/notification.service.ts +++ b/src/app/api/notification/notification.service.ts @@ -14,6 +14,7 @@ import APIError from '@api/core/exceptions/api' import { BaseService } from '@api/core/services/base.service' import { NotificationTaskActions } from '@api/core/types/tasks' import { getEmailDetails, getInProductNotificationDetails, mergeEmailOverride } from '@api/notification/notification.helpers' +import { resolveIuNotificationSetting } from '@api/notification/resolveNotificationSettingId' import { AssigneeType, ClientNotification, GroupedEmailEventType, Prisma, Task } from '@prisma/client' import { randomUUID } from 'crypto' import { enqueueGroupedEmailFlush } from '@/jobs/notifications/flush-grouped-email' @@ -70,9 +71,21 @@ export class NotificationService extends BaseService { const baseEmail = opts.disableEmail ? undefined : getEmailDetails(workspace, actionUser, task, { commentId: opts?.commentId })[action] - const email = baseEmail ? mergeEmailOverride({ base: baseEmail, override: opts.emailOverride }) : baseEmail + const mergedEmail = baseEmail ? mergeEmailOverride({ base: baseEmail, override: opts.emailOverride }) : baseEmail + + const category = this.groupedEventTypeFor(action) + // IU sends carry the category's setting id so the platform gates the in-product surface per the + // IU's preference. The email surface is gated app-side: a grouped summary is cross-category and + // can't carry a per-category id, so an IU email is only buffered when the category's declared + // setting enables the email surface. + const iuSetting = + isRecipientIu && category + ? await resolveIuNotificationSetting({ copilot: this.copilot, workspaceId: task.workspaceId, category }) + : undefined + const notificationSettingId = iuSetting?.id + const email = isRecipientIu && iuSetting && !iuSetting.emailEnabled ? undefined : mergedEmail - const groupedType = email && recipientId ? this.groupedEventTypeFor(action) : null + const groupedType = email && recipientId ? category : null if (groupedType) { const association = AssociationsSchema.parse(task.associations)?.[0] await this.bufferGroupedEmailEvent({ @@ -89,6 +102,7 @@ export class NotificationService extends BaseService { { email }, senderCompanyId, isRecipientIu, + notificationSettingId, ), }) } @@ -102,6 +116,7 @@ export class NotificationService extends BaseService { { inProduct, email }, senderCompanyId, isRecipientIu, + notificationSettingId, ) if (groupedType) notificationDetails.deliveryTargets = { inProduct } if (!inProduct && !notificationDetails.deliveryTargets?.email) return @@ -177,7 +192,7 @@ export class NotificationService extends BaseService { const baseEmail = opts?.email ? getEmailDetails(workspace, actionUserName, task, { commentId: opts?.commentId })[action] : undefined - const email = baseEmail ? mergeEmailOverride({ base: baseEmail, override: opts?.emailOverride }) : baseEmail + const mergedEmail = baseEmail ? mergeEmailOverride({ base: baseEmail, override: opts?.emailOverride }) : baseEmail // Get a list of all notifications dispatched for these taskId, clientId, companyId combinations // This will be used to filter out any duplicate notifications during creation @@ -194,9 +209,18 @@ export class NotificationService extends BaseService { const iuNotifications = [] const association = AssociationsSchema.parse(task.associations)?.[0] - // Non-null only when these emails should be diverted into the grouped buffer. - const groupedType = email ? this.groupedEventTypeFor(action) : null + const category = this.groupedEventTypeFor(action) const isRecipientIu = opts.isRecipientIu + // Resolve once per batch (not per recipient). IU sends carry the setting id so the platform + // gates the in-product surface; the email surface is gated app-side (see create()). + const iuSetting = + isRecipientIu && category + ? await resolveIuNotificationSetting({ copilot: this.copilot, workspaceId: task.workspaceId, category }) + : undefined + const notificationSettingId = iuSetting?.id + const email = isRecipientIu && iuSetting && !iuSetting.emailEnabled ? undefined : mergedEmail + // Non-null only when these emails should be diverted into the grouped buffer. + const groupedType = email ? category : null // 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 @@ -227,6 +251,7 @@ export class NotificationService extends BaseService { { email }, opts?.senderCompanyId, isRecipientIu, + notificationSettingId, ), }) if (!inProduct) continue @@ -241,6 +266,7 @@ export class NotificationService extends BaseService { { inProduct, email }, opts?.senderCompanyId, isRecipientIu, + notificationSettingId, ) if (groupedType) notificationDetails.deliveryTargets = { inProduct } @@ -715,6 +741,9 @@ export class NotificationService extends BaseService { deliveryTargets: NotificationRequestBody['deliveryTargets'], senderCompanyId?: string, isRecipientIu?: boolean, + // Set for IU payloads so the platform gates each requested surface (in-product and email) + // against the IU's per-category preference. Suppression is enforced platform-side. + notificationSettingId?: string, ): NotificationRequestBody { const associations = AssociationsSchema.parse(task.associations) const association = associations?.[0] @@ -730,6 +759,7 @@ export class NotificationService extends BaseService { delete notificationDetails.recipientCompanyId delete notificationDetails.recipientClientId notificationDetails.recipientInternalUserId = recipientId + if (notificationSettingId) notificationDetails.notificationSettingId = notificationSettingId } return notificationDetails } diff --git a/src/app/api/notification/resolveNotificationSettingId.test.ts b/src/app/api/notification/resolveNotificationSettingId.test.ts new file mode 100644 index 000000000..128fdb7a9 --- /dev/null +++ b/src/app/api/notification/resolveNotificationSettingId.test.ts @@ -0,0 +1,98 @@ +import { CopilotAPI } from '@/utils/CopilotAPI' +import { GroupedEmailEventType } from '@prisma/client' +import { __clearNotificationSettingCache, resolveIuNotificationSetting } from './resolveNotificationSettingId' + +const buildCopilot = (getNotificationSettings: jest.Mock) => ({ getNotificationSettings }) as unknown as CopilotAPI + +const settings = [ + { id: 'setting_assigned', label: 'New task assigned', surfaces: ['product', 'email'] }, + { id: 'setting_comment', label: 'New comment on a task', surfaces: ['product', 'email'] }, + { id: 'setting_completed', label: 'Task completed', surfaces: ['product', 'email'] }, +] + +beforeEach(() => __clearNotificationSettingCache()) + +describe('resolveIuNotificationSetting', () => { + it('maps each category to its declared setting by label, with email enabled', async () => { + const copilot = buildCopilot(jest.fn().mockResolvedValue({ notifications: settings })) + + expect( + await resolveIuNotificationSetting({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.ASSIGNED }), + ).toEqual({ id: 'setting_assigned', emailEnabled: true }) + expect( + await resolveIuNotificationSetting({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.COMMENT }), + ).toEqual({ id: 'setting_comment', emailEnabled: true }) + }) + + it('reports emailEnabled=false when the declared setting omits the email surface', async () => { + const copilot = buildCopilot( + jest.fn().mockResolvedValue({ notifications: [{ id: 'x', label: 'New task assigned', surfaces: ['product'] }] }), + ) + + expect( + await resolveIuNotificationSetting({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.ASSIGNED }), + ).toEqual({ id: 'x', emailEnabled: false }) + }) + + it('matches labels case-insensitively and ignoring surrounding whitespace', async () => { + const copilot = buildCopilot( + jest.fn().mockResolvedValue({ notifications: [{ id: 'x', label: ' NEW task Assigned ', surfaces: ['email'] }] }), + ) + + expect( + await resolveIuNotificationSetting({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.ASSIGNED }), + ).toEqual({ id: 'x', emailEnabled: true }) + }) + + it('returns id undefined and emailEnabled false when the category is not declared', async () => { + const copilot = buildCopilot(jest.fn().mockResolvedValue({ notifications: [settings[0]] })) + + expect( + await resolveIuNotificationSetting({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.COMMENT }), + ).toEqual({ id: undefined, emailEnabled: false }) + }) + + it('returns emailEnabled false for SHARED (no IU setting declared)', async () => { + const copilot = buildCopilot(jest.fn().mockResolvedValue({ notifications: settings })) + + expect( + await resolveIuNotificationSetting({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.SHARED }), + ).toEqual({ id: undefined, emailEnabled: false }) + }) + + it('caches the settings per workspace and does not refetch within the TTL', async () => { + const getNotificationSettings = jest.fn().mockResolvedValue({ notifications: settings }) + const copilot = buildCopilot(getNotificationSettings) + + await resolveIuNotificationSetting({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.ASSIGNED }) + await resolveIuNotificationSetting({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.COMMENT }) + + expect(getNotificationSettings).toHaveBeenCalledTimes(1) + }) + + it('fails closed (no id, emailEnabled false) when the fetch fails, without caching the failure', async () => { + const getNotificationSettings = jest + .fn() + .mockRejectedValueOnce(new Error('copilot 5xx')) + .mockResolvedValueOnce({ notifications: settings }) + const copilot = buildCopilot(getNotificationSettings) + + expect( + await resolveIuNotificationSetting({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.ASSIGNED }), + ).toEqual({ id: undefined, emailEnabled: false }) + expect( + await resolveIuNotificationSetting({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.ASSIGNED }), + ).toEqual({ id: 'setting_assigned', emailEnabled: true }) + expect(getNotificationSettings).toHaveBeenCalledTimes(2) + }) + + it('caches per workspace independently', async () => { + const getNotificationSettings = jest.fn().mockResolvedValue({ notifications: settings }) + const copilot = buildCopilot(getNotificationSettings) + + await resolveIuNotificationSetting({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.ASSIGNED }) + await resolveIuNotificationSetting({ copilot, workspaceId: 'ws_2', category: GroupedEmailEventType.ASSIGNED }) + + expect(getNotificationSettings).toHaveBeenCalledTimes(2) + }) +}) diff --git a/src/app/api/notification/resolveNotificationSettingId.ts b/src/app/api/notification/resolveNotificationSettingId.ts new file mode 100644 index 000000000..705a5cd81 --- /dev/null +++ b/src/app/api/notification/resolveNotificationSettingId.ts @@ -0,0 +1,69 @@ +import { NotificationSetting } from '@/types/common' +import { CopilotAPI } from '@/utils/CopilotAPI' +import { serializeError } from '@/utils/serializeError' +import { GroupedEmailEventType } from '@prisma/client' + +// Canonical labels the Tasks app declares on its Assembly app record (App Setup > Notifications). +export const IU_NOTIFICATION_LABELS: Partial> = { + [GroupedEmailEventType.ASSIGNED]: 'New task assigned', + [GroupedEmailEventType.COMMENT]: 'New comment on a task', + [GroupedEmailEventType.COMPLETED]: 'Task completed', +} + +// Passed on IU sends so the platform gates each requested surface against the IU's preference. +export type IuNotificationSetting = { + id: string | undefined + emailEnabled: boolean +} + +// Cache the declared settings per workspace, keyed by normalized label. +const CACHE_TTL_MS = 5 * 60 * 1000 +const cache = new Map; expiresAt: number }>() + +const normalize = (label: string): string => label.trim().toLowerCase() + +const getSettingsByLabel = async ({ + copilot, + workspaceId, +}: { + copilot: CopilotAPI + workspaceId: string +}): Promise> => { + const cached = cache.get(workspaceId) + if (cached && cached.expiresAt > Date.now()) return cached.byLabel + + const { notifications } = await copilot.getNotificationSettings() + const byLabel = new Map(notifications.map((setting) => [normalize(setting.label), setting])) + cache.set(workspaceId, { byLabel, expiresAt: Date.now() + CACHE_TTL_MS }) + return byLabel +} + +// Resolve the declared setting for an IU notification category. Returns emailEnabled=false when the +// category has no declared setting (email is not sent until it's configured in the dashboard) and, +// fail-closed, on fetch failure too — we withhold IU email rather than risk sending it past a +// preference we couldn't read. Failures are not cached, so the next send retries. +export const resolveIuNotificationSetting = async ({ + copilot, + workspaceId, + category, +}: { + copilot: CopilotAPI + workspaceId: string + category: GroupedEmailEventType +}): Promise => { + const label = IU_NOTIFICATION_LABELS[category] + if (!label) return { id: undefined, emailEnabled: false } + + try { + const byLabel = await getSettingsByLabel({ copilot, workspaceId }) + const setting = byLabel.get(normalize(label)) + if (!setting) return { id: undefined, emailEnabled: false } + return { id: setting.id, emailEnabled: setting.surfaces.includes('email') } //right now we are using setting.surfaces. We need support from assembly to expose a prop which indicates if email is turned on for the event. + } catch (e) { + console.error('resolveIuNotificationSetting | failed to resolve; withholding IU email', serializeError(e)) + return { id: undefined, emailEnabled: false } + } +} + +// Test seam: clear the per-workspace cache between cases. +export const __clearNotificationSettingCache = (): void => cache.clear() diff --git a/src/app/api/tasks/task-notifications.service.ts b/src/app/api/tasks/task-notifications.service.ts index ffbf1dc4d..89049866d 100644 --- a/src/app/api/tasks/task-notifications.service.ts +++ b/src/app/api/tasks/task-notifications.service.ts @@ -6,7 +6,6 @@ import { CopilotAPI } from '@/utils/CopilotAPI' import User from '@api/core/models/User.model' import { BaseService } from '@api/core/services/base.service' import { NotificationTaskActions } from '@api/core/types/tasks' -import { isIuEmailEnabled } from '@api/notification/isIuEmailEnabled' import { NotificationService } from '@api/notification/notification.service' import { AssigneeType, StateType, Task, WorkflowState } from '@prisma/client' import { z } from 'zod' @@ -289,13 +288,13 @@ export class TaskNotificationsService extends BaseService { // Don't do this in parallel since this can cause rate-limits, each of them has their own bottlenecks for avoiding ratelimits shouldCreateNotification && (await this.notificationService.create(NotificationTaskActions.CompletedForCompanyByIU, updatedTask, { - disableEmail: !isIuEmailEnabled(), + disableEmail: false, })) await this.notificationService.markAsReadForAllRecipients(updatedTask) } else if (updatedTask.assigneeType === AssigneeType.client) { shouldCreateNotification && (await this.notificationService.create(NotificationTaskActions.CompletedByIU, updatedTask, { - disableEmail: !isIuEmailEnabled(), + disableEmail: false, })) try { await this.notificationService.markClientNotificationAsRead(updatedTask) @@ -306,7 +305,7 @@ export class TaskNotificationsService extends BaseService { } else if (updatedTask.assigneeType === AssigneeType.internalUser) { shouldCreateNotification && (await this.notificationService.create(NotificationTaskActions.CompletedByIU, updatedTask, { - disableEmail: !isIuEmailEnabled(), + disableEmail: false, })) } } @@ -372,7 +371,7 @@ export class TaskNotificationsService extends BaseService { NotificationTaskActions.CompletedByCompanyMember, updatedTask, recipientIds, - { senderCompanyId, email: isIuEmailEnabled(), isRecipientIu: true }, + { senderCompanyId, email: true, isRecipientIu: true }, ) await this.notificationService.markAsReadForAllRecipients(updatedTask) } else { @@ -383,7 +382,7 @@ export class TaskNotificationsService extends BaseService { ) await this.notificationService.createBulkNotification(NotificationTaskActions.Completed, updatedTask, recipientIds, { senderCompanyId, - email: isIuEmailEnabled(), + email: true, isRecipientIu: true, }) await this.notificationService.markClientNotificationAsRead(updatedTask) @@ -492,7 +491,7 @@ export class TaskNotificationsService extends BaseService { // In future when reassignment is supported, change this logic to support reassigned to client as well notificationType, task, - { disableEmail: task.assigneeType === AssigneeType.internalUser && !isIuEmailEnabled(), emailOverride }, + { disableEmail: false, emailOverride }, ) // Create a new entry in ClientNotifications table so we can mark as read on // behalf of client later diff --git a/src/config/index.ts b/src/config/index.ts index d75521224..3813ba9f0 100644 --- a/src/config/index.ts +++ b/src/config/index.ts @@ -53,9 +53,6 @@ export const assemblyApiDomain = z.string().url().parse(process.env.NEXT_PUBLIC_ // Substring stripped from the task title when building the reminder email subject for // subject-override workspaces, and the value it's replaced with. Configured via env so the // workspace-specific phrasing isn't hardcoded (OUT-3919). -// Bypasses the platform preference check while OUT-3929 is pending. Set true in dev/staging. -export const iuEmailAlwaysEnabled = process.env.IU_EMAIL_ALWAYS_ENABLED === 'true' - export const reminderSubjectSearch = process.env.REMINDER_SUBJECT_SEARCH || '' export const reminderSubjectReplacement = process.env.REMINDER_SUBJECT_REPLACEMENT || '' diff --git a/src/jobs/notifications/send-comment-create-notifications.ts b/src/jobs/notifications/send-comment-create-notifications.ts index 43ca2e1ca..713b8cbab 100644 --- a/src/jobs/notifications/send-comment-create-notifications.ts +++ b/src/jobs/notifications/send-comment-create-notifications.ts @@ -1,7 +1,6 @@ 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' @@ -52,7 +51,7 @@ 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: isIuEmailEnabled(), + email: true, disableInProduct: false, commentId: comment.id, senderCompanyId, diff --git a/src/jobs/notifications/send-reply-create-notifications.ts b/src/jobs/notifications/send-reply-create-notifications.ts index 0bc4a69a5..8e0a34f4c 100644 --- a/src/jobs/notifications/send-reply-create-notifications.ts +++ b/src/jobs/notifications/send-reply-create-notifications.ts @@ -5,10 +5,10 @@ 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 { resolveIuNotificationSetting } from '@/app/api/notification/resolveNotificationSettingId' import User from '@api/core/models/User.model' import { TasksService } from '@api/tasks/tasks.service' -import { Comment, CommentInitiator, Task } from '@prisma/client' +import { Comment, CommentInitiator, GroupedEmailEventType, Task } from '@prisma/client' import { logger, task } from '@trigger.dev/sdk/v3' import { z } from 'zod' @@ -47,6 +47,15 @@ export const sendReplyCreateNotifications = task({ const deliveryTargets = await getNotificationDetails(copilot, user, comment) + // Replies are the COMMENT category. IU sends carry this setting id so the platform gates both + // surfaces against the recipient IU's preference (this job dispatches directly, no grouping, so + // the platform handles the email surface too — no app-side surfaces check needed here). + const { id: notificationSettingId } = await resolveIuNotificationSetting({ + copilot, + workspaceId: user.workspaceId, + category: GroupedEmailEventType.COMMENT, + }) + const notificationPromises: Promise[] = [] const queueNotificationPromise = (promise: Promise): void => { notificationPromises.push(copilotBottleneck.schedule(() => promise)) @@ -59,7 +68,7 @@ export const sendReplyCreateNotifications = task({ // Queue notifications to every unique reply initiator for (let initiator of threadInitiators) { - const promise = getInitiatorNotificationPromises( + const promise = getInitiatorNotificationPromises({ copilot, initiator, senderId, @@ -70,8 +79,9 @@ export const sendReplyCreateNotifications = task({ // However, it is very safe to assume that client users can ONLY reply to comments in tasks // assigned to their company, or to them. In both cases, payload.task.companyId works // For IU tasks, this will be undefined - payload.task.companyId || undefined, - ) + initiatorCompanyId: payload.task.companyId || undefined, + notificationSettingId, + }) promise && queueNotificationPromise(promise) // It's certain we will get a promise here } @@ -88,15 +98,16 @@ export const sendReplyCreateNotifications = task({ (initiator) => initiator.initiatorId === parentComment.initiatorId, ) if (!isParentCommentDeleted && !parentInitiatorIsCurrentUser && !isNotificationAlreadySent) { - const typedPromise = getInitiatorNotificationPromises( + const typedPromise = getInitiatorNotificationPromises({ copilot, - parentComment, + initiator: parentComment, senderId, senderType, senderCompanyId, deliveryTargets, - payload.task.companyId || undefined, - ) + initiatorCompanyId: payload.task.companyId || undefined, + notificationSettingId, + }) // If there is no "initiatorType" for parentComment we have to be slightly creative (coughhackycough) const promise = typedPromise ?? @@ -108,6 +119,7 @@ export const sendReplyCreateNotifications = task({ senderType, senderCompanyId, deliveryTargets, + notificationSettingId, ) queueNotificationPromise(promise) } @@ -149,27 +161,40 @@ const getNotificationDetails = async (copilot: CopilotAPI, user: User, comment: return deliveryTargets } -const getInitiatorNotificationPromises = ( - copilot: CopilotAPI, +const getInitiatorNotificationPromises = ({ + copilot, // Initiator in this context means previous initiators that were active in the thread, NOT the currently commenting user - initiator: { initiatorId: string; initiatorType: CommentInitiator | null }, - senderId: string, - senderType: NotificationSender, - senderCompanyId: string | undefined, - deliveryTargets: { inProduct: Record<'title', any>; email: object }, - initiatorCompanyId?: string, + initiator, + senderId, + senderType, + senderCompanyId, + deliveryTargets, + initiatorCompanyId, // Forces recipient branch when initiator.initiatorType is unset (legacy comments) - assume?: CommentInitiator, -) => { + assume, + notificationSettingId, +}: { + copilot: CopilotAPI + initiator: { initiatorId: string; initiatorType: CommentInitiator | null } + senderId: string + senderType: NotificationSender + senderCompanyId: string | undefined + deliveryTargets: { inProduct: Record<'title', any>; email: object } + initiatorCompanyId?: string + assume?: CommentInitiator + notificationSettingId?: string +}) => { const base = { senderId, senderType, senderCompanyId } let body: NotificationRequestBody if (initiator.initiatorType === CommentInitiator.internalUser || assume === CommentInitiator.internalUser) { body = { ...base, recipientInternalUserId: initiator.initiatorId, + // Both surfaces are always requested; the platform gates them per the IU's preference. + ...(notificationSettingId ? { notificationSettingId } : {}), deliveryTargets: { inProduct: deliveryTargets.inProduct, - ...(isIuEmailEnabled() && { email: deliveryTargets.email }), + email: deliveryTargets.email, }, } } else if (initiator.initiatorType === CommentInitiator.client || assume === CommentInitiator.client) { @@ -206,34 +231,27 @@ const getNotificationToUntypedInitiator = async ( senderType: NotificationSender, senderCompanyId: string | undefined, deliveryTargets: { inProduct: Record<'title', any>; email: object }, + notificationSettingId?: string, ) => { + const shared = { + copilot, + initiator: parentComment, + senderId, + senderType, + senderCompanyId, + deliveryTargets, + initiatorCompanyId: task.companyId || undefined, + notificationSettingId, + } try { await copilot.getInternalUser(parentComment.initiatorId) // `assume` guarantees a non-null promise - return getInitiatorNotificationPromises( - copilot, - parentComment, - senderId, - senderType, - senderCompanyId, - deliveryTargets, - task.companyId || undefined, - CommentInitiator.internalUser, - )! + return getInitiatorNotificationPromises({ ...shared, assume: CommentInitiator.internalUser })! } catch (e) { console.error(e) } - return getInitiatorNotificationPromises( - copilot, - parentComment, - senderId, - senderType, - senderCompanyId, - deliveryTargets, - task.companyId || undefined, - CommentInitiator.client, - )!.catch((e) => { + return getInitiatorNotificationPromises({ ...shared, assume: CommentInitiator.client })!.catch((e) => { console.error(e) return undefined }) diff --git a/src/types/common.ts b/src/types/common.ts index 79acc7bd9..119adb64f 100644 --- a/src/types/common.ts +++ b/src/types/common.ts @@ -206,6 +206,9 @@ export const NotificationRequestBodySchema = z recipientInternalUserId: z.string().optional(), recipientClientId: z.string().optional(), recipientCompanyId: z.string().optional(), + // When set, the platform resolves each requested surface against the recipient IU's preference + // for this setting and silently drops suppressed surfaces (IU recipients only). + notificationSettingId: z.string().optional(), deliveryTargets: z .object({ inProduct: z @@ -220,6 +223,20 @@ export const NotificationRequestBodySchema = z }) .superRefine(validateNotificationRecipient) +// Declared notification settings for the app. +export const NotificationSettingSchema = z.object({ + id: z.string(), + label: z.string(), + surfaces: z.array(z.enum(['product', 'email'])), + default: z.object({ product: z.boolean().optional(), email: z.boolean().optional() }).optional(), +}) +export type NotificationSetting = z.infer + +export const NotificationSettingsResponseSchema = z.object({ + notifications: z.array(NotificationSettingSchema).default([]), +}) +export type NotificationSettingsResponse = z.infer + export const ScrapMediaRequestSchema = z.object({ filePath: z.string(), taskId: z.string().uuid().optional(), diff --git a/src/utils/CopilotAPI.ts b/src/utils/CopilotAPI.ts index 20b721548..71f6ac8d5 100644 --- a/src/utils/CopilotAPI.ts +++ b/src/utils/CopilotAPI.ts @@ -32,6 +32,8 @@ import { NotificationRequestBody, NotificationResponseSchema, NotificationResponseType, + NotificationSettingsResponse, + NotificationSettingsResponseSchema, Token, TokenSchema, WorkspaceResponse, @@ -346,6 +348,21 @@ export class CopilotAPI { }) } + // Declared notification settings for this app in the current workspace. + async _getNotificationSettings(): Promise { + console.info('CopilotAPI#_getNotificationSettings', this.token) + const appId = z.string({ message: 'Missing AppID in environment' }).parse(APP_ID) + const installs = await this.copilot.listAppInstalls() + const install = installs.find((entry) => entry.appId === appId) + if (!install?.id) { + console.info('CopilotAPI#_getNotificationSettings | No matching app install in workspace; no settings') + return { notifications: [] } + } + const workspaceId = await this._resolveWorkspaceId() + const response = await this._manualFetch(`installs/${install.id}/notification-settings`, undefined, workspaceId) + return NotificationSettingsResponseSchema.parse(response) + } + async dispatchWebhook( eventName: DISPATCHABLE_EVENT, { @@ -410,6 +427,7 @@ export class CopilotAPI { bulkDeleteNotifications = this.wrapWithRetry(this._bulkDeleteNotifications) manualFetch = this.wrapWithRetry(this._manualFetch) getIUNotification = this.wrapWithRetry(this._getIUNotification) + getNotificationSettings = this.wrapWithRetry(this._getNotificationSettings) } const cachedFetchInternalUser = cache( From a716d211bee4cd02f2ec05fe2f1d39a7ca513552 Mon Sep 17 00:00:00 2001 From: arpandhakal-lgtm Date: Wed, 8 Jul 2026 16:55:12 +0545 Subject: [PATCH 15/20] refactor(notifications): interim IU gating + buffer replies; address Greptile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reworks IU notification gating per the platform's real mechanism (per-IU prefs are only enforced at send time via notificationSettingId; there's no read API yet). - resolver back to id-only (resolveIuNotificationSettingId); drops the surfaces/ emailEnabled buffer-time gate — gating now lives on the send + at flush - create()/createBulkNotification always buffer IU emails and attach the category id to the in-product dispatch (platform gates in-product per IU) and to the buffered email (for the flush single-category gate) - flush: attach notificationSettingId to an IU grouped summary only when the whole window is one category (mixed windows can't gate against one id); add a filterEventsForIuPreferences seam (pass-through, TODO(OUT-3929)) for when Assembly exposes a per-IU preference read endpoint - replies now buffer as COMMENT events (were direct sends) so they group + gate like top-level comments; IU in-product still fires immediately. Removes the ungated direct-reply path (Greptile P1) - Greptile P2: stop logging the raw token in _getNotificationSettings Windows stay cross-category (combined email). True per-IU gating of a mixed summary is blocked on the read endpoint being confirmed with Assembly. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../notification/notification.service.test.ts | 21 +- .../api/notification/notification.service.ts | 31 ++- .../resolveNotificationSettingId.test.ts | 61 +++--- .../resolveNotificationSettingId.ts | 51 +++-- .../notifications/flush-grouped-email.test.ts | 29 +++ src/jobs/notifications/flush-grouped-email.ts | 20 +- src/jobs/notifications/send-grouped-email.ts | 5 + .../send-reply-create-notifications.ts | 191 +++++++++--------- src/utils/CopilotAPI.ts | 2 +- 9 files changed, 215 insertions(+), 196 deletions(-) diff --git a/src/app/api/notification/notification.service.test.ts b/src/app/api/notification/notification.service.test.ts index 6cb1acab2..a18e674bf 100644 --- a/src/app/api/notification/notification.service.test.ts +++ b/src/app/api/notification/notification.service.test.ts @@ -455,28 +455,13 @@ describe('guard: IU notification setting gating', () => { expect(mockGroupedCreateMany.mock.calls[0][0].data[0].windowKey).toMatch(new RegExp(`^${task.assigneeId}:iu:[^:]+$`)) }) - it('does NOT buffer the IU email when the category setting omits the email surface, but still fires in-product with the id', async () => { - mockGetNotificationSettings.mockResolvedValue({ - notifications: [{ id: 'setting_assigned', label: 'New task assigned', surfaces: ['product'] }], - }) - const task = makeTask({ assigneeType: AssigneeType.internalUser, clientId: null }) - await buildService().create(NotificationTaskActions.Assigned, task, { disableEmail: false }) - - expect(mockGroupedCreateMany).not.toHaveBeenCalled() - expect(mockEnqueueFlush).not.toHaveBeenCalled() - const sent = mockCreateNotification.mock.calls[0][0] - expect(sent.recipientInternalUserId).toBe(task.assigneeId) - expect(sent.notificationSettingId).toBe('setting_assigned') - expect(deliveryTargetsOf(0).inProduct).toBeDefined() - expect(deliveryTargetsOf(0).email).toBeUndefined() - }) - - it('does NOT buffer the IU email when the category has no declared setting (email off until declared)', async () => { + it('still buffers the IU email but attaches no id when the category is not declared (gating deferred to flush)', async () => { mockGetNotificationSettings.mockResolvedValue({ notifications: [] }) const task = makeTask({ assigneeType: AssigneeType.internalUser, clientId: null }) await buildService().create(NotificationTaskActions.Assigned, task, { disableEmail: false }) - expect(mockGroupedCreateMany).not.toHaveBeenCalled() + expect(mockGroupedCreateMany).toHaveBeenCalledTimes(1) + expect(mockGroupedCreateMany.mock.calls[0][0].data[0].individualEmail.notificationSettingId).toBeUndefined() expect(mockCreateNotification.mock.calls[0][0].notificationSettingId).toBeUndefined() expect(deliveryTargetsOf(0).inProduct).toBeDefined() }) diff --git a/src/app/api/notification/notification.service.ts b/src/app/api/notification/notification.service.ts index 0668270e3..c5bdb5464 100644 --- a/src/app/api/notification/notification.service.ts +++ b/src/app/api/notification/notification.service.ts @@ -14,7 +14,7 @@ import APIError from '@api/core/exceptions/api' import { BaseService } from '@api/core/services/base.service' import { NotificationTaskActions } from '@api/core/types/tasks' import { getEmailDetails, getInProductNotificationDetails, mergeEmailOverride } from '@api/notification/notification.helpers' -import { resolveIuNotificationSetting } from '@api/notification/resolveNotificationSettingId' +import { resolveIuNotificationSettingId } from '@api/notification/resolveNotificationSettingId' import { AssigneeType, ClientNotification, GroupedEmailEventType, Prisma, Task } from '@prisma/client' import { randomUUID } from 'crypto' import { enqueueGroupedEmailFlush } from '@/jobs/notifications/flush-grouped-email' @@ -71,19 +71,16 @@ export class NotificationService extends BaseService { const baseEmail = opts.disableEmail ? undefined : getEmailDetails(workspace, actionUser, task, { commentId: opts?.commentId })[action] - const mergedEmail = baseEmail ? mergeEmailOverride({ base: baseEmail, override: opts.emailOverride }) : baseEmail + const email = baseEmail ? mergeEmailOverride({ base: baseEmail, override: opts.emailOverride }) : baseEmail const category = this.groupedEventTypeFor(action) - // IU sends carry the category's setting id so the platform gates the in-product surface per the - // IU's preference. The email surface is gated app-side: a grouped summary is cross-category and - // can't carry a per-category id, so an IU email is only buffered when the category's declared - // setting enables the email surface. - const iuSetting = + // IU sends carry the category's setting id. It gates the in-product surface per the IU's + // preference immediately, and rides along on the buffered email so a single-category grouped + // flush can gate the email too (see flush-grouped-email). + const notificationSettingId = isRecipientIu && category - ? await resolveIuNotificationSetting({ copilot: this.copilot, workspaceId: task.workspaceId, category }) + ? await resolveIuNotificationSettingId({ copilot: this.copilot, workspaceId: task.workspaceId, category }) : undefined - const notificationSettingId = iuSetting?.id - const email = isRecipientIu && iuSetting && !iuSetting.emailEnabled ? undefined : mergedEmail const groupedType = email && recipientId ? category : null if (groupedType) { @@ -192,7 +189,7 @@ export class NotificationService extends BaseService { const baseEmail = opts?.email ? getEmailDetails(workspace, actionUserName, task, { commentId: opts?.commentId })[action] : undefined - const mergedEmail = baseEmail ? mergeEmailOverride({ base: baseEmail, override: opts?.emailOverride }) : baseEmail + const email = baseEmail ? mergeEmailOverride({ base: baseEmail, override: opts?.emailOverride }) : baseEmail // Get a list of all notifications dispatched for these taskId, clientId, companyId combinations // This will be used to filter out any duplicate notifications during creation @@ -211,14 +208,12 @@ export class NotificationService extends BaseService { const association = AssociationsSchema.parse(task.associations)?.[0] const category = this.groupedEventTypeFor(action) const isRecipientIu = opts.isRecipientIu - // Resolve once per batch (not per recipient). IU sends carry the setting id so the platform - // gates the in-product surface; the email surface is gated app-side (see create()). - const iuSetting = + // Resolve once per batch (not per recipient). The id gates the in-product surface per IU and + // rides along on the buffered email for the flush-time single-category gate (see create()). + const notificationSettingId = isRecipientIu && category - ? await resolveIuNotificationSetting({ copilot: this.copilot, workspaceId: task.workspaceId, category }) + ? await resolveIuNotificationSettingId({ copilot: this.copilot, workspaceId: task.workspaceId, category }) : undefined - const notificationSettingId = iuSetting?.id - const email = isRecipientIu && iuSetting && !iuSetting.emailEnabled ? undefined : mergedEmail // Non-null only when these emails should be diverted into the grouped buffer. const groupedType = email ? category : null @@ -663,7 +658,7 @@ export class NotificationService extends BaseService { } } - private async bufferGroupedEmailEvent(args: { + async bufferGroupedEmailEvent(args: { task: Task recipientId: string companyId?: string diff --git a/src/app/api/notification/resolveNotificationSettingId.test.ts b/src/app/api/notification/resolveNotificationSettingId.test.ts index 128fdb7a9..9e664b4d0 100644 --- a/src/app/api/notification/resolveNotificationSettingId.test.ts +++ b/src/app/api/notification/resolveNotificationSettingId.test.ts @@ -1,6 +1,6 @@ import { CopilotAPI } from '@/utils/CopilotAPI' import { GroupedEmailEventType } from '@prisma/client' -import { __clearNotificationSettingCache, resolveIuNotificationSetting } from './resolveNotificationSettingId' +import { __clearNotificationSettingCache, resolveIuNotificationSettingId } from './resolveNotificationSettingId' const buildCopilot = (getNotificationSettings: jest.Mock) => ({ getNotificationSettings }) as unknown as CopilotAPI @@ -12,26 +12,19 @@ const settings = [ beforeEach(() => __clearNotificationSettingCache()) -describe('resolveIuNotificationSetting', () => { - it('maps each category to its declared setting by label, with email enabled', async () => { +describe('resolveIuNotificationSettingId', () => { + it('maps each category to its declared setting id by label', async () => { const copilot = buildCopilot(jest.fn().mockResolvedValue({ notifications: settings })) expect( - await resolveIuNotificationSetting({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.ASSIGNED }), - ).toEqual({ id: 'setting_assigned', emailEnabled: true }) + await resolveIuNotificationSettingId({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.ASSIGNED }), + ).toBe('setting_assigned') expect( - await resolveIuNotificationSetting({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.COMMENT }), - ).toEqual({ id: 'setting_comment', emailEnabled: true }) - }) - - it('reports emailEnabled=false when the declared setting omits the email surface', async () => { - const copilot = buildCopilot( - jest.fn().mockResolvedValue({ notifications: [{ id: 'x', label: 'New task assigned', surfaces: ['product'] }] }), - ) - + await resolveIuNotificationSettingId({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.COMMENT }), + ).toBe('setting_comment') expect( - await resolveIuNotificationSetting({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.ASSIGNED }), - ).toEqual({ id: 'x', emailEnabled: false }) + await resolveIuNotificationSettingId({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.COMPLETED }), + ).toBe('setting_completed') }) it('matches labels case-insensitively and ignoring surrounding whitespace', async () => { @@ -40,37 +33,37 @@ describe('resolveIuNotificationSetting', () => { ) expect( - await resolveIuNotificationSetting({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.ASSIGNED }), - ).toEqual({ id: 'x', emailEnabled: true }) + await resolveIuNotificationSettingId({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.ASSIGNED }), + ).toBe('x') }) - it('returns id undefined and emailEnabled false when the category is not declared', async () => { + it('returns undefined when the category is not declared', async () => { const copilot = buildCopilot(jest.fn().mockResolvedValue({ notifications: [settings[0]] })) expect( - await resolveIuNotificationSetting({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.COMMENT }), - ).toEqual({ id: undefined, emailEnabled: false }) + await resolveIuNotificationSettingId({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.COMMENT }), + ).toBeUndefined() }) - it('returns emailEnabled false for SHARED (no IU setting declared)', async () => { + it('returns undefined for SHARED (no IU setting declared)', async () => { const copilot = buildCopilot(jest.fn().mockResolvedValue({ notifications: settings })) expect( - await resolveIuNotificationSetting({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.SHARED }), - ).toEqual({ id: undefined, emailEnabled: false }) + await resolveIuNotificationSettingId({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.SHARED }), + ).toBeUndefined() }) - it('caches the settings per workspace and does not refetch within the TTL', async () => { + it('caches the label map per workspace and does not refetch within the TTL', async () => { const getNotificationSettings = jest.fn().mockResolvedValue({ notifications: settings }) const copilot = buildCopilot(getNotificationSettings) - await resolveIuNotificationSetting({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.ASSIGNED }) - await resolveIuNotificationSetting({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.COMMENT }) + await resolveIuNotificationSettingId({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.ASSIGNED }) + await resolveIuNotificationSettingId({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.COMMENT }) expect(getNotificationSettings).toHaveBeenCalledTimes(1) }) - it('fails closed (no id, emailEnabled false) when the fetch fails, without caching the failure', async () => { + it('returns undefined when the fetch fails, without caching the failure', async () => { const getNotificationSettings = jest .fn() .mockRejectedValueOnce(new Error('copilot 5xx')) @@ -78,11 +71,11 @@ describe('resolveIuNotificationSetting', () => { const copilot = buildCopilot(getNotificationSettings) expect( - await resolveIuNotificationSetting({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.ASSIGNED }), - ).toEqual({ id: undefined, emailEnabled: false }) + await resolveIuNotificationSettingId({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.ASSIGNED }), + ).toBeUndefined() expect( - await resolveIuNotificationSetting({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.ASSIGNED }), - ).toEqual({ id: 'setting_assigned', emailEnabled: true }) + await resolveIuNotificationSettingId({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.ASSIGNED }), + ).toBe('setting_assigned') expect(getNotificationSettings).toHaveBeenCalledTimes(2) }) @@ -90,8 +83,8 @@ describe('resolveIuNotificationSetting', () => { const getNotificationSettings = jest.fn().mockResolvedValue({ notifications: settings }) const copilot = buildCopilot(getNotificationSettings) - await resolveIuNotificationSetting({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.ASSIGNED }) - await resolveIuNotificationSetting({ copilot, workspaceId: 'ws_2', category: GroupedEmailEventType.ASSIGNED }) + await resolveIuNotificationSettingId({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.ASSIGNED }) + await resolveIuNotificationSettingId({ copilot, workspaceId: 'ws_2', category: GroupedEmailEventType.ASSIGNED }) expect(getNotificationSettings).toHaveBeenCalledTimes(2) }) diff --git a/src/app/api/notification/resolveNotificationSettingId.ts b/src/app/api/notification/resolveNotificationSettingId.ts index 705a5cd81..b0f8c1018 100644 --- a/src/app/api/notification/resolveNotificationSettingId.ts +++ b/src/app/api/notification/resolveNotificationSettingId.ts @@ -1,48 +1,45 @@ -import { NotificationSetting } from '@/types/common' import { CopilotAPI } from '@/utils/CopilotAPI' import { serializeError } from '@/utils/serializeError' import { GroupedEmailEventType } from '@prisma/client' // Canonical labels the Tasks app declares on its Assembly app record (App Setup > Notifications). +// Must match the declared setting labels exactly. SHARED is absent — shared notifications only ever +// target clients, never IUs. export const IU_NOTIFICATION_LABELS: Partial> = { [GroupedEmailEventType.ASSIGNED]: 'New task assigned', [GroupedEmailEventType.COMMENT]: 'New comment on a task', [GroupedEmailEventType.COMPLETED]: 'Task completed', } -// Passed on IU sends so the platform gates each requested surface against the IU's preference. -export type IuNotificationSetting = { - id: string | undefined - emailEnabled: boolean -} - -// Cache the declared settings per workspace, keyed by normalized label. +// Cache only the stable label -> id map per workspace (ids are stable per the platform docs). We +// never cache an IU's on/off preference — the platform evaluates that on every send. The TTL only +// bounds how long a newly declared setting's id takes to be picked up. const CACHE_TTL_MS = 5 * 60 * 1000 -const cache = new Map; expiresAt: number }>() +const cache = new Map; expiresAt: number }>() const normalize = (label: string): string => label.trim().toLowerCase() -const getSettingsByLabel = async ({ +const getLabelToId = async ({ copilot, workspaceId, }: { copilot: CopilotAPI workspaceId: string -}): Promise> => { +}): Promise> => { const cached = cache.get(workspaceId) - if (cached && cached.expiresAt > Date.now()) return cached.byLabel + if (cached && cached.expiresAt > Date.now()) return cached.labelToId const { notifications } = await copilot.getNotificationSettings() - const byLabel = new Map(notifications.map((setting) => [normalize(setting.label), setting])) - cache.set(workspaceId, { byLabel, expiresAt: Date.now() + CACHE_TTL_MS }) - return byLabel + const labelToId = new Map(notifications.map((setting) => [normalize(setting.label), setting.id])) + cache.set(workspaceId, { labelToId, expiresAt: Date.now() + CACHE_TTL_MS }) + return labelToId } -// Resolve the declared setting for an IU notification category. Returns emailEnabled=false when the -// category has no declared setting (email is not sent until it's configured in the dashboard) and, -// fail-closed, on fetch failure too — we withhold IU email rather than risk sending it past a -// preference we couldn't read. Failures are not cached, so the next send retries. -export const resolveIuNotificationSetting = async ({ +// Resolve the declared setting id for an IU notification category so a send can pass it and let the +// platform gate each requested surface per the IU's preference. Returns undefined when the category +// isn't declared or the fetch fails — callers then send without an id (no per-IU gating on that +// send). Failures are not cached, so the next send retries. +export const resolveIuNotificationSettingId = async ({ copilot, workspaceId, category, @@ -50,18 +47,16 @@ export const resolveIuNotificationSetting = async ({ copilot: CopilotAPI workspaceId: string category: GroupedEmailEventType -}): Promise => { +}): Promise => { const label = IU_NOTIFICATION_LABELS[category] - if (!label) return { id: undefined, emailEnabled: false } + if (!label) return undefined try { - const byLabel = await getSettingsByLabel({ copilot, workspaceId }) - const setting = byLabel.get(normalize(label)) - if (!setting) return { id: undefined, emailEnabled: false } - return { id: setting.id, emailEnabled: setting.surfaces.includes('email') } //right now we are using setting.surfaces. We need support from assembly to expose a prop which indicates if email is turned on for the event. + const labelToId = await getLabelToId({ copilot, workspaceId }) + return labelToId.get(normalize(label)) } catch (e) { - console.error('resolveIuNotificationSetting | failed to resolve; withholding IU email', serializeError(e)) - return { id: undefined, emailEnabled: false } + console.error('resolveIuNotificationSettingId | failed to resolve; sending without gating', serializeError(e)) + return undefined } } diff --git a/src/jobs/notifications/flush-grouped-email.test.ts b/src/jobs/notifications/flush-grouped-email.test.ts index 3f7565a7a..ec3bcfafe 100644 --- a/src/jobs/notifications/flush-grouped-email.test.ts +++ b/src/jobs/notifications/flush-grouped-email.test.ts @@ -132,6 +132,35 @@ describe('flushGroupedEmailRun', () => { expect(result).toMatchObject({ recipients: 1, sent: 1, sentGrouped: 1 }) }) + const iuRow = (settingId?: string) => + row({ + recipientClientId: null, + recipientCompanyId: null, + recipientIuId: 'iu_recipient', + individualEmail: { + senderId: 'actor_1', + recipientInternalUserId: 'iu_recipient', + notificationSettingId: settingId, + deliveryTargets: { email: { subject: 'A task' } }, + }, + }) + + it('gates an IU grouped summary with the setting id when the whole window is one category', async () => { + mockQueryRaw.mockResolvedValue([iuRow('setting_comment'), iuRow('setting_comment')]) + + await flushGroupedEmailRun(payload) + + expect(mockSendGroupedEmail.mock.calls[0][0]).toMatchObject({ notificationSettingId: 'setting_comment' }) + }) + + it('sends an IU grouped summary without an id when the window mixes categories', async () => { + mockQueryRaw.mockResolvedValue([iuRow('setting_assigned'), iuRow('setting_comment')]) + + await flushGroupedEmailRun(payload) + + expect(mockSendGroupedEmail.mock.calls[0][0].notificationSettingId).toBeUndefined() + }) + it('replays the original individual email when the window has a single live event', async () => { mockQueryRaw.mockResolvedValue([row()]) diff --git a/src/jobs/notifications/flush-grouped-email.ts b/src/jobs/notifications/flush-grouped-email.ts index c776d4c37..df4894145 100644 --- a/src/jobs/notifications/flush-grouped-email.ts +++ b/src/jobs/notifications/flush-grouped-email.ts @@ -87,6 +87,21 @@ const senderFromEvents = (events: WindowEvent[]): EventSender | undefined => { return { senderId: email.senderId, senderType: email.senderType, senderCompanyId: email.senderCompanyId } } +// Seam for per-IU category filtering. Today it's a pass-through: the platform can't gate a mixed +// grouped summary, and there's no API to read an IU's per-setting preferences. Once Assembly exposes +// that endpoint, fetch the recipient IU's prefs here and drop events for disabled categories so the +// summary only carries allowed ones (an all-disabled recipient then sends nothing). +// TODO(OUT-3929): implement per-IU filtering when the read endpoint lands. +const filterEventsForIuPreferences = (events: WindowEvent[]): WindowEvent[] => events + +// The platform can only gate a send that carries one setting id. A grouped summary gets an id only +// when every live event shares the same one (i.e. a single-category window); a mixed window sends +// without an id and is not gated per-IU until the filter seam above is implemented. +const singleCategorySettingId = (events: WindowEvent[]): string | undefined => { + const ids = events.map((e) => e.individualEmail?.notificationSettingId) + return ids.every((id) => id && id === ids[0]) ? (ids[0] ?? undefined) : undefined +} + const sendIndividualEmail = async (copilot: CopilotAPI, payload: NotificationRequestBody): Promise => { try { await copilot.createNotification(payload) @@ -216,7 +231,8 @@ export const flushGroupedEmailRun = async (payload: FlushGroupedEmailPayload) => } for (const group of iuGroups) { - const liveEvents = group.events.filter((e) => liveTaskIds.has(e.taskId)) + const liveEvents = filterEventsForIuPreferences(group.events.filter((e) => liveTaskIds.has(e.taskId))) + // A single event replays its buffered email verbatim (it already carries its own setting id). const singleEmail = liveEvents.length === 1 ? liveEvents[0].individualEmail : null Sentry.addBreadcrumb({ @@ -237,6 +253,8 @@ export const flushGroupedEmailRun = async (payload: FlushGroupedEmailPayload) => senderType: sender?.senderType, senderCompanyId: sender?.senderCompanyId, recipientInternalUserId: group.recipientIuId, + // Gate the summary per-IU only when the whole window is one category. + notificationSettingId: singleCategorySettingId(liveEvents), copilot, }) sent += 1 diff --git a/src/jobs/notifications/send-grouped-email.ts b/src/jobs/notifications/send-grouped-email.ts index 93588d6c9..086b8f3ee 100644 --- a/src/jobs/notifications/send-grouped-email.ts +++ b/src/jobs/notifications/send-grouped-email.ts @@ -16,6 +16,9 @@ export type SendGroupedEmailArgs = { recipientClientId?: string | null recipientCompanyId?: string | null recipientInternalUserId?: string | null + // Set only for a single-category IU window so the platform can gate this summary per the IU's + // preference. A mixed-category window carries no id (can't gate against one setting). + notificationSettingId?: string copilot: CopilotAPI } @@ -27,6 +30,7 @@ export const sendGroupedEmail = async ({ recipientClientId, recipientCompanyId, recipientInternalUserId, + notificationSettingId, copilot, }: SendGroupedEmailArgs): Promise => { const email = renderGroupedEmail(content) @@ -38,6 +42,7 @@ export const sendGroupedEmail = async ({ recipientClientId: recipientClientId ?? undefined, recipientCompanyId: recipientCompanyId ?? undefined, recipientInternalUserId: recipientInternalUserId ?? undefined, + notificationSettingId, deliveryTargets: { email: { subject: email.subject, diff --git a/src/jobs/notifications/send-reply-create-notifications.ts b/src/jobs/notifications/send-reply-create-notifications.ts index 8e0a34f4c..dc468bd1d 100644 --- a/src/jobs/notifications/send-reply-create-notifications.ts +++ b/src/jobs/notifications/send-reply-create-notifications.ts @@ -5,7 +5,8 @@ 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 { resolveIuNotificationSetting } from '@/app/api/notification/resolveNotificationSettingId' +import { NotificationService } from '@/app/api/notification/notification.service' +import { resolveIuNotificationSettingId } from '@/app/api/notification/resolveNotificationSettingId' import User from '@api/core/models/User.model' import { TasksService } from '@api/tasks/tasks.service' import { Comment, CommentInitiator, GroupedEmailEventType, Task } from '@prisma/client' @@ -36,6 +37,7 @@ export const sendReplyCreateNotifications = task({ const commentsRepo = new CommentRepository(user) const copilot = new CopilotAPI(user.token) + const notificationService = new NotificationService(user) const senderId = z .string() @@ -47,20 +49,37 @@ export const sendReplyCreateNotifications = task({ const deliveryTargets = await getNotificationDetails(copilot, user, comment) - // Replies are the COMMENT category. IU sends carry this setting id so the platform gates both - // surfaces against the recipient IU's preference (this job dispatches directly, no grouping, so - // the platform handles the email surface too — no app-side surfaces check needed here). - const { id: notificationSettingId } = await resolveIuNotificationSetting({ + // Replies are the COMMENT category. Reply emails are buffered as COMMENT grouped events (like + // top-level comments), and IU sends carry this setting id so the platform gates each surface per + // the recipient IU's preference. + const notificationSettingId = await resolveIuNotificationSettingId({ copilot, workspaceId: user.workspaceId, category: GroupedEmailEventType.COMMENT, }) const notificationPromises: Promise[] = [] - const queueNotificationPromise = (promise: Promise): void => { + const queueNotificationPromise = (promise: Promise): void => { notificationPromises.push(copilotBottleneck.schedule(() => promise)) } + const shared = { + copilot, + notificationService, + task: payload.task, + senderId, + senderType, + senderCompanyId, + deliveryTargets, + commentId: comment.id, + // NOTE: We are sending payload.task.companyId here. This might sound silly, i agree. + // However, it is very safe to assume that client users can ONLY reply to comments in tasks + // assigned to their company, or to them. In both cases, payload.task.companyId works + // For IU tasks, this will be undefined + initiatorCompanyId: payload.task.companyId || undefined, + notificationSettingId, + } + // Get all initiators involved in thread except the current user const threadInitiators = (await commentsRepo.getFirstCommentInitiators([comment.parentId], 10_000)).filter( (initiator) => initiator.initiatorId !== senderId, @@ -68,20 +87,7 @@ export const sendReplyCreateNotifications = task({ // Queue notifications to every unique reply initiator for (let initiator of threadInitiators) { - const promise = getInitiatorNotificationPromises({ - copilot, - initiator, - senderId, - senderType, - senderCompanyId, - deliveryTargets, - // NOTE: We are sending payload.task.companyId here. This might sound silly, i agree. - // However, it is very safe to assume that client users can ONLY reply to comments in tasks - // assigned to their company, or to them. In both cases, payload.task.companyId works - // For IU tasks, this will be undefined - initiatorCompanyId: payload.task.companyId || undefined, - notificationSettingId, - }) + const promise = getInitiatorNotificationPromises({ ...shared, initiator }) promise && queueNotificationPromise(promise) // It's certain we will get a promise here } @@ -98,29 +104,9 @@ export const sendReplyCreateNotifications = task({ (initiator) => initiator.initiatorId === parentComment.initiatorId, ) if (!isParentCommentDeleted && !parentInitiatorIsCurrentUser && !isNotificationAlreadySent) { - const typedPromise = getInitiatorNotificationPromises({ - copilot, - initiator: parentComment, - senderId, - senderType, - senderCompanyId, - deliveryTargets, - initiatorCompanyId: payload.task.companyId || undefined, - notificationSettingId, - }) + const typedPromise = getInitiatorNotificationPromises({ ...shared, initiator: parentComment }) // If there is no "initiatorType" for parentComment we have to be slightly creative (coughhackycough) - const promise = - typedPromise ?? - getNotificationToUntypedInitiator( - copilot, - parentComment, - payload.task, - senderId, - senderType, - senderCompanyId, - deliveryTargets, - notificationSettingId, - ) + const promise = typedPromise ?? getNotificationToUntypedInitiator({ ...shared, parentComment }) queueNotificationPromise(promise) } } @@ -161,53 +147,81 @@ const getNotificationDetails = async (copilot: CopilotAPI, user: User, comment: return deliveryTargets } +type ReplyDispatchArgs = { + copilot: CopilotAPI + notificationService: NotificationService + task: Task + // Initiator in this context means previous initiators that were active in the thread, NOT the currently commenting user + initiator: { initiatorId: string; initiatorType: CommentInitiator | null } + senderId: string + senderType: NotificationSender + senderCompanyId: string | undefined + deliveryTargets: { inProduct: Record<'title', any>; email: object } + initiatorCompanyId?: string + commentId: string + // Forces recipient branch when initiator.initiatorType is unset (legacy comments) + assume?: CommentInitiator + notificationSettingId?: string +} + const getInitiatorNotificationPromises = ({ copilot, - // Initiator in this context means previous initiators that were active in the thread, NOT the currently commenting user + notificationService, + task: parentTask, initiator, senderId, senderType, senderCompanyId, deliveryTargets, initiatorCompanyId, - // Forces recipient branch when initiator.initiatorType is unset (legacy comments) + commentId, assume, notificationSettingId, -}: { - copilot: CopilotAPI - initiator: { initiatorId: string; initiatorType: CommentInitiator | null } - senderId: string - senderType: NotificationSender - senderCompanyId: string | undefined - deliveryTargets: { inProduct: Record<'title', any>; email: object } - initiatorCompanyId?: string - assume?: CommentInitiator - notificationSettingId?: string -}) => { +}: ReplyDispatchArgs) => { const base = { senderId, senderType, senderCompanyId } - let body: NotificationRequestBody - if (initiator.initiatorType === CommentInitiator.internalUser || assume === CommentInitiator.internalUser) { - body = { + const isIu = initiator.initiatorType === CommentInitiator.internalUser || assume === CommentInitiator.internalUser + const isClient = initiator.initiatorType === CommentInitiator.client || assume === CommentInitiator.client + if (!isIu && !isClient) return null + + if (isIu) { + // IU: fire the in-product notification now (the platform gates it per the IU's preference via the + // setting id) and buffer the email as a COMMENT event so it groups + gates like top-level comments. + const iuBase = { ...base, recipientInternalUserId: initiator.initiatorId, - // Both surfaces are always requested; the platform gates them per the IU's preference. ...(notificationSettingId ? { notificationSettingId } : {}), - deliveryTargets: { - inProduct: deliveryTargets.inProduct, - email: deliveryTargets.email, - }, - } - } else if (initiator.initiatorType === CommentInitiator.client || assume === CommentInitiator.client) { - body = { - ...base, - recipientClientId: initiator.initiatorId, - recipientCompanyId: initiatorCompanyId, - deliveryTargets: { email: deliveryTargets.email }, } - } else { - return null + const inProductBody: NotificationRequestBody = { ...iuBase, deliveryTargets: { inProduct: deliveryTargets.inProduct } } + const emailBody: NotificationRequestBody = { ...iuBase, deliveryTargets: { email: deliveryTargets.email } } + return Promise.all([ + createNotificationWithCompanyFallback(copilot, inProductBody), + notificationService.bufferGroupedEmailEvent({ + task: parentTask, + recipientId: initiator.initiatorId, + isRecipientIu: true, + eventType: GroupedEmailEventType.COMMENT, + commentId, + individualEmail: emailBody, + }), + ]) } - return createNotificationWithCompanyFallback(copilot, body) + + // Client: buffer the reply email only (clients get no in-product reply notification today). + const clientEmailBody: NotificationRequestBody = { + ...base, + recipientClientId: initiator.initiatorId, + recipientCompanyId: initiatorCompanyId, + deliveryTargets: { email: deliveryTargets.email }, + } + return notificationService.bufferGroupedEmailEvent({ + task: parentTask, + recipientId: initiator.initiatorId, + companyId: initiatorCompanyId, + isRecipientIu: false, + eventType: GroupedEmailEventType.COMMENT, + commentId, + individualEmail: clientEmailBody, + }) } // Single-company workspaces reject senderCompanyId; retry without it on that specific error. @@ -223,35 +237,20 @@ const createNotificationWithCompanyFallback = async (copilot: CopilotAPI, body: } } -const getNotificationToUntypedInitiator = async ( - copilot: CopilotAPI, - parentComment: Comment, - task: Task, - senderId: string, - senderType: NotificationSender, - senderCompanyId: string | undefined, - deliveryTargets: { inProduct: Record<'title', any>; email: object }, - notificationSettingId?: string, -) => { - const shared = { - copilot, - initiator: parentComment, - senderId, - senderType, - senderCompanyId, - deliveryTargets, - initiatorCompanyId: task.companyId || undefined, - notificationSettingId, - } +const getNotificationToUntypedInitiator = async ({ + parentComment, + ...shared +}: Omit & { parentComment: Comment }) => { + const withInitiator = { ...shared, initiator: parentComment } try { - await copilot.getInternalUser(parentComment.initiatorId) + await shared.copilot.getInternalUser(parentComment.initiatorId) // `assume` guarantees a non-null promise - return getInitiatorNotificationPromises({ ...shared, assume: CommentInitiator.internalUser })! + return getInitiatorNotificationPromises({ ...withInitiator, assume: CommentInitiator.internalUser })! } catch (e) { console.error(e) } - return getInitiatorNotificationPromises({ ...shared, assume: CommentInitiator.client })!.catch((e) => { + return getInitiatorNotificationPromises({ ...withInitiator, assume: CommentInitiator.client })!.catch((e) => { console.error(e) return undefined }) diff --git a/src/utils/CopilotAPI.ts b/src/utils/CopilotAPI.ts index 71f6ac8d5..684605a41 100644 --- a/src/utils/CopilotAPI.ts +++ b/src/utils/CopilotAPI.ts @@ -350,7 +350,7 @@ export class CopilotAPI { // Declared notification settings for this app in the current workspace. async _getNotificationSettings(): Promise { - console.info('CopilotAPI#_getNotificationSettings', this.token) + console.info('CopilotAPI#_getNotificationSettings') const appId = z.string({ message: 'Missing AppID in environment' }).parse(APP_ID) const installs = await this.copilot.listAppInstalls() const install = installs.find((entry) => entry.appId === appId) From 68b151f895453c31fa64b311ec6c5ed4123510fb Mon Sep 17 00:00:00 2001 From: arpandhakal-lgtm Date: Wed, 8 Jul 2026 18:44:52 +0545 Subject: [PATCH 16/20] fix(notifications): break CommentToIU so it doesn't fall through to default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit getNotificationParties' CommentToIU case had no break and fell through to default, which strict-parses me().id and task.assigneeId — throwing (ZodError "Required") when me() returns null or the task is unassigned. The comment job only consumes recipientIds/senderCompanyId (both set in the CommentToIU case), so the fallthrough's values were unused anyway. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/app/api/notification/notification.service.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/app/api/notification/notification.service.ts b/src/app/api/notification/notification.service.ts index c5bdb5464..cf2928887 100644 --- a/src/app/api/notification/notification.service.ts +++ b/src/app/api/notification/notification.service.ts @@ -601,6 +601,7 @@ export class NotificationService extends BaseService { ) .map((iu) => iu.id) } + break default: const userInfo = await this.copilot.me() senderId = z.string().parse(userInfo?.id) From 49955e80fc216f753d859b9e53a46cc6c485d31a Mon Sep 17 00:00:00 2001 From: arpandhakal-lgtm Date: Wed, 8 Jul 2026 18:49:44 +0545 Subject: [PATCH 17/20] chore(notifications): log createNotification payload during email flush Logs the payload for both flush send paths (individual replay + grouped summary) via the trigger logger, for debugging IU/CU email delivery. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/jobs/notifications/flush-grouped-email.ts | 1 + src/jobs/notifications/send-grouped-email.ts | 2 ++ 2 files changed, 3 insertions(+) diff --git a/src/jobs/notifications/flush-grouped-email.ts b/src/jobs/notifications/flush-grouped-email.ts index df4894145..496ebc4c9 100644 --- a/src/jobs/notifications/flush-grouped-email.ts +++ b/src/jobs/notifications/flush-grouped-email.ts @@ -103,6 +103,7 @@ const singleCategorySettingId = (events: WindowEvent[]): string | undefined => { } const sendIndividualEmail = async (copilot: CopilotAPI, payload: NotificationRequestBody): Promise => { + logger.log('flush-grouped-email: createNotification payload (individual replay)', { payload }) try { await copilot.createNotification(payload) } catch (e: unknown) { diff --git a/src/jobs/notifications/send-grouped-email.ts b/src/jobs/notifications/send-grouped-email.ts index 086b8f3ee..c866dd5e6 100644 --- a/src/jobs/notifications/send-grouped-email.ts +++ b/src/jobs/notifications/send-grouped-email.ts @@ -5,6 +5,7 @@ import { renderGroupedEmail } from '@/app/api/notification/groupedEmail.renderer import { NotificationRequestBody, NotificationSender } from '@/types/common' import { isMessagableError } from '@/utils/copilotError' import { CopilotAPI } from '@/utils/CopilotAPI' +import { logger } from '@trigger.dev/sdk/v3' export type SendGroupedEmailArgs = { content: GroupedEmailContent @@ -53,6 +54,7 @@ export const sendGroupedEmail = async ({ }, } + logger.log('flush-grouped-email: createNotification payload (grouped summary)', { payload }) try { const notification = await copilot.createNotification(payload) return notification.id From b64baa5323b77b979a02255b54e1938dbaba3b13 Mon Sep 17 00:00:00 2001 From: arpandhakal-lgtm Date: Wed, 8 Jul 2026 21:41:52 +0545 Subject: [PATCH 18/20] fix(notifications): treat a suppressed notification as a no-op When a notificationSettingId is passed and the recipient IU has every requested surface turned off, the platform suppresses the notification: the call succeeds (2xx) with no created object. _createNotification now returns null in that case instead of throwing on the strict schema parse, and callers skip the DB save. Non-suppressible callers (reminders, webhook, backfill, validate-count) guard the null defensively (they never pass a settingId). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/app/api/notification/notification.service.test.ts | 11 +++++++++++ src/app/api/notification/notification.service.ts | 6 ++++-- .../validate-count/validateCount.service.ts | 4 +++- src/app/api/webhook/webhook.service.ts | 4 +++- src/cmd/backfill-missed-emails/index.ts | 1 + src/jobs/notifications/send-grouped-email.ts | 6 +++--- src/jobs/notifications/send-grouped-reminder-email.ts | 2 ++ src/jobs/notifications/send-reminder-email.ts | 2 ++ src/utils/CopilotAPI.ts | 9 ++++++++- 9 files changed, 37 insertions(+), 8 deletions(-) diff --git a/src/app/api/notification/notification.service.test.ts b/src/app/api/notification/notification.service.test.ts index a18e674bf..3d5e8b42f 100644 --- a/src/app/api/notification/notification.service.test.ts +++ b/src/app/api/notification/notification.service.test.ts @@ -465,6 +465,17 @@ describe('guard: IU notification setting gating', () => { expect(mockCreateNotification.mock.calls[0][0].notificationSettingId).toBeUndefined() expect(deliveryTargetsOf(0).inProduct).toBeDefined() }) + + it('treats a suppressed (null) createNotification response as a no-op — no throw, no save', async () => { + // Platform dropped the only requested surface for this IU (preference off) → no created object. + mockCreateNotification.mockResolvedValue(null) + const task = makeTask({ assigneeType: AssigneeType.internalUser, clientId: null }) + + const result = await buildService().create(NotificationTaskActions.Assigned, task, { disableEmail: false }) + + expect(mockCreateNotification).toHaveBeenCalledTimes(1) + expect(result).toBeUndefined() + }) }) describe('guard: IU completion emails', () => { diff --git a/src/app/api/notification/notification.service.ts b/src/app/api/notification/notification.service.ts index cf2928887..c2836798b 100644 --- a/src/app/api/notification/notification.service.ts +++ b/src/app/api/notification/notification.service.ts @@ -119,7 +119,7 @@ export class NotificationService extends BaseService { if (!inProduct && !notificationDetails.deliveryTargets?.email) return console.info('NotificationService#create | Creating single notification:', notificationDetails) - let notification: NotificationCreatedResponse + let notification: NotificationCreatedResponse | null try { notification = await this.copilot.createNotification(notificationDetails) } catch (e: unknown) { @@ -127,6 +127,8 @@ export class NotificationService extends BaseService { } console.info('NotificationService#create | Created single notification:', notification) + // Suppressed by the recipient IU's preference — nothing was created, so there's nothing to save. + if (!notification) return // 3. Save notification to ClientNotification or InternalUserNotification table. Check for notification.recipientClientId too if (task.assigneeType === AssigneeType.client && !!notification.recipientClientId && !opts.disableInProduct) { @@ -266,7 +268,7 @@ export class NotificationService extends BaseService { if (groupedType) notificationDetails.deliveryTargets = { inProduct } console.info('NotificationService#bulkCreate | Creating single notification:', notificationDetails) - let notification: NotificationCreatedResponse + let notification: NotificationCreatedResponse | null try { notification = await this.copilot.createNotification(notificationDetails) } catch (e: unknown) { diff --git a/src/app/api/notification/validate-count/validateCount.service.ts b/src/app/api/notification/validate-count/validateCount.service.ts index 4729a6238..349b989b6 100644 --- a/src/app/api/notification/validate-count/validateCount.service.ts +++ b/src/app/api/notification/validate-count/validateCount.service.ts @@ -171,8 +171,10 @@ export class ValidateCountService extends NotificationService { // Now track those to ClientNotifications table const newClientNotificationData = [] for (const i in newNotifications) { + const notification = newNotifications[i] + if (!notification) continue newClientNotificationData.push({ - notificationId: newNotifications[i].id, + notificationId: notification.id, taskId: tasksWithoutNotifications[i].id, clientId, companyId: tasksWithoutNotifications[i].companyId, diff --git a/src/app/api/webhook/webhook.service.ts b/src/app/api/webhook/webhook.service.ts index 4dbd35833..a73586890 100644 --- a/src/app/api/webhook/webhook.service.ts +++ b/src/app/api/webhook/webhook.service.ts @@ -121,9 +121,11 @@ class WebhookService extends BaseService { const insertPromises = [] const notificationService = new NotificationService(this.user) for (let i = 0; i < notifications.length; i++) { + const notification = notifications[i] + if (!notification) continue insertPromises.push( // This is assuming a 1:1 map for tasks and notifications - dbBottleneck.schedule(() => notificationService.addToClientNotifications(tasks[i], notifications[i])), + dbBottleneck.schedule(() => notificationService.addToClientNotifications(tasks[i], notification)), ) } await Promise.all(insertPromises) diff --git a/src/cmd/backfill-missed-emails/index.ts b/src/cmd/backfill-missed-emails/index.ts index b9678d985..1c30cba76 100644 --- a/src/cmd/backfill-missed-emails/index.ts +++ b/src/cmd/backfill-missed-emails/index.ts @@ -31,6 +31,7 @@ const dispatchNotification = async (copilot: CopilotAPI, taskId: string, payload // <<- Emails are triggered here. Proceed with caution ->> const notification = await copilot.createNotification(payload) + if (!notification) return await db.clientNotification.create({ data: { clientId: payload.recipientClientId!, diff --git a/src/jobs/notifications/send-grouped-email.ts b/src/jobs/notifications/send-grouped-email.ts index c866dd5e6..14b23f725 100644 --- a/src/jobs/notifications/send-grouped-email.ts +++ b/src/jobs/notifications/send-grouped-email.ts @@ -33,7 +33,7 @@ export const sendGroupedEmail = async ({ recipientInternalUserId, notificationSettingId, copilot, -}: SendGroupedEmailArgs): Promise => { +}: SendGroupedEmailArgs): Promise => { const email = renderGroupedEmail(content) const payload: NotificationRequestBody = { @@ -57,12 +57,12 @@ export const sendGroupedEmail = async ({ logger.log('flush-grouped-email: createNotification payload (grouped summary)', { payload }) try { const notification = await copilot.createNotification(payload) - return notification.id + return notification?.id } catch (e: unknown) { // Account for workspaces without multi-companies, which reject senderCompanyId (mirrors NotificationService). if (isMessagableError(e) && e.body?.message === 'sender company ID is invalid based on sender') { const notification = await copilot.createNotification({ ...payload, senderCompanyId: undefined }) - return notification.id + return notification?.id } throw e } diff --git a/src/jobs/notifications/send-grouped-reminder-email.ts b/src/jobs/notifications/send-grouped-reminder-email.ts index c8f78feb9..37cc33961 100644 --- a/src/jobs/notifications/send-grouped-reminder-email.ts +++ b/src/jobs/notifications/send-grouped-reminder-email.ts @@ -37,5 +37,7 @@ export const sendGroupedReminderEmail = async ({ } const notification = await copilot.createNotification(payload) + // Reminder emails never pass a notificationSettingId, so the platform can't suppress them. + if (!notification) throw new Error('sendGroupedReminderEmail: notification was unexpectedly suppressed') return notification.id } diff --git a/src/jobs/notifications/send-reminder-email.ts b/src/jobs/notifications/send-reminder-email.ts index dcbdaae04..b3f5d1ee5 100644 --- a/src/jobs/notifications/send-reminder-email.ts +++ b/src/jobs/notifications/send-reminder-email.ts @@ -64,5 +64,7 @@ export const sendReminderEmail = async ({ } const notification = await copilot.createNotification(payload) + // Reminder emails never pass a notificationSettingId, so the platform can't suppress them. + if (!notification) throw new Error('sendReminderEmail: notification was unexpectedly suppressed') return notification.id } diff --git a/src/utils/CopilotAPI.ts b/src/utils/CopilotAPI.ts index 684605a41..82e4ed7e6 100644 --- a/src/utils/CopilotAPI.ts +++ b/src/utils/CopilotAPI.ts @@ -230,7 +230,7 @@ export class CopilotAPI { return InternalUsersSchema.parse(await this.copilot.retrieveInternalUser({ id })) } - async _createNotification(requestBody: NotificationRequestBody): Promise { + async _createNotification(requestBody: NotificationRequestBody): Promise { console.info('CopilotAPI#_createNotification', this.token) // Direct REST call instead of the SDK: the SDK request type omits `deliveryTargets.email.htmlBody`, // so HTML email bodies only reach Copilot when we post the request body ourselves. @@ -246,6 +246,13 @@ export class CopilotAPI { method: 'POST', body: requestBody, }) + // When a notificationSettingId is supplied and the recipient IU has every requested surface + // turned off, the platform suppresses the notification: the call succeeds (2xx) but returns no + // created object. Treat that as a no-op rather than failing the schema parse. + if (!notification?.id) { + console.info('CopilotAPI#_createNotification | no notification created (suppressed by recipient preference)') + return null + } return NotificationCreatedResponseSchema.parse(notification) } From 88ada0ca125ee1c1595eceeaae360c8f4107edbd Mon Sep 17 00:00:00 2001 From: arpandhakal-lgtm Date: Wed, 8 Jul 2026 22:04:57 +0545 Subject: [PATCH 19/20] chore(notifications): ship IU notifications ungated for now Stop attaching notificationSettingId (set undefined; resolve calls commented) so IUs receive all email + in-product notifications without platform gating. Per-IU gating is blocked on Copilot exposing a preference-read endpoint; re-enable by restoring the commented resolve calls (and implementing flush-time preference filtering). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../notification/notification.service.test.ts | 26 ++++++------------- .../api/notification/notification.service.ts | 21 +++++---------- .../send-reply-create-notifications.ts | 14 ++++------ 3 files changed, 20 insertions(+), 41 deletions(-) diff --git a/src/app/api/notification/notification.service.test.ts b/src/app/api/notification/notification.service.test.ts index 3d5e8b42f..acda3543b 100644 --- a/src/app/api/notification/notification.service.test.ts +++ b/src/app/api/notification/notification.service.test.ts @@ -432,20 +432,21 @@ describe('guard: IU wiring boundaries', () => { }) }) -describe('guard: IU notification setting gating', () => { - it('resolves the id by action category and attaches it to both the buffered email and the in-product dispatch', async () => { +describe('guard: IU notifications ship ungated (settingId gating disabled)', () => { + it('does not attach notificationSettingId to the in-product dispatch or the buffered email', async () => { const task = makeTask({ assigneeType: AssigneeType.internalUser, clientId: null }) await buildService().create(NotificationTaskActions.Assigned, task, { disableEmail: false }) - expect(mockGroupedCreateMany.mock.calls[0][0].data[0].individualEmail.notificationSettingId).toBe('setting_assigned') - expect(mockCreateNotification.mock.calls[0][0].notificationSettingId).toBe('setting_assigned') + expect(mockCreateNotification.mock.calls[0][0].notificationSettingId).toBeUndefined() + expect(mockGroupedCreateMany.mock.calls[0][0].data[0].individualEmail.notificationSettingId).toBeUndefined() }) - it('resolves a different id per category (COMPLETED vs ASSIGNED)', async () => { + it('still buffers the IU email and fires the in-product notification', async () => { const task = makeTask({ assigneeType: AssigneeType.internalUser, clientId: null }) - await buildService().create(NotificationTaskActions.CompletedByIU, task, { disableEmail: false }) + await buildService().create(NotificationTaskActions.Assigned, task, { disableEmail: false }) - expect(mockCreateNotification.mock.calls[0][0].notificationSettingId).toBe('setting_completed') + expect(mockGroupedCreateMany).toHaveBeenCalledTimes(1) + expect(deliveryTargetsOf(0).inProduct).toBeDefined() }) it('keeps IU grouped windows cross-category (window key is not scoped by event type)', async () => { @@ -455,17 +456,6 @@ describe('guard: IU notification setting gating', () => { expect(mockGroupedCreateMany.mock.calls[0][0].data[0].windowKey).toMatch(new RegExp(`^${task.assigneeId}:iu:[^:]+$`)) }) - it('still buffers the IU email but attaches no id when the category is not declared (gating deferred to flush)', async () => { - mockGetNotificationSettings.mockResolvedValue({ notifications: [] }) - const task = makeTask({ assigneeType: AssigneeType.internalUser, clientId: null }) - await buildService().create(NotificationTaskActions.Assigned, task, { disableEmail: false }) - - expect(mockGroupedCreateMany).toHaveBeenCalledTimes(1) - expect(mockGroupedCreateMany.mock.calls[0][0].data[0].individualEmail.notificationSettingId).toBeUndefined() - expect(mockCreateNotification.mock.calls[0][0].notificationSettingId).toBeUndefined() - expect(deliveryTargetsOf(0).inProduct).toBeDefined() - }) - it('treats a suppressed (null) createNotification response as a no-op — no throw, no save', async () => { // Platform dropped the only requested surface for this IU (preference off) → no created object. mockCreateNotification.mockResolvedValue(null) diff --git a/src/app/api/notification/notification.service.ts b/src/app/api/notification/notification.service.ts index c2836798b..2505dde21 100644 --- a/src/app/api/notification/notification.service.ts +++ b/src/app/api/notification/notification.service.ts @@ -14,7 +14,7 @@ import APIError from '@api/core/exceptions/api' import { BaseService } from '@api/core/services/base.service' import { NotificationTaskActions } from '@api/core/types/tasks' import { getEmailDetails, getInProductNotificationDetails, mergeEmailOverride } from '@api/notification/notification.helpers' -import { resolveIuNotificationSettingId } from '@api/notification/resolveNotificationSettingId' +// import { resolveIuNotificationSettingId } from '@api/notification/resolveNotificationSettingId' import { AssigneeType, ClientNotification, GroupedEmailEventType, Prisma, Task } from '@prisma/client' import { randomUUID } from 'crypto' import { enqueueGroupedEmailFlush } from '@/jobs/notifications/flush-grouped-email' @@ -74,13 +74,9 @@ export class NotificationService extends BaseService { const email = baseEmail ? mergeEmailOverride({ base: baseEmail, override: opts.emailOverride }) : baseEmail const category = this.groupedEventTypeFor(action) - // IU sends carry the category's setting id. It gates the in-product surface per the IU's - // preference immediately, and rides along on the buffered email so a single-category grouped - // flush can gate the email too (see flush-grouped-email). - const notificationSettingId = - isRecipientIu && category - ? await resolveIuNotificationSettingId({ copilot: this.copilot, workspaceId: task.workspaceId, category }) - : undefined + // Gating disabled until Copilot exposes a per-IU preference read endpoint — ship IUs ungated. + const notificationSettingId = undefined + // const notificationSettingId = isRecipientIu && category ? await resolveIuNotificationSettingId({ copilot: this.copilot, workspaceId: task.workspaceId, category }) : undefined const groupedType = email && recipientId ? category : null if (groupedType) { @@ -210,12 +206,9 @@ export class NotificationService extends BaseService { const association = AssociationsSchema.parse(task.associations)?.[0] const category = this.groupedEventTypeFor(action) const isRecipientIu = opts.isRecipientIu - // Resolve once per batch (not per recipient). The id gates the in-product surface per IU and - // rides along on the buffered email for the flush-time single-category gate (see create()). - const notificationSettingId = - isRecipientIu && category - ? await resolveIuNotificationSettingId({ copilot: this.copilot, workspaceId: task.workspaceId, category }) - : undefined + // Gating disabled until Copilot exposes a per-IU preference read endpoint — ship IUs ungated. + const notificationSettingId = undefined + // const notificationSettingId = isRecipientIu && category ? await resolveIuNotificationSettingId({ copilot: this.copilot, workspaceId: task.workspaceId, category }) : undefined // Non-null only when these emails should be diverted into the grouped buffer. const groupedType = email ? category : null diff --git a/src/jobs/notifications/send-reply-create-notifications.ts b/src/jobs/notifications/send-reply-create-notifications.ts index dc468bd1d..812d34653 100644 --- a/src/jobs/notifications/send-reply-create-notifications.ts +++ b/src/jobs/notifications/send-reply-create-notifications.ts @@ -6,7 +6,7 @@ import { isMessagableError } from '@/utils/copilotError' import { CommentRepository } from '@/app/api/comments/comment.repository' import { CommentService } from '@/app/api/comments/comment.service' import { NotificationService } from '@/app/api/notification/notification.service' -import { resolveIuNotificationSettingId } from '@/app/api/notification/resolveNotificationSettingId' +// import { resolveIuNotificationSettingId } from '@/app/api/notification/resolveNotificationSettingId' import User from '@api/core/models/User.model' import { TasksService } from '@api/tasks/tasks.service' import { Comment, CommentInitiator, GroupedEmailEventType, Task } from '@prisma/client' @@ -49,14 +49,10 @@ export const sendReplyCreateNotifications = task({ const deliveryTargets = await getNotificationDetails(copilot, user, comment) - // Replies are the COMMENT category. Reply emails are buffered as COMMENT grouped events (like - // top-level comments), and IU sends carry this setting id so the platform gates each surface per - // the recipient IU's preference. - const notificationSettingId = await resolveIuNotificationSettingId({ - copilot, - workspaceId: user.workspaceId, - category: GroupedEmailEventType.COMMENT, - }) + // Replies are buffered as COMMENT grouped events like top-level comments. + // Gating disabled until Copilot exposes a per-IU preference read endpoint — ship IUs ungated. + const notificationSettingId = undefined + // const notificationSettingId = await resolveIuNotificationSettingId({ copilot, workspaceId: user.workspaceId, category: GroupedEmailEventType.COMMENT }) const notificationPromises: Promise[] = [] const queueNotificationPromise = (promise: Promise): void => { From 74c7b396c3ab3fb3ba046b7fe26eea9754194ba7 Mon Sep 17 00:00:00 2001 From: arpandhakal-lgtm Date: Mon, 13 Jul 2026 15:39:43 +0545 Subject: [PATCH 20/20] fix(OUT-3929): applied requested changes --- .../api/notification/notification.service.ts | 32 +++++++++---------- .../api/tasks/task-notifications.service.ts | 18 +++-------- 2 files changed, 20 insertions(+), 30 deletions(-) diff --git a/src/app/api/notification/notification.service.ts b/src/app/api/notification/notification.service.ts index 2505dde21..f65b28211 100644 --- a/src/app/api/notification/notification.service.ts +++ b/src/app/api/notification/notification.service.ts @@ -29,12 +29,12 @@ export class NotificationService extends BaseService { action: NotificationTaskActions, task: Task, opts: { - disableEmail: boolean + disableEmail?: boolean disableInProduct?: boolean commentId?: string senderCompanyId?: string emailOverride?: EmailNotificationDetails - } = { disableEmail: false }, + } = {}, ) { try { const isAssignedToIu = @@ -74,7 +74,7 @@ export class NotificationService extends BaseService { const email = baseEmail ? mergeEmailOverride({ base: baseEmail, override: opts.emailOverride }) : baseEmail const category = this.groupedEventTypeFor(action) - // Gating disabled until Copilot exposes a per-IU preference read endpoint — ship IUs ungated. + // TODO(OUT-3929): re-enable per-IU gating once Copilot exposes a preference-read endpoint — ship IUs ungated for now. const notificationSettingId = undefined // const notificationSettingId = isRecipientIu && category ? await resolveIuNotificationSettingId({ copilot: this.copilot, workspaceId: task.workspaceId, category }) : undefined @@ -115,12 +115,7 @@ export class NotificationService extends BaseService { if (!inProduct && !notificationDetails.deliveryTargets?.email) return console.info('NotificationService#create | Creating single notification:', notificationDetails) - let notification: NotificationCreatedResponse | null - try { - notification = await this.copilot.createNotification(notificationDetails) - } catch (e: unknown) { - notification = await this.handleIfSenderCompanyIdError(e, notificationDetails) - } + const notification = await this.dispatchNotification(notificationDetails) console.info('NotificationService#create | Created single notification:', notification) // Suppressed by the recipient IU's preference — nothing was created, so there's nothing to save. @@ -206,7 +201,7 @@ export class NotificationService extends BaseService { const association = AssociationsSchema.parse(task.associations)?.[0] const category = this.groupedEventTypeFor(action) const isRecipientIu = opts.isRecipientIu - // Gating disabled until Copilot exposes a per-IU preference read endpoint — ship IUs ungated. + // TODO(OUT-3929): re-enable per-IU gating once Copilot exposes a preference-read endpoint — ship IUs ungated for now. const notificationSettingId = undefined // const notificationSettingId = isRecipientIu && category ? await resolveIuNotificationSettingId({ copilot: this.copilot, workspaceId: task.workspaceId, category }) : undefined // Non-null only when these emails should be diverted into the grouped buffer. @@ -261,12 +256,7 @@ export class NotificationService extends BaseService { if (groupedType) notificationDetails.deliveryTargets = { inProduct } console.info('NotificationService#bulkCreate | Creating single notification:', notificationDetails) - let notification: NotificationCreatedResponse | null - try { - notification = await this.copilot.createNotification(notificationDetails) - } catch (e: unknown) { - notification = await this.handleIfSenderCompanyIdError(e, notificationDetails) - } + const notification = await this.dispatchNotification(notificationDetails) console.info('NotificationService#bulkCreate | Created single notification:', notification) if (!notification) { @@ -707,6 +697,16 @@ export class NotificationService extends BaseService { } } + private async dispatchNotification( + notificationDetails: NotificationRequestBody, + ): Promise { + try { + return await this.copilot.createNotification(notificationDetails) + } catch (e: unknown) { + return await this.handleIfSenderCompanyIdError(e, notificationDetails) + } + } + private async handleIfSenderCompanyIdError(e: unknown, notificationDetails: NotificationRequestBody) { // Account for workspaces that don't have multi-companies enabled, thus don't support the senderCompanyId key // Yes, this is hacky. No, I don't have a choice (I can't find out if workspace has single/multi company at all from the Copilot API) diff --git a/src/app/api/tasks/task-notifications.service.ts b/src/app/api/tasks/task-notifications.service.ts index 89049866d..bfefd01ff 100644 --- a/src/app/api/tasks/task-notifications.service.ts +++ b/src/app/api/tasks/task-notifications.service.ts @@ -287,15 +287,10 @@ export class TaskNotificationsService extends BaseService { else if (updatedTask.assigneeType === AssigneeType.company) { // Don't do this in parallel since this can cause rate-limits, each of them has their own bottlenecks for avoiding ratelimits shouldCreateNotification && - (await this.notificationService.create(NotificationTaskActions.CompletedForCompanyByIU, updatedTask, { - disableEmail: false, - })) + (await this.notificationService.create(NotificationTaskActions.CompletedForCompanyByIU, updatedTask)) await this.notificationService.markAsReadForAllRecipients(updatedTask) } else if (updatedTask.assigneeType === AssigneeType.client) { - shouldCreateNotification && - (await this.notificationService.create(NotificationTaskActions.CompletedByIU, updatedTask, { - disableEmail: false, - })) + shouldCreateNotification && (await this.notificationService.create(NotificationTaskActions.CompletedByIU, updatedTask)) try { await this.notificationService.markClientNotificationAsRead(updatedTask) return @@ -303,10 +298,7 @@ export class TaskNotificationsService extends BaseService { console.error(`Failed to find ClientNotification for task ${updatedTask.id}`, e) } } else if (updatedTask.assigneeType === AssigneeType.internalUser) { - shouldCreateNotification && - (await this.notificationService.create(NotificationTaskActions.CompletedByIU, updatedTask, { - disableEmail: false, - })) + shouldCreateNotification && (await this.notificationService.create(NotificationTaskActions.CompletedByIU, updatedTask)) } } @@ -406,7 +398,6 @@ export class TaskNotificationsService extends BaseService { const notification = await this.notificationService.create(notificationType, task, { disableInProduct: true, - disableEmail: false, }) // Create a new entry in ClientNotifications table so we can mark as read on // behalf of client later @@ -432,7 +423,6 @@ export class TaskNotificationsService extends BaseService { private sendUserTaskCompletedSharedNotification = async (task: Task) => { const notification = await this.notificationService.create(NotificationTaskActions.CompletedToSharedCU, task, { disableInProduct: true, - disableEmail: false, }) if (!notification) { console.error('Completed-shared notification failed to trigger for task:', task) @@ -491,7 +481,7 @@ export class TaskNotificationsService extends BaseService { // In future when reassignment is supported, change this logic to support reassigned to client as well notificationType, task, - { disableEmail: false, emailOverride }, + { emailOverride }, ) // Create a new entry in ClientNotifications table so we can mark as read on // behalf of client later