Skip to content

Commit fd0aa1c

Browse files
Marfuenclaude
Marfuen
and
claude
authored
fix(api): scope task status-change emails to assignee, not whole org (#2669)
* fix(api): scope task status-change emails to assignee, not whole org notifyStatusChange and notifyBulkStatusChange were emailing every non-platform-admin member of the org on any task status change, then leaning on isUserUnsubscribed to filter. That filter had gaps (multi-role users, custom roles, unsaved matrix state), so employees with "Task Assignments" unchecked still received status-change emails. Now: - Single status change: emails only the task's assignee. If the task has no assignee, falls back to owners + admins. Actor always excluded, isUserUnsubscribed still honored. - Bulk status change: groups tasks by assignee and sends each one a bulk email with the count of THEIR tasks. Unassigned tasks are routed to owners/admins with the unassigned count. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(roles): add permission-based member filter helper RolesService.filterMembersWithPermission(orgId, members, resource, action) returns the subset of members whose combined (built-in + custom) role permissions grant the requested resource:action. One batched organizationRole.findMany query regardless of member count. Matches better-auth's hasPermissionFn semantics: comma-separated role strings treated as a union; unknown role names skipped silently. Uses BUILT_IN_ROLE_PERMISSIONS (derived from the same role.statements that better-auth initializes with) so answers stay in lockstep with the runtime permission guard. Enables upcoming migration of notifier recipient selection from hardcoded role-string matching (role.includes('admin')) to permission-based filtering — so custom roles like "Compliance Manager" with task:update automatically qualify. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 2ba9a3d commit fd0aa1c

4 files changed

Lines changed: 721 additions & 139 deletions

File tree

‎apps/api/src/roles/roles.service.spec.ts‎

Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -520,4 +520,197 @@ describe('RolesService', () => {
520520
);
521521
});
522522
});
523+
524+
describe('filterMembersWithPermission', () => {
525+
const organizationId = 'org_1';
526+
527+
it('returns empty array when members list is empty', async () => {
528+
const result = await service.filterMembersWithPermission(
529+
organizationId,
530+
[],
531+
'task',
532+
'update',
533+
);
534+
expect(result).toEqual([]);
535+
expect(mockDb.organizationRole.findMany).not.toHaveBeenCalled();
536+
});
537+
538+
it('keeps built-in roles that grant the permission (owner has task:update)', async () => {
539+
const members = [
540+
{ id: 'm1', role: 'owner' },
541+
{ id: 'm2', role: 'admin' },
542+
];
543+
const result = await service.filterMembersWithPermission(
544+
organizationId,
545+
members,
546+
'task',
547+
'update',
548+
);
549+
expect(result.map((m) => m.id).sort()).toEqual(['m1', 'm2']);
550+
});
551+
552+
it('excludes built-in roles that lack the permission (employee has no task perms)', async () => {
553+
const members = [
554+
{ id: 'm1', role: 'employee' },
555+
{ id: 'm2', role: 'contractor' },
556+
{ id: 'm3', role: 'owner' },
557+
];
558+
const result = await service.filterMembersWithPermission(
559+
organizationId,
560+
members,
561+
'task',
562+
'update',
563+
);
564+
expect(result.map((m) => m.id)).toEqual(['m3']);
565+
});
566+
567+
it('excludes auditor for task:update but keeps them for task:read', async () => {
568+
const members = [{ id: 'm1', role: 'auditor' }];
569+
570+
const forUpdate = await service.filterMembersWithPermission(
571+
organizationId,
572+
members,
573+
'task',
574+
'update',
575+
);
576+
expect(forUpdate).toEqual([]);
577+
578+
const forRead = await service.filterMembersWithPermission(
579+
organizationId,
580+
members,
581+
'task',
582+
'read',
583+
);
584+
expect(forRead.map((m) => m.id)).toEqual(['m1']);
585+
});
586+
587+
it('treats comma-separated roles as a union (employee,admin gets included)', async () => {
588+
const members = [{ id: 'm1', role: 'employee,admin' }];
589+
const result = await service.filterMembersWithPermission(
590+
organizationId,
591+
members,
592+
'task',
593+
'update',
594+
);
595+
expect(result.map((m) => m.id)).toEqual(['m1']);
596+
});
597+
598+
it('includes a member whose custom role grants the permission', async () => {
599+
(mockDb.organizationRole.findMany as jest.Mock).mockResolvedValue([
600+
{
601+
name: 'compliance-lead',
602+
permissions: JSON.stringify({
603+
task: ['read', 'update'],
604+
app: ['read'],
605+
}),
606+
},
607+
]);
608+
const members = [{ id: 'm1', role: 'compliance-lead' }];
609+
const result = await service.filterMembersWithPermission(
610+
organizationId,
611+
members,
612+
'task',
613+
'update',
614+
);
615+
expect(result.map((m) => m.id)).toEqual(['m1']);
616+
expect(mockDb.organizationRole.findMany).toHaveBeenCalledTimes(1);
617+
});
618+
619+
it('excludes a member whose custom role lacks the permission', async () => {
620+
(mockDb.organizationRole.findMany as jest.Mock).mockResolvedValue([
621+
{
622+
name: 'readonly',
623+
permissions: JSON.stringify({ task: ['read'] }),
624+
},
625+
]);
626+
const members = [{ id: 'm1', role: 'readonly' }];
627+
const result = await service.filterMembersWithPermission(
628+
organizationId,
629+
members,
630+
'task',
631+
'update',
632+
);
633+
expect(result).toEqual([]);
634+
});
635+
636+
it('excludes members with null, empty, or unknown roles', async () => {
637+
(mockDb.organizationRole.findMany as jest.Mock).mockResolvedValue([]);
638+
const members = [
639+
{ id: 'm1', role: null },
640+
{ id: 'm2', role: '' },
641+
{ id: 'm3', role: 'nonexistent-role' },
642+
];
643+
const result = await service.filterMembersWithPermission(
644+
organizationId,
645+
members,
646+
'task',
647+
'update',
648+
);
649+
expect(result).toEqual([]);
650+
});
651+
652+
it('makes exactly one DB query regardless of member count', async () => {
653+
(mockDb.organizationRole.findMany as jest.Mock).mockResolvedValue([
654+
{
655+
name: 'custom-a',
656+
permissions: JSON.stringify({ task: ['update'] }),
657+
},
658+
]);
659+
const members = Array.from({ length: 25 }, (_, i) => ({
660+
id: `m${i}`,
661+
role: i % 2 === 0 ? 'custom-a' : 'employee',
662+
}));
663+
const result = await service.filterMembersWithPermission(
664+
organizationId,
665+
members,
666+
'task',
667+
'update',
668+
);
669+
expect(result.length).toBe(13); // 0,2,4,...,24 → 13 members
670+
expect(mockDb.organizationRole.findMany).toHaveBeenCalledTimes(1);
671+
});
672+
673+
it('skips the DB query when all roles are built-in', async () => {
674+
const members = [
675+
{ id: 'm1', role: 'owner' },
676+
{ id: 'm2', role: 'admin,auditor' },
677+
{ id: 'm3', role: 'employee' },
678+
];
679+
await service.filterMembersWithPermission(
680+
organizationId,
681+
members,
682+
'app',
683+
'read',
684+
);
685+
expect(mockDb.organizationRole.findMany).not.toHaveBeenCalled();
686+
});
687+
688+
it('parses permissions that are already objects (not strings)', async () => {
689+
(mockDb.organizationRole.findMany as jest.Mock).mockResolvedValue([
690+
{
691+
name: 'object-role',
692+
permissions: { task: ['update'] },
693+
},
694+
]);
695+
const members = [{ id: 'm1', role: 'object-role' }];
696+
const result = await service.filterMembersWithPermission(
697+
organizationId,
698+
members,
699+
'task',
700+
'update',
701+
);
702+
expect(result.map((m) => m.id)).toEqual(['m1']);
703+
});
704+
705+
it('trims whitespace around comma-separated role names', async () => {
706+
const members = [{ id: 'm1', role: 'employee , admin' }];
707+
const result = await service.filterMembersWithPermission(
708+
organizationId,
709+
members,
710+
'task',
711+
'update',
712+
);
713+
expect(result.map((m) => m.id)).toEqual(['m1']);
714+
});
715+
});
523716
});

