From 577d98b5470100dc364b044bb50181f4252bc753 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 29 Jun 2026 13:37:13 +0000 Subject: [PATCH] Bypass grouped emails for public overrides Co-authored-by: Neil Raina --- docs/PUBLIC_TASK_EMAIL_OVERRIDE.md | 12 ++--- .../notification/notification.service.test.ts | 46 +++++++++++++++++++ .../api/notification/notification.service.ts | 4 +- 3 files changed, 51 insertions(+), 11 deletions(-) diff --git a/docs/PUBLIC_TASK_EMAIL_OVERRIDE.md b/docs/PUBLIC_TASK_EMAIL_OVERRIDE.md index fbec8d242..2922164bf 100644 --- a/docs/PUBLIC_TASK_EMAIL_OVERRIDE.md +++ b/docs/PUBLIC_TASK_EMAIL_OVERRIDE.md @@ -68,12 +68,6 @@ POST /api/tasks/public (controller: email pulled off the parsed DTO) The merge (`{ ...default, ...override }`) is why omitted fields fall back to the default copy. -The merged `email` is what gets buffered as `GroupedEmailEvent.individualEmail` during the -5-minute grouping window, so the override is preserved through the grouped-email pipeline. - -## Known limitation - -Task-assignment emails are buffered for ~5 minutes and grouped per recipient (see the grouped -email flow). The override is replayed **verbatim** only when the recipient has a **single** -buffered event in that window. If multiple events group into one summary email, that summary -uses the default grouped copy and the override is **not** applied. +When an override is provided, the email bypasses the 5-minute grouped-email buffer and is sent +as an individual task email. That keeps the custom copy and CTA tied to the task that was just +created. diff --git a/src/app/api/notification/notification.service.test.ts b/src/app/api/notification/notification.service.test.ts index ce7c59d75..e84d7c85d 100644 --- a/src/app/api/notification/notification.service.test.ts +++ b/src/app/api/notification/notification.service.test.ts @@ -146,6 +146,28 @@ describe('NotificationService grouped-email interception', () => { expect(mockEnqueueFlush).not.toHaveBeenCalled() }) + it('sends email overrides immediately so their task CTA is not grouped away', async () => { + const task = makeTask() + + await buildService().create(NotificationTaskActions.Assigned, task, { + disableEmail: false, + emailOverride: { + subject: 'Action Required: Evaluation Ready', + title: 'Review evaluation', + }, + }) + + expect(mockGroupedCreateMany).not.toHaveBeenCalled() + expect(mockEnqueueFlush).not.toHaveBeenCalled() + expect(mockCreateNotification).toHaveBeenCalledTimes(1) + expect(deliveryTargetsOf(0).inProduct).toBeDefined() + expect(deliveryTargetsOf(0).email).toMatchObject({ + subject: 'Action Required: Evaluation Ready', + title: 'Review evaluation', + ctaParams: { taskId: task.id }, + }) + }) + it('keys the window on (clientId, companyId) so a multi-company client does not get merged', async () => { const companyB = '99999999-9999-9999-9999-999999999999' await buildService().create(NotificationTaskActions.Assigned, makeTask({ companyId: companyB })) @@ -251,6 +273,30 @@ describe('NotificationService grouped-email interception', () => { expect(recipients).toEqual(['cu_a', 'cu_b', 'cu_c']) }) + it('sends bulk email overrides immediately for every recipient', async () => { + const task = makeTask() + const recipients = ['33333333-3333-3333-3333-333333333333', '44444444-4444-4444-4444-444444444444'] + + await buildService().createBulkNotification(NotificationTaskActions.AssignedToCompany, task, recipients, { + email: true, + emailOverride: { + subject: 'Action Required: Evaluation Ready', + title: 'Review evaluation', + }, + }) + + expect(mockGroupedCreateMany).not.toHaveBeenCalled() + expect(mockEnqueueFlush).not.toHaveBeenCalled() + expect(mockCreateNotification).toHaveBeenCalledTimes(2) + for (const call of mockCreateNotification.mock.calls) { + expect(call[0].deliveryTargets.email).toMatchObject({ + subject: 'Action Required: Evaluation Ready', + title: 'Review evaluation', + ctaParams: { taskId: task.id }, + }) + } + }) + it('falls back to the association company when the shared task has no companyId', async () => { const assocCompany = '88888888-8888-8888-8888-888888888888' const task = makeTask({ diff --git a/src/app/api/notification/notification.service.ts b/src/app/api/notification/notification.service.ts index e37a311e7..a4fd9adc2 100644 --- a/src/app/api/notification/notification.service.ts +++ b/src/app/api/notification/notification.service.ts @@ -64,7 +64,7 @@ export class NotificationService extends BaseService { const email = baseEmail ? mergeEmailOverride({ base: baseEmail, override: opts.emailOverride }) : baseEmail // Non-null only when this CU email should be diverted into the grouped buffer. - const groupedType = email && recipientId ? this.groupedEventTypeFor(action) : null + const groupedType = email && recipientId && !opts.emailOverride ? this.groupedEventTypeFor(action) : null if (groupedType) { const association = AssociationsSchema.parse(task.associations)?.[0] await this.bufferGroupedEmailEvent({ @@ -178,7 +178,7 @@ export class NotificationService extends BaseService { const association = AssociationsSchema.parse(task.associations)?.[0] // Non-null only when these CU emails should be diverted into the grouped buffer. - const groupedType = email ? this.groupedEventTypeFor(action) : null + const groupedType = email && !opts?.emailOverride ? this.groupedEventTypeFor(action) : 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