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..acda3543b 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,42 @@ describe('guard: IU wiring boundaries', () => { }) }) +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(mockCreateNotification.mock.calls[0][0].notificationSettingId).toBeUndefined() + expect(mockGroupedCreateMany.mock.calls[0][0].data[0].individualEmail.notificationSettingId).toBeUndefined() + }) + + 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.Assigned, task, { disableEmail: false }) + + 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 () => { + 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('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', () => { // 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..f65b28211 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 { 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' @@ -28,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 = @@ -72,7 +73,12 @@ export class NotificationService extends BaseService { : getEmailDetails(workspace, actionUser, task, { commentId: opts?.commentId })[action] const email = baseEmail ? mergeEmailOverride({ base: baseEmail, override: opts.emailOverride }) : baseEmail - const groupedType = email && recipientId ? this.groupedEventTypeFor(action) : null + const category = this.groupedEventTypeFor(action) + // 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 + + const groupedType = email && recipientId ? category : null if (groupedType) { const association = AssociationsSchema.parse(task.associations)?.[0] await this.bufferGroupedEmailEvent({ @@ -89,6 +95,7 @@ export class NotificationService extends BaseService { { email }, senderCompanyId, isRecipientIu, + notificationSettingId, ), }) } @@ -102,19 +109,17 @@ export class NotificationService extends BaseService { { inProduct, email }, senderCompanyId, isRecipientIu, + notificationSettingId, ) if (groupedType) notificationDetails.deliveryTargets = { inProduct } if (!inProduct && !notificationDetails.deliveryTargets?.email) return console.info('NotificationService#create | Creating single notification:', notificationDetails) - let notification: NotificationCreatedResponse - 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. + 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) { @@ -194,9 +199,13 @@ 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 + // 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. + 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 +236,7 @@ export class NotificationService extends BaseService { { email }, opts?.senderCompanyId, isRecipientIu, + notificationSettingId, ), }) if (!inProduct) continue @@ -241,16 +251,12 @@ export class NotificationService extends BaseService { { inProduct, email }, opts?.senderCompanyId, isRecipientIu, + notificationSettingId, ) if (groupedType) notificationDetails.deliveryTargets = { inProduct } console.info('NotificationService#bulkCreate | Creating single notification:', notificationDetails) - let notification: NotificationCreatedResponse - 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) { @@ -580,6 +586,7 @@ export class NotificationService extends BaseService { ) .map((iu) => iu.id) } + break default: const userInfo = await this.copilot.me() senderId = z.string().parse(userInfo?.id) @@ -637,7 +644,7 @@ export class NotificationService extends BaseService { } } - private async bufferGroupedEmailEvent(args: { + async bufferGroupedEmailEvent(args: { task: Task recipientId: string companyId?: string @@ -690,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) @@ -715,6 +732,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 +750,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..9e664b4d0 --- /dev/null +++ b/src/app/api/notification/resolveNotificationSettingId.test.ts @@ -0,0 +1,91 @@ +import { CopilotAPI } from '@/utils/CopilotAPI' +import { GroupedEmailEventType } from '@prisma/client' +import { __clearNotificationSettingCache, resolveIuNotificationSettingId } 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('resolveIuNotificationSettingId', () => { + it('maps each category to its declared setting id by label', async () => { + const copilot = buildCopilot(jest.fn().mockResolvedValue({ notifications: settings })) + + expect( + await resolveIuNotificationSettingId({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.ASSIGNED }), + ).toBe('setting_assigned') + expect( + await resolveIuNotificationSettingId({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.COMMENT }), + ).toBe('setting_comment') + expect( + await resolveIuNotificationSettingId({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.COMPLETED }), + ).toBe('setting_completed') + }) + + 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 resolveIuNotificationSettingId({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.ASSIGNED }), + ).toBe('x') + }) + + it('returns undefined when the category is not declared', async () => { + const copilot = buildCopilot(jest.fn().mockResolvedValue({ notifications: [settings[0]] })) + + expect( + await resolveIuNotificationSettingId({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.COMMENT }), + ).toBeUndefined() + }) + + it('returns undefined for SHARED (no IU setting declared)', async () => { + const copilot = buildCopilot(jest.fn().mockResolvedValue({ notifications: settings })) + + expect( + await resolveIuNotificationSettingId({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.SHARED }), + ).toBeUndefined() + }) + + 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 resolveIuNotificationSettingId({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.ASSIGNED }) + await resolveIuNotificationSettingId({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.COMMENT }) + + expect(getNotificationSettings).toHaveBeenCalledTimes(1) + }) + + it('returns undefined 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 resolveIuNotificationSettingId({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.ASSIGNED }), + ).toBeUndefined() + expect( + await resolveIuNotificationSettingId({ copilot, workspaceId: 'ws_1', category: GroupedEmailEventType.ASSIGNED }), + ).toBe('setting_assigned') + expect(getNotificationSettings).toHaveBeenCalledTimes(2) + }) + + it('caches per workspace independently', async () => { + const getNotificationSettings = jest.fn().mockResolvedValue({ notifications: settings }) + const copilot = buildCopilot(getNotificationSettings) + + 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 new file mode 100644 index 000000000..b0f8c1018 --- /dev/null +++ b/src/app/api/notification/resolveNotificationSettingId.ts @@ -0,0 +1,64 @@ +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', +} + +// 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 normalize = (label: string): string => label.trim().toLowerCase() + +const getLabelToId = async ({ + copilot, + workspaceId, +}: { + copilot: CopilotAPI + workspaceId: string +}): Promise> => { + const cached = cache.get(workspaceId) + if (cached && cached.expiresAt > Date.now()) return cached.labelToId + + const { notifications } = await copilot.getNotificationSettings() + 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 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, +}: { + copilot: CopilotAPI + workspaceId: string + category: GroupedEmailEventType +}): Promise => { + const label = IU_NOTIFICATION_LABELS[category] + if (!label) return undefined + + try { + const labelToId = await getLabelToId({ copilot, workspaceId }) + return labelToId.get(normalize(label)) + } catch (e) { + console.error('resolveIuNotificationSettingId | failed to resolve; sending without gating', serializeError(e)) + return undefined + } +} + +// Test seam: clear the per-workspace cache between cases. +export const __clearNotificationSettingCache = (): void => cache.clear() 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/tasks/task-notifications.service.ts b/src/app/api/tasks/task-notifications.service.ts index ffbf1dc4d..bfefd01ff 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' @@ -288,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: !isIuEmailEnabled(), - })) + (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: !isIuEmailEnabled(), - })) + shouldCreateNotification && (await this.notificationService.create(NotificationTaskActions.CompletedByIU, updatedTask)) try { await this.notificationService.markClientNotificationAsRead(updatedTask) return @@ -304,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: !isIuEmailEnabled(), - })) + shouldCreateNotification && (await this.notificationService.create(NotificationTaskActions.CompletedByIU, updatedTask)) } } @@ -372,7 +363,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 +374,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) @@ -407,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 @@ -433,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) @@ -492,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: task.assigneeType === AssigneeType.internalUser && !isIuEmailEnabled(), emailOverride }, + { emailOverride }, ) // Create a new entry in ClientNotifications table so we can mark as read on // behalf of client later 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/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/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..496ebc4c9 100644 --- a/src/jobs/notifications/flush-grouped-email.ts +++ b/src/jobs/notifications/flush-grouped-email.ts @@ -87,7 +87,23 @@ 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 => { + logger.log('flush-grouped-email: createNotification payload (individual replay)', { payload }) try { await copilot.createNotification(payload) } catch (e: unknown) { @@ -216,7 +232,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 +254,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-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-grouped-email.ts b/src/jobs/notifications/send-grouped-email.ts index 93588d6c9..14b23f725 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 @@ -16,6 +17,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,8 +31,9 @@ export const sendGroupedEmail = async ({ recipientClientId, recipientCompanyId, recipientInternalUserId, + notificationSettingId, copilot, -}: SendGroupedEmailArgs): Promise => { +}: SendGroupedEmailArgs): Promise => { const email = renderGroupedEmail(content) const payload: NotificationRequestBody = { @@ -38,6 +43,7 @@ export const sendGroupedEmail = async ({ recipientClientId: recipientClientId ?? undefined, recipientCompanyId: recipientCompanyId ?? undefined, recipientInternalUserId: recipientInternalUserId ?? undefined, + notificationSettingId, deliveryTargets: { email: { subject: email.subject, @@ -48,14 +54,15 @@ 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/jobs/notifications/send-reply-create-notifications.ts b/src/jobs/notifications/send-reply-create-notifications.ts index 0bc4a69a5..812d34653 100644 --- a/src/jobs/notifications/send-reply-create-notifications.ts +++ b/src/jobs/notifications/send-reply-create-notifications.ts @@ -5,10 +5,11 @@ 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 { 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, Task } from '@prisma/client' +import { Comment, CommentInitiator, GroupedEmailEventType, Task } from '@prisma/client' import { logger, task } from '@trigger.dev/sdk/v3' import { z } from 'zod' @@ -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,11 +49,33 @@ export const sendReplyCreateNotifications = task({ const deliveryTargets = await getNotificationDetails(copilot, user, 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 => { + 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, @@ -59,19 +83,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 - payload.task.companyId || undefined, - ) + const promise = getInitiatorNotificationPromises({ ...shared, initiator }) promise && queueNotificationPromise(promise) // It's certain we will get a promise here } @@ -88,27 +100,9 @@ export const sendReplyCreateNotifications = task({ (initiator) => initiator.initiatorId === parentComment.initiatorId, ) if (!isParentCommentDeleted && !parentInitiatorIsCurrentUser && !isNotificationAlreadySent) { - const typedPromise = getInitiatorNotificationPromises( - copilot, - parentComment, - senderId, - senderType, - senderCompanyId, - deliveryTargets, - payload.task.companyId || undefined, - ) + 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, - ) + const promise = typedPromise ?? getNotificationToUntypedInitiator({ ...shared, parentComment }) queueNotificationPromise(promise) } } @@ -149,40 +143,81 @@ const getNotificationDetails = async (copilot: CopilotAPI, user: User, comment: return deliveryTargets } -const getInitiatorNotificationPromises = ( - copilot: CopilotAPI, +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, + 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, -) => { + assume?: CommentInitiator + notificationSettingId?: string +} + +const getInitiatorNotificationPromises = ({ + copilot, + notificationService, + task: parentTask, + initiator, + senderId, + senderType, + senderCompanyId, + deliveryTargets, + initiatorCompanyId, + commentId, + assume, + notificationSettingId, +}: 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, - deliveryTargets: { - inProduct: deliveryTargets.inProduct, - ...(isIuEmailEnabled() && { email: deliveryTargets.email }), - }, - } - } else if (initiator.initiatorType === CommentInitiator.client || assume === CommentInitiator.client) { - body = { - ...base, - recipientClientId: initiator.initiatorId, - recipientCompanyId: initiatorCompanyId, - deliveryTargets: { email: deliveryTargets.email }, + ...(notificationSettingId ? { notificationSettingId } : {}), } - } 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. @@ -198,42 +233,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 }, -) => { +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( - copilot, - parentComment, - senderId, - senderType, - senderCompanyId, - deliveryTargets, - task.companyId || undefined, - CommentInitiator.internalUser, - )! + return getInitiatorNotificationPromises({ ...withInitiator, 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({ ...withInitiator, 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..82e4ed7e6 100644 --- a/src/utils/CopilotAPI.ts +++ b/src/utils/CopilotAPI.ts @@ -32,6 +32,8 @@ import { NotificationRequestBody, NotificationResponseSchema, NotificationResponseType, + NotificationSettingsResponse, + NotificationSettingsResponseSchema, Token, TokenSchema, WorkspaceResponse, @@ -228,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. @@ -244,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) } @@ -346,6 +355,21 @@ export class CopilotAPI { }) } + // Declared notification settings for this app in the current workspace. + async _getNotificationSettings(): Promise { + 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) + 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 +434,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(