fix(chat): Also send a 202 when editing and deleting and a bot is ena…

…bled Signed-off-by: Joas Schilling <[email protected]>
nextcloud · Jan 10, 2024 · 91d2280 · 91d2280
1 parent 14220aa
commit 91d2280
Show file tree

Hide file tree

Showing 5 changed files with 41 additions and 10 deletions.
diff --git 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
@@ -41,13 +41,15 @@
 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;
 use OCA\Talk\ResponseDefinitions;
 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<Http::STATUS_OK|Http::STATUS_ACCEPTED, TalkChatMessageWithParent, array{X-Chat-Last-Common-Read?: numeric-string}>|DataResponse<Http::STATUS_BAD_REQUEST|Http::STATUS_FORBIDDEN|Http::STATUS_NOT_FOUND|Http::STATUS_METHOD_NOT_ALLOWED, array<empty>, 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<Http::STATUS_OK|Http::STATUS_ACCEPTED, TalkChatMessageWithParent, array{X-Chat-Last-Common-Read?: numeric-string}>|DataResponse<Http::STATUS_BAD_REQUEST|Http::STATUS_FORBIDDEN|Http::STATUS_NOT_FOUND|Http::STATUS_METHOD_NOT_ALLOWED, array<empty>, 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
@@ -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
@@ -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
@@ -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,