Better chat notification handling

lib/Chat/ChatManager.php

@@ -363,10 +363,6 @@ public function getHistory(Room $chat, int $offset, int $limit, bool $includeLas
 	 *         timeout expired.
 	 */
 	public function waitForNewMessages(Room $chat, int $offset, int $limit, int $timeout, ?IUser $user, bool $includeLastKnown): array {
-		if ($user instanceof IUser) {
-			$this->notifier->markMentionNotificationsRead($chat, $user->getUID());
-		}
-
 		if ($this->cache instanceof NullCache
 			|| $this->cache instanceof ArrayCache) {
 			return $this->waitForNewMessagesWithDatabase($chat, $offset, $limit, $timeout, $includeLastKnown);

lib/Chat/Notifier.php

@@ -218,31 +218,6 @@ public function removePendingNotificationsForRoom(Room $chat): void {
 		}
 	}
 
-	/**
-	 * Removes all the pending mention notifications for the room
-	 *
-	 * @param Room $chat
-	 * @param string $userId
-	 */
-	public function markMentionNotificationsRead(Room $chat, ?string $userId): void {
-		if ($userId === null || $userId === '') {
-			return;
-		}
-
-		$shouldFlush = $this->notificationManager->defer();
-		$notification = $this->notificationManager->createNotification();
-
-		$notification
-			->setApp('spreed')
-			->setObject('chat', $chat->getToken())
-			->setUser($userId);
-
-		$this->notificationManager->markProcessed($notification);
-		if ($shouldFlush) {
-			$this->notificationManager->flush();
-		}
-	}
-
 	/**
 	 * Returns the IDs of the users mentioned in the given comment.
 	 *
@@ -359,7 +334,6 @@ protected function shouldMentionedUserBeNotified(string $userId, IComment $comme
 	 * 1. The participant is not a guest
 	 * 2. The participant is not the writing user
 	 * 3. The participant was not mentioned already
-	 * 4. The participant must not be active in the room
 	 *
 	 * @param Participant $participant
 	 * @param IComment $comment
@@ -377,15 +351,6 @@ protected function shouldParticipantBeNotified(Participant $participant, ICommen
 			return false;
 		}
 
-		if (\in_array($userId, $alreadyNotifiedUsers, true)) {
-			return false;
-		}
-
-		if ($participant->getSession() instanceof Session) {
-			// User is online
-			return false;
-		}
-
-		return true;
+		return !\in_array($userId, $alreadyNotifiedUsers, true);
 	}
 }

lib/Notification/Listener.php

@@ -86,6 +86,7 @@ public static function register(IEventDispatcher $dispatcher): void {
 			/** @var self $listener */
 			$listener = \OC::$server->query(self::class);
 			$listener->markInvitationRead($event->getRoom());
+			$listener->markChatNotificationsRead($event->getRoom());
 		};
 		$dispatcher->addListener(Room::EVENT_AFTER_ROOM_CONNECT, $listener);
 
@@ -190,6 +191,33 @@ public function markInvitationRead(Room $room): void {
 		}
 	}
 
+	/**
+	 * Chat notification: "{actor} mentioned you conversation {conversation}"
+	 *
+	 * @param Room $room
+	 */
+	public function markChatNotificationsRead(Room $room): void {
+		$currentUser = $this->userSession->getUser();
+		if (!$currentUser instanceof IUser) {
+			return;
+		}
+
+		$shouldFlush = $this->notificationManager->defer();
+		$notification = $this->notificationManager->createNotification();
+		try {
+			$notification->setApp('spreed')
+				->setObject('chat', $room->getToken())
+				->setUser($currentUser->getUID());
+			$this->notificationManager->markProcessed($notification);
+		} catch (\InvalidArgumentException $e) {
+			$this->logger->error($e->getMessage(), ['exception' => $e]);
+		}
+
+		if ($shouldFlush) {
+			$this->notificationManager->flush();
+		}
+	}
+
 	/**
 	 * Call notification: "{user} wants to talk with you"
 	 *

lib/Notification/Notifier.php

@@ -283,6 +283,7 @@ protected function parseChatMessage(INotification $notification, Room $room, Par
 			throw new \InvalidArgumentException('Unknown object type');
 		}
 
+
 		$subjectParameters = $notification->getSubjectParameters();
 
 		$richSubjectUser = null;
@@ -314,6 +315,12 @@ protected function parseChatMessage(INotification $notification, Room $room, Par
 			throw new AlreadyProcessedException();
 		}
 
+		if (!$this->notificationManager->isPreparingPushNotification()
+			&& $participant->getAttendee()->getLastReadMessage() >= $messageParameters['commentId']) {
+			// Mark notification processed when the user read the message already
+			throw new AlreadyProcessedException();
+		}
+
 		try {
 			$comment = $this->commentManager->get($messageParameters['commentId']);
 		} catch (NotFoundException $e) {

tests/php/Chat/ChatManagerTest.php

@@ -265,10 +265,6 @@ public function testWaitForNewMessages() {
 			->with('chat', 1234, $offset, 'asc', $limit)
 			->willReturn($expected);
 
-		$this->notifier->expects($this->once())
-			->method('markMentionNotificationsRead')
-			->with($chat, 'userId');
-
 		/** @var IUser|\PHPUnit_Framework_MockObject_MockObject $user */
 		$user = $this->createMock(IUser::class);
 		$user->expects($this->any())
@@ -303,10 +299,6 @@ public function testWaitForNewMessagesWithWaiting() {
 				$expected
 			);
 
-		$this->notifier->expects($this->once())
-			->method('markMentionNotificationsRead')
-			->with($chat, 'userId');
-
 		/** @var IUser|\PHPUnit_Framework_MockObject_MockObject $user */
 		$user = $this->createMock(IUser::class);
 		$user->expects($this->any())