From d76631499a5764178d1fdad81d296fe3c967962a Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 28 Jun 2021 14:37:31 +0200 Subject: [PATCH 1/5] Clear history Signed-off-by: Joas Schilling --- appinfo/routes.php | 9 ++++++++ docs/chat.md | 1 + lib/Chat/ChatManager.php | 15 ++++++++++++ lib/Controller/ChatController.php | 38 ++++++++++++++++++++++++++++++- 4 files changed, 62 insertions(+), 1 deletion(-) diff --git a/appinfo/routes.php b/appinfo/routes.php index 13dc7f1f43..26ec3ccbde 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -162,6 +162,15 @@ return [ 'token' => '^[a-z0-9]{4,30}$', ], ], + [ + 'name' => 'Chat#clearHistory', + 'url' => '/api/{apiVersion}/chat/{token}', + 'verb' => 'DELETE', + 'requirements' => [ + 'apiVersion' => 'v1', + 'token' => '^[a-z0-9]{4,30}$', + ], + ], [ 'name' => 'Chat#deleteMessage', 'url' => '/api/{apiVersion}/chat/{token}/{messageId}', diff --git a/docs/chat.md b/docs/chat.md index d102a1b181..4d7f37eb0c 100644 --- a/docs/chat.md +++ b/docs/chat.md @@ -268,6 +268,7 @@ See [OCP\RichObjectStrings\Definitions](https://github.com/nextcloud/server/blob * `guest_moderator_promoted` - {actor} promoted {user} to moderator * `guest_moderator_demoted` - {actor} demoted {user} from moderator * `message_deleted` - Message deleted by {actor} (Should not be shown to the user) +* `cleared_history` - {actor} removed the history of the conversation * `file_shared` - {file} * `object_shared` - {object} * `matterbridge_config_added` - {actor} set up Matterbridge to synchronize this conversation with other chats diff --git a/lib/Chat/ChatManager.php b/lib/Chat/ChatManager.php index 63a0b3ab66..f0ff88feaf 100644 --- a/lib/Chat/ChatManager.php +++ b/lib/Chat/ChatManager.php @@ -279,6 +279,21 @@ class ChatManager { ); } + public function clearHistory(Room $chat, string $actorType, string $actorId): IComment { + $this->commentsManager->deleteCommentsAtObject('chat', (string) $chat->getId()); + + // TODO reset read markers so they don't error when being used as offset + + return $this->addSystemMessage( + $chat, + $actorType, + $actorId, + json_encode(['message' => 'cleared_history', 'parameters' => []]), + $this->timeFactory->getDateTime(), + false + ); + } + /** * @param Room $chat * @param string $parentId diff --git a/lib/Controller/ChatController.php b/lib/Controller/ChatController.php index 62db43832e..af9e13b310 100644 --- a/lib/Controller/ChatController.php +++ b/lib/Controller/ChatController.php @@ -592,7 +592,43 @@ class ChatController extends AEnvironmentAwareController { $bridge = $this->matterbridgeManager->getBridgeOfRoom($this->room); - $response = new DataResponse($data, $bridge['enabled'] ? Http::STATUS_ACCEPTED: Http::STATUS_OK); + $response = new DataResponse($data, $bridge['enabled'] ? Http::STATUS_ACCEPTED : Http::STATUS_OK); + if ($this->participant->getAttendee()->getReadPrivacy() === Participant::PRIVACY_PUBLIC) { + $response->addHeader('X-Chat-Last-Common-Read', $this->chatManager->getLastCommonReadMessage($this->room)); + } + return $response; + } + + /** + * @NoAdminRequired + * @RequireModeratorParticipant + * @RequireReadWriteConversation + * + * @return DataResponse + */ + public function clearHistory(): DataResponse { + $attendee = $this->participant->getAttendee(); + if (!$this->participant->hasModeratorPermissions(false) + || $this->room->getType() === Room::ONE_TO_ONE_CALL) { + // Actor is not a moderator or not the owner of the message + return new DataResponse([], Http::STATUS_FORBIDDEN); + } + + $systemMessageComment = $this->chatManager->clearHistory( + $this->room, + $attendee->getActorType(), + $attendee->getActorId() + ); + + $systemMessage = $this->messageParser->createMessage($this->room, $this->participant, $systemMessageComment, $this->l); + $this->messageParser->parseMessage($systemMessage); + + + $data = $systemMessage->toArray(); + + $bridge = $this->matterbridgeManager->getBridgeOfRoom($this->room); + + $response = new DataResponse($data, $bridge['enabled'] ? Http::STATUS_ACCEPTED : Http::STATUS_OK); if ($this->participant->getAttendee()->getReadPrivacy() === Participant::PRIVACY_PUBLIC) { $response->addHeader('X-Chat-Last-Common-Read', $this->chatManager->getLastCommonReadMessage($this->room)); } From 81922eef896b9a3b34e7ea68ac99f554760a6d93 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 29 Jun 2021 14:34:53 +0200 Subject: [PATCH 2/5] Remove shares and fix the message markers Signed-off-by: Joas Schilling --- docs/chat.md | 28 ++++++++++++++++++++++++++-- lib/Chat/ChatManager.php | 13 ++++++++++++- lib/Chat/Notifier.php | 12 +++++++----- lib/Chat/Parser/SystemMessage.php | 5 +++++ lib/Service/ParticipantService.php | 9 +++++++++ 5 files changed, 59 insertions(+), 8 deletions(-) diff --git a/docs/chat.md b/docs/chat.md index 4d7f37eb0c..4cc9de0e2f 100644 --- a/docs/chat.md +++ b/docs/chat.md @@ -153,6 +153,30 @@ See [OCP\RichObjectStrings\Definitions](https://github.com/nextcloud/server/blob * Response: [See official OCS Share API docs](https://docs.nextcloud.com/server/latest/developer_manual/client_apis/OCS/ocs-share-api.html?highlight=sharing#create-a-new-share) +## Clear chat history + +* Required capability: `clear-history` +* Method: `DELETE` +* Endpoint: `/chat/{token}` + +* Response: + - Status code: + + `200 OK` - When deleting was successful + + `202 Accepted` - When deleting was successful but Matterbridge is enabled so the message was leaked to other services + + `403 Forbidden` When the user is not a moderator + + `404 Not Found` When the conversation could not be found for the participant + + - Header: + + field | type | Description + ---|---|--- + `X-Chat-Last-Common-Read` | int | ID of the last message read by every user that has read privacy set to public. When the user themself has it set to private the value the header is not set (only available with `chat-read-status` capability) + + - Data: + The full message array of the new system message "You cleared the history of the conversation", as defined in [Receive chat messages of a conversation](#receive-chat-messages-of-a-conversation) + When rendering this message the client should also remove all messages from any cache/storage of the device. + + ## Deleting a chat message * Required capability: `delete-messages` @@ -179,7 +203,7 @@ See [OCP\RichObjectStrings\Definitions](https://github.com/nextcloud/server/blob - Data: The full message array of the new system message "You deleted a message", as defined in [Receive chat messages of a conversation](#receive-chat-messages-of-a-conversation) The parent message is the object of the deleted message with the replaced text "Message deleted by you". - This message should not be displayed to the user but instead be used to remove the original message from any cache/storage of the device. + This message should **NOT** be displayed to the user but instead be used to remove the original message from any cache/storage of the device. ## Mark chat as read @@ -268,7 +292,7 @@ See [OCP\RichObjectStrings\Definitions](https://github.com/nextcloud/server/blob * `guest_moderator_promoted` - {actor} promoted {user} to moderator * `guest_moderator_demoted` - {actor} demoted {user} from moderator * `message_deleted` - Message deleted by {actor} (Should not be shown to the user) -* `cleared_history` - {actor} removed the history of the conversation +* `cleared_history` - {actor} cleared the history of the conversation * `file_shared` - {file} * `object_shared` - {object} * `matterbridge_config_added` - {actor} set up Matterbridge to synchronize this conversation with other chats diff --git a/lib/Chat/ChatManager.php b/lib/Chat/ChatManager.php index f0ff88feaf..2b12079e33 100644 --- a/lib/Chat/ChatManager.php +++ b/lib/Chat/ChatManager.php @@ -32,6 +32,7 @@ use OCA\Talk\Model\Attendee; use OCA\Talk\Participant; use OCA\Talk\Room; use OCA\Talk\Service\ParticipantService; +use OCA\Talk\Share\RoomShareProvider; use OCP\AppFramework\Utility\ITimeFactory; use OCP\Comments\IComment; use OCP\Comments\ICommentsManager; @@ -69,6 +70,8 @@ class ChatManager { private $connection; /** @var INotificationManager */ private $notificationManager; + /** @var RoomShareProvider */ + private $shareProvider; /** @var ParticipantService */ private $participantService; /** @var Notifier */ @@ -84,6 +87,7 @@ class ChatManager { IEventDispatcher $dispatcher, IDBConnection $connection, INotificationManager $notificationManager, + RoomShareProvider $shareProvider, ParticipantService $participantService, Notifier $notifier, ICacheFactory $cacheFactory, @@ -92,6 +96,7 @@ class ChatManager { $this->dispatcher = $dispatcher; $this->connection = $connection; $this->notificationManager = $notificationManager; + $this->shareProvider = $shareProvider; $this->participantService = $participantService; $this->notifier = $notifier; $this->cache = $cacheFactory->createDistributed('talk/lastmsgid'); @@ -282,7 +287,11 @@ class ChatManager { public function clearHistory(Room $chat, string $actorType, string $actorId): IComment { $this->commentsManager->deleteCommentsAtObject('chat', (string) $chat->getId()); - // TODO reset read markers so they don't error when being used as offset + $this->shareProvider->deleteInRoom($chat->getToken()); + + $this->notifier->removePendingNotificationsForRoom($chat, true); + + $this->participantService->resetChatDetails($chat); return $this->addSystemMessage( $chat, @@ -498,6 +507,8 @@ class ChatManager { public function deleteMessages(Room $chat): void { $this->commentsManager->deleteCommentsAtObject('chat', (string) $chat->getId()); + $this->shareProvider->deleteInRoom($chat->getToken()); + $this->notifier->removePendingNotificationsForRoom($chat); } diff --git a/lib/Chat/Notifier.php b/lib/Chat/Notifier.php index 235ffe1287..e3aa088be6 100644 --- a/lib/Chat/Notifier.php +++ b/lib/Chat/Notifier.php @@ -197,7 +197,7 @@ class Notifier { * * @param Room $chat */ - public function removePendingNotificationsForRoom(Room $chat): void { + public function removePendingNotificationsForRoom(Room $chat, bool $chatOnly = false): void { $notification = $this->notificationManager->createNotification(); $shouldFlush = $this->notificationManager->defer(); @@ -207,11 +207,13 @@ class Notifier { $notification->setObject('chat', $chat->getToken()); $this->notificationManager->markProcessed($notification); - $notification->setObject('room', $chat->getToken()); - $this->notificationManager->markProcessed($notification); + if ($chatOnly) { + $notification->setObject('room', $chat->getToken()); + $this->notificationManager->markProcessed($notification); - $notification->setObject('call', $chat->getToken()); - $this->notificationManager->markProcessed($notification); + $notification->setObject('call', $chat->getToken()); + $this->notificationManager->markProcessed($notification); + } if ($shouldFlush) { $this->notificationManager->flush(); diff --git a/lib/Chat/Parser/SystemMessage.php b/lib/Chat/Parser/SystemMessage.php index 03cf8929f1..031a4b9439 100644 --- a/lib/Chat/Parser/SystemMessage.php +++ b/lib/Chat/Parser/SystemMessage.php @@ -402,6 +402,11 @@ class SystemMessage { if ($currentUserIsActor) { $parsedMessage = $this->l->t('You deleted a message'); } + } elseif ($message === 'cleared_history') { + $parsedMessage = $this->l->t('{actor} cleared the history of the conversation'); + if ($currentUserIsActor) { + $parsedMessage = $this->l->t('You cleared the history of the conversation'); + } } else { throw new \OutOfBoundsException('Unknown subject'); } diff --git a/lib/Service/ParticipantService.php b/lib/Service/ParticipantService.php index b5c18054db..0b69ab3ae4 100644 --- a/lib/Service/ParticipantService.php +++ b/lib/Service/ParticipantService.php @@ -787,6 +787,15 @@ class ParticipantService { $query->execute(); } + public function resetChatDetails(Room $room): void { + $query = $this->connection->getQueryBuilder(); + $query->update('talk_attendees') + ->set('last_read_message', $query->createNamedParameter(0, IQueryBuilder::PARAM_INT)) + ->set('last_mention_message', $query->createNamedParameter(0, IQueryBuilder::PARAM_INT)) + ->where($query->expr()->eq('room_id', $query->createNamedParameter($room->getId(), IQueryBuilder::PARAM_INT))); + $query->executeStatement(); + } + public function updateReadPrivacyForActor(string $actorType, string $actorId, int $readPrivacy): void { $query = $this->connection->getQueryBuilder(); $query->update('talk_attendees') From bbed7ef20200e626eb070520b61c5fff9c07e7f3 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 7 Jul 2021 16:01:25 +0200 Subject: [PATCH 3/5] Add integration test Signed-off-by: Joas Schilling --- .../features/bootstrap/FeatureContext.php | 16 ++++++++++ .../integration/features/chat/delete.feature | 32 ++++++++++++++++++- 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index e3acd1ec2d..16160049c9 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -1277,6 +1277,22 @@ class FeatureContext implements Context, SnippetAcceptingContext { $this->assertStatusCode($this->response, $statusCode); } + /** + * @Then /^user "([^"]*)" deletes chat history for room "([^"]*)" with (\d+)(?: \((v1)\))?$/ + * + * @param string $user + * @param string $identifier + * @param int $statusCode + * @param string $apiVersion + */ + public function userDeletesHistoryFromRoom(string $user, string $identifier, int $statusCode, string $apiVersion = 'v1'): void { + $this->setCurrentUser($user); + $this->sendRequest( + 'DELETE', '/apps/spreed/api/' . $apiVersion . '/chat/' . self::$identifierToToken[$identifier] + ); + $this->assertStatusCode($this->response, $statusCode); + } + /** * @Then /^user "([^"]*)" reads message "([^"]*)" in room "([^"]*)" with (\d+)(?: \((v1)\))?$/ * diff --git a/tests/integration/features/chat/delete.feature b/tests/integration/features/chat/delete.feature index 904f7b0eab..5096ce582f 100644 --- a/tests/integration/features/chat/delete.feature +++ b/tests/integration/features/chat/delete.feature @@ -148,7 +148,7 @@ Feature: chat/reply Then user "participant2" received a system messages in room "group room" to delete "Message 1" Scenario: Can only delete own messages in one-to-one - Given user "participant1" creates room "room1" + Given user "participant1" creates room "room1" (v4) | roomType | 1 | | invite | participant2 | And user "participant1" sends message "Message 1" to room "room1" with 201 @@ -177,3 +177,33 @@ Feature: chat/reply | room | actorType | actorId | actorDisplayName | message | messageParameters | | room1 | users | participant2 | participant2-displayname | Message deleted by you | {"actor":{"type":"user","id":"participant2","name":"participant2-displayname"}} | | room1 | users | participant1 | participant1-displayname | Message deleted by author | {"actor":{"type":"user","id":"participant1","name":"participant1-displayname"}} | + + Scenario: Clear chat history as a moderator + Given user "participant1" creates room "room1" (v4) + | roomType | 2 | + | roomName | room | + And user "participant1" adds user "participant2" to room "room1" with 200 (v4) + And user "participant1" sends message "Message 1" to room "room1" with 201 + And user "participant2" sends message "Message 2" to room "room1" with 201 + Then user "participant1" sees the following messages in room "room1" with 200 + | room | actorType | actorId | actorDisplayName | message | messageParameters | + | room1 | users | participant2 | participant2-displayname | Message 2 | [] | + | room1 | users | participant1 | participant1-displayname | Message 1 | [] | + Then user "participant2" sees the following messages in room "room1" with 200 + | room | actorType | actorId | actorDisplayName | message | messageParameters | + | room1 | users | participant2 | participant2-displayname | Message 2 | [] | + | room1 | users | participant1 | participant1-displayname | Message 1 | [] | + And user "participant2" deletes chat history for room "room1" with 403 + Then user "participant1" sees the following messages in room "room1" with 200 + | room | actorType | actorId | actorDisplayName | message | messageParameters | + | room1 | users | participant2 | participant2-displayname | Message 2 | [] | + | room1 | users | participant1 | participant1-displayname | Message 1 | [] | + And user "participant1" deletes chat history for room "room1" with 200 + Then user "participant1" sees the following messages in room "room1" with 200 + | room | actorType | actorId | actorDisplayName | message | messageParameters | + Then user "participant1" sees the following system messages in room "room1" with 200 (v1) + | room | actorType | actorId | actorDisplayName | systemMessage | + | room1 | users | participant1 | participant1-displayname | cleared_history | + Then user "participant2" sees the following system messages in room "room1" with 200 (v1) + | room | actorType | actorId | actorDisplayName | systemMessage | + | room1 | users | participant1 | participant1-displayname | cleared_history | From 31a32b678086a6d70e7a3f4635cdb04a2b38b7b7 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 7 Jul 2021 16:38:24 +0200 Subject: [PATCH 4/5] Add unit tests Signed-off-by: Joas Schilling --- lib/Chat/Notifier.php | 2 +- tests/php/Chat/ChatManagerTest.php | 99 +++++++++++++++++++++++++++--- tests/php/Chat/NotifierTest.php | 57 ++++++++++++----- 3 files changed, 136 insertions(+), 22 deletions(-) diff --git a/lib/Chat/Notifier.php b/lib/Chat/Notifier.php index e3aa088be6..c881d5b88e 100644 --- a/lib/Chat/Notifier.php +++ b/lib/Chat/Notifier.php @@ -207,7 +207,7 @@ class Notifier { $notification->setObject('chat', $chat->getToken()); $this->notificationManager->markProcessed($notification); - if ($chatOnly) { + if (!$chatOnly) { $notification->setObject('room', $chat->getToken()); $this->notificationManager->markProcessed($notification); diff --git a/tests/php/Chat/ChatManagerTest.php b/tests/php/Chat/ChatManagerTest.php index 6bf7447ef1..d64e5f04ba 100644 --- a/tests/php/Chat/ChatManagerTest.php +++ b/tests/php/Chat/ChatManagerTest.php @@ -30,6 +30,7 @@ use OCA\Talk\Chat\Notifier; use OCA\Talk\Participant; use OCA\Talk\Room; use OCA\Talk\Service\ParticipantService; +use OCA\Talk\Share\RoomShareProvider; use OCP\AppFramework\Utility\ITimeFactory; use OCP\Comments\IComment; use OCP\Comments\ICommentsManager; @@ -51,6 +52,8 @@ class ChatManagerTest extends TestCase { protected $dispatcher; /** @var INotificationManager|MockObject */ protected $notificationManager; + /** @var RoomShareProvider|MockObject */ + protected $shareProvider; /** @var ParticipantService|MockObject */ protected $participantService; /** @var Notifier|MockObject */ @@ -66,6 +69,7 @@ class ChatManagerTest extends TestCase { $this->commentsManager = $this->createMock(CommentsManager::class); $this->dispatcher = $this->createMock(IEventDispatcher::class); $this->notificationManager = $this->createMock(INotificationManager::class); + $this->shareProvider = $this->createMock(RoomShareProvider::class); $this->participantService = $this->createMock(ParticipantService::class); $this->notifier = $this->createMock(Notifier::class); $this->timeFactory = $this->createMock(ITimeFactory::class); @@ -76,6 +80,44 @@ class ChatManagerTest extends TestCase { $this->dispatcher, \OC::$server->getDatabaseConnection(), $this->notificationManager, + $this->shareProvider, + $this->participantService, + $this->notifier, + $cacheFactory, + $this->timeFactory + ); + } + + /** + * @param string[] $methods + * @return ChatManager|MockObject + */ + protected function getManager(array $methods = []): ChatManager { + $cacheFactory = $this->createMock(ICacheFactory::class); + + if (!empty($methods)) { + return $this->getMockBuilder(ChatManager::class) + ->setConstructorArgs([ + $this->commentsManager, + $this->dispatcher, + \OC::$server->getDatabaseConnection(), + $this->notificationManager, + $this->shareProvider, + $this->participantService, + $this->notifier, + $cacheFactory, + $this->timeFactory, + ]) + ->setMethods($methods) + ->getMock(); + } + + return new ChatManager( + $this->commentsManager, + $this->dispatcher, + \OC::$server->getDatabaseConnection(), + $this->notificationManager, + $this->shareProvider, $this->participantService, $this->notifier, $cacheFactory, @@ -221,7 +263,7 @@ class ChatManagerTest extends TestCase { $this->assertCommentEquals($commentExpected, $return); } - public function testGetHistory() { + public function testGetHistory(): void { $offset = 1; $limit = 42; $expected = [ @@ -245,7 +287,7 @@ class ChatManagerTest extends TestCase { $this->assertEquals($expected, $comments); } - public function testWaitForNewMessages() { + public function testWaitForNewMessages(): void { $offset = 1; $limit = 42; $timeout = 23; @@ -269,7 +311,7 @@ class ChatManagerTest extends TestCase { ->method('markMentionNotificationsRead') ->with($chat, 'userId'); - /** @var IUser|\PHPUnit_Framework_MockObject_MockObject $user */ + /** @var IUser|MockObject $user */ $user = $this->createMock(IUser::class); $user->expects($this->any()) ->method('getUID') @@ -280,7 +322,7 @@ class ChatManagerTest extends TestCase { $this->assertEquals($expected, $comments); } - public function testWaitForNewMessagesWithWaiting() { + public function testWaitForNewMessagesWithWaiting(): void { $offset = 1; $limit = 42; $timeout = 23; @@ -307,7 +349,7 @@ class ChatManagerTest extends TestCase { ->method('markMentionNotificationsRead') ->with($chat, 'userId'); - /** @var IUser|\PHPUnit_Framework_MockObject_MockObject $user */ + /** @var IUser|MockObject $user */ $user = $this->createMock(IUser::class); $user->expects($this->any()) ->method('getUID') @@ -318,7 +360,7 @@ class ChatManagerTest extends TestCase { $this->assertEquals($expected, $comments); } - public function testGetUnreadCount() { + public function testGetUnreadCount(): void { /** @var Room|MockObject $chat */ $chat = $this->createMock(Room::class); $chat->expects($this->atLeastOnce()) @@ -332,7 +374,7 @@ class ChatManagerTest extends TestCase { $this->chatManager->getUnreadCount($chat, 42); } - public function testDeleteMessages() { + public function testDeleteMessages(): void { $chat = $this->createMock(Room::class); $chat->expects($this->any()) ->method('getId') @@ -348,4 +390,47 @@ class ChatManagerTest extends TestCase { $this->chatManager->deleteMessages($chat); } + + public function testClearHistory(): void { + $chat = $this->createMock(Room::class); + $chat->expects($this->any()) + ->method('getId') + ->willReturn(1234); + $chat->expects($this->any()) + ->method('getToken') + ->willReturn('t0k3n'); + + $this->commentsManager->expects($this->once()) + ->method('deleteCommentsAtObject') + ->with('chat', 1234); + + $this->shareProvider->expects($this->once()) + ->method('deleteInRoom') + ->with('t0k3n'); + + $this->notifier->expects($this->once()) + ->method('removePendingNotificationsForRoom') + ->with($chat, true); + + $this->participantService->expects($this->once()) + ->method('resetChatDetails') + ->with($chat); + + $date = new \DateTime(); + $this->timeFactory->method('getDateTime') + ->willReturn($date); + + $manager = $this->getManager(['addSystemMessage']); + $manager->expects($this->once()) + ->method('addSystemMessage') + ->with( + $chat, + 'users', + 'admin', + json_encode(['message' => 'cleared_history', 'parameters' => []]), + $date, + false + ); + $manager->clearHistory($chat, 'users', 'admin'); + } } diff --git a/tests/php/Chat/NotifierTest.php b/tests/php/Chat/NotifierTest.php index d0214a305e..299e6aedd3 100644 --- a/tests/php/Chat/NotifierTest.php +++ b/tests/php/Chat/NotifierTest.php @@ -83,7 +83,7 @@ class NotifierTest extends TestCase { ); } - private function newComment($id, $actorType, $actorId, $creationDateTime, $message) { + private function newComment($id, $actorType, $actorId, $creationDateTime, $message): IComment { // $mentionMatches[0] contains the whole matches, while // $mentionMatches[1] contains the matched subpattern. $mentionMatches = []; @@ -107,7 +107,7 @@ class NotifierTest extends TestCase { return $comment; } - private function newNotification($room, IComment $comment) { + private function newNotification($room, IComment $comment): INotification { $notification = $this->createMock(INotification::class); $notification->expects($this->once()) @@ -140,7 +140,7 @@ class NotifierTest extends TestCase { return $notification; } - public function testNotifyMentionedUsers() { + public function testNotifyMentionedUsers(): void { $comment = $this->newComment(108, 'users', 'testUser', new \DateTime('@' . 1000000016), 'Mention @anotherUser'); $room = $this->createMock(Room::class); @@ -183,7 +183,7 @@ class NotifierTest extends TestCase { $this->notifier->notifyMentionedUsers($room, $comment, []); } - public function testNotNotifyMentionedUserIfReplyToAuthor() { + public function testNotNotifyMentionedUserIfReplyToAuthor(): void { $comment = $this->newComment(108, 'users', 'testUser', new \DateTime('@' . 1000000016), 'Mention @anotherUser'); $room = $this->createMock(Room::class); @@ -211,7 +211,7 @@ class NotifierTest extends TestCase { $this->notifier->notifyMentionedUsers($room, $comment, ['anotherUser']); } - public function testNotifyMentionedUsersByGuest() { + public function testNotifyMentionedUsersByGuest(): void { $comment = $this->newComment(108, 'guests', 'testSpreedSession', new \DateTime('@' . 1000000016), 'Mention @anotherUser'); $room = $this->createMock(Room::class); @@ -254,7 +254,7 @@ class NotifierTest extends TestCase { $this->notifier->notifyMentionedUsers($room, $comment, []); } - public function testNotifyMentionedUsersWithLongMessageStartMention() { + public function testNotifyMentionedUsersWithLongMessageStartMention(): void { $comment = $this->newComment(108, 'users', 'testUser', new \DateTime('@' . 1000000016), '123456789 @anotherUserWithOddLengthName 123456789-123456789-123456789-123456789-123456789-123456789'); @@ -298,7 +298,7 @@ class NotifierTest extends TestCase { $this->notifier->notifyMentionedUsers($room, $comment, []); } - public function testNotifyMentionedUsersWithLongMessageMiddleMention() { + public function testNotifyMentionedUsersWithLongMessageMiddleMention(): void { $comment = $this->newComment(108, 'users', 'testUser', new \DateTime('@' . 1000000016), '123456789-123456789-123456789-1234 @anotherUserWithOddLengthName 6789-123456789-123456789-123456789'); @@ -342,7 +342,7 @@ class NotifierTest extends TestCase { $this->notifier->notifyMentionedUsers($room, $comment, []); } - public function testNotifyMentionedUsersWithLongMessageEndMention() { + public function testNotifyMentionedUsersWithLongMessageEndMention(): void { $comment = $this->newComment(108, 'users', 'testUser', new \DateTime('@' . 1000000016), '123456789-123456789-123456789-123456789-123456789-123456789 @anotherUserWithOddLengthName 123456789'); @@ -386,7 +386,7 @@ class NotifierTest extends TestCase { $this->notifier->notifyMentionedUsers($room, $comment, []); } - public function testNotifyMentionedUsersToSelf() { + public function testNotifyMentionedUsersToSelf(): void { $comment = $this->newComment(108, 'users', 'testUser', new \DateTime('@' . 1000000016), 'Mention @testUser'); $room = $this->createMock(Room::class); @@ -406,7 +406,7 @@ class NotifierTest extends TestCase { $this->notifier->notifyMentionedUsers($room, $comment, []); } - public function testNotifyMentionedUsersToUnknownUser() { + public function testNotifyMentionedUsersToUnknownUser(): void { $comment = $this->newComment(108, 'users', 'testUser', new \DateTime('@' . 1000000016), 'Mention @unknownUser'); $room = $this->createMock(Room::class); @@ -427,7 +427,7 @@ class NotifierTest extends TestCase { $this->notifier->notifyMentionedUsers($room, $comment, []); } - public function testNotifyMentionedUsersToUserNotInvitedToChat() { + public function testNotifyMentionedUsersToUserNotInvitedToChat(): void { $comment = $this->newComment(108, 'users', 'testUser', new \DateTime('@' . 1000000016), 'Mention @userNotInOneToOneChat'); $room = $this->createMock(Room::class); @@ -458,7 +458,7 @@ class NotifierTest extends TestCase { $this->notifier->notifyMentionedUsers($room, $comment, []); } - public function testNotifyMentionedUsersNoMentions() { + public function testNotifyMentionedUsersNoMentions(): void { $comment = $this->newComment(108, 'users', 'testUser', new \DateTime('@' . 1000000016), 'No mentions'); $room = $this->createMock(Room::class); @@ -475,7 +475,7 @@ class NotifierTest extends TestCase { $this->notifier->notifyMentionedUsers($room, $comment, []); } - public function testNotifyMentionedUsersSeveralMentions() { + public function testNotifyMentionedUsersSeveralMentions(): void { $comment = $this->newComment(108, 'users', 'testUser', new \DateTime('@' . 1000000016), 'Mention @anotherUser, and @unknownUser, and @testUser, and @userAbleToJoin'); $room = $this->createMock(Room::class); @@ -524,7 +524,7 @@ class NotifierTest extends TestCase { $this->notifier->notifyMentionedUsers($room, $comment, []); } - public function testRemovePendingNotificationsForRoom() { + public function testRemovePendingNotificationsForRoom(): void { $notification = $this->createMock(INotification::class); $room = $this->createMock(Room::class); @@ -556,4 +556,33 @@ class NotifierTest extends TestCase { $this->notifier->removePendingNotificationsForRoom($room); } + + public function testRemovePendingNotificationsForChatOnly(): void { + $notification = $this->createMock(INotification::class); + + $room = $this->createMock(Room::class); + $room->expects($this->any()) + ->method('getToken') + ->willReturn('Token123'); + + $this->notificationManager->expects($this->once()) + ->method('createNotification') + ->willReturn($notification); + + $notification->expects($this->once()) + ->method('setApp') + ->with('spreed') + ->willReturnSelf(); + + $notification->expects($this->exactly(1)) + ->method('setObject') + ->with('chat', 'Token123') + ->willReturnSelf(); + + $this->notificationManager->expects($this->exactly(1)) + ->method('markProcessed') + ->with($notification); + + $this->notifier->removePendingNotificationsForRoom($room, true); + } } From b138414888dc8a558bacd9cfe3cf6735117f40b7 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 7 Jul 2021 17:02:04 +0200 Subject: [PATCH 5/5] Change system message string Signed-off-by: Joas Schilling --- docs/chat.md | 2 +- lib/Chat/ChatManager.php | 2 +- lib/Chat/Parser/SystemMessage.php | 2 +- tests/integration/features/chat/delete.feature | 4 ++-- tests/php/Chat/ChatManagerTest.php | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/chat.md b/docs/chat.md index 4cc9de0e2f..9885ce79c7 100644 --- a/docs/chat.md +++ b/docs/chat.md @@ -292,7 +292,7 @@ See [OCP\RichObjectStrings\Definitions](https://github.com/nextcloud/server/blob * `guest_moderator_promoted` - {actor} promoted {user} to moderator * `guest_moderator_demoted` - {actor} demoted {user} from moderator * `message_deleted` - Message deleted by {actor} (Should not be shown to the user) -* `cleared_history` - {actor} cleared the history of the conversation +* `history_cleared` - {actor} cleared the history of the conversation * `file_shared` - {file} * `object_shared` - {object} * `matterbridge_config_added` - {actor} set up Matterbridge to synchronize this conversation with other chats diff --git a/lib/Chat/ChatManager.php b/lib/Chat/ChatManager.php index 2b12079e33..c7de834b30 100644 --- a/lib/Chat/ChatManager.php +++ b/lib/Chat/ChatManager.php @@ -297,7 +297,7 @@ class ChatManager { $chat, $actorType, $actorId, - json_encode(['message' => 'cleared_history', 'parameters' => []]), + json_encode(['message' => 'history_cleared', 'parameters' => []]), $this->timeFactory->getDateTime(), false ); diff --git a/lib/Chat/Parser/SystemMessage.php b/lib/Chat/Parser/SystemMessage.php index 031a4b9439..f3d72a845f 100644 --- a/lib/Chat/Parser/SystemMessage.php +++ b/lib/Chat/Parser/SystemMessage.php @@ -402,7 +402,7 @@ class SystemMessage { if ($currentUserIsActor) { $parsedMessage = $this->l->t('You deleted a message'); } - } elseif ($message === 'cleared_history') { + } elseif ($message === 'history_cleared') { $parsedMessage = $this->l->t('{actor} cleared the history of the conversation'); if ($currentUserIsActor) { $parsedMessage = $this->l->t('You cleared the history of the conversation'); diff --git a/tests/integration/features/chat/delete.feature b/tests/integration/features/chat/delete.feature index 5096ce582f..518c0f1732 100644 --- a/tests/integration/features/chat/delete.feature +++ b/tests/integration/features/chat/delete.feature @@ -203,7 +203,7 @@ Feature: chat/reply | room | actorType | actorId | actorDisplayName | message | messageParameters | Then user "participant1" sees the following system messages in room "room1" with 200 (v1) | room | actorType | actorId | actorDisplayName | systemMessage | - | room1 | users | participant1 | participant1-displayname | cleared_history | + | room1 | users | participant1 | participant1-displayname | history_cleared | Then user "participant2" sees the following system messages in room "room1" with 200 (v1) | room | actorType | actorId | actorDisplayName | systemMessage | - | room1 | users | participant1 | participant1-displayname | cleared_history | + | room1 | users | participant1 | participant1-displayname | history_cleared | diff --git a/tests/php/Chat/ChatManagerTest.php b/tests/php/Chat/ChatManagerTest.php index d64e5f04ba..ea58a8b59f 100644 --- a/tests/php/Chat/ChatManagerTest.php +++ b/tests/php/Chat/ChatManagerTest.php @@ -427,7 +427,7 @@ class ChatManagerTest extends TestCase { $chat, 'users', 'admin', - json_encode(['message' => 'cleared_history', 'parameters' => []]), + json_encode(['message' => 'history_cleared', 'parameters' => []]), $date, false );