Merge pull request #12130 from nextcloud/backport/12121/stable29

[stable29] fix(conversations): Include rooms in "modified since" updates when at…

lib/Controller/RoomController.php

@@ -214,9 +214,24 @@ public function getRooms(int $noStatusUpdate = 0, bool $includeStatus = false, i
 		$rooms = $this->manager->getRoomsForUser($this->userId, $sessionIds, true);
 
 		if ($modifiedSince !== 0) {
-			$rooms = array_filter($rooms, static function (Room $room) use ($includeStatus, $modifiedSince): bool {
-				return ($includeStatus && $room->getType() === Room::TYPE_ONE_TO_ONE)
-					|| ($room->getLastActivity() && $room->getLastActivity()->getTimestamp() >= $modifiedSince);
+			$rooms = array_filter($rooms, function (Room $room) use ($includeStatus, $modifiedSince): bool {
+				if ($includeStatus && $room->getType() === Room::TYPE_ONE_TO_ONE) {
+					// Always include 1-1s to update the user status
+					return true;
+				}
+				if ($room->getCallFlag() !== Participant::FLAG_DISCONNECTED) {
+					// Always include active calls
+					return true;
+				}
+				if ($room->getLastActivity() && $room->getLastActivity()->getTimestamp() >= $modifiedSince) {
+					// Include rooms which had activity
+					return true;
+				}
+
+				// Include rooms where only attendee level things changed,
+				// e.g. favorite, read-marker update, notification setting
+				$attendee = $room->getParticipant($this->userId)->getAttendee();
+				return $attendee->getLastAttendeeActivity() >= $modifiedSince;
 			});
 		}
 

lib/Migration/Version19000Date20240416104656.php

@@ -0,0 +1,58 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2024 Joas Schilling <coding@schilljs.com>
+ *
+ * @author Joas Schilling <coding@schilljs.com>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OCA\Talk\Migration;
+
+use Closure;
+use OCP\DB\ISchemaWrapper;
+use OCP\DB\Types;
+use OCP\Migration\IOutput;
+use OCP\Migration\SimpleMigrationStep;
+
+/**
+ * Add a column to track the last read marker update so conversations marked
+ * as (un)read on other devices are properly updated with the "lazy" update.
+ */
+class Version19000Date20240416104656 extends SimpleMigrationStep {
+	/**
+	 * @param IOutput $output
+	 * @param Closure(): ISchemaWrapper $schemaClosure
+	 * @param array $options
+	 * @return null|ISchemaWrapper
+	 */
+	public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
+		/** @var ISchemaWrapper $schema */
+		$schema = $schemaClosure();
+
+		$table = $schema->getTable('talk_attendees');
+		$table->addColumn('last_attendee_activity', Types::BIGINT, [
+			'default' => 0,
+			'unsigned' => true,
+		]);
+
+		return $schema;
+	}
+}

lib/Model/Attendee.php

@@ -70,6 +70,8 @@
  * @method int getState()
  * @method void setUnreadMessages(int $unreadMessages)
  * @method int getUnreadMessages()
+ * @method void setLastAttendeeActivity(int $lastAttendeeActivity)
+ * @method int getLastAttendeeActivity()
  */
 class Attendee extends Entity {
 	public const ACTOR_USERS = 'users';
@@ -178,6 +180,9 @@ class Attendee extends Entity {
 	/** @var int */
 	protected $unreadMessages;
 
+	/** @var int */
+	protected $lastAttendeeActivity;
+
 	public function __construct() {
 		$this->addType('roomId', 'int');
 		$this->addType('actorType', 'string');
@@ -201,6 +206,7 @@ public function __construct() {
 		$this->addType('callId', 'string');
 		$this->addType('state', 'int');
 		$this->addType('unreadMessages', 'int');
+		$this->addType('lastAttendeeActivity', 'int');
 	}
 
 	public function getDisplayName(): string {
@@ -232,6 +238,7 @@ public function asArray(): array {
 			'invited_cloud_id' => $this->getInvitedCloudId(),
 			'phone_number' => $this->getPhoneNumber(),
 			'call_id' => $this->getCallId(),
+			'last_attendee_activity' => $this->getLastAttendeeActivity(),
 		];
 	}
 }

lib/Model/AttendeeMapper.php

@@ -301,6 +301,7 @@ public function createAttendeeFromRow(array $row): Attendee {
 			'call_id' => $row['call_id'],
 			'state' => (int) $row['state'],
 			'unread_messages' => (int) $row['unread_messages'],
+			'last_attendee_activity' => (int) $row['last_attendee_activity'],
 		]);
 	}
 }

lib/Model/SelectHelper.php

@@ -90,6 +90,7 @@ public function selectAttendeesTable(IQueryBuilder $query, string $alias = 'a'):
 			->addSelect($alias . 'call_id')
 			->addSelect($alias . 'state')
 			->addSelect($alias . 'unread_messages')
+			->addSelect($alias . 'last_attendee_activity')
 			->selectAlias($alias . 'id', 'a_id');
 	}
 