‎apps/api/src/roles/roles.service.ts‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -467,6 +467,71 @@ export class RolesService {
467467
return combined;
468468
}
469469

470+
/**
471+
* Filter a list of members down to those whose combined role permissions
472+
* grant the given `resource:action`. Built-in role definitions come from
473+
* `BUILT_IN_ROLE_PERMISSIONS`; custom roles are fetched in a single batched
474+
* `organizationRole.findMany` keyed by the distinct role names present in
475+
* the input.
476+
*
477+
* Matches better-auth's `hasPermissionFn` semantics: comma-separated roles
478+
* in `member.role` are treated as a union (ANY role granting the permission
479+
* is sufficient). Unknown role names are skipped silently.
480+
*/
481+
async filterMembersWithPermission<M extends { role: string | null }>(
482+
organizationId: string,
483+
members: M[],
484+
resource: string,
485+
action: string,
486+
): Promise<M[]> {
487+
if (members.length === 0) return [];
488+
489+
const distinctRoles = new Set<string>();
490+
for (const m of members) {
491+
if (!m.role) continue;
492+
for (const r of m.role.split(',').map((r) => r.trim()).filter(Boolean)) {
493+
distinctRoles.add(r);
494+
}
495+
}
496+
if (distinctRoles.size === 0) return [];
497+
498+
const customRoleNames = [...distinctRoles].filter(
499+
(r) => !BUILT_IN_ROLES.includes(r),
500+
);
501+
502+
const customRoles =
503+
customRoleNames.length > 0
504+
? await db.organizationRole.findMany({
505+
where: { organizationId, name: { in: customRoleNames } },
506+
select: { name: true, permissions: true },
507+
})
508+
: [];
509+
510+
const permsByRole = new Map<string, Record<string, string[]>>();
511+
for (const name of distinctRoles) {
512+
if (BUILT_IN_ROLES.includes(name)) {
513+
permsByRole.set(name, BUILT_IN_ROLE_PERMISSIONS[name] ?? {});
514+
} else {
515+
const custom = customRoles.find((c) => c.name === name);
516+
if (!custom) continue;
517+
const perms =
518+
typeof custom.permissions === 'string'
519+
? (JSON.parse(custom.permissions) as Record<string, string[]>)
520+
: (custom.permissions as Record<string, string[]>);
521+
permsByRole.set(name, perms);
522+
}
523+
}
524+
525+
return members.filter((m) => {
526+
if (!m.role) return false;
527+
const roles = m.role
528+
.split(',')
529+
.map((r) => r.trim())
530+
.filter(Boolean);
531+
return roles.some((r) => permsByRole.get(r)?.[resource]?.includes(action));
532+
});
533+
}
534+
470535
/**
471536
* Get merged obligations for a list of custom role names.
472537
* Used by the frontend to resolve effective obligations for custom roles.

0 commit comments

Comments
 (0)