Merge pull request #41266 from nextcloud/feat/contactsmenu/user-status-sorting

feat(contactsmenu): Sort by user status

apps/user_status/lib/ContactsMenu/StatusProvider.php

@@ -44,6 +44,7 @@ class StatusProvider implements IBulkProvider {
 		);
 
 		$statuses = $this->statusService->findByUserIds($uids);
+		/** @var array<string, UserStatus> $indexed */
 		$indexed = array_combine(
 			array_map(fn(UserStatus $status) => $status->getUserId(), $statuses),
 			$statuses
@@ -56,6 +57,7 @@ class StatusProvider implements IBulkProvider {
 				$entry->setStatus(
 					$status->getStatus(),
 					$status->getCustomMessage(),
+					$status->getStatusMessageTimestamp(),
 					$status->getCustomIcon(),
 				);
 			}

lib/private/Contacts/ContactsMenu/ContactsStore.php

@@ -33,6 +33,8 @@ namespace OC\Contacts\ContactsMenu;
 
 use OC\KnownUser\KnownUserService;
 use OC\Profile\ProfileManager;
+use OCA\UserStatus\Db\UserStatus;
+use OCA\UserStatus\Service\StatusService;
 use OCP\Contacts\ContactsMenu\IContactsStore;
 use OCP\Contacts\ContactsMenu\IEntry;
 use OCP\Contacts\IManager;
@@ -42,10 +44,17 @@ use OCP\IURLGenerator;
 use OCP\IUser;
 use OCP\IUserManager;
 use OCP\L10N\IFactory as IL10NFactory;
+use function array_column;
+use function array_fill_keys;
+use function array_filter;
+use function array_key_exists;
+use function array_merge;
+use function count;
 
 class ContactsStore implements IContactsStore {
 	public function __construct(
 		private IManager $contactsManager,
+		private ?StatusService $userStatusService,
 		private IConfig $config,
 		private ProfileManager $profileManager,
 		private IUserManager $userManager,
@@ -70,15 +79,75 @@ class ContactsStore implements IContactsStore {
 		if ($offset !== null) {
 			$options['offset'] = $offset;
 		}
+		// Status integration only works without pagination and filters
+		if ($offset === null && ($filter === null || $filter === '')) {
+			$recentStatuses = $this->userStatusService?->findAllRecentStatusChanges($limit, $offset) ?? [];
+		} else {
+			$recentStatuses = [];
+		}
 
-		$allContacts = $this->contactsManager->search(
-			$filter ?? '',
-			[
-				'FN',
-				'EMAIL'
-			],
-			$options
-		);
+		// Search by status if there is no filter and statuses are available
+		if (!empty($recentStatuses)) {
+			$allContacts = array_filter(array_map(function (UserStatus $userStatus) use ($options) {
+				// UID is ambiguous with federation. We have to use the federated cloud ID to an exact match of
+				// A local user
+				$user = $this->userManager->get($userStatus->getUserId());
+				if ($user === null) {
+					return null;
+				}
+
+				$contact = $this->contactsManager->search(
+					$user->getCloudId(),
+					[
+						'CLOUD',
+					],
+					array_merge(
+						$options,
+						[
+							'limit' => 1,
+							'offset' => 0,
+						],
+					),
+				)[0] ?? null;
+				if ($contact !== null) {
+					$contact[Entry::PROPERTY_STATUS_MESSAGE_TIMESTAMP] = $userStatus->getStatusMessageTimestamp();
+				}
+				return $contact;
+			}, $recentStatuses));
+			if ($limit !== null && count($allContacts) < $limit) {
+				// More contacts were requested
+				$fromContacts = $this->contactsManager->search(
+					$filter ?? '',
+					[
+						'FN',
+						'EMAIL'
+					],
+					array_merge(
+						$options,
+						[
+							'limit' => $limit - count($allContacts),
+						],
+					),
+				);
+
+				// Create hash map of all status contacts
+				$existing = array_fill_keys(array_column($allContacts, 'URI'), null);
+				// Append the ones that are new
+				$allContacts = array_merge(
+					$allContacts,
+					array_filter($fromContacts, fn (array $contact): bool => !array_key_exists($contact['URI'], $existing))
+				);
+			}
+		} else {
+			$allContacts = $this->contactsManager->search(
+				$filter ?? '',
+				[
+					'FN',
+					'EMAIL'
+				],
+				$options
+			);
+		}
 
 		$userId = $user->getUID();
 		$contacts = array_filter($allContacts, function ($contact) use ($userId) {

lib/private/Contacts/ContactsMenu/Entry.php

@@ -32,6 +32,8 @@ use OCP\Contacts\ContactsMenu\IEntry;
 use function array_merge;
 
 class Entry implements IEntry {
+	public const PROPERTY_STATUS_MESSAGE_TIMESTAMP = 'statusMessageTimestamp';
+
 	/** @var string|int|null */
 	private $id = null;
 
@@ -53,6 +55,7 @@ class Entry implements IEntry {
 
 	private ?string $status = null;
 	private ?string $statusMessage = null;
+	private ?int $statusMessageTimestamp = null;
 	private ?string $statusIcon = null;
 
 	public function setId(string $id): void {
@@ -109,9 +112,11 @@ class Entry implements IEntry {
 
 	public function setStatus(string $status,
 		string $statusMessage = null,
+		int $statusMessageTimestamp = null,
 		string $icon = null): void {
 		$this->status = $status;
 		$this->statusMessage = $statusMessage;
+		$this->statusMessageTimestamp = $statusMessageTimestamp;
 		$this->statusIcon = $icon;
 	}
 
@@ -159,7 +164,7 @@ class Entry implements IEntry {
 	}
 
 	/**
-	 * @return array{id: int|string|null, fullName: string, avatar: string|null, topAction: mixed, actions: array, lastMessage: '', emailAddresses: string[], profileTitle: string|null, profileUrl: string|null, status: string|null, statusMessage: null|string, statusIcon: null|string, isUser: bool, uid: mixed}
+	 * @return array{id: int|string|null, fullName: string, avatar: string|null, topAction: mixed, actions: array, lastMessage: '', emailAddresses: string[], profileTitle: string|null, profileUrl: string|null, status: string|null, statusMessage: null|string, statusMessageTimestamp: null|int, statusIcon: null|string, isUser: bool, uid: mixed}
 	 */
 	public function jsonSerialize(): array {
 		$topAction = !empty($this->actions) ? $this->actions[0]->jsonSerialize() : null;
@@ -179,9 +184,18 @@ class Entry implements IEntry {
 			'profileUrl' => $this->profileUrl,
 			'status' => $this->status,
 			'statusMessage' => $this->statusMessage,
+			'statusMessageTimestamp' => $this->statusMessageTimestamp,
 			'statusIcon' => $this->statusIcon,
 			'isUser' => $this->getProperty('isUser') === true,
 			'uid' => $this->getProperty('UID'),
 		];
 	}
+
+	public function getStatusMessage(): ?string {
+		return $this->statusMessage;
+	}
+
+	public function getStatusMessageTimestamp(): ?int {
+		return $this->statusMessageTimestamp;
+	}
 }

lib/private/Contacts/ContactsMenu/Manager.php

@@ -82,8 +82,19 @@ class Manager {
 	 * @return IEntry[]
 	 */
 	private function sortEntries(array $entries): array {
-		usort($entries, function (IEntry $entryA, IEntry $entryB) {
-			return strcasecmp($entryA->getFullName(), $entryB->getFullName());
+		usort($entries, function (Entry $entryA, Entry $entryB) {
+			$aStatusTimestamp = $entryA->getProperty(Entry::PROPERTY_STATUS_MESSAGE_TIMESTAMP);
+			$bStatusTimestamp = $entryB->getProperty(Entry::PROPERTY_STATUS_MESSAGE_TIMESTAMP);
+			if (!$aStatusTimestamp && !$bStatusTimestamp) {
+				return strcasecmp($entryA->getFullName(), $entryB->getFullName());
+			}
+			if ($aStatusTimestamp === null) {
+				return 1;
+			}
+			if ($bStatusTimestamp === null) {
+				return -1;
+			}
+			return $bStatusTimestamp - $aStatusTimestamp;
 		});
 		return $entries;
 	}

lib/public/Contacts/ContactsMenu/IEntry.php

@@ -64,6 +64,7 @@ interface IEntry extends JsonSerializable {
 	 */
 	public function setStatus(string $status,
 		string $statusMessage = null,
+		int $statusMessageTimestamp = null,
 		string $icon = null): void;
 
 	/**

tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php

@@ -1,4 +1,7 @@
 <?php
+
+declare(strict_types=1);
+
 /**
  * @copyright 2017 Christoph Wurst <christoph@winzerhof-wurst.at>
  * @copyright 2017 Lukas Reschke <lukas@statuscode.ch>
@@ -28,6 +31,8 @@ namespace Tests\Contacts\ContactsMenu;
 use OC\Contacts\ContactsMenu\ContactsStore;
 use OC\KnownUser\KnownUserService;
 use OC\Profile\ProfileManager;
+use OCA\UserStatus\Db\UserStatus;
+use OCA\UserStatus\Service\StatusService;
 use OCP\Contacts\IManager;
 use OCP\IConfig;
 use OCP\IGroupManager;
@@ -40,6 +45,7 @@ use Test\TestCase;
 
 class ContactsStoreTest extends TestCase {
 	private ContactsStore $contactsStore;
+	private StatusService|MockObject $statusService;
 	/** @var IManager|MockObject */
 	private $contactsManager;
 	/** @var ProfileManager */
@@ -61,6 +67,7 @@ class ContactsStoreTest extends TestCase {
 		parent::setUp();
 
 		$this->contactsManager = $this->createMock(IManager::class);
+		$this->statusService = $this->createMock(StatusService::class);
 		$this->userManager = $this->createMock(IUserManager::class);
 		$this->profileManager = $this->createMock(ProfileManager::class);
 		$this->urlGenerator = $this->createMock(IURLGenerator::class);
@@ -70,13 +77,14 @@ class ContactsStoreTest extends TestCase {
 		$this->l10nFactory = $this->createMock(IL10NFactory::class);
 		$this->contactsStore = new ContactsStore(
 			$this->contactsManager,
+			$this->statusService,
 			$this->config,
 			$this->profileManager,
 			$this->userManager,
 			$this->urlGenerator,
 			$this->groupManager,
 			$this->knownUserService,
-			$this->l10nFactory
+			$this->l10nFactory,
 		);
 	}
 
@@ -964,4 +972,118 @@ class ContactsStoreTest extends TestCase {
 
 		$this->assertEquals(null, $entry);
 	}
+
+	public function testGetRecentStatusFirst(): void {
+		$user = $this->createMock(IUser::class);
+		$status1 = new UserStatus();
+		$status1->setUserId('user1');
+		$status2 = new UserStatus();
+		$status2->setUserId('user2');
+		$this->statusService->expects(self::once())
+			->method('findAllRecentStatusChanges')
+			->willReturn([
+				$status1,
+				$status2,
+			]);
+		$user1 = $this->createMock(IUser::class);
+		$user1->method('getCloudId')->willReturn('user1@localcloud');
+		$user2 = $this->createMock(IUser::class);
+		$user2->method('getCloudId')->willReturn('user2@localcloud');
+		$this->userManager->expects(self::exactly(2))
+			->method('get')
+			->willReturnCallback(function ($uid) use ($user1, $user2) {
+				return match ($uid) {
+					'user1' => $user1,
+					'user2' => $user2,
+				};
+			});
+		$this->contactsManager
+			->expects(self::exactly(3))
+			->method('search')
+			->willReturnCallback(function ($uid, $searchProps, $options) {
+				return match ([$uid, $options['limit'] ?? null]) {
+					['user1@localcloud', 1] => [
+						[
+							'UID' => 'user1',
+							'URI' => 'user1.vcf',
+						],
+					],
+					['user2@localcloud' => [], 1], // Simulate not found
+					['', 4] => [
+						[
+							'UID' => 'contact1',
+							'URI' => 'contact1.vcf',
+						],
+						[
+							'UID' => 'contact2',
+							'URI' => 'contact2.vcf',
+						],
+					],
+					default => [],
+				};
+			});
+
+		$contacts = $this->contactsStore->getContacts(
+			$user,
+			null,
+			5,
+		);
+
+		self::assertCount(3, $contacts);
+		self::assertEquals('user1', $contacts[0]->getProperty('UID'));
+		self::assertEquals('contact1', $contacts[1]->getProperty('UID'));
+		self::assertEquals('contact2', $contacts[2]->getProperty('UID'));
+	}
+
+	public function testPaginateRecentStatus(): void {
+		$user = $this->createMock(IUser::class);
+		$status1 = new UserStatus();
+		$status1->setUserId('user1');
+		$status2 = new UserStatus();
+		$status2->setUserId('user2');
+		$status3 = new UserStatus();
+		$status3->setUserId('user3');
+		$this->statusService->expects(self::never())
+			->method('findAllRecentStatusChanges');
+		$this->contactsManager
+			->expects(self::exactly(2))
+			->method('search')
+			->willReturnCallback(function ($uid, $searchProps, $options) {
+				return match ([$uid, $options['limit'] ?? null, $options['offset'] ?? null]) {
+					['', 2, 0] => [
+						[
+							'UID' => 'contact1',
+							'URI' => 'contact1.vcf',
+						],
+						[
+							'UID' => 'contact2',
+							'URI' => 'contact2.vcf',
+						],
+					],
+					['', 2, 3] => [
+						[
+							'UID' => 'contact3',
+							'URI' => 'contact3.vcf',
+						],
+					],
+					default => [],
+				};
+			});
+
+		$page1 = $this->contactsStore->getContacts(
+			$user,
+			null,
+			2,
+			0,
+		);
+		$page2 = $this->contactsStore->getContacts(
+			$user,
+			null,
+			2,
+			3,
+		);
+
+		self::assertCount(2, $page1);
+		self::assertCount(1, $page2);
+	}
 }

tests/lib/Contacts/ContactsMenu/EntryTest.php

@@ -105,6 +105,7 @@ class EntryTest extends TestCase {
 			'profileUrl' => null,
 			'status' => null,
 			'statusMessage' => null,
+			'statusMessageTimestamp' => null,
 			'statusIcon' => null,
 			'isUser' => false,
 			'uid' => null,

tests/lib/Contacts/ContactsMenu/ManagerTest.php

@@ -26,10 +26,10 @@ namespace Tests\Contacts\ContactsMenu;
 
 use OC\Contacts\ContactsMenu\ActionProviderStore;
 use OC\Contacts\ContactsMenu\ContactsStore;
+use OC\Contacts\ContactsMenu\Entry;
 use OC\Contacts\ContactsMenu\Manager;
 use OCP\App\IAppManager;
 use OCP\Constants;
-use OCP\Contacts\ContactsMenu\IEntry;
 use OCP\Contacts\ContactsMenu\IProvider;
 use OCP\IConfig;
 use OCP\IUser;
@@ -65,7 +65,7 @@ class ManagerTest extends TestCase {
 	private function generateTestEntries(): array {
 		$entries = [];
 		foreach (range('Z', 'A') as $char) {
-			$entry = $this->createMock(IEntry::class);
+			$entry = $this->createMock(Entry::class);
 			$entry->expects($this->any())
 				->method('getFullName')
 				->willReturn('Contact ' . $char);