lib/Service/ParticipantService.php

@@ -145,6 +145,7 @@ public function updateParticipantType(Room $room, Participant $participant, int
 			$attendee->setPermissions(Attendee::PERMISSIONS_DEFAULT);
 		}
 
+		$attendee->setLastAttendeeActivity($this->timeFactory->getTime());
 		$this->attendeeMapper->update($attendee);
 
 		// XOR so we don't move the participant in and out when they are changed from moderator to owner or vice-versa
@@ -232,6 +233,7 @@ public function updatePermissions(Room $room, Participant $participant, string $
 		if ($attendee->getParticipantType() === Participant::USER_SELF_JOINED) {
 			$attendee->setParticipantType(Participant::USER);
 		}
+		$attendee->setLastAttendeeActivity($this->timeFactory->getTime());
 		$this->attendeeMapper->update($attendee);
 
 		$event = new ParticipantModifiedEvent($room, $participant, AParticipantModifiedEvent::PROPERTY_PERMISSIONS, $newPermissions, $oldPermissions);
@@ -247,6 +249,7 @@ public function updateAllPermissions(Room $room, string $method, int $newState):
 	public function updateLastReadMessage(Participant $participant, int $lastReadMessage): void {
 		$attendee = $participant->getAttendee();
 		$attendee->setLastReadMessage($lastReadMessage);
+		$attendee->setLastAttendeeActivity($this->timeFactory->getTime());
 		$this->attendeeMapper->update($attendee);
 	}
 
@@ -256,12 +259,14 @@ public function updateUnreadInfoForProxyParticipant(Participant $participant, in
 		$attendee->setLastReadMessage($lastReadMessageId);
 		$attendee->setLastMentionMessage($hasMention ? 1 : 0);
 		$attendee->setLastMentionDirect($hadDirectMention ? 1 : 0);
+		$attendee->setLastAttendeeActivity($this->timeFactory->getTime());
 		$this->attendeeMapper->update($attendee);
 	}
 
 	public function updateFavoriteStatus(Participant $participant, bool $isFavorite): void {
 		$attendee = $participant->getAttendee();
 		$attendee->setFavorite($isFavorite);
+		$attendee->setLastAttendeeActivity($this->timeFactory->getTime());
 		$this->attendeeMapper->update($attendee);
 	}
 
@@ -281,6 +286,7 @@ public function updateNotificationLevel(Participant $participant, int $level): v
 
 		$attendee = $participant->getAttendee();
 		$attendee->setNotificationLevel($level);
+		$attendee->setLastAttendeeActivity($this->timeFactory->getTime());
 		$this->attendeeMapper->update($attendee);
 	}
 
@@ -298,6 +304,7 @@ public function updateNotificationCalls(Participant $participant, int $level): v
 
 		$attendee = $participant->getAttendee();
 		$attendee->setNotificationCalls($level);
+		$attendee->setLastAttendeeActivity($this->timeFactory->getTime());
 		$this->attendeeMapper->update($attendee);
 	}
 
@@ -794,6 +801,7 @@ public function generatePinForParticipant(Room $room, Participant $participant):
 			&& ($attendee->getActorType() === Attendee::ACTOR_USERS || $attendee->getActorType() === Attendee::ACTOR_EMAILS)
 			&& !$attendee->getPin()) {
 			$attendee->setPin($this->generatePin());
+			$attendee->setLastAttendeeActivity($this->timeFactory->getTime());
 			$this->attendeeMapper->update($attendee);
 		}
 	}
@@ -1330,6 +1338,7 @@ public function resetChatDetails(Room $room): void {
 			->set('last_read_message', $update->createNamedParameter(0, IQueryBuilder::PARAM_INT))
 			->set('last_mention_message', $update->createNamedParameter(0, IQueryBuilder::PARAM_INT))
 			->set('last_mention_direct', $update->createNamedParameter(0, IQueryBuilder::PARAM_INT))
+			->set('last_attendee_activity', $update->createNamedParameter($this->timeFactory->getTime(), IQueryBuilder::PARAM_INT))
 			->where($update->expr()->eq('room_id', $update->createNamedParameter($room->getId(), IQueryBuilder::PARAM_INT)));
 		$update->executeStatement();
 	}

tests/integration/features/bootstrap/FeatureContext.php

