From cc6a6d26749655de0aae5a84d2a4423ee5815b72 Mon Sep 17 00:00:00 2001 From: arpandhakal-lgtm Date: Wed, 8 Jul 2026 15:35:19 +0545 Subject: [PATCH 1/7] 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 2/7] 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 3/7] 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 4/7] 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 5/7] 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 6/7] 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 7/7] 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