From 91d2280ee4c89e53bf70e50d5ac12eff3028e7d8 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 10 Jan 2024 13:59:11 +0100 Subject: [PATCH] fix(chat): Also send a 202 when editing and deleting and a bot is enabled Signed-off-by: Joas Schilling --- docs/chat.md | 4 ++-- lib/Controller/ChatController.php | 23 ++++++++++++++----- openapi.json | 4 ++-- .../integration/features/chat-1/bots.feature | 16 +++++++++++++ tests/php/Controller/ChatControllerTest.php | 4 ++++ 5 files changed, 41 insertions(+), 10 deletions(-) diff --git a/docs/chat.md b/docs/chat.md index 3ecd79da616c..328978801cfd 100644 --- a/docs/chat.md +++ b/docs/chat.md @@ -289,7 +289,7 @@ See [OCP\RichObjectStrings\Definitions](https://github.com/nextcloud/server/blob * 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 + + `202 Accepted` - When deleting was successful, but a bot or Matterbridge is configured, so the information can be replicated to other services + `400 Bad Request` The message is already older than 6 hours + `403 Forbidden` When the message is not from the current user and the user not a moderator + `403 Forbidden` When the conversation is read-only @@ -323,7 +323,7 @@ See [OCP\RichObjectStrings\Definitions](https://github.com/nextcloud/server/blob * Response: - Status code: + `200 OK` - When editing was successful - + `202 Accepted` - When editing was successful but Matterbridge is enabled so the message was leaked to other services + + `202 Accepted` - When editing was successful, but a bot or Matterbridge is configured, so the information can be replicated to other services + `400 Bad Request` The message is already older than 24 hours or another reason why editing is not okay + `403 Forbidden` When the message is not from the current user and the user not a moderator + `403 Forbidden` When the conversation is read-only diff --git a/lib/Controller/ChatController.php b/lib/Controller/ChatController.php index 914535ce9c0b..e6bc809b3fe1 100644 --- a/lib/Controller/ChatController.php +++ b/lib/Controller/ChatController.php @@ -41,6 +41,7 @@ use OCA\Talk\Middleware\Attribute\RequireReadWriteConversation; use OCA\Talk\Model\Attachment; use OCA\Talk\Model\Attendee; +use OCA\Talk\Model\Bot; use OCA\Talk\Model\Message; use OCA\Talk\Model\Session; use OCA\Talk\Participant; @@ -48,6 +49,7 @@ use OCA\Talk\Room; use OCA\Talk\Service\AttachmentService; use OCA\Talk\Service\AvatarService; +use OCA\Talk\Service\BotService; use OCA\Talk\Service\ParticipantService; use OCA\Talk\Service\ReminderService; use OCA\Talk\Service\SessionService; @@ -110,6 +112,7 @@ public function __construct( private IManager $autoCompleteManager, private IUserStatusManager $statusManager, protected MatterbridgeManager $matterbridgeManager, + protected BotService $botService, private SearchPlugin $searchPlugin, private ISearchResult $searchResult, protected ITimeFactory $timeFactory, @@ -673,7 +676,7 @@ protected function loadSelfReactions(array $messages, array $commentIdToIndex): * @return DataResponse|DataResponse, array{}> * * 200: Message deleted successfully - * 202: Message deleted successfully, but Matterbridge is configured, so the information can be replicated elsewhere + * 202: Message deleted successfully, but a bot or Matterbridge is configured, so the information can be replicated elsewhere * 400: Deleting message is not possible * 403: Missing permissions to delete message * 404: Message not found @@ -738,13 +741,17 @@ public function deleteMessage(int $messageId): DataResponse { $data = $systemMessage->toArray($this->getResponseFormat()); $data['parent'] = $message->toArray($this->getResponseFormat()); - $bridge = $this->matterbridgeManager->getBridgeOfRoom($this->room); + $hasBotOrBridge = !empty($this->botService->getBotsForToken($this->room->getToken(), Bot::FEATURE_WEBHOOK)); + if (!$hasBotOrBridge) { + $bridge = $this->matterbridgeManager->getBridgeOfRoom($this->room); + $hasBotOrBridge = $bridge['enabled']; + } $headers = []; if ($this->participant->getAttendee()->getReadPrivacy() === Participant::PRIVACY_PUBLIC) { $headers = ['X-Chat-Last-Common-Read' => (string) $this->chatManager->getLastCommonReadMessage($this->room)]; } - return new DataResponse($data, $bridge['enabled'] ? Http::STATUS_ACCEPTED : Http::STATUS_OK, $headers); + return new DataResponse($data, $hasBotOrBridge ? Http::STATUS_ACCEPTED : Http::STATUS_OK, $headers); } /** @@ -756,7 +763,7 @@ public function deleteMessage(int $messageId): DataResponse { * @return DataResponse|DataResponse, array{}> * * 200: Message edited successfully - * 202: Message edited successfully, but Matterbridge is configured, so the information can be replicated elsewhere + * 202: Message edited successfully, but a bot or Matterbridge is configured, so the information can be replicated to other services * 400: Editing message is not possible * 403: Missing permissions to edit message * 404: Message not found @@ -818,13 +825,17 @@ public function editMessage(int $messageId, string $message): DataResponse { $data = $systemMessage->toArray($this->getResponseFormat()); $data['parent'] = $parseMessage->toArray($this->getResponseFormat()); - $bridge = $this->matterbridgeManager->getBridgeOfRoom($this->room); + $hasBotOrBridge = !empty($this->botService->getBotsForToken($this->room->getToken(), Bot::FEATURE_WEBHOOK)); + if (!$hasBotOrBridge) { + $bridge = $this->matterbridgeManager->getBridgeOfRoom($this->room); + $hasBotOrBridge = $bridge['enabled']; + } $headers = []; if ($this->participant->getAttendee()->getReadPrivacy() === Participant::PRIVACY_PUBLIC) { $headers = ['X-Chat-Last-Common-Read' => (string) $this->chatManager->getLastCommonReadMessage($this->room)]; } - return new DataResponse($data, $bridge['enabled'] ? Http::STATUS_ACCEPTED : Http::STATUS_OK, $headers); + return new DataResponse($data, $hasBotOrBridge ? Http::STATUS_ACCEPTED : Http::STATUS_OK, $headers); } /** diff --git a/openapi.json b/openapi.json index 1345de55b645..3c75b158fb02 100644 --- a/openapi.json +++ b/openapi.json @@ -5962,7 +5962,7 @@ } }, "202": { - "description": "Message deleted successfully, but Matterbridge is configured, so the information can be replicated elsewhere", + "description": "Message deleted successfully, but a bot or Matterbridge is configured, so the information can be replicated elsewhere", "headers": { "X-Chat-Last-Common-Read": { "schema": { @@ -6218,7 +6218,7 @@ } }, "202": { - "description": "Message edited successfully, but Matterbridge is configured, so the information can be replicated elsewhere", + "description": "Message edited successfully, but a bot or Matterbridge is configured, so the information can be replicated to other services", "headers": { "X-Chat-Last-Common-Read": { "schema": { diff --git a/tests/integration/features/chat-1/bots.feature b/tests/integration/features/chat-1/bots.feature index 4e16e67576f2..3489a0a959ad 100644 --- a/tests/integration/features/chat-1/bots.feature +++ b/tests/integration/features/chat-1/bots.feature @@ -320,3 +320,19 @@ Feature: chat/bots | reaction | 👍 | Then user "participant1" retrieve reactions "👍" of message "Message 1" in room "room1" with 200 | actorType | actorId | actorDisplayName | reaction | + + Scenario: Editing and deleting messages returns a 202 status code with a bot + When user "participant1" creates room "room" (v4) + | roomType | 2 | + | roomName | room | + And user "participant1" sends message "Message 1" to room "room" with 201 + Then user "participant1" edits message "Message 1" in room "room" to "Message 1 - Edit 1" with 200 + And user "participant1" deletes message "Message 1 - Edit 1" from room "room" with 200 + Given invoking occ with "talk:bot:install Bot Secret1234567890123456789012345678901234567890 https://localhost/bot1 --feature=webhook" + And the command was successful + And read bot ids from OCC + And invoking occ with "talk:bot:setup BOT(Bot) ROOM(room)" + And the command was successful + When user "participant1" sends message "Message 2" to room "room" with 201 + Then user "participant1" edits message "Message 2" in room "room" to "Message 2 - Edit 2" with 202 + And user "participant1" deletes message "Message 2 - Edit 2" from room "room" with 202 diff --git a/tests/php/Controller/ChatControllerTest.php b/tests/php/Controller/ChatControllerTest.php index a5bbe6da6b69..a1eea3c08715 100644 --- a/tests/php/Controller/ChatControllerTest.php +++ b/tests/php/Controller/ChatControllerTest.php @@ -36,6 +36,7 @@ use OCA\Talk\Room; use OCA\Talk\Service\AttachmentService; use OCA\Talk\Service\AvatarService; +use OCA\Talk\Service\BotService; use OCA\Talk\Service\ParticipantService; use OCA\Talk\Service\ReminderService; use OCA\Talk\Service\SessionService; @@ -95,6 +96,7 @@ class ChatControllerTest extends TestCase { protected $statusManager; /** @var MatterbridgeManager|MockObject */ protected $matterbridgeManager; + protected BotService|MockObject $botService; /** @var SearchPlugin|MockObject */ protected $searchPlugin; /** @var ISearchResult|MockObject */ @@ -138,6 +140,7 @@ public function setUp(): void { $this->autoCompleteManager = $this->createMock(IManager::class); $this->statusManager = $this->createMock(IUserStatusManager::class); $this->matterbridgeManager = $this->createMock(MatterbridgeManager::class); + $this->botService = $this->createMock(BotService::class); $this->searchPlugin = $this->createMock(SearchPlugin::class); $this->searchResult = $this->createMock(ISearchResult::class); $this->eventDispatcher = $this->createMock(IEventDispatcher::class); @@ -179,6 +182,7 @@ private function recreateChatController() { $this->autoCompleteManager, $this->statusManager, $this->matterbridgeManager, + $this->botService, $this->searchPlugin, $this->searchResult, $this->timeFactory,