Skip to content

Commit 577d98b

Browse files
Bypass grouped emails for public overrides
Co-authored-by: Neil Raina <makeitraina@users.noreply.github.com>
1 parent 14f61c7 commit 577d98b

3 files changed

Lines changed: 51 additions & 11 deletions

File tree

docs/PUBLIC_TASK_EMAIL_OVERRIDE.md

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -68,12 +68,6 @@ POST /api/tasks/public (controller: email pulled off the parsed DTO)
6868

6969
The merge (`{ ...default, ...override }`) is why omitted fields fall back to the default copy.
7070

71-
The merged `email` is what gets buffered as `GroupedEmailEvent.individualEmail` during the
72-
5-minute grouping window, so the override is preserved through the grouped-email pipeline.
73-
74-
## Known limitation
75-
76-
Task-assignment emails are buffered for ~5 minutes and grouped per recipient (see the grouped
77-
email flow). The override is replayed **verbatim** only when the recipient has a **single**
78-
buffered event in that window. If multiple events group into one summary email, that summary
79-
uses the default grouped copy and the override is **not** applied.
71+
When an override is provided, the email bypasses the 5-minute grouped-email buffer and is sent
72+
as an individual task email. That keeps the custom copy and CTA tied to the task that was just
73+
created.

src/app/api/notification/notification.service.test.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,28 @@ describe('NotificationService grouped-email interception', () => {
146146
expect(mockEnqueueFlush).not.toHaveBeenCalled()
147147
})
148148

149+
it('sends email overrides immediately so their task CTA is not grouped away', async () => {
150+
const task = makeTask()
151+
152+
await buildService().create(NotificationTaskActions.Assigned, task, {
153+
disableEmail: false,
154+
emailOverride: {
155+
subject: 'Action Required: Evaluation Ready',
156+
title: 'Review evaluation',
157+
},
158+
})
159+
160+
expect(mockGroupedCreateMany).not.toHaveBeenCalled()
161+
expect(mockEnqueueFlush).not.toHaveBeenCalled()
162+
expect(mockCreateNotification).toHaveBeenCalledTimes(1)
163+
expect(deliveryTargetsOf(0).inProduct).toBeDefined()
164+
expect(deliveryTargetsOf(0).email).toMatchObject({
165+
subject: 'Action Required: Evaluation Ready',
166+
title: 'Review evaluation',
167+
ctaParams: { taskId: task.id },
168+
})
169+
})
170+
149171
it('keys the window on (clientId, companyId) so a multi-company client does not get merged', async () => {
150172
const companyB = '99999999-9999-9999-9999-999999999999'
151173
await buildService().create(NotificationTaskActions.Assigned, makeTask({ companyId: companyB }))
@@ -251,6 +273,30 @@ describe('NotificationService grouped-email interception', () => {
251273
expect(recipients).toEqual(['cu_a', 'cu_b', 'cu_c'])
252274
})
253275

276+
it('sends bulk email overrides immediately for every recipient', async () => {
277+
const task = makeTask()
278+
const recipients = ['33333333-3333-3333-3333-333333333333', '44444444-4444-4444-4444-444444444444']
279+
280+
await buildService().createBulkNotification(NotificationTaskActions.AssignedToCompany, task, recipients, {
281+
email: true,
282+
emailOverride: {
283+
subject: 'Action Required: Evaluation Ready',
284+
title: 'Review evaluation',
285+
},
286+
})
287+
288+
expect(mockGroupedCreateMany).not.toHaveBeenCalled()
289+
expect(mockEnqueueFlush).not.toHaveBeenCalled()
290+
expect(mockCreateNotification).toHaveBeenCalledTimes(2)
291+
for (const call of mockCreateNotification.mock.calls) {
292+
expect(call[0].deliveryTargets.email).toMatchObject({
293+
subject: 'Action Required: Evaluation Ready',
294+
title: 'Review evaluation',
295+
ctaParams: { taskId: task.id },
296+
})
297+
}
298+
})
299+
254300
it('falls back to the association company when the shared task has no companyId', async () => {
255301
const assocCompany = '88888888-8888-8888-8888-888888888888'
256302
const task = makeTask({

src/app/api/notification/notification.service.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ export class NotificationService extends BaseService {
6464
const email = baseEmail ? mergeEmailOverride({ base: baseEmail, override: opts.emailOverride }) : baseEmail
6565

6666
// Non-null only when this CU email should be diverted into the grouped buffer.
67-
const groupedType = email && recipientId ? this.groupedEventTypeFor(action) : null
67+
const groupedType = email && recipientId && !opts.emailOverride ? this.groupedEventTypeFor(action) : null
6868
if (groupedType) {
6969
const association = AssociationsSchema.parse(task.associations)?.[0]
7070
await this.bufferGroupedEmailEvent({
@@ -178,7 +178,7 @@ export class NotificationService extends BaseService {
178178

179179
const association = AssociationsSchema.parse(task.associations)?.[0]
180180
// Non-null only when these CU emails should be diverted into the grouped buffer.
181-
const groupedType = email ? this.groupedEventTypeFor(action) : null
181+
const groupedType = email && !opts?.emailOverride ? this.groupedEventTypeFor(action) : null
182182

183183
// NOTE: The reason we are skipping using NotificationService#create and implementing notification dispatch + save manually is because
184184
// we can just do one `createMany` DB call instead of one per notification, saving a ton of DB calls

0 commit comments

Comments
 (0)