Skip to content

Commit

Permalink
Do not notify mentioned users that can not participate in the chat
Browse files Browse the repository at this point in the history 
Now mentioned users are notified only if they are already participants
of a private chat (a one to one chat, a group chat, or a password
protected public chat), or if the message was written in a public chat.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
  • Loading branch information
@danxuliu
danxuliu committed Nov 2, 2017
1 parent 13b33cb commit 59f7687
Show file tree
Hide file tree
Showing 2 changed files with 257 additions and 2 deletions.
53 changes: 52 additions & 1 deletion lib/Chat/Notifier.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@

namespace OCA\Spreed\Chat;

use OCA\Spreed\Exceptions\ParticipantNotFoundException;
use OCA\Spreed\Manager;
use OCA\Spreed\Room;
use OCP\Comments\IComment;
use OCP\Notification\IManager as INotificationManager;
use OCP\Notification\INotification;
Expand All @@ -43,14 +46,20 @@ class Notifier {
/** @var IUserManager */
private $userManager;

/** @var Manager */
private $manager;

/**
* @param INotificationManager $notificationManager
* @param IUserManager $userManager
* @param Manager $manager
*/
public function __construct(INotificationManager $notificationManager,
IUserManager $userManager) {
IUserManager $userManager,
Manager $manager) {
$this->notificationManager = $notificationManager;
$this->userManager = $userManager;
$this->manager = $manager;
}

/**
Expand All @@ -59,6 +68,9 @@ public function __construct(INotificationManager $notificationManager,
* The comment must be a chat message comment. That is, its "objectId" must
* be the room ID.
*
* Not every user mentioned in the message is notified, but only those that
* are able to participate in the room.
*
* @param IComment $comment
*/
public function notifyMentionedUsers(IComment $comment) {
Expand Down Expand Up @@ -196,6 +208,45 @@ private function shouldUserBeNotified($userId, $comment) {
return false;
}

$roomId = $comment->getObjectId();
if (!$this->isUserAbleToParticipateInRoom($userId, $roomId)) {
return false;
}

return true;
}

/**
* Returns whether the user with the given ID can participate in the room
* with the given ID or not.
*
* A user can participate in a one to one or a group room if she is a
* participant already. For public rooms, a user can participate if the room
* has no password, or if it has a password and the user is a participant
* already.
*
* @param string $userId
* @param int $roomId
* @return bool true if the user is active in the room, false otherwise.
*/
private function isUserAbleToParticipateInRoom($userId, $roomId) {
$room = $this->manager->getRoomById($roomId);

$roomType = $room->getType();
if ($roomType === Room::UNKNOWN_CALL) {
return false;
}

if ($roomType === Room::PUBLIC_CALL && !$room->hasPassword()) {
return true;
}

try {
$room->getParticipant($userId);
} catch (ParticipantNotFoundException $exception) {
return false;
}

return true;
}

Expand Down
206 changes: 205 additions & 1 deletion tests/php/Chat/NotifierTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@
namespace OCA\Spreed\Tests\php\Chat;

use OCA\Spreed\Chat\Notifier;
use OCA\Spreed\Exceptions\ParticipantNotFoundException;
use OCA\Spreed\Manager;
use OCA\Spreed\Room;
use OCP\Comments\IComment;
use OCP\Notification\IManager as INotificationManager;
use OCP\Notification\INotification;
Expand All @@ -37,6 +40,9 @@ class NotifierTest extends \Test\TestCase {
/** @var \OCP\IUserManager|\PHPUnit_Framework_MockObject_MockObject */
protected $userManager;

/** @var \OCA\Spreed\Manager|\PHPUnit_Framework_MockObject_MockObject */
protected $manager;

/** @var \OCA\Spreed\Chat\Notifier */
protected $notifier;

Expand All @@ -56,8 +62,11 @@ public function setUp() {
return true;
}));

$this->manager = $this->createMock(Manager::class);

$this->notifier = new Notifier($this->notificationManager,
$this->userManager);
$this->userManager,
$this->manager);
}

