fix(notifications): restrict task assignment notifications to assignee by MShreyash09 · Pull Request #3894 · Leantime/leantime

MShreyash09 · 2026-10-11T09:36:49Z

Description

Currently, task creation and task reassignment events enter the general project notification pipeline (Projects::notifyProjectUsers()). As a result, assignment notifications are sent to all project members who have notifications enabled, and broadcasted to project chat channels (Slack, Discord, Mattermost, Telegram).

This pull request introduces an optional $recipientIds allow-list to the Notification model:

Notification Model: Added public ?array $recipientIds = null. When null, the existing project-wide notification flow remains 100% backward-compatible.
Projects Service (notifyProjectUsers):
- Sanitizes and enforces $recipientIds when present.
- Suppresses mention/collaborator bypass so third-party users are not inadvertently added to private assignments.
- Skips project-wide messenger notifications for targeted assignments.
- Scopes in-app notifications to the allow-list.
Tickets Service:
- In quickAddTicket() & addTicket(): Restricts assignment notifications to the assigned user (editorId).
- In updateTicket(): Detects if editorId changed, sets action = 'assigned', and targets only the incoming assignee.
Unit Tests: Added test coverage in ProjectsServiceTest verifying allow-list filtering, bypass suppression, and messenger skipping.

Link to ticket

