Skip to content

Commit 75c8888

Browse files
github-actions[bot]Marfuenclaude
authored
fix(digest): key policy acknowledgment rollup by email, not user.id (#2624)
User.email is not @unique in the schema, so the same person can end up with multiple user rows — typically when invited to separate orgs through different flows. Keying the rollup on user.id split those duplicates into one email per row, which is why a user with memberships across 8 orgs received two separate digests within a minute of each other (one per duplicated user record, each branded from a different primaryOrgId). Key the dedup map on the normalized email address instead so one person always gets one digest regardless of how many user rows back their memberships. Added a regression test covering two orgs, same email, different user.ids. Co-authored-by: Mariano Fuentes <marfuen98@gmail.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 99a65df commit 75c8888

2 files changed

Lines changed: 90 additions & 4 deletions

File tree

apps/app/src/trigger/tasks/task/policy-acknowledgment-digest.test.ts

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -421,6 +421,87 @@ describe('policyAcknowledgmentDigest', () => {
421421
});
422422
});
423423

424+
it('rolls up across orgs by email when the same person has multiple User records (schema allows duplicate emails)', async () => {
425+
// Regression: User.email is not @unique in the Prisma schema, so one
426+
// person can end up with multiple user rows — typically when they get
427+
// invited to separate orgs through different flows. Keying the rollup
428+
// on user.id split those duplicates into one email each. Rollup must
429+
// collapse by normalized email instead.
430+
mockFindMany.mockResolvedValueOnce([
431+
{
432+
id: 'org_1',
433+
name: 'Acme',
434+
policy: [
435+
{
436+
id: 'pol_a',
437+
name: 'Access Control',
438+
signedBy: [],
439+
visibility: 'ALL',
440+
visibleToDepartments: [],
441+
},
442+
],
443+
members: [
444+
{
445+
id: 'mem_1',
446+
department: 'it',
447+
user: {
448+
id: 'usr_alice_first',
449+
name: 'Alice',
450+
email: 'alice@example.com',
451+
role: null,
452+
},
453+
},
454+
],
455+
},
456+
{
457+
id: 'org_2',
458+
name: 'Beta',
459+
policy: [
460+
{
461+
id: 'pol_b',
462+
name: 'Backup',
463+
signedBy: [],
464+
visibility: 'ALL',
465+
visibleToDepartments: [],
466+
},
467+
],
468+
members: [
469+
{
470+
id: 'mem_2',
471+
department: 'hr',
472+
user: {
473+
// Different user row, same email — Alice was re-invited under
474+
// a separate user record.
475+
id: 'usr_alice_second',
476+
name: 'Alice',
477+
email: 'ALICE@example.com',
478+
role: null,
479+
},
480+
},
481+
],
482+
},
483+
]);
484+
485+
const result = await taskUnderTest.run({ timestamp: new Date() } as never);
486+
487+
expect(mockSendEmailViaApi).toHaveBeenCalledTimes(1);
488+
const call = mockSendEmailViaApi.mock.calls[0][0] as {
489+
to: string;
490+
subject: string;
491+
organizationId: string;
492+
};
493+
expect(call.subject).toBe(
494+
'You have 2 policies to review across 2 organizations',
495+
);
496+
expect(call.organizationId).toBe('org_1');
497+
expect(result).toMatchObject({
498+
success: true,
499+
orgsProcessed: 2,
500+
recipients: 1,
501+
emailsSent: 1,
502+
});
503+
});
504+
424505
it('drops a single org from the rollup when the user is unsubscribed there, but still emails about other orgs', async () => {
425506
mockFindMany.mockResolvedValueOnce([
426507
{

apps/app/src/trigger/tasks/task/policy-acknowledgment-digest.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -122,8 +122,12 @@ export const policyAcknowledgmentDigest = schedules.task({
122122
// counts 2 — same semantic as the pre-rollup implementation.
123123
let orgsSkippedUnsubscribed = 0;
124124

125-
// Rollup across orgs, keyed by user id so one person = one email even
126-
// when they hold separate member records in multiple organizations.
125+
// Rollup across orgs, keyed by normalized email so one person = one
126+
// email even when they hold separate member records in multiple
127+
// organizations. Keyed on email (not user.id) because User.email is
128+
// not @unique in the schema — the same person can end up with multiple
129+
// user rows, typically when invited to separate orgs through different
130+
// flows, and keying on user.id split those duplicates into one email each.
127131
const rollup = new Map<string, RollupEntry>();
128132

129133
for (const org of organizations) {
@@ -178,11 +182,12 @@ export const policyAcknowledgmentDigest = schedules.task({
178182
continue;
179183
}
180184

181-
const existing = rollup.get(member.user.id);
185+
const rollupKey = member.user.email.trim().toLowerCase();
186+
const existing = rollup.get(rollupKey);
182187
if (existing) {
183188
existing.orgs.push({ id: org.id, name: org.name, policies });
184189
} else {
185-
rollup.set(member.user.id, {
190+
rollup.set(rollupKey, {
186191
email: member.user.email,
187192
userName: member.user.name ?? '',
188193
primaryOrgId: org.id,

0 commit comments

Comments
 (0)