Merge pull request #11242 from nextcloud/backport/11224/stable28

[stable28] fix(shares): reply to message with attachments

docs/chat.md

@@ -195,6 +195,7 @@ See [OCP\RichObjectStrings\Definitions](https://github.com/nextcloud/server/blob
 |---------------|--------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
 | `messageType` | string | A message type to show the message in different styles. Currently known: `voice-message` and `comment`                                                                  |
 | `caption`     | string | A caption message that should be shown together with the shared file (only available with `media-caption` capability)                                                   |
+| `replyTo`     | int    | The message ID this caption message is a reply to (only allowed for messages from the same conversation and when the message type is not `system` or `command`)         |
 | `silent`      | bool   | If sent silent the message will not create chat notifications even for mentions (only available with `media-caption` capability, yes `media-caption` not `silent-send`) |
 
 * 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)

lib/Chat/ChatManager.php

@@ -140,7 +140,7 @@ public function addSystemMessage(
 		\DateTime $creationDateTime,
 		bool $sendNotifications,
 		?string $referenceId = null,
-		?int $parentId = null,
+		?IComment $replyTo = null,
 		bool $shouldSkipLastMessageUpdate = false,
 		bool $silent = false,
 	): IComment {
@@ -153,8 +153,8 @@ public function addSystemMessage(
 				$comment->setReferenceId($referenceId);
 			}
 		}
