Merge pull request #12319 from nextcloud/bugfix/12307/no-notification…

…-for-useless-system

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

lib/Federation/CloudFederationProviderTalk.php

@@ -36,6 +36,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;
@@ -79,6 +80,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;
@@ -385,7 +387,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']]));

lib/Notification/FederationChatNotifier.php

@@ -63,9 +63,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);
+			}
 		}
 	}
 
@@ -111,6 +113,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
 	 */

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 |

tests/php/Federation/FederationTest.php

@@ -135,6 +135,7 @@ public function setUp(): void {
 			$this->proxyCacheMessageService,
 			$this->federationChatNotifier,
 			$this->userConverter,
+			$this->timeFactory,
 			$this->cacheFactory,
 		);
 	}