diff --git a/lib/Controller/ChatController.php b/lib/Controller/ChatController.php index 13e40592b15..896a45031c9 100644 --- a/lib/Controller/ChatController.php +++ b/lib/Controller/ChatController.php @@ -30,6 +30,7 @@ use OCA\Talk\Chat\AutoComplete\Sorter; use OCA\Talk\Chat\ChatManager; use OCA\Talk\Chat\MessageParser; +use OCA\Talk\Chat\Notifier; use OCA\Talk\Chat\ReactionManager; use OCA\Talk\Exceptions\CannotReachRemoteException; use OCA\Talk\Federation\Authenticator; @@ -130,6 +131,7 @@ public function __construct( private IL10N $l, protected Authenticator $federationAuthenticator, protected ProxyCacheMessageService $pcmService, + protected Notifier $notifier, ) { parent::__construct($appName, $request); } @@ -1086,14 +1088,19 @@ public function clearHistory(): DataResponse { #[PublicPage] #[RequireAuthenticatedParticipant] public function setReadMarker(?int $lastReadMessage = null): DataResponse { + $setToMessage = $lastReadMessage ?? $this->room->getLastMessageId(); + if ($setToMessage === $this->room->getLastMessageId() + && $this->participant->getAttendee()->getActorType() === Attendee::ACTOR_USERS) { + $this->notifier->markMentionNotificationsRead($this->room, $this->participant->getAttendee()->getActorId()); + } + if ($this->room->isFederatedConversation()) { /** @var \OCA\Talk\Federation\Proxy\TalkV1\Controller\ChatController $proxy */ $proxy = \OCP\Server::get(\OCA\Talk\Federation\Proxy\TalkV1\Controller\ChatController::class); return $proxy->setReadMarker($this->room, $this->participant, $this->getResponseFormat(), $lastReadMessage); } - $lastReadMessage = $lastReadMessage ?? $this->room->getLastMessageId(); - $this->participantService->updateLastReadMessage($this->participant, $lastReadMessage); + $this->participantService->updateLastReadMessage($this->participant, $setToMessage); $attendee = $this->participant->getAttendee(); $headers = $lastCommonRead = []; diff --git a/lib/Federation/Proxy/TalkV1/Controller/ChatController.php b/lib/Federation/Proxy/TalkV1/Controller/ChatController.php index 781a5b6605f..e926e59072a 100644 --- a/lib/Federation/Proxy/TalkV1/Controller/ChatController.php +++ b/lib/Federation/Proxy/TalkV1/Controller/ChatController.php @@ -26,9 +26,11 @@ namespace OCA\Talk\Federation\Proxy\TalkV1\Controller; +use OCA\Talk\Chat\Notifier; use OCA\Talk\Exceptions\CannotReachRemoteException; use OCA\Talk\Federation\Proxy\TalkV1\ProxyRequest; use OCA\Talk\Federation\Proxy\TalkV1\UserConverter; +use OCA\Talk\Model\Attendee; use OCA\Talk\Participant; use OCA\Talk\ResponseDefinitions; use OCA\Talk\Room; @@ -52,6 +54,7 @@ public function __construct( protected UserConverter $userConverter, protected ParticipantService $participantService, protected RoomFormatter $roomFormatter, + protected Notifier $notifier, ICacheFactory $cacheFactory, ) { $this->proxyCacheMessages = $cacheFactory->isAvailable() ? $cacheFactory->createDistributed('talk/pcm/') : null; @@ -139,6 +142,11 @@ public function receiveMessages( int $markNotificationsAsRead): DataResponse { $cacheKey = sha1(json_encode([$room->getRemoteServer(), $room->getRemoteToken()])); + + if ($lookIntoFuture && $markNotificationsAsRead && $participant->getAttendee()->getActorType() === Attendee::ACTOR_USERS) { + $this->notifier->markMentionNotificationsRead($room, $participant->getAttendee()->getActorId()); + } + if ($lookIntoFuture) { if ($this->proxyCacheMessages instanceof ICache) { for ($i = 0; $i <= $timeout; $i++) { @@ -232,6 +240,10 @@ public function getMessageContext(Room $room, Participant $participant, int $mes ], ); + if ($participant->getAttendee()->getActorType() === Attendee::ACTOR_USERS) { + $this->notifier->markMentionNotificationsRead($room, $participant->getAttendee()->getActorId()); + } + if ($proxy->getStatusCode() === Http::STATUS_NOT_MODIFIED) { return new DataResponse([], Http::STATUS_NOT_MODIFIED); } diff --git a/lib/Notification/Listener.php b/lib/Notification/Listener.php index b078879e90f..f622dd865ca 100644 --- a/lib/Notification/Listener.php +++ b/lib/Notification/Listener.php @@ -160,7 +160,7 @@ protected function markInvitationRead(Room $room, IUser $user): void { * Reaction: "{user} reacted with {reaction} in {call}" * * We should not mark reactions read based on the read-status of the comment - * they apply to, but the point in time when the reaction as done. + * they apply to, but the point in time when the reaction was done. * However, these messages are not visible and don't update the read marker, * so we purge them on joining the conversation. * This already happened before on the initial loading of a chat with diff --git a/tests/integration/features/federation/chat.feature b/tests/integration/features/federation/chat.feature index 21d652edac9..08002cfce35 100644 --- a/tests/integration/features/federation/chat.feature +++ b/tests/integration/features/federation/chat.feature @@ -226,6 +226,13 @@ Feature: federation/chat | spreed | chat | room/Hi @all bye | participant1-displayname mentioned everyone in conversation room | Hi room bye | | spreed | chat | room/Hi @"federated_user/participant2@{$REMOTE_URL}" bye | participant1-displayname mentioned you in conversation room | Hi @participant2-displayname bye | | spreed | chat | room/Message 1-1 | participant1-displayname replied to your message in conversation room | Message 1-1 | + When next message request has the following parameters set + | timeout | 0 | + | lookIntoFuture | 1 | + | lastKnownMessageId | Hi @all bye | + And user "participant2" sees the following messages in room "LOCAL::room" with 304 + Then user "participant2" has the following notifications + | app | object_type | object_id | subject | message | Scenario: Mentioning a federated user as a guest also triggers a notification for them Given the following "spreed" app config is set @@ -248,9 +255,13 @@ Feature: federation/chat Given user "participant2" leaves room "LOCAL::room" with 200 (v4) And user "guest" joins room "room" with 200 (v4) When user "guest" sends message 'Hi @"federated_user/participant2@{$REMOTE_URL}" bye' to room "room" with 201 + When user "guest" sends message "Message 2" to room "room" with 201 Then user "participant2" has the following notifications | app | object_type | object_id | subject | message | | spreed | chat | room/Hi @"federated_user/participant2@{$REMOTE_URL}" bye | A guest mentioned you in conversation room | Hi @participant2-displayname bye | + Then user "participant2" reads message "Message 2" in room "LOCAL::room" with 200 (v1) + Then user "participant2" has the following notifications + | app | object_type | object_id | subject | message | Scenario: Mentioning a federated user as a federated user that is a local user to the mentioned one also triggers a notification for them Given the following "spreed" app config is set diff --git a/tests/php/Controller/ChatControllerTest.php b/tests/php/Controller/ChatControllerTest.php index 76147899cae..feb8588098a 100644 --- a/tests/php/Controller/ChatControllerTest.php +++ b/tests/php/Controller/ChatControllerTest.php @@ -26,6 +26,7 @@ use OCA\Talk\Chat\AutoComplete\SearchPlugin; use OCA\Talk\Chat\ChatManager; use OCA\Talk\Chat\MessageParser; +use OCA\Talk\Chat\Notifier; use OCA\Talk\Chat\ReactionManager; use OCA\Talk\Controller\ChatController; use OCA\Talk\Federation\Authenticator; @@ -117,6 +118,7 @@ class ChatControllerTest extends TestCase { private $l; private Authenticator|MockObject $federationAuthenticator; private ProxyCacheMessageService|MockObject $pcmService; + private Notifier|MockObject $notifier; /** @var Room|MockObject */ protected $room; @@ -157,6 +159,7 @@ public function setUp(): void { $this->l = $this->createMock(IL10N::class); $this->federationAuthenticator = $this->createMock(Authenticator::class); $this->pcmService = $this->createMock(ProxyCacheMessageService::class); + $this->notifier = $this->createMock(Notifier::class); $this->room = $this->createMock(Room::class); @@ -202,6 +205,7 @@ private function recreateChatController() { $this->l, $this->federationAuthenticator, $this->pcmService, + $this->notifier, ); }