diff --git a/lib/Chat/ChatManager.php b/lib/Chat/ChatManager.php index 0869bdac642..5b3fc73a2ec 100644 --- a/lib/Chat/ChatManager.php +++ b/lib/Chat/ChatManager.php @@ -508,11 +508,38 @@ public function editMessage(Room $chat, IComment $comment, Participant $particip $metaData['last_edited_by_id'] = $participant->getAttendee()->getActorId(); $metaData['last_edited_time'] = $editTime->getTimestamp(); $comment->setMetaData($metaData); + + $mentionsBefore = $comment->getMentions(); + $usersDirectlyMentionedBefore = $this->notifier->getMentionedUserIds($comment); + $usersToNotifyBefore = $this->notifier->getUsersToNotify($chat, $comment, []); $comment->setMessage($message, self::MAX_CHAT_LENGTH); + $mentionsAfter = $comment->getMentions(); + $this->commentsManager->save($comment); $this->referenceManager->invalidateCache($chat->getToken()); - // TODO update mentions/notifications + $removedMentions = empty($mentionsAfter) ? $mentionsBefore : array_udiff($mentionsBefore, $mentionsAfter, [$this, 'compareMention']); + $addedMentions = empty($mentionsBefore) ? $mentionsAfter : array_udiff($mentionsAfter, $mentionsBefore, [$this, 'compareMention']); + + // FIXME Not needed when it was silent, once it's stored in metadata + if (!empty($removedMentions)) { + $usersToNotifyAfter = $this->notifier->getUsersToNotify($chat, $comment, []); + $removedUsersMentioned = array_udiff($usersToNotifyBefore, $usersToNotifyAfter, [$this, 'compareMention']); + $userIds = array_column($removedUsersMentioned, 'id'); + $this->notifier->removeMentionNotificationAfterEdit($chat, $comment, $userIds); + } + + // FIXME silent support, once it's stored in metadata + if (!empty($addedMentions)) { + $usersDirectlyMentionedAfter = $this->notifier->getMentionedUserIds($comment); + $addedUsersDirectMentioned = array_diff($usersDirectlyMentionedAfter, $usersDirectlyMentionedBefore); + + $alreadyNotifiedUsers = $this->notifier->notifyMentionedUsers($chat, $comment, $usersToNotifyBefore, silent: false); + if (!empty($alreadyNotifiedUsers)) { + $userIds = array_column($alreadyNotifiedUsers, 'id'); + $this->participantService->markUsersAsMentioned($chat, $userIds, (int) $comment->getId(), $addedUsersDirectMentioned); + } + } return $this->addSystemMessage( $chat, @@ -527,6 +554,13 @@ public function editMessage(Room $chat, IComment $comment, Participant $particip ); } + protected static function compareMention(array $mention1, array $mention2): int { + if ($mention1['type'] === $mention2['type']) { + return $mention1['id'] <=> $mention2['id']; + } + return $mention1['type'] <=> $mention2['type']; + } + public function clearHistory(Room $chat, string $actorType, string $actorId): IComment { $this->commentsManager->deleteCommentsAtObject('chat', (string) $chat->getId()); diff --git a/lib/Chat/Notifier.php b/lib/Chat/Notifier.php index 72974cdb12d..8ba24bd1547 100644 --- a/lib/Chat/Notifier.php +++ b/lib/Chat/Notifier.php @@ -70,7 +70,7 @@ public function __construct( * are able to participate in the room. * * @param array[] $alreadyNotifiedUsers - * @psalm-param array $alreadyNotifiedUsers + * @psalm-param array $alreadyNotifiedUsers * @return string[] Users that were mentioned * @psalm-return array */ @@ -117,11 +117,11 @@ public function notifyMentionedUsers(Room $chat, IComment $comment, array $alrea * @param Room $chat * @param IComment $comment * @param array $alreadyNotifiedUsers - * @psalm-param array $alreadyNotifiedUsers + * @psalm-param array $alreadyNotifiedUsers * @return array - * @psalm-return array + * @psalm-return array */ - private function getUsersToNotify(Room $chat, IComment $comment, array $alreadyNotifiedUsers): array { + public function getUsersToNotify(Room $chat, IComment $comment, array $alreadyNotifiedUsers): array { $usersToNotify = $this->getMentionedUsers($comment); $usersToNotify = $this->getMentionedGroupMembers($chat, $comment, $usersToNotify); $usersToNotify = $this->addMentionAllToList($chat, $usersToNotify); @@ -132,11 +132,11 @@ private function getUsersToNotify(Room $chat, IComment $comment, array $alreadyN /** * @param array $usersToNotify - * @psalm-param array $usersToNotify + * @psalm-param array $usersToNotify * @param array $alreadyNotifiedUsers - * @psalm-param array $alreadyNotifiedUsers + * @psalm-param array $alreadyNotifiedUsers * @return array - * @psalm-return array + * @psalm-return array */ private function removeAlreadyNotifiedUsers(array $usersToNotify, array $alreadyNotifiedUsers): array { return array_filter($usersToNotify, static function (array $userToNotify) use ($alreadyNotifiedUsers): bool { @@ -364,6 +364,36 @@ public function markMentionNotificationsRead(Room $chat, ?string $userId): void } } + /** + * Remove all mention notifications of users that got their mention removed + * + * @param list $userIds + */ + public function removeMentionNotificationAfterEdit(Room $chat, IComment $comment, array $userIds): void { + $shouldFlush = $this->notificationManager->defer(); + $notification = $this->notificationManager->createNotification(); + + $notification + ->setApp('spreed') + ->setObject('chat', $chat->getToken()) + // FIXME message_parameters are not handled by notification app, so this removes all notifications :( + ->setMessage('comment', [ + 'commentId' => $comment->getId(), + ]); + + foreach (['mention_all', 'mention_direct'] as $subject) { + $notification->setSubject($subject); + foreach ($userIds as $userId) { + $notification->setUser($userId); + $this->notificationManager->markProcessed($notification); + } + } + + if ($shouldFlush) { + $this->notificationManager->flush(); + } + } + /** * Returns the IDs of the users mentioned in the given comment. * diff --git a/lib/Service/ParticipantService.php b/lib/Service/ParticipantService.php index 645d6e80049..6d7d20bef6d 100644 --- a/lib/Service/ParticipantService.php +++ b/lib/Service/ParticipantService.php @@ -1267,7 +1267,8 @@ public function markUsersAsMentioned(Room $room, array $userIds, int $messageId, ->set('last_mention_message', $update->createNamedParameter($messageId, IQueryBuilder::PARAM_INT)) ->where($update->expr()->eq('room_id', $update->createNamedParameter($room->getId(), IQueryBuilder::PARAM_INT))) ->andWhere($update->expr()->eq('actor_type', $update->createNamedParameter(Attendee::ACTOR_USERS))) - ->andWhere($update->expr()->in('actor_id', $update->createNamedParameter($userIds, IQueryBuilder::PARAM_STR_ARRAY))); + ->andWhere($update->expr()->in('actor_id', $update->createNamedParameter($userIds, IQueryBuilder::PARAM_STR_ARRAY))) + ->andWhere($update->expr()->lt('last_mention_message', $update->createNamedParameter($messageId, IQueryBuilder::PARAM_INT))); $update->executeStatement(); if (!empty($usersDirectlyMentioned)) { @@ -1276,7 +1277,8 @@ public function markUsersAsMentioned(Room $room, array $userIds, int $messageId, ->set('last_mention_direct', $update->createNamedParameter($messageId, IQueryBuilder::PARAM_INT)) ->where($update->expr()->eq('room_id', $update->createNamedParameter($room->getId(), IQueryBuilder::PARAM_INT))) ->andWhere($update->expr()->eq('actor_type', $update->createNamedParameter(Attendee::ACTOR_USERS))) - ->andWhere($update->expr()->in('actor_id', $update->createNamedParameter($usersDirectlyMentioned, IQueryBuilder::PARAM_STR_ARRAY))); + ->andWhere($update->expr()->in('actor_id', $update->createNamedParameter($usersDirectlyMentioned, IQueryBuilder::PARAM_STR_ARRAY))) + ->andWhere($update->expr()->lt('last_mention_direct', $update->createNamedParameter($messageId, IQueryBuilder::PARAM_INT))); $update->executeStatement(); } } diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index 5ca28622c5c..460c76e1c53 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -3140,7 +3140,7 @@ public function userNotifications(string $user, TableNode $body = null): void { if ($body === null) { self::$lastNotifications = []; - Assert::assertCount(0, $data); + Assert::assertCount(0, $data, json_encode($data, JSON_PRETTY_PRINT)); return; } @@ -3149,7 +3149,7 @@ public function userNotifications(string $user, TableNode $body = null): void { } private function assertNotifications($notifications, TableNode $formData) { - Assert::assertCount(count($formData->getHash()), $notifications, 'Notifications count does not match'); + Assert::assertCount(count($formData->getHash()), $notifications, 'Notifications count does not match:' . "\n" . json_encode($notifications, JSON_PRETTY_PRINT)); Assert::assertEquals($formData->getHash(), array_map(function ($notification, $expectedNotification) { $data = []; if (isset($expectedNotification['object_id'])) { @@ -3181,7 +3181,7 @@ private function assertNotifications($notifications, TableNode $formData) { } return $data; - }, $notifications, $formData->getHash())); + }, $notifications, $formData->getHash()), json_encode($notifications, JSON_PRETTY_PRINT)); } /** diff --git a/tests/integration/features/chat-1/edit-message.feature b/tests/integration/features/chat-1/edit-message.feature index 4af56cd0908..b6b7cf95ca4 100644 --- a/tests/integration/features/chat-1/edit-message.feature +++ b/tests/integration/features/chat-1/edit-message.feature @@ -72,3 +72,148 @@ Feature: chat-1/edit-message Then user "participant1" sees the following messages in room "room" with 200 | room | actorType | actorId | actorDisplayName | message | messageParameters | | room | users | participant1 | participant1-displayname | Caption 1 - Edit 1 | "IGNORE" | + + Scenario: Notification handling - None vs Direct mention + Given user "participant1" creates room "room" (v4) + | roomType | 2 | + | roomName | room | + And user "participant1" adds user "participant2" to room "room" with 200 (v4) + # Join and leave to clear the invite notification + Given user "participant2" joins room "room" with 200 (v4) + And user "participant2" sends message "Message 1" to room "room" with 201 + Then user "participant1" sees the following messages in room "room" with 200 + | room | actorType | actorId | actorDisplayName | message | messageParameters | + | room | users | participant2 | participant2-displayname | Message 1 | [] | + Then user "participant1" is participant of the following rooms (v4) + | id | unreadMessages | unreadMention | unreadMentionDirect | + | room | 1 | 0 | 0 | + Then user "participant1" has the following notifications + And user "participant2" edits message "Message 1" in room "room" to "Message 1 - Edit @participant1" with 200 + Then user "participant1" sees the following messages in room "room" with 200 + | room | actorType | actorId | actorDisplayName | message | messageParameters | + | room | users | participant2 | participant2-displayname | Message 1 - Edit {mention-user1} | {"mention-user1":{"type":"user","id":"participant1","name":"participant1-displayname"}} | + Then user "participant1" has the following notifications + | app | object_type | object_id | subject | + | spreed | chat | room/Message 1 - Edit {mention-user1} | participant2-displayname mentioned you in conversation room | + Then user "participant1" is participant of the following rooms (v4) + | id | unreadMessages | unreadMention | unreadMentionDirect | + | room | 1 | 1 | 1 | + And user "participant2" edits message "Message 1" in room "room" to "Message 1 - Edit 2" with 200 + Then user "participant1" sees the following messages in room "room" with 200 + | room | actorType | actorId | actorDisplayName | message | messageParameters | + | room | users | participant2 | participant2-displayname | Message 1 - Edit 2 | [] | + Then user "participant1" has the following notifications + Then user "participant1" is participant of the following rooms (v4) + | id | unreadMessages | unreadMention | unreadMentionDirect | + | room | 1 | 1 | 1 | + + Scenario: Notification handling - None vs All + Given user "participant1" creates room "room" (v4) + | roomType | 2 | + | roomName | room | + And user "participant1" adds user "participant2" to room "room" with 200 (v4) + # Join and leave to clear the invite notification + Given user "participant2" joins room "room" with 200 (v4) + And user "participant2" sends message "Message 1" to room "room" with 201 + Then user "participant1" sees the following messages in room "room" with 200 + | room | actorType | actorId | actorDisplayName | message | messageParameters | + | room | users | participant2 | participant2-displayname | Message 1 | [] | + Then user "participant1" has the following notifications + Then user "participant1" is participant of the following rooms (v4) + | id | unreadMessages | unreadMention | unreadMentionDirect | + | room | 1 | 0 | 0 | + And user "participant2" edits message "Message 1" in room "room" to "Message 1 - Edit @all" with 200 + Then user "participant1" sees the following messages in room "room" with 200 + | room | actorType | actorId | actorDisplayName | message | messageParameters | + | room | users | participant2 | participant2-displayname | Message 1 - Edit {mention-call1} | "IGNORE" | + Then user "participant1" has the following notifications + | app | object_type | object_id | subject | + | spreed | chat | room/Message 1 - Edit {mention-call1} | participant2-displayname mentioned everyone in conversation room | + Then user "participant1" is participant of the following rooms (v4) + | id | unreadMessages | unreadMention | unreadMentionDirect | + | room | 1 | 1 | 0 | + And user "participant2" edits message "Message 1" in room "room" to "Message 1 - Edit 2" with 200 + Then user "participant1" sees the following messages in room "room" with 200 + | room | actorType | actorId | actorDisplayName | message | messageParameters | + | room | users | participant2 | participant2-displayname | Message 1 - Edit 2 | [] | + Then user "participant1" has the following notifications + Then user "participant1" is participant of the following rooms (v4) + | id | unreadMessages | unreadMention | unreadMentionDirect | + | room | 1 | 1 | 0 | + + Scenario: Notification handling - Direct mention vs All + Given user "participant1" creates room "room" (v4) + | roomType | 2 | + | roomName | room | + And user "participant1" adds user "participant2" to room "room" with 200 (v4) + # Join and leave to clear the invite notification + Given user "participant2" joins room "room" with 200 (v4) + And user "participant2" sends message "Message 1 - @participant1" to room "room" with 201 + Then user "participant1" sees the following messages in room "room" with 200 + | room | actorType | actorId | actorDisplayName | message | messageParameters | + | room | users | participant2 | participant2-displayname | Message 1 - {mention-user1} | {"mention-user1":{"type":"user","id":"participant1","name":"participant1-displayname"}} | + Then user "participant1" has the following notifications + | app | object_type | object_id | subject | + | spreed | chat | room/Message 1 - {mention-user1} | participant2-displayname mentioned you in conversation room | + Then user "participant1" is participant of the following rooms (v4) + | id | unreadMessages | unreadMention | unreadMentionDirect | + | room | 1 | 1 | 1 | + And user "participant2" edits message "Message 1 - @participant1" in room "room" to "Message 1 - Edit @all" with 200 + Then user "participant1" sees the following messages in room "room" with 200 + | room | actorType | actorId | actorDisplayName | message | messageParameters | + | room | users | participant2 | participant2-displayname | Message 1 - Edit {mention-call1} | "IGNORE" | + Then user "participant1" has the following notifications + | app | object_type | object_id | subject | + | spreed | chat | room/Message 1 - Edit {mention-call1} | participant2-displayname mentioned you in conversation room | + Then user "participant1" is participant of the following rooms (v4) + | id | unreadMessages | unreadMention | unreadMentionDirect | + | room | 1 | 1 | 1 | + And user "participant2" edits message "Message 1 - @participant1" in room "room" to "Message 1 - Edit @participant1" with 200 + Then user "participant1" sees the following messages in room "room" with 200 + | room | actorType | actorId | actorDisplayName | message | messageParameters | + | room | users | participant2 | participant2-displayname | Message 1 - Edit {mention-user1} | {"mention-user1":{"type":"user","id":"participant1","name":"participant1-displayname"}} | + Then user "participant1" has the following notifications + | app | object_type | object_id | subject | + | spreed | chat | room/Message 1 - Edit {mention-user1} | participant2-displayname mentioned you in conversation room | + Then user "participant1" is participant of the following rooms (v4) + | id | unreadMessages | unreadMention | unreadMentionDirect | + | room | 1 | 1 | 1 | + + + Scenario: Notification handling - All vs Direct mention + Given user "participant1" creates room "room" (v4) + | roomType | 2 | + | roomName | room | + And user "participant1" adds user "participant2" to room "room" with 200 (v4) + # Join and leave to clear the invite notification + Given user "participant2" joins room "room" with 200 (v4) + And user "participant2" sends message "Message 1 - @all" to room "room" with 201 + Then user "participant1" sees the following messages in room "room" with 200 + | room | actorType | actorId | actorDisplayName | message | messageParameters | + | room | users | participant2 | participant2-displayname | Message 1 - {mention-call1} | "IGNORE" | + Then user "participant1" has the following notifications + | app | object_type | object_id | subject | + | spreed | chat | room/Message 1 - {mention-call1} | participant2-displayname mentioned everyone in conversation room | + Then user "participant1" is participant of the following rooms (v4) + | id | unreadMessages | unreadMention | unreadMentionDirect | + | room | 1 | 1 | 0 | + And user "participant2" edits message "Message 1 - @all" in room "room" to "Message 1 - Edit @participant1" with 200 + Then user "participant1" sees the following messages in room "room" with 200 + | room | actorType | actorId | actorDisplayName | message | messageParameters | + | room | users | participant2 | participant2-displayname | Message 1 - Edit {mention-user1} | {"mention-user1":{"type":"user","id":"participant1","name":"participant1-displayname"}} | + Then user "participant1" has the following notifications + | app | object_type | object_id | subject | + | spreed | chat | room/Message 1 - Edit {mention-user1} | participant2-displayname mentioned everyone in conversation room | + Then user "participant1" is participant of the following rooms (v4) + | id | unreadMessages | unreadMention | unreadMentionDirect | + | room | 1 | 1 | 1 | + And user "participant2" edits message "Message 1 - Edit @participant1" in room "room" to "Message 1 - Edit @all" with 200 + Then user "participant1" sees the following messages in room "room" with 200 + | room | actorType | actorId | actorDisplayName | message | messageParameters | + | room | users | participant2 | participant2-displayname | Message 1 - Edit {mention-call1} | "IGNORE" | + Then user "participant1" has the following notifications + | app | object_type | object_id | subject | + | spreed | chat | room/Message 1 - Edit {mention-call1} | participant2-displayname mentioned everyone in conversation room | + Then user "participant1" is participant of the following rooms (v4) + | id | unreadMessages | unreadMention | unreadMentionDirect | + | room | 1 | 1 | 1 |