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.

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(reactions): wrong attribution for federated reactions #14161

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
16 changes: 12 additions & 4 deletions lib/Notification/Notifier.php
Original file line number Diff line number Diff line change
Expand Up @@ -537,18 +537,26 @@ protected function parseChatMessage(INotification $notification, Room $room, Par
}
} elseif ($subjectParameters['userType'] === Attendee::ACTOR_FEDERATED_USERS) {
try {
$cloudId = $this->cloudIdManager->resolveCloudId($message->getActorId());
$cloudId = $this->cloudIdManager->resolveCloudId($subjectParameters['userId']);
$cloudParticipant = $this->participantService->getParticipantByActor($room, Attendee::ACTOR_FEDERATED_USERS, $subjectParameters['userId']);
$richSubjectUser = [
'type' => 'user',
'id' => $cloudId->getUser(),
'name' => $message->getActorDisplayName(),
'name' => $cloudParticipant->getAttendee()->getDisplayName(),
'server' => $cloudId->getRemote(),
];
} catch (ParticipantNotFoundException) {
$richSubjectUser = [
'type' => 'user',
'id' => $cloudId->getUser(),
'name' => $cloudId->getDisplayId(),
'server' => $cloudId->getRemote(),
];
} catch (\InvalidArgumentException) {
$richSubjectUser = [
'type' => 'highlight',
'id' => $message->getActorId(),
'name' => $message->getActorId(),
'id' => $subjectParameters['userId'],
'name' => $subjectParameters['userId'],
];
}
} elseif ($subjectParameters['userType'] === Attendee::ACTOR_BOTS) {
Expand Down
8 changes: 4 additions & 4 deletions tests/integration/features/federation/chat.feature
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,8 @@ Feature: federation/chat
And user "participant1" adds federated_user "participant2@REMOTE" to room "room" with 200 (v4)
And using server "REMOTE"
And user "participant2" has the following invitations (v1)
| remoteServerUrl | remoteToken | state | inviterCloudId | inviterDisplayName |
| LOCAL | room | 0 | participant1@http://localhost:8080 | participant1-displayname |
| remoteServerUrl | remoteToken | state | inviterCloudId | inviterDisplayName |
| LOCAL | room | 0 | participant1@http://localhost:8080 | participant1@localhost:8080 |
And user "participant2" accepts invite to room "room" of server "LOCAL" with 200 (v1)
| id | name | type | remoteServer | remoteToken |
| LOCAL::room | room | 2 | LOCAL | room |
Expand Down Expand Up @@ -276,8 +276,8 @@ Feature: federation/chat
Then using server "REMOTE"
Then user "participant2" has the following notifications
| app | object_type | object_id | subject | message |
| spreed | chat | LOCAL::room/Hi @all bye | participant1-displayname mentioned everyone in conversation room | Hi room bye |
| spreed | chat | LOCAL::room/Hi @"federated_user/participant2@{$REMOTE_URL}" bye | participant1-displayname mentioned you in conversation room | Hi @participant2-displayname bye |
| spreed | chat | LOCAL::room/Hi @all bye | participant1@localhost:8080 mentioned everyone in conversation room | Hi room bye |
| spreed | chat | LOCAL::room/Hi @"federated_user/participant2@{$REMOTE_URL}" bye | participant1@localhost:8080 mentioned you in conversation room | Hi @participant2-displayname bye |
| spreed | chat | LOCAL::room/Message 1-1 | participant1-displayname replied to your message in conversation room | Message 1-1 |
When next message request has the following parameters set
| timeout | 0 |
Expand Down
7 changes: 5 additions & 2 deletions tests/integration/features/federation/reminder.feature
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,11 @@ Feature: federation/reminder
| app | object_type | object_id | subject |
And using server "REMOTE"
And user "participant2" has the following notifications
| app | object_type | object_id | subject |
| spreed | reminder | LOCAL::room/Message 1 | Reminder: participant1-displayname in conversation room |
| app | object_type | object_id | subject |
| spreed | reminder | LOCAL::room/Message 1 | Reminder: participant1@localhost:8080 in conversation room |
# This is the previous version of the line. localhost:8080 is not the expected result but the username
# is resolved to it because of missing display name resolution for federation. Revert this when it's been fixed
# | spreed | reminder | LOCAL::room/Message 1 | Reminder: participant1-displayname in conversation room |
# Participant1 sets timestamp to past so it should trigger now
When using server "LOCAL"
And user "participant1" sets reminder for message "Message 2" in room "room" for time 1234567 with 201 (v1)
Expand Down
Loading