Merge pull request #12371 from nextcloud/backport/12319/stable29

[stable29] fix(federation): Don't send notifications for most system messages in…
nextcloud · May 16, 2024 · fb5e6df · fb5e6df
2 parents 3f35d79 + 463325f
commit fb5e6df
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 4 deletions.
diff --git a/lib/Federation/CloudFederationProviderTalk.php b/lib/Federation/CloudFederationProviderTalk.php
@@ -53,6 +53,7 @@
 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 @@ public function __construct(
 		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 @@ private function messagePosted(int $remoteAttendeeId, array $notification): arra
 				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
@@ -80,9 +80,11 @@ public function handleChatMessage(Room $room, Participant $participant, ProxyCac
 			}
 		} 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 @@ protected function isMentionedAll(Room $room, ProxyCacheMessage $message): bool
 		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
@@ -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
@@ -169,6 +169,7 @@ public function setUp(): void {
 			$this->proxyCacheMessageService,
 			$this->federationChatNotifier,
 			$this->userConverter,
+			$this->timeFactory,
 			$this->cacheFactory,
 		);
 	}