fix(notifications): Fix Undefined variable $comment at Notifier.php#693

Signed-off-by: Joas Schilling <coding@schilljs.com>

3 files changed, 41 insertions, 13 deletions

diff --git a/lib/Notification/Notifier.php b/lib/Notification/Notifier.php
index 939c7b8da..c3b404521 100644
--- a/lib/Notification/Notifier.php
+++ b/lib/Notification/Notifier.php
@@ -603,7 +603,7 @@ class Notifier implements INotifier {
 			}
 			$richSubjectParameters['message'] = [
 				'type' => 'highlight',
-				'id' => $message->getComment()->getId(),
+				'id' => $message->getMessageId(),
 				'name' => $shortenMessage,
 			];
 			if ($notification->getSubject() === 'reminder') {
@@ -621,7 +621,7 @@ class Notifier implements INotifier {
 					$subject = $l->t('Reminder: Deleted user in {call}') . "\n{message}";
 				} else {
 					try {
-						$richSubjectParameters['guest'] = $this->getGuestParameter($room, $comment->getActorId());
+						$richSubjectParameters['guest'] = $this->getGuestParameter($room, $message->getActorId());
 						// TRANSLATORS Reminder for a message from a guest in conversation {call}
 						$subject = $l->t('Reminder: {guest} (guest) in {call}') . "\n{message}";
 					} catch (ParticipantNotFoundException $e) {
@@ -644,7 +644,7 @@ class Notifier implements INotifier {
 					$subject = $l->t('Deleted user reacted with {reaction} in {call}') . "\n{message}";
 				} else {
 					try {
-						$richSubjectParameters['guest'] = $this->getGuestParameter($room, $comment->getActorId());
+						$richSubjectParameters['guest'] = $this->getGuestParameter($room, $message->getActorId());
 						$subject = $l->t('{guest} (guest) reacted with {reaction} in {call}') . "\n{message}";
 					} catch (ParticipantNotFoundException $e) {
 						$subject = $l->t('Guest reacted with {reaction} in {call}') . "\n{message}";
@@ -659,7 +659,7 @@ class Notifier implements INotifier {
 					$subject = $l->t('Deleted user in {call}') . "\n{message}";
 				} else {
 					try {
-						$richSubjectParameters['guest'] = $this->getGuestParameter($room, $comment->getActorId());
+						$richSubjectParameters['guest'] = $this->getGuestParameter($room, $message->getActorId());
 						$subject = $l->t('{guest} (guest) in {call}') . "\n{message}";
 					} catch (ParticipantNotFoundException $e) {
 						$subject = $l->t('Guest in {call}') . "\n{message}";
@@ -675,7 +675,7 @@ class Notifier implements INotifier {
 				$subject = $l->t('A deleted user sent a message in conversation {call}');
 			} else {
 				try {
-					$richSubjectParameters['guest'] = $this->getGuestParameter($room, $comment->getActorId());
+					$richSubjectParameters['guest'] = $this->getGuestParameter($room, $message->getActorId());
 					$subject = $l->t('{guest} (guest) sent a message in conversation {call}');
 				} catch (ParticipantNotFoundException $e) {
 					$subject = $l->t('A guest sent a message in conversation {call}');
@@ -690,7 +690,7 @@ class Notifier implements INotifier {
 				$subject = $l->t('A deleted user replied to your message in conversation {call}');
 			} else {
 				try {
-					$richSubjectParameters['guest'] = $this->getGuestParameter($room, $comment->getActorId());
+					$richSubjectParameters['guest'] = $this->getGuestParameter($room, $message->getActorId());
 					$subject = $l->t('{guest} (guest) replied to your message in conversation {call}');
 				} catch (ParticipantNotFoundException $e) {
 					$subject = $l->t('A guest replied to your message in conversation {call}');
@@ -715,7 +715,7 @@ class Notifier implements INotifier {
 				$subject = $l->t('Reminder: A deleted user in conversation {call}');
 			} else {
 				try {
-					$richSubjectParameters['guest'] = $this->getGuestParameter($room, $comment->getActorId());
+					$richSubjectParameters['guest'] = $this->getGuestParameter($room, $message->getActorId());
 					$subject = $l->t('Reminder: {guest} (guest) in conversation {call}');
 				} catch (ParticipantNotFoundException) {
 					$subject = $l->t('Reminder: A guest in conversation {call}');
@@ -736,7 +736,7 @@ class Notifier implements INotifier {
 				$subject = $l->t('A deleted user reacted with {reaction} to your message in conversation {call}');
 			} else {
 				try {
-					$richSubjectParameters['guest'] = $this->getGuestParameter($room, $comment->getActorId());
+					$richSubjectParameters['guest'] = $this->getGuestParameter($room, $message->getActorId());
 					$subject = $l->t('{guest} (guest) reacted with {reaction} to your message in conversation {call}');
 				} catch (ParticipantNotFoundException $e) {
 					$subject = $l->t('A guest reacted with {reaction} to your message in conversation {call}');
@@ -776,7 +776,7 @@ class Notifier implements INotifier {
 			}
 		} else {
 			try {
-				$richSubjectParameters['guest'] = $this->getGuestParameter($room, $comment->getActorId());
+				$richSubjectParameters['guest'] = $this->getGuestParameter($room, $message->getActorId());
 				if ($notification->getSubject() === 'mention_group') {
 					$groupName = $this->groupManager->getDisplayName($subjectParameters['sourceId']) ?? $subjectParameters['sourceId'];
 					$richSubjectParameters['group'] = [
@@ -821,7 +821,7 @@ class Notifier implements INotifier {
 						[
 							'apiVersion' => 'v1',
 							'token' => $room->getToken(),
-							'messageId' => $comment->getId(),
+							'messageId' => $message->getMessageId(),
 						]
 					),
 					IAction::TYPE_DELETE
diff --git a/tests/integration/features/federation/chat.feature b/tests/integration/features/federation/chat.feature
index 0c12fb3f0..9405dbe26 100644
--- a/tests/integration/features/federation/chat.feature
+++ b/tests/integration/features/federation/chat.feature
@@ -222,6 +222,31 @@ Feature: federation/chat
       | spreed | chat        | room/Hi @"federated_user/participant2@{$REMOTE_URL}" bye | participant1-displayname mentioned you in conversation room | Hi @participant2-displayname bye |
       | spreed | chat        | room/Message 1-1         | participant1-displayname replied to your message in conversation room  | Message 1-1 |
 
+  Scenario: Mentioning a federated user as a guest also triggers a notification for them
+    Given the following "spreed" app config is set
+      | federation_enabled | yes |
+    Given user "participant1" creates room "room" (v4)
+      | roomType | 3 |
+      | roomName | room |
+    And user "participant1" adds federated_user "participant2" to room "room" with 200 (v4)
+    And user "participant2" has the following invitations (v1)
+      | remoteServerUrl | remoteToken | state | inviterCloudId                     | inviterDisplayName       |
+      | LOCAL           | room        | 0     | participant1@http://localhost:8080 | participant1-displayname |
+    And user "participant2" accepts invite to room "room" of server "LOCAL" with 200 (v1)
+      | id   | name | type | remoteServer | remoteToken |
+      | room | room | 3    | LOCAL        | room        |
+    Then user "participant2" is participant of the following rooms (v4)
+      | id   | type |
+      | room | 3    |
+    # Join and leave to clear the invite notification
+    Given user "participant2" joins room "LOCAL::room" with 200 (v4)
+    Given user "participant2" leaves room "LOCAL::room" with 200 (v4)
+    And user "guest" joins room "room" with 200 (v4)
+    When user "guest" sends message 'Hi @"federated_user/participant2@{$REMOTE_URL}" bye' to room "room" with 201
+    Then user "participant2" has the following notifications
+      | app    | object_type | object_id                | subject                                                                | message     |
+      | spreed | chat        | room/Hi @"federated_user/participant2@{$REMOTE_URL}" bye | A guest mentioned you in conversation room | Hi @participant2-displayname bye |
+
   Scenario: Reaction on federated chat messages
     Given the following "spreed" app config is set
       | federation_enabled | yes |
diff --git a/tests/php/Notification/NotifierTest.php b/tests/php/Notification/NotifierTest.php
index db22eff66..e574a8aee 100644
--- a/tests/php/Notification/NotifierTest.php
+++ b/tests/php/Notification/NotifierTest.php
@@ -966,9 +966,6 @@ class NotifierTest extends TestCase {
 		$comment->expects($this->any())
 			->method('getActorId')
 			->willReturn('random-hash');
-		$comment->expects($this->any())
-			->method('getId')
-			->willReturn('123456789');
 		$this->commentsManager->expects($this->once())
 			->method('get')
 			->with('23')
@@ -1010,6 +1007,12 @@ class NotifierTest extends TestCase {
 			->willReturn(true);
 		$chatMessage->method('getComment')
 			->willReturn($comment);
+		$chatMessage->expects($this->any())
+			->method('getMessageId')
+			->willReturn(123456789);
+		$chatMessage->expects($this->any())
+			->method('getActorId')
+			->willReturn('random-hash');
 
 		$this->messageParser->expects($this->once())
 			->method('createMessage')