diff --git a/lib/Notification/NotificationHelper.php b/lib/Notification/NotificationHelper.php index bba2dcf306..b014a88a56 100644 --- a/lib/Notification/NotificationHelper.php +++ b/lib/Notification/NotificationHelper.php @@ -200,9 +200,28 @@ public function sendBoardShared(int $boardId, Acl $acl, bool $markAsRead = false } public function sendMention(IComment $comment): void { - foreach ($comment->getMentions() as $mention) { - $card = $this->cardMapper->find($comment->getObjectId()); - $boardId = $this->cardMapper->findBoardId($card->getId()); + $mentions = $comment->getMentions(); + if (empty($mentions)) { + return; + } + + $card = $this->cardMapper->find($comment->getObjectId()); + $boardId = $this->cardMapper->findBoardId($card->getId()); + $boardUsers = $this->permissionService->findUsers($boardId); + + foreach ($mentions as $mention) { + if (($mention['type'] ?? 'users') !== 'users') { + // skip non-user mentions + continue; + } + $mentionedUserId = (string)$mention['id']; + if ($mentionedUserId === $this->currentUser) { + continue; + } + if (!array_key_exists($mentionedUserId, $boardUsers)) { + // skip users that don't have access to the board + continue; + } $notification = $this->notificationManager->createNotification(); $notification ->setApp('deck') diff --git a/tests/unit/Notification/NotificationHelperTest.php b/tests/unit/Notification/NotificationHelperTest.php index 30483f11a9..11c006ae96 100644 --- a/tests/unit/Notification/NotificationHelperTest.php +++ b/tests/unit/Notification/NotificationHelperTest.php @@ -489,8 +489,8 @@ public function testSendMention() { $comment->expects($this->once()) ->method('getMentions') ->willReturn([ - ['id' => 'user1'], - ['id' => 'user2'] + ['id' => 'user1', 'type' => 'users'], + ['id' => 'user2', 'type' => 'users'], ]); $card = new Card(); $card->setId(123); @@ -503,6 +503,13 @@ public function testSendMention() { ->method('findBoardId') ->with(123) ->willReturn(1); + $this->permissionService->expects($this->once()) + ->method('findUsers') + ->with(1) + ->willReturn([ + 'user1' => $this->createUserMock('user1'), + 'user2' => $this->createUserMock('user2'), + ]); $notification1 = $this->createMock(INotification::class); $notification1->expects($this->once())->method('setApp')->with('deck')->willReturn($notification1); @@ -527,4 +534,149 @@ public function testSendMention() { $this->notificationHelper->sendMention($comment); } + + /** + * When a comment mentions the current user ('admin'), that mention must + * be silently skipped while other mentioned users still get notified. + */ + public function testSendMentionSkipsSelf() { + $comment = $this->createMock(IComment::class); + $comment->expects($this->any()) + ->method('getObjectId') + ->willReturn(123); + $comment->expects($this->any()) + ->method('getMessage') + ->willReturn('@admin @user1 This is a message.'); + $comment->expects($this->once()) + ->method('getMentions') + ->willReturn([ + ['id' => 'admin', 'type' => 'users'], + ['id' => 'user1', 'type' => 'users'], + ]); + $card = new Card(); + $card->setId(123); + $card->setTitle('MyCard'); + $this->cardMapper->expects($this->any()) + ->method('find') + ->with(123) + ->willReturn($card); + $this->cardMapper->expects($this->any()) + ->method('findBoardId') + ->with(123) + ->willReturn(1); + $this->permissionService->expects($this->once()) + ->method('findUsers') + ->with(1) + ->willReturn([ + 'admin' => $this->createUserMock('admin'), + 'user1' => $this->createUserMock('user1'), + ]); + + // Only one notification should be created — for user1, not for admin + $notification = $this->createMock(INotification::class); + $notification->expects($this->once())->method('setApp')->with('deck')->willReturn($notification); + $notification->expects($this->once())->method('setUser')->with('user1')->willReturn($notification); + $notification->expects($this->once())->method('setObject')->with('card', 123)->willReturn($notification); + $notification->expects($this->once())->method('setSubject')->with('card-comment-mentioned', ['MyCard', 1, 'admin'])->willReturn($notification); + $notification->expects($this->once())->method('setDateTime')->willReturn($notification); + + $this->notificationManager->expects($this->once()) + ->method('createNotification') + ->willReturn($notification); + $this->notificationManager->expects($this->once()) + ->method('notify') + ->with($notification); + + $this->notificationHelper->sendMention($comment); + } + + /** + * Unsharing a board (markAsRead=true) must still clean up stale + * notifications via markProcessed, even when the participant is the + * acting user. This ensures pre-existing self-notifications get removed. + */ + public function testSendBoardSharedSelfMarkAsReadStillCleansUp() { + $board = new Board(); + $board->setId(123); + $board->setTitle('MyBoardTitle'); + $acl = new Acl(); + $acl->setParticipant('admin'); + $acl->setType(Acl::PERMISSION_TYPE_USER); + $this->boardMapper->expects($this->once()) + ->method('find') + ->with(123) + ->willReturn($board); + + $notification = $this->createMock(INotification::class); + $notification->expects($this->once())->method('setApp')->with('deck')->willReturn($notification); + $notification->expects($this->once())->method('setUser')->with('admin')->willReturn($notification); + $notification->expects($this->once())->method('setObject')->with('board', 123)->willReturn($notification); + $notification->expects($this->once())->method('setSubject')->with('board-shared', ['MyBoardTitle', 'admin'])->willReturn($notification); + + $this->notificationManager->expects($this->once()) + ->method('createNotification') + ->willReturn($notification); + // markProcessed must be called to clean up any stale notification + $this->notificationManager->expects($this->once()) + ->method('markProcessed') + ->with($notification); + $this->notificationManager->expects($this->never()) + ->method('notify'); + + $this->notificationHelper->sendBoardShared(123, $acl, true); + } + + /** + * A mention of a user who is not a board member must not trigger a notification. + */ + public function testSendMentionSkipsNonBoardMembers(): void { + $comment = $this->createMock(IComment::class); + $comment->expects($this->any()) + ->method('getObjectId') + ->willReturn(123); + $comment->expects($this->any()) + ->method('getMessage') + ->willReturn('@user1 @outsider This is a message.'); + $comment->expects($this->once()) + ->method('getMentions') + ->willReturn([ + ['id' => 'user1', 'type' => 'users'], + ['id' => 'outsider', 'type' => 'users'], + ]); + + $card = new Card(); + $card->setId(123); + $card->setTitle('MyCard'); + $this->cardMapper->expects($this->once()) + ->method('find') + ->with(123) + ->willReturn($card); + $this->cardMapper->expects($this->once()) + ->method('findBoardId') + ->with(123) + ->willReturn(1); + // Only user1 is a board member; outsider is not + $this->permissionService->expects($this->once()) + ->method('findUsers') + ->with(1) + ->willReturn([ + 'user1' => $this->createUserMock('user1'), + ]); + + // Exactly one notification must be created and sent — for user1 only + $notification = $this->createMock(INotification::class); + $notification->expects($this->once())->method('setApp')->with('deck')->willReturn($notification); + $notification->expects($this->once())->method('setUser')->with('user1')->willReturn($notification); + $notification->expects($this->once())->method('setObject')->with('card', 123)->willReturn($notification); + $notification->expects($this->once())->method('setSubject')->with('card-comment-mentioned', ['MyCard', 1, 'admin'])->willReturn($notification); + $notification->expects($this->once())->method('setDateTime')->willReturn($notification); + + $this->notificationManager->expects($this->once()) + ->method('createNotification') + ->willReturn($notification); + $this->notificationManager->expects($this->once()) + ->method('notify') + ->with($notification); + $this->notificationHelper->sendMention($comment); + } }