From 71080a8d20d5e7ee9e77768cdfa47ec07a200393 Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Fri, 3 Nov 2023 12:56:51 +0100 Subject: [PATCH] feat(contactsmenu): Sort by user status If user_status is not enabled, fall back to sorting by contact name. Signed-off-by: Christoph Wurst --- .../lib/ContactsMenu/StatusProvider.php | 2 + .../Contacts/ContactsMenu/ContactsStore.php | 85 ++++++++++-- lib/private/Contacts/ContactsMenu/Entry.php | 16 ++- lib/private/Contacts/ContactsMenu/Manager.php | 15 ++- lib/public/Contacts/ContactsMenu/IEntry.php | 1 + .../ContactsMenu/ContactsStoreTest.php | 124 +++++++++++++++++- tests/lib/Contacts/ContactsMenu/EntryTest.php | 1 + .../lib/Contacts/ContactsMenu/ManagerTest.php | 4 +- 8 files changed, 234 insertions(+), 14 deletions(-) diff --git a/apps/user_status/lib/ContactsMenu/StatusProvider.php b/apps/user_status/lib/ContactsMenu/StatusProvider.php index 3c37d6b080ff4..f1316f02df840 100644 --- a/apps/user_status/lib/ContactsMenu/StatusProvider.php +++ b/apps/user_status/lib/ContactsMenu/StatusProvider.php @@ -44,6 +44,7 @@ public function process(array $entries): void { ); $statuses = $this->statusService->findByUserIds($uids); + /** @var array $indexed */ $indexed = array_combine( array_map(fn(UserStatus $status) => $status->getUserId(), $statuses), $statuses @@ -56,6 +57,7 @@ public function process(array $entries): void { $entry->setStatus( $status->getStatus(), $status->getCustomMessage(), + $status->getStatusMessageTimestamp(), $status->getCustomIcon(), ); } diff --git a/lib/private/Contacts/ContactsMenu/ContactsStore.php b/lib/private/Contacts/ContactsMenu/ContactsStore.php index 189d7eede1cb5..eeb6ae56bc1e6 100644 --- a/lib/private/Contacts/ContactsMenu/ContactsStore.php +++ b/lib/private/Contacts/ContactsMenu/ContactsStore.php @@ -33,6 +33,8 @@ 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\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 @@ public function getContacts(IUser $user, ?string $filter, ?int $limit = null, ?i 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) { diff --git a/lib/private/Contacts/ContactsMenu/Entry.php b/lib/private/Contacts/ContactsMenu/Entry.php index ee55a74b051af..954f46e12967b 100644 --- a/lib/private/Contacts/ContactsMenu/Entry.php +++ b/lib/private/Contacts/ContactsMenu/Entry.php @@ -32,6 +32,8 @@ 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 @@ public function addAction(IAction $action): void { 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 @@ public function getProperty(string $key): 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, 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 @@ public function jsonSerialize(): array { '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; + } } diff --git a/lib/private/Contacts/ContactsMenu/Manager.php b/lib/private/Contacts/ContactsMenu/Manager.php index 22dbb52404626..5cf9a07c8e3b6 100644 --- a/lib/private/Contacts/ContactsMenu/Manager.php +++ b/lib/private/Contacts/ContactsMenu/Manager.php @@ -82,8 +82,19 @@ public function findOne(IUser $user, int $shareType, string $shareWith): ?IEntry * @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; } diff --git a/lib/public/Contacts/ContactsMenu/IEntry.php b/lib/public/Contacts/ContactsMenu/IEntry.php index cd68394998504..1307e2c74f7bf 100644 --- a/lib/public/Contacts/ContactsMenu/IEntry.php +++ b/lib/public/Contacts/ContactsMenu/IEntry.php @@ -64,6 +64,7 @@ public function addAction(IAction $action): void; */ public function setStatus(string $status, string $statusMessage = null, + int $statusMessageTimestamp = null, string $icon = null): void; /** diff --git a/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php b/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php index ccb888a6c3d6d..e3d6ea9922668 100644 --- a/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php +++ b/tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php @@ -1,4 +1,7 @@ * @copyright 2017 Lukas Reschke @@ -28,6 +31,8 @@ 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 @@ class ContactsStoreTest extends TestCase { private ContactsStore $contactsStore; + private StatusService|MockObject $statusService; /** @var IManager|MockObject */ private $contactsManager; /** @var ProfileManager */ @@ -61,6 +67,7 @@ protected function setUp(): void { 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 @@ protected function setUp(): void { $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 @@ public function testFindOneNoMatches() { $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); + } } diff --git a/tests/lib/Contacts/ContactsMenu/EntryTest.php b/tests/lib/Contacts/ContactsMenu/EntryTest.php index 7b1ca5d532097..253ec3213654e 100644 --- a/tests/lib/Contacts/ContactsMenu/EntryTest.php +++ b/tests/lib/Contacts/ContactsMenu/EntryTest.php @@ -105,6 +105,7 @@ public function testJsonSerialize() { 'profileUrl' => null, 'status' => null, 'statusMessage' => null, + 'statusMessageTimestamp' => null, 'statusIcon' => null, 'isUser' => false, 'uid' => null, diff --git a/tests/lib/Contacts/ContactsMenu/ManagerTest.php b/tests/lib/Contacts/ContactsMenu/ManagerTest.php index eb776a6e39d4f..2ea3966ad4f87 100644 --- a/tests/lib/Contacts/ContactsMenu/ManagerTest.php +++ b/tests/lib/Contacts/ContactsMenu/ManagerTest.php @@ -26,10 +26,10 @@ 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 @@ protected function setUp(): void { 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);