Skip to content

Commit 6c229f7

Browse files
nickvergessennickvergessen
authored
Merge pull request #5041 from nextcloud/bugfix/noid/mention-sub-name-users
Mention sub name users
2 parents 71634c6 + e65fa8d commit 6c229f7

File tree

3 files changed

+99
-13
lines changed

3 files changed

+99
-13
lines changed

‎lib/Chat/Parser/UserMention.php

+8-5
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,11 @@ public function parseMessage(Message $chatMessage): void {
8585
$mentionTypeCount = [];
8686

8787
$mentions = $comment->getMentions();
88+
// TODO This can be removed once getMentions() returns sorted results (Nextcloud 21+)
89+
usort($mentions, static function ($m1, $m2) {
90+
return mb_strlen($m2['id']) <=> mb_strlen($m1['id']);
91+
});
92+
8893
foreach ($mentions as $mention) {
8994
if ($mention['type'] === 'user' && $mention['id'] === 'all') {
9095
$mention['type'] = 'call';
@@ -108,12 +113,10 @@ public function parseMessage(Message $chatMessage): void {
108113
// index of the mentions of that type.
109114
$mentionParameterId = 'mention-' . $mention['type'] . $mentionTypeCount[$mention['type']];
110115

111-
if (strpos($mention['id'], ' ') !== false || strpos($mention['id'], 'guest/') === 0) {
112-
$placeholder = '@"' . $mention['id'] . '"';
113-
} else {
114-
$placeholder = '@' . $mention['id'];
116+
$message = str_replace('@"' . $mention['id'] . '"', '{' . $mentionParameterId . '}', $message);
117+
if (strpos($mention['id'], ' ') === false && strpos($mention['id'], 'guest/') !== 0) {
118+
$message = str_replace('@' . $mention['id'], '{' . $mentionParameterId . '}', $message);
115119
}
116-
$message = str_replace($placeholder, '{' . $mentionParameterId . '}', $message);
117120

118121
if ($mention['type'] === 'call') {
119122
$userId = '';

‎tests/integration/features/chat/rich-messages.feature

+12
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ Feature: chat/public
33
Given user "participant1" exists
44
Given user "participant2" exists
55
Given user "participant3" exists
6+
Given user "participant3a" exists
67

78
Scenario: message without enrichable references has empty parameters
89
Given user "participant1" creates room "public room"
@@ -48,3 +49,14 @@ Feature: chat/public
4849
Then user "participant1" sees the following messages in room "public room" with 200
4950
| room | actorType | actorId | actorDisplayName | message | messageParameters |
5051
| 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"}} |
52+
53+
Scenario: message with mentions of subname users (uid1 is fully part of uid2)
54+
Given user "participant1" creates room "public room"
55+
| roomType | 3 |
56+
| roomName | room |
57+
When user "participant1" sends message "Mention to @participant3 and @participant3a" to room "public room" with 201
58+
When user "participant1" sends message "Mention to @participant3a and @participant3" to room "public room" with 201
59+
Then user "participant1" sees the following messages in room "public room" with 200
60+
| room | actorType | actorId | actorDisplayName | message | messageParameters |
61+
| 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"}} |
62+
| 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"}} |

‎tests/php/Chat/Parser/UserMentionTest.php

+79-8
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,80 @@ public function testGetRichMessageWithDuplicatedMention() {
174174
$this->assertEquals($expectedMessageParameters, $chatMessage->getMessageParameters());
175175
}
176176

177+
public function dataGetRichMessageWithMentionsFullyIncludedInOtherMentions() {
178+
// Based on valid characters from server/lib/private/User/Manager.php
179+
return [
180+
['testUser', 'testUser1', false],
181+
['testUser', 'testUser1', true],
182+
['testUser', 'testUser_1', false],
183+
['testUser', 'testUser_1', true],
184+
['testUser', 'testUser.1', false],
185+
['testUser', 'testUser.1', true],
186+
['testUser', 'testUser@1', false],
187+
['testUser', 'testUser@1', true],
188+
['testUser', 'testUser-1', false],
189+
['testUser', 'testUser-1', true],
190+
['testUser', 'testUser\'1', false],
191+
['testUser', 'testUser\'1', true],
192+
];
193+
}
194+
195+
/**
196+
* @dataProvider dataGetRichMessageWithMentionsFullyIncludedInOtherMentions
197+
*/
198+
public function testGetRichMessageWithMentionsFullyIncludedInOtherMentions(string $baseId, string $longerId, bool $quoted) {
199+
$mentions = [
200+
['type' => 'user', 'id' => $baseId],
201+
['type' => 'user', 'id' => $longerId],
202+
];
203+
$comment = $this->newComment($mentions);
204+
205+
$this->commentsManager->expects($this->exactly(2))
206+
->method('resolveDisplayName')
207+
->willReturnCallback(function ($type, $id) {
208+
return $id . ' display name';
209+
});
210+
211+
$this->userManager->expects($this->exactly(2))
212+
->method('get')
213+
->withConsecutive(
214+
[$longerId],
215+
[$baseId]
216+
)
217+
->willReturn($this->createMock(IUser::class));
218+
219+
/** @var Room|MockObject $room */
220+
$room = $this->createMock(Room::class);
221+
/** @var Participant|MockObject $participant */
222+
$participant = $this->createMock(Participant::class);
223+
/** @var IL10N|MockObject $l */
224+
$l = $this->createMock(IL10N::class);
225+
$chatMessage = new Message($room, $participant, $comment, $l);
226+
if ($quoted) {
227+
$chatMessage->setMessage('Mention to @"' . $baseId . '" and @"' . $longerId . '"', []);
228+
} else {
229+
$chatMessage->setMessage('Mention to @' . $baseId . ' and @' . $longerId, []);
230+
}
231+
232+
$this->parser->parseMessage($chatMessage);
233+
234+
$expectedMessageParameters = [
235+
'mention-user1' => [
236+
'type' => 'user',
237+
'id' => $longerId,
238+
'name' => $longerId . ' display name'
239+
],
240+
'mention-user2' => [
241+
'type' => 'user',
242+
'id' => $baseId,
243+
'name' => $baseId . ' display name'
244+
],
245+
];
246+
247+
$this->assertEquals('Mention to {mention-user2} and {mention-user1}', $chatMessage->getMessage());
248+
$this->assertEquals($expectedMessageParameters, $chatMessage->getMessageParameters());
249+
}
250+
177251
public function testGetRichMessageWithSeveralMentions() {
178252
$mentions = [
179253
['type' => 'user', 'id' => 'testUser1'],
@@ -249,15 +323,12 @@ public function testGetRichMessageWithNonExistingUserMention() {
249323
->with('user', 'testUser')
250324
->willReturn('testUser display name');
251325

252-
$this->userManager->expects($this->at(0))
253-
->method('get')
254-
->with('me')
255-
->willReturn(null);
256-
257-
$this->userManager->expects($this->at(1))
326+
$this->userManager->expects($this->exactly(2))
258327
->method('get')
259-
->with('testUser')
260-
->willReturn($this->createMock(IUser::class));
328+
->willReturnMap([
329+
['me', null],
330+
['testUser', $this->createMock(IUser::class)],
331+
]);
261332

262333
/** @var Room|MockObject $room */
263334
$room = $this->createMock(Room::class);

0 commit comments

Comments
 (0)