private function newComment($id, $actorType, $actorId, $creationDateTime, $message) {
Expand All @@ -73,6 +82,7 @@ private function newComment($id, $actorType, $actorId, $creationDateTime, $messa
$comment = $this->createMock(IComment::class);

$comment->method('getId')->willReturn($id);
$comment->method('getObjectId')->willReturn('roomId');
$comment->method('getActorType')->willReturn($actorType);
$comment->method('getActorId')->willReturn($actorId);
$comment->method('getCreationDateTime')->willReturn($creationDateTime);
Expand Down Expand Up @@ -127,6 +137,21 @@ public function testNotifyMentionedUsers() {
->with($comment->getMessage())
->willReturnSelf();

$room = $this->createMock(Room::class);
$this->manager->expects($this->once())
->method('getRoomById')
->with('roomId')
->willReturn($room);

$room->expects($this->once())
->method('getType')
->willReturn(Room::ONE_TO_ONE_CALL);

$room->expects($this->once())
->method('getParticipant')
->with('anotherUser')
->willReturn(true);

$this->notificationManager->expects($this->once())
->method('notify')
->with($notification);
Expand All @@ -153,6 +178,20 @@ public function testNotifyMentionedUsersByGuest() {
->with($comment->getMessage())
->willReturnSelf();

$room = $this->createMock(Room::class);
$this->manager->expects($this->once())
->method('getRoomById')
->with('roomId')
->willReturn($room);

$room->expects($this->once())
->method('getType')
->willReturn(Room::PUBLIC_CALL);

$room->expects($this->once())
->method('hasPassword')
->willReturn(false);

$this->notificationManager->expects($this->once())
->method('notify')
->with($notification);
Expand Down Expand Up @@ -180,6 +219,21 @@ public function testNotifyMentionedUsersWithLongMessageStartMention() {
->with('123456789 @anotherUserWithOddLengthName 123456789-123456789-1234', ['ellipsisEnd'])
->willReturnSelf();

$room = $this->createMock(Room::class);
$this->manager->expects($this->once())
->method('getRoomById')
->with('roomId')
->willReturn($room);

$room->expects($this->once())
->method('getType')
->willReturn(Room::GROUP_CALL);

$room->expects($this->once())
->method('getParticipant')
->with('anotherUserWithOddLengthName')
->willReturn(true);

$this->notificationManager->expects($this->once())
->method('notify')
->with($notification);
Expand Down Expand Up @@ -207,6 +261,21 @@ public function testNotifyMentionedUsersWithLongMessageMiddleMention() {
->with('89-123456789-1234 @anotherUserWithOddLengthName 6789-123456789-1', ['ellipsisStart', 'ellipsisEnd'])
->willReturnSelf();

$room = $this->createMock(Room::class);
$this->manager->expects($this->once())
->method('getRoomById')
->with('roomId')
->willReturn($room);

$room->expects($this->once())
->method('getType')
->willReturn(Room::GROUP_CALL);

$room->expects($this->once())
->method('getParticipant')
->with('anotherUserWithOddLengthName')
->willReturn(true);

$this->notificationManager->expects($this->once())
->method('notify')
->with($notification);
Expand Down Expand Up @@ -234,6 +303,21 @@ public function testNotifyMentionedUsersWithLongMessageEndMention() {
->with('6789-123456789-123456789 @anotherUserWithOddLengthName 123456789', ['ellipsisStart'])
->willReturnSelf();

$room = $this->createMock(Room::class);
$this->manager->expects($this->once())
->method('getRoomById')
->with('roomId')
->willReturn($room);

$room->expects($this->once())
->method('getType')
->willReturn(Room::GROUP_CALL);

$room->expects($this->once())
->method('getParticipant')
->with('anotherUserWithOddLengthName')
->willReturn(true);

$this->notificationManager->expects($this->once())
->method('notify')
->with($notification);
Expand Down Expand Up @@ -265,6 +349,112 @@ public function testNotifyMentionedUsersToUnknownUser() {
$this->notifier->notifyMentionedUsers($comment);
}

public function testNotifyMentionedUsersToUserNotInvitedToPrivateChat() {
$comment = $this->newComment(108, 'users', 'testUser', new \DateTime('@' . 1000000016), 'Mention @userNotInOneToOneChat');

$this->notificationManager->expects($this->never())
->method('createNotification');

$room = $this->createMock(Room::class);
$this->manager->expects($this->once())
->method('getRoomById')
->with('roomId')
->willReturn($room);

$room->expects($this->once())
->method('getType')
->willReturn(Room::ONE_TO_ONE_CALL);

$room->expects($this->once())
->method('getParticipant')
->with('userNotInOneToOneChat')
->will($this->throwException(new ParticipantNotFoundException()));

$this->notificationManager->expects($this->never())
->method('createNotification');

$this->notificationManager->expects($this->never())
->method('notify');

$this->notifier->notifyMentionedUsers($comment);
}

public function testNotifyMentionedUsersToUserNotInvitedToPasswordProtectedPublicChat() {
$comment = $this->newComment(108, 'users', 'testUser', new \DateTime('@' . 1000000016), 'Mention @userNotInvitedToPasswordProtectedPublicChat');

$this->notificationManager->expects($this->never())
->method('createNotification');

$room = $this->createMock(Room::class);
$this->manager->expects($this->once())
->method('getRoomById')
->with('roomId')
->willReturn($room);

$room->expects($this->once())
->method('getType')
->willReturn(Room::PUBLIC_CALL);

$room->expects($this->once())
->method('hasPassword')
->willReturn(true);

$room->expects($this->once())
->method('getParticipant')
->with('userNotInvitedToPasswordProtectedPublicChat')
->will($this->throwException(new ParticipantNotFoundException()));

$this->notificationManager->expects($this->never())
->method('notify');

$this->notifier->notifyMentionedUsers($comment);
}

public function testNotifyMentionedUsersToUserInvitedToPasswordProtectedPublicChat() {
$comment = $this->newComment(108, 'users', 'testUser', new \DateTime('@' . 1000000016), 'Mention @userInvitedToPasswordProtectedPublicChat');

$notification = $this->newNotification($comment);

$this->notificationManager->expects($this->once())
->method('createNotification')
->willReturn($notification);

$notification->expects($this->once())
->method('setUser')
->with('userInvitedToPasswordProtectedPublicChat')
->willReturnSelf();

$notification->expects($this->once())
->method('setMessage')
->with($comment->getMessage())
->willReturnSelf();

$room = $this->createMock(Room::class);
$this->manager->expects($this->once())
->method('getRoomById')
->with('roomId')
->willReturn($room);

$room->expects($this->once())
->method('getType')
->willReturn(Room::PUBLIC_CALL);

$room->expects($this->once())
->method('hasPassword')
->willReturn(true);

$room->expects($this->once())
->method('getParticipant')
->with('userInvitedToPasswordProtectedPublicChat')
->willReturn(true);

$this->notificationManager->expects($this->once())
->method('notify')
->with($notification);

$this->notifier->notifyMentionedUsers($comment);
}

public function testNotifyMentionedUsersNoMentions() {
$comment = $this->newComment(108, 'users', 'testUser', new \DateTime('@' . 1000000016), 'No mentions');

Expand Down Expand Up @@ -310,6 +500,20 @@ public function testNotifyMentionedUsersSeveralMentions() {
->with('notherUser, and @unknownUser, and @testUser, and @userAbleToJoin')
->willReturnSelf();

$room = $this->createMock(Room::class);
$this->manager->expects($this->exactly(2))
->method('getRoomById')
->with('roomId')
->willReturn($room);

$room->expects($this->exactly(2))
->method('getType')
->willReturn(Room::PUBLIC_CALL);

$room->expects($this->exactly(2))
->method('hasPassword')
->willReturn(false);

$this->notificationManager->expects($this->exactly(2))
->method('notify')
->withConsecutive(
Expand Down

0 comments on commit 59f7687

Please sign in to comment.