Skip to content

Commit

Permalink
Only notify participants when they are not active
Browse files Browse the repository at this point in the history 
Also removes notifications for public rooms where the user was not invited.

Signed-off-by: Joas Schilling <coding@schilljs.com>
  • Loading branch information
@nickvergessen
nickvergessen committed Nov 17, 2017
1 parent bef6d2a commit 2a68f59
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 144 deletions.
56 changes: 19 additions & 37 deletions lib/Chat/Notifier.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
namespace OCA\Spreed\Chat;

use OCA\Spreed\Exceptions\ParticipantNotFoundException;
use OCA\Spreed\Exceptions\RoomNotFoundException;
use OCA\Spreed\Manager;
use OCA\Spreed\Room;
use OCP\Comments\IComment;
Expand Down Expand Up @@ -201,56 +202,37 @@ private function getNotificationMessage(IComment $comment, $mentionedUserId) {
return [substr($message, $mentionMiddleIndex - ($maximumLength / 2), $maximumLength), ['ellipsisStart', 'ellipsisEnd']];
}

private function shouldUserBeNotified($userId, IComment $comment) {
if ($userId === $comment->getActorId()) {
// Do not notify the user if she mentioned herself
return false;
}

if (!$this->userManager->userExists($userId)) {
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.
* Determinates whether a user should be notified about the mention:
*
* 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.
* 1. The user did not mention themself
* 2. The user must exist
* 3. The user must be a participant of the room
* 4. The user must not be active in the room
*
* @param string $userId
* @param int $roomId
* @return bool true if the user is active in the room, false otherwise.
* @param IComment $comment
* @return bool
*/
private function isUserAbleToParticipateInRoom($userId, $roomId) {
$room = $this->manager->getRoomById($roomId);

$roomType = $room->getType();
if ($roomType === Room::UNKNOWN_CALL) {
private function shouldUserBeNotified($userId, IComment $comment) {
if ($userId === $comment->getActorId()) {
// Do not notify the user if they mentioned themself
return false;
}

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

try {
$room->getParticipant($userId);
} catch (ParticipantNotFoundException $exception) {
$room = $this->manager->getRoomById($comment->getObjectId());
$participant = $room->getParticipant($userId);
} catch (RoomNotFoundException $e) {
return false;
} catch (ParticipantNotFoundException $e) {
return false;
}

return true;
return $participant->getSessionId() === '0';
}

}
172 changes: 65 additions & 107 deletions tests/php/Chat/NotifierTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
use OCA\Spreed\Chat\Notifier;
use OCA\Spreed\Exceptions\ParticipantNotFoundException;
use OCA\Spreed\Manager;
use OCA\Spreed\Participant;
use OCA\Spreed\Room;
use OCP\Comments\IComment;
use OCP\Notification\IManager as INotificationManager;
Expand Down Expand Up @@ -146,14 +147,15 @@ public function testNotifyMentionedUsers() {
->with('roomId')
->willReturn($room);

$room->expects($this->once())
->method('getType')
->willReturn(Room::ONE_TO_ONE_CALL);
$participant = $this->createMock(Participant::class);
$participant->expects($this->once())
->method('getSessionId')
->willReturn('0');

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

$this->notificationManager->expects($this->once())
->method('notify')
Expand All @@ -162,6 +164,35 @@ public function testNotifyMentionedUsers() {
$this->notifier->notifyMentionedUsers($comment);
}

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

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

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


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

$participant = $this->createMock(Participant::class);
$participant->expects($this->once())
->method('getSessionId')
->willReturn('1');

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

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

public function testNotifyMentionedUsersByGuest() {
$comment = $this->newComment(108, 'guests', 'testSpreedSession', new \DateTime('@' . 1000000016), 'Mention @anotherUser');

Expand All @@ -187,13 +218,15 @@ public function testNotifyMentionedUsersByGuest() {
->with('roomId')
->willReturn($room);

$room->expects($this->once())
->method('getType')
->willReturn(Room::PUBLIC_CALL);
$participant = $this->createMock(Participant::class);
$participant->expects($this->once())
->method('getSessionId')
->willReturn('0');

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

$this->notificationManager->expects($this->once())
->method('notify')
Expand Down Expand Up @@ -228,14 +261,15 @@ public function testNotifyMentionedUsersWithLongMessageStartMention() {
->with('roomId')
->willReturn($room);

$room->expects($this->once())
->method('getType')
->willReturn(Room::GROUP_CALL);
$participant = $this->createMock(Participant::class);
$participant->expects($this->once())
->method('getSessionId')
->willReturn('0');

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

$this->notificationManager->expects($this->once())
->method('notify')
Expand Down Expand Up @@ -270,14 +304,15 @@ public function testNotifyMentionedUsersWithLongMessageMiddleMention() {
->with('roomId')
->willReturn($room);

$room->expects($this->once())
->method('getType')
->willReturn(Room::GROUP_CALL);
$participant = $this->createMock(Participant::class);
$participant->expects($this->once())
->method('getSessionId')
->willReturn('0');

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

$this->notificationManager->expects($this->once())
->method('notify')
Expand Down Expand Up @@ -312,14 +347,15 @@ public function testNotifyMentionedUsersWithLongMessageEndMention() {
->with('roomId')
->willReturn($room);

$room->expects($this->once())
->method('getType')
->willReturn(Room::GROUP_CALL);
$participant = $this->createMock(Participant::class);
$participant->expects($this->once())
->method('getSessionId')
->willReturn('0');

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

$this->notificationManager->expects($this->once())
->method('notify')
Expand Down Expand Up @@ -352,7 +388,7 @@ public function testNotifyMentionedUsersToUnknownUser() {
$this->notifier->notifyMentionedUsers($comment);
}

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

$this->notificationManager->expects($this->never())
Expand All @@ -364,10 +400,6 @@ public function testNotifyMentionedUsersToUserNotInvitedToPrivateChat() {
->with('roomId')
->willReturn($room);

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

$room->expects($this->once())
->method('getParticipant')
->with('userNotInOneToOneChat')
Expand All @@ -382,82 +414,6 @@ public function testNotifyMentionedUsersToUserNotInvitedToPrivateChat() {
$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 @@ -509,13 +465,15 @@ public function testNotifyMentionedUsersSeveralMentions() {
->with('roomId')
->willReturn($room);

$room->expects($this->exactly(2))
->method('getType')
->willReturn(Room::PUBLIC_CALL);
$participant = $this->createMock(Participant::class);
$participant->expects($this->exactly(2))
->method('getSessionId')
->willReturn('0');

$room->expects($this->exactly(2))
->method('hasPassword')
->willReturn(false);
->method('getParticipant')
->withConsecutive(['anotherUser'], ['userAbleToJoin'])
->willReturn($participant);

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

0 comments on commit 2a68f59

Please sign in to comment.