N/A (or link your issue if one was created on GitHub, e.g. Fixes #xxxx)

Type

Fix
Feature
Cleanup

Screenshot of the result

N/A (Backend notification routing change; no UI modifications)

- Add optional recipientIds allow-list to Notification model - Filter candidate users by recipientIds in Projects::notifyProjectUsers() - Disable mention and collaborator filter bypass when in targeted recipient mode - Suppress project-wide chat messenger broadcasts for targeted assignment notifications - Target assignee on task creation in quickAddTicket() and addTicket() - Detect assignee changes in updateTicket() and target only the new assignee - Add unit test coverage in ProjectsServiceTest

CLAassistant · 2026-10-11T09:36:57Z

All committers have signed the CLA.

Copilot

🟡 Changes recommended

Valid assignees with inherited project access are excluded from targeted notifications.

2 open findings

What changed in this PR

Adds targeted task-assignment notifications while preserving project-wide behavior for other notifications.

Changes:

Introduces an optional recipient allow-list.
Targets ticket creation/reassignment notifications to assignees.
Adds notification-routing unit coverage.

File	Description
`app/Domain/Notifications/Models/Notification.php`	Adds the recipient allow-list.
`app/Domain/Projects/Services/Projects.php`	Applies targeted routing across notification channels.
`app/Domain/Tickets/Services/Tickets.php`	Targets assignment notifications to assignees.
`tests/Unit/app/Domain/Projects/Services/ProjectsServiceTest.php`	Tests targeted delivery behavior.

🧠 Review effort: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

…d update PHPDoc - Build candidate recipients from getUsersWithAccessToProject() when recipientIds is set - Update in-app notification candidate selection to support indirect access - Update notifyProjectUsers PHPDoc regarding mention/collaborator bypass contract - Add unit test coverage for indirect project access notifications

Copilot

🟡 Changes recommended

Valid admin or owner assignees can be omitted from targeted notifications, and key routing behavior lacks coverage.

2 open findings

2 resolved since last review

Previously missed (1)

In code that hasn't changed since last review

Test reassignment notification targeting and unchanged assignments

app/Domain/Tickets/Services/Tickets.php:2633

There is no service-level test for this new reassignment decision: the added tests inject recipientIds directly and do not verify that updateTicket() targets only a genuinely changed, non-empty assignee while ordinary updates remain project-wide. Add tests that capture the Notification passed to notifyProjectUsers() for reassignment, unchanged/omitted editorId, and unassignment.

🧠 Review effort: Balanced

… predicate and assert in-app recipients - Build targeted candidates by validating allow-listed IDs against isUserAssignedToProject() and fetching user rows via UserRepository - Support direct team members, indirect project settings ('all'/'clients'), and global admins/owners for targeted notifications - Reuse validated candidate rows for in-app notification routing - Add assertion for in-app notification recipients in targeted notification test - Add tests for unassigned admin/owner notifications, unauthorized recipient dropping, and author self-assignment exclusion

Copilot

🔵 Needs a closer look

Recipient sanitization can coerce malformed values into unintended user IDs, and reassignment integration lacks direct coverage.

0 open findings

2 resolved since last review

Previously missed (2)

In code that hasn't changed since last review

Reject malformed allow-list entries before casting user IDs

app/Domain/Projects/Services/Projects.php:360

intval broadens malformed allow-list entries instead of rejecting them: for example, true and [7] both become user ID 1, while 2.9 becomes ID 2. Since notificationFilter and plugins can mutate this public model, malformed input can send private assignment details to an unintended user. Accept only positive integers (and, if needed, digit-only strings) before casting.

Add service test coverage for ticket reassignment notifications

app/Domain/Tickets/Services/Tickets.php:2655

No ticket-service test exercises this reassignment branch. The added ProjectsServiceTest injects recipientIds directly, so it would still pass if updateTicket() failed to set the allow-list, targeted an unchanged assignee, or retained the project-wide path. Add a service test that captures notifyProjectUsers() and verifies changed versus unchanged editorId values.

🧠 Review effort: Balanced

… add reassignment test coverage - Validate allow-list entries strictly (positive integers or digit strings) before casting - Add unit test verifying malformed entries are rejected without unintended coercion - Add TicketsService test coverage for ticket reassignment notifications - Add test coverage verifying non-reassignment and unassignment notifications do not target recipientIds

Copilot

🔵 Needs a closer look

Inline reassignment through patchTicket() still does not notify the incoming assignee.

0 open findings

Previously missed (1)

In code that hasn't changed since last review

Notify assignees on inline ticket reassignment

app/Domain/Tickets/Services/Tickets.php:2633

Reassignment targeting is only implemented in updateTicket(), but the current assignee chip and kanban assignee swimlane persist editorId through Tickets.Tickets.patchTicket (the chip contract is documented at app/Views/Templates/components/forms/chip.blade.php:21-23, and the swimlane call is at app/Domain/Tickets/Js/ticketsController.js:1391-1395). That path reaches patch(), which does not build an assignment notification, so common inline reassignments still deliver no notification to the incoming assignee. Move the assignment-notification construction into a shared helper and invoke it after successful editorId changes from both updateTicket() and patch() (while avoiding duplicate sends), with coverage for the patch path.

🧠 Review effort: Balanced

Copilot

🟢 Approval recommended

Static review identified no blocking issues in the scoped notification changes; automated tests were not run.

0 open findings

🧠 Review effort: Balanced

Copilot

🟡 Changes recommended

Allow-list validation can abort dispatch on malformed entries and accept values that resolve to unintended recipients.

1 open issue

Validate allowlist entries before notification dispatch · New

_{Review effort: Balanced}

+        if (is_array($notification->recipientIds)) {
+            // recipientIds is typed as array<int>|null – filter to positive IDs only.
+            $targetedRecipientIds = array_values(array_unique(
+                array_filter($notification->recipientIds, static fn (int $id): bool => $id > 0)


…tIds)

Copilot

🟡 Changes recommended

Targeted in-app notifications are dropped, and personal webhooks bypass the recipient allow-list.

3 open issues

_{Review effort: Balanced}

+            foreach ($targetedUsers ?? [] as $user) {
+                if ((int) $user['id'] !== (int) $notification->authorId) {
+                    $allUsersToNotify[] = $user;
+                }
+            }


+                if ($this->isUserAssignedToProject((int) $targetUserId, (int) $notification->projectId)) {
+                    $user = $this->userRepo->getUser((int) $targetUserId);
+                    if ($user !== false && ($user['notifications'] ?? 1) != 0 && strtolower($user['status'] ?? '') === 'a') {
+                        $users[] = (int) $user['id'];


MShreyash09 requested a review from a team as a code owner October 11, 2026 09:36

MShreyash09 requested review from marcelfolaron and a balanced review from Copilot and removed request for a team October 11, 2026 09:36

Copilot started reviewing on behalf of MShreyash09 October 11, 2026 09:37

View session

Copilot AI reviewed Oct 11, 2026

View reviewed changes

Comment thread app/Domain/Projects/Services/Projects.php Outdated

Comment thread app/Domain/Projects/Services/Projects.php

Copilot AI balanced review requested due to automatic review settings October 11, 2026 09:51

Copilot started reviewing on behalf of MShreyash09 October 11, 2026 09:52

View session

Copilot AI reviewed Oct 11, 2026

View reviewed changes

Comment thread app/Domain/Projects/Services/Projects.php Outdated

Comment thread tests/Unit/app/Domain/Projects/Services/ProjectsServiceTest.php

Copilot AI balanced review requested due to automatic review settings October 11, 2026 10:14

Copilot started reviewing on behalf of MShreyash09 October 11, 2026 10:15

View session

Copilot AI reviewed Oct 11, 2026

View reviewed changes

Copilot AI balanced review requested due to automatic review settings October 11, 2026 10:26

Copilot started reviewing on behalf of MShreyash09 October 11, 2026 10:26

View session

Copilot AI reviewed Oct 11, 2026

View reviewed changes

Merge branch 'master' into fix/task-assignee-targeted-notifications

9895b74

Copilot AI balanced review requested due to automatic review settings October 15, 2026 16:46

Copilot started reviewing on behalf of MShreyash09 October 15, 2026 16:47

View session

Copilot AI reviewed Oct 15, 2026

View reviewed changes

fix(phpstan): remove impossible type checks in recipientIds filtering

04636d2

Copilot AI balanced review requested due to automatic review settings October 21, 2026 02:00

Copilot started reviewing on behalf of MShreyash09 October 21, 2026 02:01

View session

Copilot AI reviewed Oct 21, 2026

View reviewed changes

merge: resolve conflicts with origin/master (activeMembers + recipien…

17f0632

…tIds)

Copilot AI balanced review requested due to automatic review settings October 21, 2026 02:06

Copilot started reviewing on behalf of MShreyash09 October 21, 2026 02:06

View session

Copilot AI reviewed Oct 21, 2026

View reviewed changes

Navigation Menu

Repository navigation

fix(notifications): restrict task assignment notifications to assignee - #3894

fix(notifications): restrict task assignment notifications to assignee#3894
MShreyash09 wants to merge 7 commits into
Leantime:masterfrom
MShreyash09:fix/task-assignee-targeted-notifications

MShreyash09 commented Oct 11, 2026

Uh oh!

CLAassistant commented Oct 11, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

MShreyash09 commented Oct 11, 2026

Description

Link to ticket

Type

Screenshot of the result

Uh oh!

CLAassistant commented Oct 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

CLAassistant commented Oct 11, 2026 •

edited

Loading