diff --git a/apps/app/src/trigger/tasks/task/policy-acknowledgment-digest-helpers.test.ts b/apps/app/src/trigger/tasks/task/policy-acknowledgment-digest-helpers.test.ts index 8ab24c108..f9a9026e0 100644 --- a/apps/app/src/trigger/tasks/task/policy-acknowledgment-digest-helpers.test.ts +++ b/apps/app/src/trigger/tasks/task/policy-acknowledgment-digest-helpers.test.ts @@ -37,14 +37,21 @@ const itOnlyPolicy: DigestPolicy = { describe('computePendingPolicies', () => { it('returns no pending policies when member has signed all applicable policies', () => { - const policies: DigestPolicy[] = [{ ...allPolicy, signedBy: ['usr_alice'] }]; + const policies: DigestPolicy[] = [{ ...allPolicy, signedBy: ['mem_alice'] }]; expect(computePendingPolicies(alice, policies)).toEqual([]); }); it('returns policies where the member id is missing from signedBy[]', () => { const policies: DigestPolicy[] = [ - { ...allPolicy, signedBy: ['usr_bob'] }, - { ...itOnlyPolicy, id: 'pol_2', name: 'Second', signedBy: ['usr_alice'] }, + { ...allPolicy, signedBy: ['mem_bob'] }, + { ...itOnlyPolicy, id: 'pol_2', name: 'Second', signedBy: ['mem_alice'] }, + ]; + expect(computePendingPolicies(alice, policies).map((p) => p.id)).toEqual(['pol_all']); + }); + + it('does not treat the member user id as a signature (signedBy stores member ids, not user ids)', () => { + const policies: DigestPolicy[] = [ + { ...allPolicy, signedBy: ['usr_alice'] }, ]; expect(computePendingPolicies(alice, policies).map((p) => p.id)).toEqual(['pol_all']); }); diff --git a/apps/app/src/trigger/tasks/task/policy-acknowledgment-digest-helpers.ts b/apps/app/src/trigger/tasks/task/policy-acknowledgment-digest-helpers.ts index 624f2fe0e..f35f81604 100644 --- a/apps/app/src/trigger/tasks/task/policy-acknowledgment-digest-helpers.ts +++ b/apps/app/src/trigger/tasks/task/policy-acknowledgment-digest-helpers.ts @@ -101,7 +101,9 @@ export function computePendingPolicies( policies: DigestPolicy[], ): DigestPolicy[] { return policies.filter((policy) => { - if (policy.signedBy.includes(member.user.id)) return false; + // signedBy stores member ids (see apps/portal/src/actions/accept-policies.ts), + // not user ids — every other consumer checks against member.id. + if (policy.signedBy.includes(member.id)) return false; if (policy.visibility === 'ALL') return true; if (!member.department) return false; return policy.visibleToDepartments.includes(member.department); diff --git a/apps/app/src/trigger/tasks/task/policy-acknowledgment-digest.test.ts b/apps/app/src/trigger/tasks/task/policy-acknowledgment-digest.test.ts index 2f2c59d92..01ffd6f35 100644 --- a/apps/app/src/trigger/tasks/task/policy-acknowledgment-digest.test.ts +++ b/apps/app/src/trigger/tasks/task/policy-acknowledgment-digest.test.ts @@ -51,7 +51,8 @@ const taskUnderTest = policyAcknowledgmentDigest as unknown as { emailsSent: number; emailsFailed: number; orgsProcessed: number; - emailsSkippedUnsubscribed: number; + recipients: number; + orgsSkippedUnsubscribed: number; }>; }; @@ -79,7 +80,7 @@ describe('policyAcknowledgmentDigest', () => { { id: 'pol_b', name: 'Backup', - signedBy: ['usr_alice'], + signedBy: ['mem_alice'], visibility: 'ALL', visibleToDepartments: [], }, @@ -111,7 +112,7 @@ describe('policyAcknowledgmentDigest', () => { expect(result).toMatchObject({ success: true, emailsSent: 1, - emailsSkippedUnsubscribed: 0, + orgsSkippedUnsubscribed: 0, }); }); @@ -124,7 +125,7 @@ describe('policyAcknowledgmentDigest', () => { { id: 'pol_a', name: 'Access Control', - signedBy: ['usr_alice'], + signedBy: ['mem_alice'], visibility: 'ALL', visibleToDepartments: [], }, @@ -333,11 +334,94 @@ describe('policyAcknowledgmentDigest', () => { expect(result).toMatchObject({ success: true, emailsSent: 0, - emailsSkippedUnsubscribed: 1, + orgsSkippedUnsubscribed: 1, }); }); - it('sends a separate email per org when a user belongs to multiple orgs', async () => { + it('rolls up pending policies across orgs into a single email when a user belongs to multiple orgs', async () => { + mockFindMany.mockResolvedValueOnce([ + { + id: 'org_1', + name: 'Acme', + policy: [ + { + id: 'pol_a', + name: 'Access Control', + signedBy: [], + visibility: 'ALL', + visibleToDepartments: [], + }, + ], + members: [ + { + id: 'mem_1', + department: 'it', + user: { + id: 'usr_alice', + name: 'Alice', + email: 'alice@example.com', + role: null, + }, + }, + ], + }, + { + id: 'org_2', + name: 'Beta', + policy: [ + { + id: 'pol_b', + name: 'Backup', + signedBy: [], + visibility: 'ALL', + visibleToDepartments: [], + }, + { + id: 'pol_c', + name: 'Change Mgmt', + signedBy: [], + visibility: 'ALL', + visibleToDepartments: [], + }, + ], + members: [ + { + id: 'mem_2', + department: 'hr', + user: { + id: 'usr_alice', + name: 'Alice', + email: 'alice@example.com', + role: null, + }, + }, + ], + }, + ]); + + const result = await taskUnderTest.run({ timestamp: new Date() } as never); + + expect(mockSendEmailViaApi).toHaveBeenCalledTimes(1); + const call = mockSendEmailViaApi.mock.calls[0][0] as { + to: string; + subject: string; + organizationId: string; + }; + expect(call.to).toBe('alice@example.com'); + expect(call.subject).toBe( + 'You have 3 policies to review across 2 organizations', + ); + // x-organization-id falls back to the first org the user had policies in. + expect(call.organizationId).toBe('org_1'); + expect(result).toMatchObject({ + success: true, + orgsProcessed: 2, + recipients: 1, + emailsSent: 1, + }); + }); + + it('drops a single org from the rollup when the user is unsubscribed there, but still emails about other orgs', async () => { mockFindMany.mockResolvedValueOnce([ { id: 'org_1', @@ -390,21 +474,137 @@ describe('policyAcknowledgmentDigest', () => { ], }, ]); + // Alice is unsubscribed from policy notifications in org_1 only. + mockGetUnsubscribedEmails.mockImplementation( + async (_db, _emails, _pref, orgId) => + orgId === 'org_1' + ? new Set(['alice@example.com']) + : new Set(), + ); const result = await taskUnderTest.run({ timestamp: new Date() } as never); - expect(sendEmailViaApi).toHaveBeenCalledTimes(2); - const orgs = mockSendEmailViaApi.mock.calls - .map((c) => (c[0] as { organizationId: string }).organizationId) - .sort(); - expect(orgs).toEqual(['org_1', 'org_2']); + expect(mockSendEmailViaApi).toHaveBeenCalledTimes(1); + const call = mockSendEmailViaApi.mock.calls[0][0] as { + subject: string; + organizationId: string; + }; + expect(call.subject).toBe('You have 1 policy to review at Beta'); + expect(call.organizationId).toBe('org_2'); expect(result).toMatchObject({ success: true, orgsProcessed: 2, - emailsSent: 2, + recipients: 1, + emailsSent: 1, + orgsSkippedUnsubscribed: 1, }); }); + it('does not send any email to a user who is unsubscribed in every org they belong to', async () => { + mockFindMany.mockResolvedValueOnce([ + { + id: 'org_1', + name: 'Acme', + policy: [ + { + id: 'pol_a', + name: 'A', + signedBy: [], + visibility: 'ALL', + visibleToDepartments: [], + }, + ], + members: [ + { + id: 'mem_1', + department: 'it', + user: { + id: 'usr_alice', + name: 'Alice', + email: 'alice@example.com', + role: null, + }, + }, + ], + }, + { + id: 'org_2', + name: 'Beta', + policy: [ + { + id: 'pol_b', + name: 'B', + signedBy: [], + visibility: 'ALL', + visibleToDepartments: [], + }, + ], + members: [ + { + id: 'mem_2', + department: 'hr', + user: { + id: 'usr_alice', + name: 'Alice', + email: 'alice@example.com', + role: null, + }, + }, + ], + }, + ]); + mockGetUnsubscribedEmails.mockResolvedValue( + new Set(['alice@example.com']), + ); + + const result = await taskUnderTest.run({ timestamp: new Date() } as never); + + expect(mockSendEmailViaApi).not.toHaveBeenCalled(); + expect(result).toMatchObject({ + success: true, + recipients: 0, + emailsSent: 0, + orgsSkippedUnsubscribed: 2, + }); + }); + + it('does not email a multi-role member (e.g. owner,employee) who has already signed every policy', async () => { + // Regression: signedBy stores member ids (from the portal accept action), + // not user ids. The digest must compare against member.id, otherwise + // every compliance-obligated member looks "pending" on every policy. + mockFindMany.mockResolvedValueOnce([ + { + id: 'org_1', + name: 'Acme', + policy: Array.from({ length: 10 }, (_, i) => ({ + id: `pol_${i}`, + name: `Policy ${i}`, + signedBy: ['mem_owner_employee'], + visibility: 'ALL', + visibleToDepartments: [], + })), + members: [ + { + id: 'mem_owner_employee', + role: 'owner,employee', + department: 'eng', + user: { + id: 'usr_owner_employee', + name: 'Owner Employee', + email: 'multi@example.com', + role: null, + }, + }, + ], + }, + ]); + + const result = await taskUnderTest.run({ timestamp: new Date() } as never); + + expect(mockSendEmailViaApi).not.toHaveBeenCalled(); + expect(result).toMatchObject({ success: true, emailsSent: 0 }); + }); + it('sends emails in batches of up to 25', async () => { // Create 60 members in one org, all with pending policies, all subscribed. const members = Array.from({ length: 60 }, (_, i) => ({ @@ -443,4 +643,40 @@ describe('policyAcknowledgmentDigest', () => { expect(mockSendEmailViaApi).toHaveBeenCalledTimes(60); expect(result).toMatchObject({ success: true, emailsSent: 60 }); }); + + it('filters out dead orgs at the DB query (hasAccess, onboardingCompleted, 90-day session activity)', async () => { + mockFindMany.mockResolvedValueOnce([]); + + await taskUnderTest.run({ timestamp: new Date() } as never); + + expect(mockFindMany).toHaveBeenCalledTimes(1); + const args = mockFindMany.mock.calls[0][0] as { + where: { + hasAccess?: boolean; + onboardingCompleted?: boolean; + members?: { + some?: { + deactivated?: boolean; + user?: { + sessions?: { some?: { updatedAt?: { gte?: Date } } }; + }; + }; + }; + }; + }; + + expect(args.where.hasAccess).toBe(true); + expect(args.where.onboardingCompleted).toBe(true); + expect(args.where.members?.some?.deactivated).toBe(false); + const gte = args.where.members?.some?.user?.sessions?.some?.updatedAt?.gte; + expect(gte).toBeInstanceOf(Date); + + // Mirror the task's local-time setDate(-90) so the assertion survives + // DST transitions during the 90-day window. + const expected = new Date(); + expected.setDate(expected.getDate() - 90); + expect(Math.abs((gte as Date).getTime() - expected.getTime())).toBeLessThan( + 5_000, + ); + }); }); diff --git a/apps/app/src/trigger/tasks/task/policy-acknowledgment-digest.ts b/apps/app/src/trigger/tasks/task/policy-acknowledgment-digest.ts index 045230f0c..19c29be29 100644 --- a/apps/app/src/trigger/tasks/task/policy-acknowledgment-digest.ts +++ b/apps/app/src/trigger/tasks/task/policy-acknowledgment-digest.ts @@ -1,7 +1,11 @@ import { db } from '@db/server'; import { logger, schedules } from '@trigger.dev/sdk'; -import { PolicyAcknowledgmentDigestEmail } from '@trycompai/email'; +import { + PolicyAcknowledgmentDigestEmail, + computePolicyAcknowledgmentDigestSubject, + type PolicyAcknowledgmentDigestOrg, +} from '@trycompai/email'; import { getUnsubscribedEmails } from '@trycompai/email/lib/check-unsubscribe'; import { sendEmailViaApi } from '../../lib/send-email-via-api'; @@ -19,6 +23,9 @@ const getPortalBase = () => ); const EMAIL_BATCH_SIZE = 25; +// Skip orgs that look abandoned — same threshold weekly-task-reminder uses so +// we don't keep hitting dead addresses and burning domain reputation. +const ORG_INACTIVITY_DAYS = 90; async function sendInBatches( sends: Array<() => Promise>, @@ -32,14 +39,30 @@ async function sendInBatches( return results; } +interface RollupEntry { + email: string; + userName: string; + // First org (in iteration order) a policy was added for this user — used + // as the x-organization-id header when sending. The body lists all orgs. + primaryOrgId: string; + orgs: PolicyAcknowledgmentDigestOrg[]; +} + export const policyAcknowledgmentDigest = schedules.task({ id: 'policy-acknowledgment-digest', machine: 'large-1x', cron: '0 14 * * *', // Once daily at 14:00 UTC maxDuration: 1000 * 60 * 15, // 15 minutes run: async () => { + const inactivityCutoff = new Date(); + inactivityCutoff.setDate( + inactivityCutoff.getDate() - ORG_INACTIVITY_DAYS, + ); + const organizations = await db.organization.findMany({ where: { + hasAccess: true, + onboardingCompleted: true, policy: { some: { status: 'published', @@ -47,6 +70,16 @@ export const policyAcknowledgmentDigest = schedules.task({ isRequiredToSign: true, }, }, + members: { + some: { + deactivated: false, + user: { + sessions: { + some: { updatedAt: { gte: inactivityCutoff } }, + }, + }, + }, + }, }, select: { id: true, @@ -80,14 +113,18 @@ export const policyAcknowledgmentDigest = schedules.task({ }); logger.info( - `Checking ${organizations.length} orgs for pending acknowledgments`, + `Checking ${organizations.length} active orgs for pending acknowledgments (skipped orgs with no sessions in ${ORG_INACTIVITY_DAYS} days)`, ); const portalBase = getPortalBase(); - let emailsSent = 0; - let emailsFailed = 0; - let emailsSkippedUnsubscribed = 0; let orgsProcessed = 0; + // Per-org drops from the unsubscribe filter. A user opted-out in 2 orgs + // counts 2 — same semantic as the pre-rollup implementation. + let orgsSkippedUnsubscribed = 0; + + // Rollup across orgs, keyed by user id so one person = one email even + // when they hold separate member records in multiple organizations. + const rollup = new Map(); for (const org of organizations) { orgsProcessed += 1; @@ -99,45 +136,31 @@ export const policyAcknowledgmentDigest = schedules.task({ if (complianceMembers.length === 0) continue; - // Compute pending policies for each member first (no sends yet) - type PendingEntry = { + const pendingByMember: Array<{ member: DigestMember; - policies: Array<{ id: string; name: string; url: string }>; - subject: string; - emailElement: ReturnType; - }; - - const pending: PendingEntry[] = []; - const emailsWithPending: string[] = []; + policies: PolicyAcknowledgmentDigestOrg['policies']; + }> = []; for (const member of complianceMembers) { const pendingPolicies = computePendingPolicies(member, org.policy); if (pendingPolicies.length === 0) continue; - const policies = pendingPolicies.map((p) => ({ - id: p.id, - name: p.name, - url: `${portalBase}/${org.id}/policy/${p.id}`, - })); - const countLabel = - policies.length === 1 ? '1 policy' : `${policies.length} policies`; - const subject = `You have ${countLabel} to review at ${org.name}`; - - const emailElement = PolicyAcknowledgmentDigestEmail({ - email: member.user.email, - userName: member.user.name ?? '', - organizationName: org.name, - organizationId: org.id, - policies, + pendingByMember.push({ + member, + policies: pendingPolicies.map((p) => ({ + id: p.id, + name: p.name, + url: `${portalBase}/${org.id}/policy/${p.id}`, + })), }); - - emailsWithPending.push(member.user.email); - pending.push({ member, policies, subject, emailElement }); } - if (pending.length === 0) continue; + if (pendingByMember.length === 0) continue; - // Batch unsubscribe check — 3 DB queries total for this org + // One unsubscribe query per org, batched across members. + const emailsWithPending = pendingByMember.map( + (p) => p.member.user.email, + ); const unsubscribedEmails = await getUnsubscribedEmails( db, emailsWithPending, @@ -145,56 +168,80 @@ export const policyAcknowledgmentDigest = schedules.task({ org.id, ); - // Build thunks for subscribed members only - const sends: Array<() => Promise> = []; - - for (const entry of pending) { - if (unsubscribedEmails.has(entry.member.user.email)) { + for (const { member, policies } of pendingByMember) { + if (unsubscribedEmails.has(member.user.email)) { logger.debug( - 'User unsubscribed from policy notifications, skipping', - { email: entry.member.user.email, orgId: org.id }, + 'User unsubscribed from policy notifications for this org, omitting from digest', + { email: member.user.email, orgId: org.id }, ); - emailsSkippedUnsubscribed += 1; + orgsSkippedUnsubscribed += 1; continue; } - sends.push(() => - sendEmailViaApi({ - to: entry.member.user.email, - subject: entry.subject, - organizationId: org.id, - react: entry.emailElement!, - }), - ); - } - - const results = await sendInBatches(sends); - for (const r of results) { - if (r.status === 'fulfilled') emailsSent += 1; - else { - emailsFailed += 1; - logger.warn('Digest email failed', { - orgId: org.id, - error: - r.reason instanceof Error ? r.reason.message : String(r.reason), + const existing = rollup.get(member.user.id); + if (existing) { + existing.orgs.push({ id: org.id, name: org.name, policies }); + } else { + rollup.set(member.user.id, { + email: member.user.email, + userName: member.user.name ?? '', + primaryOrgId: org.id, + orgs: [{ id: org.id, name: org.name, policies }], }); } } } + // Build one send per user. + const sends: Array<() => Promise> = []; + for (const entry of rollup.values()) { + const subject = computePolicyAcknowledgmentDigestSubject(entry.orgs); + const emailElement = PolicyAcknowledgmentDigestEmail({ + email: entry.email, + userName: entry.userName, + orgs: entry.orgs, + }); + if (!emailElement) continue; + + sends.push(() => + sendEmailViaApi({ + to: entry.email, + subject, + organizationId: entry.primaryOrgId, + react: emailElement, + }), + ); + } + + const results = await sendInBatches(sends); + let emailsSent = 0; + let emailsFailed = 0; + for (const r of results) { + if (r.status === 'fulfilled') emailsSent += 1; + else { + emailsFailed += 1; + logger.warn('Digest email failed', { + error: + r.reason instanceof Error ? r.reason.message : String(r.reason), + }); + } + } + logger.info('Digest complete', { orgsProcessed, + recipients: rollup.size, emailsSent, emailsFailed, - emailsSkippedUnsubscribed, + orgsSkippedUnsubscribed, }); return { success: true, orgsProcessed, + recipients: rollup.size, emailsSent, emailsFailed, - emailsSkippedUnsubscribed, + orgsSkippedUnsubscribed, }; }, }); diff --git a/packages/email/emails/policy-acknowledgment-digest.tsx b/packages/email/emails/policy-acknowledgment-digest.tsx index 21c68aca4..c7b6b5aa8 100644 --- a/packages/email/emails/policy-acknowledgment-digest.tsx +++ b/packages/email/emails/policy-acknowledgment-digest.tsx @@ -15,29 +15,56 @@ import { Logo } from '../components/logo'; import { UnsubscribeLink } from '../components/unsubscribe-link'; import { getUnsubscribeUrl } from '../lib/unsubscribe'; +export interface PolicyAcknowledgmentDigestPolicy { + id: string; + name: string; + url: string; +} + +export interface PolicyAcknowledgmentDigestOrg { + id: string; + name: string; + policies: PolicyAcknowledgmentDigestPolicy[]; +} + export interface PolicyAcknowledgmentDigestEmailProps { email: string; userName: string; - organizationName: string; - organizationId: string; - policies: { id: string; name: string; url: string }[]; + orgs: PolicyAcknowledgmentDigestOrg[]; } +const pluralizePolicies = (count: number) => + count === 1 ? '1 policy' : `${count} policies`; + +/** + * Shared subject-line builder so the trigger task and the email Preview + * header stay in sync. + */ +export const computePolicyAcknowledgmentDigestSubject = ( + orgs: PolicyAcknowledgmentDigestOrg[], +): string => { + const totalPolicies = orgs.reduce((sum, o) => sum + o.policies.length, 0); + const [firstOrg] = orgs; + if (orgs.length === 1 && firstOrg) { + return `You have ${pluralizePolicies(totalPolicies)} to review at ${firstOrg.name}`; + } + return `You have ${pluralizePolicies(totalPolicies)} to review across ${orgs.length} organizations`; +}; + export const PolicyAcknowledgmentDigestEmail = ({ email, userName, - organizationName, - organizationId, - policies, + orgs, }: PolicyAcknowledgmentDigestEmailProps) => { - if (policies.length === 0) return null; + const orgsWithPolicies = orgs.filter((o) => o.policies.length > 0); + const [firstOrg] = orgsWithPolicies; + if (!firstOrg) return null; const portalBase = ( process.env.NEXT_PUBLIC_PORTAL_URL ?? 'https://portal.trycomp.ai' ).replace(/\/+$/, ''); - const portalLink = `${portalBase}/${organizationId}`; - const countLabel = policies.length === 1 ? '1 policy' : `${policies.length} policies`; - const subjectText = `You have ${countLabel} to review at ${organizationName}`; + const subjectText = computePolicyAcknowledgmentDigestSubject(orgsWithPolicies); + const isMultiOrg = orgsWithPolicies.length > 1; return ( @@ -59,46 +86,67 @@ export const PolicyAcknowledgmentDigestEmail = ({ Hi {userName || 'there'}, - - Your organization {organizationName} has {countLabel} awaiting your - review and acknowledgment: - - -
- {policies.map((policy) => ( - - •{' '} - - {policy.name} - - - ))} -
- -
- -
+ {isMultiOrg ? ( + + The following organizations have policies awaiting your review + and acknowledgment: + + ) : ( + + Your organization{' '} + {firstOrg.name} has{' '} + {pluralizePolicies(firstOrg.policies.length)} awaiting + your review and acknowledgment: + + )} - - or copy and paste this URL into your browser{' '} - - {portalLink} - - + {orgsWithPolicies.map((org) => { + const orgPortalLink = `${portalBase}/${org.id}`; + return ( +
+ {isMultiOrg && ( + + {org.name} + + )} + {org.policies.map((policy) => ( + + •{' '} + + {policy.name} + + + ))} +
+ +
+
+ ); + })}
- This notification was intended for {email}. + This notification was intended for{' '} + {email}.
- +
diff --git a/packages/email/emails/render.test.tsx b/packages/email/emails/render.test.tsx index 262511cad..14503a298 100644 --- a/packages/email/emails/render.test.tsx +++ b/packages/email/emails/render.test.tsx @@ -140,23 +140,65 @@ const cases = [ ), }, { - name: 'policy-acknowledgment-digest', + name: 'policy-acknowledgment-digest-single-org', el: ( + ), + }, + { + name: 'policy-acknowledgment-digest-multi-org', + el: ( +