-		if ($parentId !== null) {
-			$comment->setParentId((string) $parentId);
+		if ($replyTo !== null) {
+			$comment->setParentId($replyTo->getId());
 		}
 
 		$messageDecoded = json_decode($message, true);
@@ -168,6 +168,8 @@ public function addSystemMessage(
 
 		$this->setMessageExpiration($chat, $comment);
 
+		$shouldFlush = $this->notificationManager->defer();
+
 		$event = new BeforeSystemMessageSentEvent($chat, $comment, silent: $silent, skipLastActivityUpdate: $shouldSkipLastMessageUpdate);
 		$this->dispatcher->dispatchTyped($event);
 		$event = new ChatEvent($chat, $comment, $shouldSkipLastMessageUpdate, $silent);
@@ -182,6 +184,29 @@ public function addSystemMessage(
 			}
 
 			if ($sendNotifications) {
+				/** @var ?IComment $captionComment */
+				$captionComment = null;
+				$alreadyNotifiedUsers = $usersDirectlyMentioned = [];
+				if ($messageType === 'file_shared') {
+					if (isset($messageDecoded['parameters']['metaData']['caption'])) {
+						$captionComment = clone $comment;
+						$captionComment->setMessage($messageDecoded['parameters']['metaData']['caption'], self::MAX_CHAT_LENGTH);
+						$usersDirectlyMentioned = $this->notifier->getMentionedUserIds($captionComment);
+					}
+					if ($replyTo instanceof IComment) {
+						$alreadyNotifiedUsers = $this->notifier->notifyReplyToAuthor($chat, $comment, $replyTo, $silent);
+						if ($replyTo->getActorType() === Attendee::ACTOR_USERS) {
+							$usersDirectlyMentioned[] = $replyTo->getActorId();
+						}
+					}
+				}
+
+				$alreadyNotifiedUsers = $this->notifier->notifyMentionedUsers($chat, $captionComment ?? $comment, $alreadyNotifiedUsers, $silent);
+				if (!empty($alreadyNotifiedUsers)) {
+					$userIds = array_column($alreadyNotifiedUsers, 'id');
+					$this->participantService->markUsersAsMentioned($chat, $userIds, (int) $comment->getId(), $usersDirectlyMentioned);
+				}
+
 				$this->notifier->notifyOtherParticipant($chat, $comment, [], $silent);
 			}
 
@@ -203,6 +228,10 @@ public function addSystemMessage(
 		}
 		$this->cache->remove($chat->getToken());
 
+		if ($shouldFlush) {
+			$this->notificationManager->flush();
+		}
+
 		if ($messageType === 'object_shared' || $messageType === 'file_shared') {
 			$this->attachmentService->createAttachmentEntry($chat, $comment, $messageType, $messageDecoded['parameters'] ?? []);
 		}
@@ -466,7 +495,7 @@ public function deleteMessage(Room $chat, IComment $comment, Participant $partic
 			$this->timeFactory->getDateTime(),
 			false,
 			null,
-			(int) $comment->getId(),
+			$comment,
 			true
 		);
 	}

lib/Chat/ReactionManager.php

@@ -110,7 +110,7 @@ public function addReactionMessage(Room $chat, string $actorType, string $actorI
 	 */
 	public function deleteReactionMessage(Room $chat, string $actorType, string $actorId, int $messageId, string $reaction): IComment {
 		// Just to verify that messageId is part of the room and throw error if not.
-		$this->getCommentToReact($chat, (string) $messageId);
+		$parentComment = $this->getCommentToReact($chat, (string) $messageId);
 
 		$comment = $this->commentsManager->getReactionComment(
 			$messageId,
@@ -136,7 +136,7 @@ public function deleteReactionMessage(Room $chat, string $actorType, string $act
 			$this->timeFactory->getDateTime(),
 			false,
 			null,
-			$messageId,
+			$parentComment,
 			true
 		);
 

lib/Chat/SystemMessage/Listener.php

@@ -27,6 +27,7 @@
 
 use DateInterval;
 use OCA\Talk\Chat\ChatManager;
+use OCA\Talk\Chat\MessageParser;
 use OCA\Talk\Events\AAttendeeRemovedEvent;
 use OCA\Talk\Events\AParticipantModifiedEvent;
 use OCA\Talk\Events\ARoomModifiedEvent;
@@ -51,8 +52,10 @@
 use OCA\Talk\Webinary;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\Comments\IComment;
+use OCP\Comments\NotFoundException;
 use OCP\EventDispatcher\Event;
 use OCP\EventDispatcher\IEventListener;
+use OCP\IL10N;
 use OCP\IRequest;
 use OCP\ISession;
 use OCP\IUser;
@@ -75,6 +78,8 @@ public function __construct(
 		protected ITimeFactory $timeFactory,
 		protected Manager $manager,
 		protected ParticipantService $participantService,
+		protected MessageParser $messageParser,
+		protected IL10N $l,
 	) {
 	}
 
@@ -455,12 +460,28 @@ protected function sendSystemMessage(Room $room, string $message, array $paramet
 			$referenceId = (string) $referenceId;
 		}
 
+		$parent = null;
+		$replyTo = $parameters['metaData']['replyTo'] ?? null;
+		if ($replyTo !== null) {
+			try {
+				$parentComment = $this->chatManager->getParentComment($room, (string) $replyTo);
+				$parentMessage = $this->messageParser->createMessage($room, $participant, $parentComment, $this->l);
+				$this->messageParser->parseMessage($parentMessage);
+				if ($parentMessage->isReplyable()) {
+					$parent = $parentComment;
+				}
+			} catch (NotFoundException) {
+			}
+
+		}
+
 		return $this->chatManager->addSystemMessage(
 			$room, $actorType, $actorId,
 			json_encode(['message' => $message, 'parameters' => $parameters]),
-			$this->timeFactory->getDateTime(), $message === 'file_shared',
+			$this->timeFactory->getDateTime(),
+			$message === 'file_shared',
 			$referenceId,
-			null,
+			$parent,
 			$shouldSkipLastMessageUpdate,
 			$silent,
 		);

src/components/NewMessage/NewMessage.vue

@@ -54,7 +54,7 @@
 
 			<!-- Input area -->
 			<div class="new-message-form__input">
-				<NewMessageAbsenceInfo v-if="userAbsence"
+				<NewMessageAbsenceInfo v-if="!upload && userAbsence"
 					:user-absence="userAbsence"
 					:display-name="conversation.displayName" />
 
@@ -517,6 +517,7 @@ export default {
 			if (this.upload) {
 				// Clear input content from store
 				this.$store.dispatch('setCurrentMessageInput', { token: this.token, text: '' })
+				this.$store.dispatch('removeMessageToBeReplied', this.token)
 
 				if (this.$store.getters.getInitialisedUploads(this.$store.getters.currentUploadId).length) {
 					// If dialog contains files to upload, delegate sending

src/store/fileUploadStore.js

@@ -375,6 +375,9 @@ const actions = {
 			if (options?.silent) {
 				Object.assign(rawMetadata, { silent: options.silent })
 			}
+			if (temporaryMessage.parent) {
+				Object.assign(rawMetadata, { replyTo: temporaryMessage.parent.id })
+			}
 			const metadata = JSON.stringify(rawMetadata)
 
 			try {

tests/integration/features/bootstrap/FeatureContext.php

@@ -125,6 +125,10 @@ public static function getTokenForIdentifier(string $identifier) {
 		return self::$identifierToToken[$identifier];
 	}
 
+	public static function getMessageIdForText(string $text): int {
+		return self::$textToMessageId[$text];
+	}
+
 	public static function getActorIdForPhoneNumber(string $phoneNumber): string {
 		return self::$phoneNumberToActorId[$phoneNumber];
 	}

tests/integration/features/bootstrap/SharingContext.php

@@ -775,15 +775,28 @@ private function userSharesWith(string $user, string $path, string $shareType, s
 		$parameters[] = 'shareType=' . $shareType;
 		$parameters[] = 'shareWith=' . $shareWith;
 
+		$talkMetaData = [];
+
 		if ($body instanceof TableNode) {
 			foreach ($body->getRowsHash() as $key => $value) {
 				if ($key === 'expireDate' && $value !== 'invalid date') {
 					$value = date('Y-m-d', strtotime($value));
 				}
-				$parameters[] = $key . '=' . $value;
+				if ($key === 'talkMetaData.replyTo') {
+					$value = FeatureContext::getMessageIdForText($value);
+				}
+				if (str_starts_with($key, 'talkMetaData.')) {
+					$talkMetaData[substr($key, 13)] = $value;
+				} else {
+					$parameters[] = $key . '=' . $value;
+				}
 			}
 		}
 
+		if (!empty($talkMetaData)) {
+			$parameters[] = 'talkMetaData=' . json_encode($talkMetaData);
+		}
+
 		$url .= '?' . implode('&', $parameters);
 
 		$this->sendingTo('POST', $url);

tests/integration/features/chat-1/file-share.feature

@@ -42,6 +42,45 @@ Feature: chat/file-share
       | room        | actorType | actorId      | actorDisplayName         | message         | messageParameters |
       | public room | users     | participant1 | participant1-displayname | {mention-user1} | "IGNORE"          |
 
+  Scenario: Captioned message as a reply
+    Given user "participant1" creates room "public room" (v4)
+      | roomType | 3 |
+      | roomName | room |
+    And user "participant1" adds user "participant2" to room "public room" with 200 (v4)
+    And user "participant2" sends message "Message 1" to room "public room" with 201
+    Then user "participant1" sees the following messages in room "public room" with 200
+      | room        | actorType | actorId      | actorDisplayName         | message   | messageParameters | parentMessage |
+      | public room | users     | participant2 | participant2-displayname | Message 1 | []                |               |
+    When user "participant1" shares "welcome.txt" with room "public room"
+      | talkMetaData.caption      | @participant2 |
+      | talkMetaData.replyTo      | Message 1     |
+    Then user "participant1" sees the following messages in room "public room" with 200
+      | room        | actorType | actorId      | actorDisplayName         | message         | messageParameters | parentMessage |
+      | public room | users     | participant1 | participant1-displayname | {mention-user1} | "IGNORE"          | Message 1     |
+      | public room | users     | participant2 | participant2-displayname | Message 1       | []                |               |
+
+  Scenario: Captioned message can not reply cross chats
+    Given user "participant1" creates room "room1" (v4)
+      | roomType | 3 |
+      | roomName | room |
+    Given user "participant1" creates room "room2" (v4)
+      | roomType | 3 |
+      | roomName | room |
+    And user "participant1" adds user "participant2" to room "room1" with 200 (v4)
+    And user "participant2" sends message "Message 1" to room "room1" with 201
+    Then user "participant1" sees the following messages in room "room1" with 200
+      | room  | actorType | actorId      | actorDisplayName         | message   | messageParameters | parentMessage |
+      | room1 | users     | participant2 | participant2-displayname | Message 1 | []                |               |
+    When user "participant1" shares "welcome.txt" with room "room2"
+      | talkMetaData.caption      | @participant2 |
+      | talkMetaData.replyTo      | Message 1     |
+    Then user "participant1" sees the following messages in room "room1" with 200
+      | room  | actorType | actorId      | actorDisplayName         | message   | messageParameters | parentMessage |
+      | room1 | users     | participant2 | participant2-displayname | Message 1 | []                |               |
+    Then user "participant1" sees the following messages in room "room2" with 200
+      | room  | actorType | actorId      | actorDisplayName         | message         | messageParameters | parentMessage |
+      | room2 | users     | participant1 | participant1-displayname | {mention-user1} | "IGNORE"          |               |
+
   Scenario: Can not share a file without chat permission
     Given user "participant1" creates room "public room" (v4)
       | roomType | 3 |

tests/integration/features/chat-1/notifications.feature

@@ -373,6 +373,48 @@ Feature: chat/notifications
       | app    | object_type | object_id        | subject                                                     |
       | spreed | chat        | room/Hi @all @participant2 @"group/attendees1" bye | participant1-displayname replied to your message in conversation room |
 
+  Scenario: Replying with a captioned file gives a reply notification
+    When user "participant1" creates room "room" (v4)
+      | roomType | 2 |
+      | roomName | room |
+    And user "participant1" adds user "participant2" to room "room" with 200 (v4)
+    And user "participant1" adds group "attendees1" to room "room" with 200 (v4)
+    # Join and leave to clear the invite notification
+    Given user "participant2" joins room "room" with 200 (v4)
+    Given user "participant2" leaves room "room" with 200 (v4)
+    When 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 | parentMessage |
+      | room | users     | participant2 | participant2-displayname | Message 1 | []                |               |
+    When user "participant1" shares "welcome.txt" with room "room"
+      | talkMetaData.caption      | Caption 1-1 |
+      | talkMetaData.replyTo      | Message 1   |
+    Then user "participant1" sees the following messages in room "room" with 200
+      | room | actorType | actorId      | actorDisplayName         | message     | messageParameters | parentMessage |
+      | room | users     | participant1 | participant1-displayname | Caption 1-1 | "IGNORE"          | Message 1     |
+      | room | users     | participant2 | participant2-displayname | Message 1   | []                |               |
+    Then user "participant2" has the following notifications
+      | app    | object_type | object_id        | subject                                                               |
+      | spreed | chat        | room/Caption 1-1 | participant1-displayname replied to your message in conversation room |
+
+  Scenario: Mentions in captions trigger normal mention notifications
+    When user "participant1" creates room "room" (v4)
+      | roomType | 2 |
+      | roomName | room |
+    And user "participant1" adds user "participant2" to room "room" with 200 (v4)
+    And user "participant1" adds group "attendees1" to room "room" with 200 (v4)
+    # Join and leave to clear the invite notification
+    Given user "participant2" joins room "room" with 200 (v4)
+    Given user "participant2" leaves room "room" with 200 (v4)
+    When user "participant1" shares "welcome.txt" with room "room"
+      | talkMetaData.caption      | @participant2 |
+    Then user "participant1" sees the following messages in room "room" with 200
+      | room | actorType | actorId      | actorDisplayName         | message   | messageParameters | parentMessage |
+      | room | users     | participant1 | participant1-displayname | {mention-user1} | "IGNORE"    |               |
+    Then user "participant2" has the following notifications
+      | app    | object_type | object_id            | subject                                                     |
+      | spreed | chat        | room/{mention-user1} | participant1-displayname mentioned you in conversation room |
+
   Scenario: Delete notification when the message is deleted
     When user "participant1" creates room "one-to-one room" (v4)
       | roomType | 1 |

tests/php/Chat/ChatManagerTest.php

@@ -473,7 +473,7 @@ public function testDeleteMessage(): void {
 		$chatManager = $this->getManager(['addSystemMessage']);
 		$chatManager->expects($this->once())
 			->method('addSystemMessage')
-			->with($chat, Attendee::ACTOR_USERS, 'user', $this->anything(), $this->anything(), false, null, 123456)
+			->with($chat, Attendee::ACTOR_USERS, 'user', $this->anything(), $this->anything(), false, null, $comment)
 			->willReturn($systemMessage);
 
 		$this->assertSame($systemMessage, $chatManager->deleteMessage($chat, $comment, $participant, $date));
@@ -553,7 +553,7 @@ public function testDeleteMessageFileShare(): void {
 		$chatManager = $this->getManager(['addSystemMessage']);
 		$chatManager->expects($this->once())
 			->method('addSystemMessage')
-			->with($chat, Attendee::ACTOR_USERS, 'user', $this->anything(), $this->anything(), false, null, 123456)
+			->with($chat, Attendee::ACTOR_USERS, 'user', $this->anything(), $this->anything(), false, null, $comment)
 			->willReturn($systemMessage);
 
 		$this->assertSame($systemMessage, $chatManager->deleteMessage($chat, $comment, $participant, $date));

tests/php/Chat/SystemMessage/ListenerTest.php

@@ -22,6 +22,7 @@
 namespace OCA\Talk\Tests\php\Chat\SystemMessage;
 
 use OCA\Talk\Chat\ChatManager;
+use OCA\Talk\Chat\MessageParser;
 use OCA\Talk\Chat\SystemMessage\Listener;
 use OCA\Talk\Events\AParticipantModifiedEvent;
 use OCA\Talk\Events\ARoomModifiedEvent;
@@ -37,6 +38,7 @@
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\Comments\IComment;
 use OCP\EventDispatcher\IEventDispatcher;
+use OCP\IL10N;
 use OCP\IRequest;
 use OCP\ISession;
 use OCP\IUser;
@@ -71,6 +73,8 @@ class ListenerTest extends TestCase {
 	protected $manager;
 	/** @var ParticipantService|MockObject */
 	protected $participantService;
+	/** @var MessageParser|MockObject */
+	protected $messageParser;
 	protected ?array $handlers = null;
 	protected ?\DateTime $dummyTime = null;
 
@@ -94,6 +98,13 @@ protected function setUp(): void {
 		$this->eventDispatcher = $this->createMock(IEventDispatcher::class);
 		$this->manager = $this->createMock(Manager::class);
 		$this->participantService = $this->createMock(ParticipantService::class);
+		$this->messageParser = $this->createMock(MessageParser::class);
+		$l = $this->createMock(IL10N::class);
+		$l->expects($this->any())
+			->method('t')
+			->willReturnCallback(function ($string, $args) {
+				return vsprintf($string, $args);
+			});
 
 		$this->handlers = [];
 
@@ -112,6 +123,8 @@ protected function setUp(): void {
 			$this->timeFactory,
 			$this->manager,
 			$this->participantService,
+			$this->messageParser,
+			$l,
 		);
 	}