Skip to content

Commit 7bafac4

Browse files
danxuliudanxuliu
committed
Do not notify mentioned users that can not participate in the chat
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>
1 parent 13b33cb commit 7bafac4

File tree

2 files changed

+257
-2
lines changed

2 files changed

+257
-2
lines changed

‎lib/Chat/Notifier.php

+52-1
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@
2323

2424
namespace OCA\Spreed\Chat;
2525

26+
use OCA\Spreed\Exceptions\ParticipantNotFoundException;
27+
use OCA\Spreed\Manager;
28+
use OCA\Spreed\Room;
2629
use OCP\Comments\IComment;
2730
use OCP\Notification\IManager as INotificationManager;
2831
use OCP\Notification\INotification;
@@ -43,14 +46,20 @@ class Notifier {
4346
/** @var IUserManager */
4447
private $userManager;
4548

49+
/** @var Manager */
50+
private $manager;
51+
4652
/**
4753
* @param INotificationManager $notificationManager
4854
* @param IUserManager $userManager
55+
* @param Manager $manager
4956
*/
5057
public function __construct(INotificationManager $notificationManager,
51-
IUserManager $userManager) {
58+
IUserManager $userManager,
59+
Manager $manager) {
5260
$this->notificationManager = $notificationManager;
5361
$this->userManager = $userManager;
62+
$this->manager = $manager;
5463
}
5564

5665
/**
@@ -59,6 +68,9 @@ public function __construct(INotificationManager $notificationManager,
5968
* The comment must be a chat message comment. That is, its "objectId" must
6069
* be the room ID.
6170
*
71+
* Not every user mentioned in the message is notified, but only those that
72+
* are able to participate in the room.
73+
*
6274
* @param IComment $comment
6375
*/
6476
public function notifyMentionedUsers(IComment $comment) {
@@ -196,6 +208,45 @@ private function shouldUserBeNotified($userId, $comment) {
196208
return false;
197209
}
198210

211+
$roomId = $comment->getObjectId();
212+
if (!$this->isUserAbleToParticipateInRoom($userId, $roomId)) {
213+
return false;
214+
}
215+
216+
return true;
217+
}
218+
219+
/**
220+
* Returns whether the user with the given ID can participate in the room
221+
* with the given ID or not.
222+
*
223+
* A user can participate in a one to one or a group room if she is a
224+
* participant already. For public rooms, a user can participate if the room
225+
* has no password, or if it has a password and the user is a participant
226+
* already.
227+
+
228+
+ @param string userId
229+
* @param int roomId
230+
* @return bool true if the user is active in the room, false otherwise.
231+
*/
232+
private function isUserAbleToParticipateInRoom($userId, $roomId) {
233+
$room = $this->manager->getRoomById($roomId);
234+
235+
$roomType = $room->getType();
236+
if ($roomType === Room::UNKNOWN_CALL) {
237+
return false;
238+
}
239+
240+
if ($roomType === Room::PUBLIC_CALL && !$room->hasPassword()) {
241+
return true;
242+
}
243+
244+
try {
245+
$room->getParticipant($userId);
246+
} catch (ParticipantNotFoundException $exception) {
247+
return false;
248+
}
249+
199250
return true;
200251
}
201252

‎tests/php/Chat/NotifierTest.php

+205-1
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@
2424
namespace OCA\Spreed\Tests\php\Chat;
2525

2626
use OCA\Spreed\Chat\Notifier;
27+
use OCA\Spreed\Exceptions\ParticipantNotFoundException;
28+
use OCA\Spreed\Manager;
29+
use OCA\Spreed\Room;
2730
use OCP\Comments\IComment;
2831
use OCP\Notification\IManager as INotificationManager;
2932
use OCP\Notification\INotification;
@@ -37,6 +40,9 @@ class NotifierTest extends \Test\TestCase {
3740
/** @var \OCP\IUserManager|\PHPUnit_Framework_MockObject_MockObject */
3841
protected $userManager;
3942

43+
/** @var \OCA\Spreed\Manager|\PHPUnit_Framework_MockObject_MockObject */
44+
protected $manager;
45+
4046
/** @var \OCA\Spreed\Chat\Notifier */
4147
protected $notifier;
4248

@@ -56,8 +62,11 @@ public function setUp() {
5662
return true;
5763
}));
5864

65+
$this->manager = $this->createMock(Manager::class);
66+
5967
$this->notifier = new Notifier($this->notificationManager,
60-
$this->userManager);
68+
$this->userManager,
69+
$this->manager);
6170
}
6271

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

