fix(federation): Don't send notifications for most system messages in federation

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

4 files changed, 44 insertions, 4 deletions

diff --git a/lib/Federation/CloudFederationProviderTalk.php b/lib/Federation/CloudFederationProviderTalk.php
index 20bb51f38..6cdf26530 100644
--- a/lib/Federation/CloudFederationProviderTalk.php
+++ b/lib/Federation/CloudFederationProviderTalk.php
@@ -53,6 +53,7 @@ use OCA\Talk\Service\RoomService;
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Services\IAppConfig;
+use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\DB\Exception as DBException;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Federation\Exceptions\ActionNotSupportedException;
@@ -96,6 +97,7 @@ class CloudFederationProviderTalk implements ICloudFederationProvider {
 		private ProxyCacheMessageService $pcmService,
 		private FederationChatNotifier $federationChatNotifier,
 		private UserConverter $userConverter,
+		private ITimeFactory $timeFactory,
 		ICacheFactory $cacheFactory,
 	) {
 		$this->proxyCacheMessages = $cacheFactory->isAvailable() ? $cacheFactory->createDistributed(CachePrefix::FEDERATED_PCM) : null;
@@ -402,7 +404,11 @@ class CloudFederationProviderTalk implements ICloudFederationProvider {
 				if ($notification['messageData']['remoteMessageId'] > $lastMessageId) {
 					$lastMessageId = (int) $notification['messageData']['remoteMessageId'];
 				}
-				$this->roomService->setLastMessageInfo($room, $lastMessageId, new \DateTime());
+
+				if ($notification['messageData']['systemMessage'] !== 'message_edited'
+					&& $notification['messageData']['systemMessage'] !== 'message_deleted') {
+					$this->roomService->setLastMessageInfo($room, $lastMessageId, $this->timeFactory->getDateTime());
+				}
 
 				if ($this->proxyCacheMessages instanceof ICache) {
 					$cacheKey = sha1(json_encode([$notification['remoteServerUrl'], $notification['remoteToken']]));
diff --git a/lib/Notification/FederationChatNotifier.php b/lib/Notification/FederationChatNotifier.php
index 90f466b5c..e4e9966b2 100644
--- a/lib/Notification/FederationChatNotifier.php
+++ b/lib/Notification/FederationChatNotifier.php
@@ -80,9 +80,11 @@ class FederationChatNotifier {
 			}
 		} elseif ($participant->getAttendee()->getNotificationLevel() === Participant::NOTIFY_ALWAYS
 			|| ($defaultLevel === Participant::NOTIFY_ALWAYS && $participant->getAttendee()->getNotificationLevel() === Participant::NOTIFY_DEFAULT)) {
-			$notification = $this->createNotification($room, $message, 'chat');
-			$notification->setUser($participant->getAttendee()->getActorId());
-			$this->notificationManager->notify($notification);
+			if ($this->isUserMessageOrRelevantSystemMessage($message->getSystemMessage())) {
+				$notification = $this->createNotification($room, $message, 'chat');
+				$notification->setUser($participant->getAttendee()->getActorId());
+				$this->notificationManager->notify($notification);
+			}
 		}
 	}
 
@@ -128,6 +130,14 @@ class FederationChatNotifier {
 		return false;
 	}
 
+	protected function isUserMessageOrRelevantSystemMessage(?string $systemMessage): bool {
+		return $systemMessage === null
+			|| $systemMessage === ''
+			|| $systemMessage === 'object_shared'
+			|| $systemMessage === 'poll_closed'
+			|| $systemMessage === 'file_shared';
+	}
+
 	/**
 	 * Creates a notification for the given proxy message and mentioned users
 	 */
diff --git a/tests/integration/features/federation/chat.feature b/tests/integration/features/federation/chat.feature
index de9aee824..616e91ec7 100644
--- a/tests/integration/features/federation/chat.feature
+++ b/tests/integration/features/federation/chat.feature
@@ -371,6 +371,29 @@ Feature: federation/chat
     Then user "participant2" has the following notifications
       | app    | object_type | object_id                | subject                                                                | message     |
 
+  Scenario: System messages don't trigger notifications
+    Given the following "spreed" app config is set
+      | federation_enabled | yes |
+    And 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        |
+    And user "participant2" is participant of the following rooms (v4)
+      | id   | type |
+      | room | 3    |
+    And user "participant1" sends message "Message 1" to room "room" with 201
+    When user "participant2" sets notifications to all for room "LOCAL::room" (v4)
+    And user "participant1" sets description for room "room" to "the description" with 200 (v4)
+    And user "participant1" react with "🚀" on message "Message 1" to room "room" with 201
+    Then user "participant2" has the following notifications
+      | app    | object_type | object_id                | subject                                                                | message     |
+
   Scenario: Reaction on federated chat messages
     Given the following "spreed" app config is set
       | federation_enabled | yes |
diff --git a/tests/php/Federation/FederationTest.php b/tests/php/Federation/FederationTest.php
index e0d83b681..ac9c0a860 100644
--- a/tests/php/Federation/FederationTest.php
+++ b/tests/php/Federation/FederationTest.php
@@ -169,6 +169,7 @@ class FederationTest extends TestCase {
 			$this->proxyCacheMessageService,
 			$this->federationChatNotifier,
 			$this->userConverter,
+			$this->timeFactory,
 			$this->cacheFactory,
 		);
 	}