diff --git a/lib/Chat/Parser/UserMention.php b/lib/Chat/Parser/UserMention.php index 38a20a2de01..4982a1b8e97 100644 --- a/lib/Chat/Parser/UserMention.php +++ b/lib/Chat/Parser/UserMention.php @@ -78,6 +78,11 @@ public function parseMessage(Message $chatMessage): void { $mentionTypeCount = []; $mentions = $comment->getMentions(); + // TODO This can be removed once getMentions() returns sorted results (Nextcloud 21+) + usort($mentions, static function ($m1, $m2) { + return mb_strlen($m2['id']) <=> mb_strlen($m1['id']); + }); + foreach ($mentions as $mention) { if ($mention['type'] === 'user' && $mention['id'] === 'all') { $mention['type'] = 'call'; @@ -101,12 +106,10 @@ public function parseMessage(Message $chatMessage): void { // index of the mentions of that type. $mentionParameterId = 'mention-' . $mention['type'] . $mentionTypeCount[$mention['type']]; - if (strpos($mention['id'], ' ') !== false || strpos($mention['id'], 'guest/') === 0) { - $placeholder = '@"' . $mention['id'] . '"'; - } else { - $placeholder = '@' . $mention['id']; + $message = str_replace('@"' . $mention['id'] . '"', '{' . $mentionParameterId . '}', $message); + if (strpos($mention['id'], ' ') === false && strpos($mention['id'], 'guest/') !== 0) { + $message = str_replace('@' . $mention['id'], '{' . $mentionParameterId . '}', $message); } - $message = str_replace($placeholder, '{' . $mentionParameterId . '}', $message); if ($mention['type'] === 'call') { $messageParameters[$mentionParameterId] = [ diff --git a/tests/integration/features/chat/rich-messages.feature b/tests/integration/features/chat/rich-messages.feature index 13b51323a8a..e4d07c26d2a 100644 --- a/tests/integration/features/chat/rich-messages.feature +++ b/tests/integration/features/chat/rich-messages.feature @@ -3,6 +3,7 @@ Feature: chat/public Given user "participant1" exists Given user "participant2" exists Given user "participant3" exists + Given user "participant3a" exists Scenario: message without enrichable references has empty parameters Given user "participant1" creates room "public room" @@ -48,3 +49,14 @@ Feature: chat/public Then user "participant1" sees the following messages in room "public room" with 200 | room | actorType | actorId | actorDisplayName | message | messageParameters | | public room | users | participant1 | participant1-displayname | Mention to {mention-user1}, @unknownUser, {mention-user1} again and {mention-user2} | {"mention-user1":{"type":"user","id":"participant2","name":"participant2-displayname"},"mention-user2":{"type":"user","id":"participant3","name":"participant3-displayname"}} | + + Scenario: message with mentions of subname users (uid1 is fully part of uid2) + Given user "participant1" creates room "public room" + | roomType | 3 | + | roomName | room | + When user "participant1" sends message "Mention to @participant3 and @participant3a" to room "public room" with 201 + When user "participant1" sends message "Mention to @participant3a and @participant3" 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 | + | public room | users | participant1 | participant1-displayname | Mention to {mention-user1} and {mention-user2} | {"mention-user1":{"type":"user","id":"participant3a","name":"participant3a-displayname"},"mention-user2":{"type":"user","id":"participant3","name":"participant3-displayname"}} | + | public room | users | participant1 | participant1-displayname | Mention to {mention-user2} and {mention-user1} | {"mention-user1":{"type":"user","id":"participant3a","name":"participant3a-displayname"},"mention-user2":{"type":"user","id":"participant3","name":"participant3-displayname"}} | diff --git a/tests/php/Chat/Parser/UserMentionTest.php b/tests/php/Chat/Parser/UserMentionTest.php index a25c136414e..afe9a2486d5 100644 --- a/tests/php/Chat/Parser/UserMentionTest.php +++ b/tests/php/Chat/Parser/UserMentionTest.php @@ -174,6 +174,80 @@ public function testGetRichMessageWithDuplicatedMention() { $this->assertEquals($expectedMessageParameters, $chatMessage->getMessageParameters()); } + public function dataGetRichMessageWithMentionsFullyIncludedInOtherMentions() { + // Based on valid characters from server/lib/private/User/Manager.php + return [ + ['testUser', 'testUser1', false], + ['testUser', 'testUser1', true], + ['testUser', 'testUser_1', false], + ['testUser', 'testUser_1', true], + ['testUser', 'testUser.1', false], + ['testUser', 'testUser.1', true], + ['testUser', 'testUser@1', false], + ['testUser', 'testUser@1', true], + ['testUser', 'testUser-1', false], + ['testUser', 'testUser-1', true], + ['testUser', 'testUser\'1', false], + ['testUser', 'testUser\'1', true], + ]; + } + + /** + * @dataProvider dataGetRichMessageWithMentionsFullyIncludedInOtherMentions + */ + public function testGetRichMessageWithMentionsFullyIncludedInOtherMentions(string $baseId, string $longerId, bool $quoted) { + $mentions = [ + ['type' => 'user', 'id' => $baseId], + ['type' => 'user', 'id' => $longerId], + ]; + $comment = $this->newComment($mentions); + + $this->commentsManager->expects($this->exactly(2)) + ->method('resolveDisplayName') + ->willReturnCallback(function ($type, $id) { + return $id . ' display name'; + }); + + $this->userManager->expects($this->exactly(2)) + ->method('get') + ->withConsecutive( + [$longerId], + [$baseId] + ) + ->willReturn($this->createMock(IUser::class)); + + /** @var Room|MockObject $room */ + $room = $this->createMock(Room::class); + /** @var Participant|MockObject $participant */ + $participant = $this->createMock(Participant::class); + /** @var IL10N|MockObject $l */ + $l = $this->createMock(IL10N::class); + $chatMessage = new Message($room, $participant, $comment, $l); + if ($quoted) { + $chatMessage->setMessage('Mention to @"' . $baseId . '" and @"' . $longerId . '"', []); + } else { + $chatMessage->setMessage('Mention to @' . $baseId . ' and @' . $longerId, []); + } + + $this->parser->parseMessage($chatMessage); + + $expectedMessageParameters = [ + 'mention-user1' => [ + 'type' => 'user', + 'id' => $longerId, + 'name' => $longerId . ' display name' + ], + 'mention-user2' => [ + 'type' => 'user', + 'id' => $baseId, + 'name' => $baseId . ' display name' + ], + ]; + + $this->assertEquals('Mention to {mention-user2} and {mention-user1}', $chatMessage->getMessage()); + $this->assertEquals($expectedMessageParameters, $chatMessage->getMessageParameters()); + } + public function testGetRichMessageWithSeveralMentions() { $mentions = [ ['type'=>'user', 'id'=>'testUser1'], @@ -249,15 +323,12 @@ public function testGetRichMessageWithNonExistingUserMention() { ->with('user', 'testUser') ->willReturn('testUser display name'); - $this->userManager->expects($this->at(0)) - ->method('get') - ->with('me') - ->willReturn(null); - - $this->userManager->expects($this->at(1)) + $this->userManager->expects($this->exactly(2)) ->method('get') - ->with('testUser') - ->willReturn($this->createMock(IUser::class)); + ->willReturnMap([ + ['me', null], + ['testUser', $this->createMock(IUser::class)], + ]); /** @var Room|MockObject $room */ $room = $this->createMock(Room::class);