7584
$comment->method('getId')->willReturn($id);
85+
$comment->method('getObjectId')->willReturn('roomId');
7686
$comment->method('getActorType')->willReturn($actorType);
7787
$comment->method('getActorId')->willReturn($actorId);
7888
$comment->method('getCreationDateTime')->willReturn($creationDateTime);
@@ -127,6 +137,21 @@ public function testNotifyMentionedUsers() {
127137
->with($comment->getMessage())
128138
->willReturnSelf();
129139

140+
$room = $this->createMock(Room::class);
141+
$this->manager->expects($this->once())
142+
->method('getRoomById')
143+
->with('roomId')
144+
->willReturn($room);
145+
146+
$room->expects($this->once())
147+
->method('getType')
148+
->willReturn(Room::ONE_TO_ONE_CALL);
149+
150+
$room->expects($this->once())
151+
->method('getParticipant')
152+
->with('anotherUser')
153+
->willReturn(true);
154+
130155
$this->notificationManager->expects($this->once())
131156
->method('notify')
132157
->with($notification);
@@ -153,6 +178,20 @@ public function testNotifyMentionedUsersByGuest() {
153178
->with($comment->getMessage())
154179
->willReturnSelf();
155180

181+
$room = $this->createMock(Room::class);
182+
$this->manager->expects($this->once())
183+
->method('getRoomById')
184+
->with('roomId')
185+
->willReturn($room);
186+
187+
$room->expects($this->once())
188+
->method('getType')
189+
->willReturn(Room::PUBLIC_CALL);
190+
191+
$room->expects($this->once())
192+
->method('hasPassword')
193+
->willReturn(false);
194+
156195
$this->notificationManager->expects($this->once())
157196
->method('notify')
158197
->with($notification);
@@ -180,6 +219,21 @@ public function testNotifyMentionedUsersWithLongMessageStartMention() {
180219
->with('123456789 @anotherUserWithOddLengthName 123456789-123456789-1234', ['ellipsisEnd'])
181220
->willReturnSelf();
182221

222+
$room = $this->createMock(Room::class);
223+
$this->manager->expects($this->once())
224+
->method('getRoomById')
225+
->with('roomId')
226+
->willReturn($room);
227+
228+
$room->expects($this->once())
229+
->method('getType')
230+
->willReturn(Room::GROUP_CALL);
231+
232+
$room->expects($this->once())
233+
->method('getParticipant')
234+
->with('anotherUserWithOddLengthName')
235+
->willReturn(true);
236+
183237
$this->notificationManager->expects($this->once())
184238
->method('notify')
185239
->with($notification);
@@ -207,6 +261,21 @@ public function testNotifyMentionedUsersWithLongMessageMiddleMention() {
207261
->with('89-123456789-1234 @anotherUserWithOddLengthName 6789-123456789-1', ['ellipsisStart', 'ellipsisEnd'])
208262
->willReturnSelf();
209263

264+
$room = $this->createMock(Room::class);
265+
$this->manager->expects($this->once())
266+
->method('getRoomById')
267+
->with('roomId')
268+
->willReturn($room);
269+
270+
$room->expects($this->once())
271+
->method('getType')
272+
->willReturn(Room::GROUP_CALL);
273+
274+
$room->expects($this->once())
275+
->method('getParticipant')
276+
->with('anotherUserWithOddLengthName')
277+
->willReturn(true);
278+
210279
$this->notificationManager->expects($this->once())
211280
->method('notify')
212281
->with($notification);
@@ -234,6 +303,21 @@ public function testNotifyMentionedUsersWithLongMessageEndMention() {
234303
->with('6789-123456789-123456789 @anotherUserWithOddLengthName 123456789', ['ellipsisStart'])
235304
->willReturnSelf();
236305

306+
$room = $this->createMock(Room::class);
307+
$this->manager->expects($this->once())
308+
->method('getRoomById')
309+
->with('roomId')
310+
->willReturn($room);
311+
312+
$room->expects($this->once())
313+
->method('getType')
314+
->willReturn(Room::GROUP_CALL);
315+
316+
$room->expects($this->once())
317+
->method('getParticipant')
318+
->with('anotherUserWithOddLengthName')
319+
->willReturn(true);
320+
237321
$this->notificationManager->expects($this->once())
238322
->method('notify')
239323
->with($notification);
@@ -265,6 +349,112 @@ public function testNotifyMentionedUsersToUnknownUser() {
265349
$this->notifier->notifyMentionedUsers($comment);
266350
}
267351

352+
public function testNotifyMentionedUsersToUserNotInvitedToPrivateChat() {
353+
$comment = $this->newComment(108, 'users', 'testUser', new \DateTime('@' . 1000000016), 'Mention @userNotInOneToOneChat');
354+
355+
$this->notificationManager->expects($this->never())
356+
->method('createNotification');
357+
358+
$room = $this->createMock(Room::class);
359+
$this->manager->expects($this->once())
360+
->method('getRoomById')
361+
->with('roomId')
362+
->willReturn($room);
363+
364+
$room->expects($this->once())
365+
->method('getType')
366+
->willReturn(Room::ONE_TO_ONE_CALL);
367+
368+
$room->expects($this->once())
369+
->method('getParticipant')
370+
->with('userNotInOneToOneChat')
371+
->will($this->throwException(new ParticipantNotFoundException()));
372+
373+
$this->notificationManager->expects($this->never())
374+
->method('createNotification');
375+
376+
$this->notificationManager->expects($this->never())
377+
->method('notify');
378+
379+
$this->notifier->notifyMentionedUsers($comment);
380+
}
381+
382+
public function testNotifyMentionedUsersToUserNotInvitedToPasswordProtectedPublicChat() {
383+
$comment = $this->newComment(108, 'users', 'testUser', new \DateTime('@' . 1000000016), 'Mention @userNotInvitedToPasswordProtectedPublicChat');
384+
385+
$this->notificationManager->expects($this->never())
386+
->method('createNotification');
387+
388+
$room = $this->createMock(Room::class);
389+
$this->manager->expects($this->once())
390+
->method('getRoomById')
391+
->with('roomId')
392+
->willReturn($room);
393+
394+
$room->expects($this->once())
395+
->method('getType')
396+
->willReturn(Room::PUBLIC_CALL);
397+
398+
$room->expects($this->once())
399+
->method('hasPassword')
400+
->willReturn(true);
401+
402+
$room->expects($this->once())
403+
->method('getParticipant')
404+
->with('userNotInvitedToPasswordProtectedPublicChat')
405+
->will($this->throwException(new ParticipantNotFoundException()));
406+
407+
$this->notificationManager->expects($this->never())
408+
->method('notify');
409+
410+
$this->notifier->notifyMentionedUsers($comment);
411+
}
412+
413+
public function testNotifyMentionedUsersToUserInvitedToPasswordProtectedPublicChat() {
414+
$comment = $this->newComment(108, 'users', 'testUser', new \DateTime('@' . 1000000016), 'Mention @userInvitedToPasswordProtectedPublicChat');
415+
416+
$notification = $this->newNotification($comment);
417+
418+
$this->notificationManager->expects($this->once())
419+
->method('createNotification')
420+
->willReturn($notification);
421+
422+
$notification->expects($this->once())
423+
->method('setUser')
424+
->with('userInvitedToPasswordProtectedPublicChat')
425+
->willReturnSelf();
426+
427+
$notification->expects($this->once())
428+
->method('setMessage')
429+
->with($comment->getMessage())
430+
->willReturnSelf();
431+
432+
$room = $this->createMock(Room::class);
433+
$this->manager->expects($this->once())
434+
->method('getRoomById')
435+
->with('roomId')
436+
->willReturn($room);
437+
438+
$room->expects($this->once())
439+
->method('getType')
440+
->willReturn(Room::PUBLIC_CALL);
441+
442+
$room->expects($this->once())
443+
->method('hasPassword')
444+
->willReturn(true);
445+
446+
$room->expects($this->once())
447+
->method('getParticipant')
448+
->with('userInvitedToPasswordProtectedPublicChat')
449+
->willReturn(true);
450+
451+
$this->notificationManager->expects($this->once())
452+
->method('notify')
453+
->with($notification);
454+
455+
$this->notifier->notifyMentionedUsers($comment);
456+
}
457+
268458
public function testNotifyMentionedUsersNoMentions() {
269459
$comment = $this->newComment(108, 'users', 'testUser', new \DateTime('@' . 1000000016), 'No mentions');
270460

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

503+
$room = $this->createMock(Room::class);
504+
$this->manager->expects($this->exactly(2))
505+
->method('getRoomById')
506+
->with('roomId')
507+
->willReturn($room);
508+
509+
$room->expects($this->exactly(2))
510+
->method('getType')
511+
->willReturn(Room::PUBLIC_CALL);
512+
513+
$room->expects($this->exactly(2))
514+
->method('hasPassword')
515+
->willReturn(false);
516+
313517
$this->notificationManager->expects($this->exactly(2))
314518
->method('notify')
315519
->withConsecutive(

0 commit comments

Comments
 (0)