@@ -73,6 +73,8 @@ class FeatureContext implements Context, SnippetAcceptingContext {
 	protected static array $phoneNumberToActorId;
 	/** @var array<string, mixed>|null */
 	protected static ?array $nextChatRequestParameters = null;
+	/** @var array<string, int> */
+	protected static array $modifiedSince;
 
 
 	protected static array $permissionsMap = [
@@ -185,6 +187,7 @@ public function setUp() {
 		self::$questionToPollId = [];
 		self::$lastNotifications = [];
 		self::$phoneNumberToActorId = [];
+		self::$modifiedSince = [];
 
 		$this->createdUsers = [];
 		$this->createdGroups = [];
@@ -288,17 +291,26 @@ public function userCanFindListedRoomsWithTerm(string $user, string $term, strin
 	}
 
 	/**
-	 * @Then /^user "([^"]*)" is participant of the following (unordered )?(note-to-self )?rooms \((v4)\)$/
+	 * @Then /^user "([^"]*)" is participant of the following (unordered )?(note-to-self )?(modified-since )?rooms \((v4)\)$/
 	 *
 	 * @param string $user
 	 * @param string $shouldOrder
 	 * @param string $apiVersion
 	 * @param TableNode|null $formData
 	 */
-	public function userIsParticipantOfRooms(string $user, string $shouldOrder, string $shouldFilter, string $apiVersion, TableNode $formData = null): void {
+	public function userIsParticipantOfRooms(string $user, string $shouldOrder, string $shouldFilter, string $modifiedSince, string $apiVersion, TableNode $formData = null): void {
+		$parameters = '';
+		if ($modifiedSince !== '') {
+			if (!isset(self::$modifiedSince[$user])) {
+				throw new \RuntimeException('Must run once without "modified-since" before');
+			}
+			$parameters .= '?modifiedSince=' . self::$modifiedSince[$user];
+		}
+
 		$this->setCurrentUser($user);
-		$this->sendRequest('GET', '/apps/spreed/api/' . $apiVersion . '/room');
+		$this->sendRequest('GET', '/apps/spreed/api/' . $apiVersion . '/room' . $parameters);
 		$this->assertStatusCode($this->response, 200);
+		self::$modifiedSince[$user] = time();
 
 		$rooms = $this->getDataFromResponse($this->response);
 

tests/integration/features/chat-2/unread-messages.feature

@@ -148,9 +148,24 @@ Feature: chat-2/unread-messages
     And user "participant1" sends message "Message 1" to room "group room" with 201
     And user "participant1" sends message "Message 2" to room "group room" with 201
     And user "participant1" sends message "Message 3" to room "group room" with 201
+    Then user "participant2" is participant of the following rooms (v4)
+      | id         | unreadMessages |
+      | group room | 3              |
+    And wait for 2 seconds
+    Then user "participant2" is participant of the following modified-since rooms (v4)
     And user "participant2" reads message "Message 3" in room "group room" with 200
+    And wait for 2 seconds
+    Then user "participant2" is participant of the following modified-since rooms (v4)
+      | id         | unreadMessages |
+      | group room | 0              |
+    Then user "participant2" is participant of the following modified-since rooms (v4)
     When user "participant1" marks room "group room" as unread with 200
     And user "participant2" marks room "group room" as unread with 200
+    And wait for 2 seconds
+    Then user "participant2" is participant of the following modified-since rooms (v4)
+      | id         | unreadMessages |
+      | group room | 1              |
+    Then user "participant2" is participant of the following modified-since rooms (v4)
     Then user "participant1" is participant of room "group room" (v4)
       | unreadMessages |
       | 1              |

tests/php/Chat/ChatManagerTest.php

@@ -449,6 +449,7 @@ public function testDeleteMessage(): void {
 			'invited_cloud_id' => '',
 			'state' => Invitation::STATE_ACCEPTED,
 			'unread_messages' => 0,
+			'last_attendee_activity' => 0,
 		]);
 		$chat = $this->createMock(Room::class);
 		$chat->expects($this->any())
@@ -510,6 +511,7 @@ public function testDeleteMessageFileShare(): void {
 			'invited_cloud_id' => '',
 			'state' => Invitation::STATE_ACCEPTED,
 			'unread_messages' => 0,
+			'last_attendee_activity' => 0,
 		]);
 		$chat = $this->createMock(Room::class);
 		$chat->expects($this->any())
@@ -593,6 +595,7 @@ public function testDeleteMessageFileShareNotFound(): void {
 			'invited_cloud_id' => '',
 			'state' => Invitation::STATE_ACCEPTED,
 			'unread_messages' => 0,
+			'last_attendee_activity' => 0,
 		]);
 		$chat = $this->createMock(Room::class);
 		$chat->expects($this->any())