Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Jan 15, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/chat.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
23 changes: 17 additions & 6 deletions lib/Controller/ChatController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}

/**
Expand All @@ -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, array{error: string}, array{}>|DataResponse<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, e.g. when the new message is empty or the message is too old
* 403: Missing permissions to edit message
* 404: Message not found
Expand Down Expand Up @@ -825,13 +832,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);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions openapi.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5989,7 +5989,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": {
Expand Down Expand Up @@ -6245,7 +6245,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": {
Expand Down
16 changes: 16 additions & 0 deletions tests/integration/features/chat-1/bots.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 4 additions & 0 deletions tests/php/Controller/ChatControllerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -179,6 +182,7 @@ private function recreateChatController() {
$this->autoCompleteManager,
$this->statusManager,
$this->matterbridgeManager,
$this->botService,
$this->searchPlugin,
$this->searchResult,
$this->timeFactory,
Expand Down
Loading