Skip to content

fix(api): scope task status-change emails to assignee, not whole org#2669

Merged
Marfuen merged 2 commits intomainfrom
mariano/unnecessary-task-notifications
Apr 24, 2026
Merged

fix(api): scope task status-change emails to assignee, not whole org#2669
Marfuen merged 2 commits intomainfrom
mariano/unnecessary-task-notifications

Conversation

@Marfuen
Copy link
Copy Markdown
Contributor

@Marfuen Marfuen commented Apr 24, 2026

Summary

Employees were receiving Task Status Updated emails even when their role had Task Assignments unchecked in the notifications matrix, because notifyStatusChange / notifyBulkStatusChange were emailing every non-platform-admin member of the org and leaning on isUserUnsubscribed to filter. That filter has gaps (multi-role users, custom roles, unsaved matrix state) so spam still escaped.

Now:

  • Single status change → emails only the task's assignee. If the task has no assignee, falls back to owners + admins so someone who can act on it is notified.
  • Bulk status change → groups tasks by assignee, sends each assignee a bulk email scoped to their tasks. Unassigned tasks are routed to owners + admins with the unassigned count.
  • Actor always excluded. isUserUnsubscribed('taskAssignments', orgId) still runs per recipient, so personal opt-outs and role matrix still work.
  • New getOwnerAdminRecipients helper splits member.role on commas and checks for exact 'owner' / 'admin' (safer than the previous contains matcher).

notifyAssigneeChange, notifyBulkAssigneeChange, notifyEvidenceReviewRequested, notifyAutomationFailures, task-item-assignment-notifier, comment-mention-notifier and finding-notifier were already targeting narrow audiences — untouched.

Follow-ups (separate ticket)

Found some smaller gaps worth tracking but out of scope here:

  • 4 call sites of isUserUnsubscribed pass no organizationId, so role-matrix settings don't apply: notifyEvidenceReviewRequested, notifyBulkEvidenceReviewRequested, notifyAutomationFailures, notifyBulkAutomationFailures.
  • role.includes('admin') in automation-failure flows would false-match custom roles like "Security Admin".
  • UI description for the "Task Assignments" toggle should mention that it also gates status-change emails (not just assignments).

Test plan

  • New unit tests in task-notifier.service.spec.ts — 8 cases covering assignee targeting, actor exclusion, owner/admin fallback, unsubscribe honoring, bulk per-assignee counts
  • cd apps/api && npx jest src/tasks/ — 113/113 pass
  • Manual smoke in staging: change a task's status as an admin, confirm only the assignee is emailed; unassign a task and change status, confirm owners/admins are emailed

🤖 Generated with Claude Code


Summary by cubic

Scopes task status-change emails to the assignee instead of the whole org, reducing noisy notifications. Bulk updates now send per-assignee summaries; actor is excluded and unsubscribe/matrix settings still apply.

  • Bug Fixes

    • Single status change: email only the task assignee; if unassigned, notify owners/admins; always exclude the actor.
    • Bulk status change: group tasks by assignee and send each recipient a count of their tasks; route unassigned tasks to owners/admins.
    • Honor isUserUnsubscribed with organizationId; add safer owner/admin detection via a helper that parses comma-separated roles.
  • New Features

    • Added RolesService.filterMembersWithPermission(orgId, members, resource, action) to filter members by effective permissions (supports comma-separated and custom roles with one batched query), enabling future migration from role-string checks to permission-based recipient selection.

Written for commit 55c2ee6. Summary will update on new commits.

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>
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 24, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
comp-framework-editor Ready Ready Preview, Comment Apr 24, 2026 4:43pm
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
app Skipped Skipped Apr 24, 2026 4:43pm
portal Skipped Skipped Apr 24, 2026 4:43pm

Request Review

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 2 files

Requires human review: Logic change to notification routing and recipient selection (scoping to assignees vs. org) is a business logic modification that should be verified by a human.

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>
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 issues found across 2 files (changes from recent commits).

Requires human review: This PR modifies core notification business logic by changing recipient selection for task status updates and introduces a new permission-filtering utility in RolesService.

@Marfuen Marfuen merged commit fd0aa1c into main Apr 24, 2026
11 checks passed
@Marfuen Marfuen deleted the mariano/unnecessary-task-notifications branch April 24, 2026 16:46
claudfuen pushed a commit that referenced this pull request Apr 24, 2026
## [3.33.1](v3.33.0...v3.33.1) (2026-04-24)

### Bug Fixes

* **api:** scope task status-change emails to assignee, not whole org ([#2669](#2669)) ([fd0aa1c](fd0aa1c))
@claudfuen
Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 3.33.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants