From da0fabcc10b8013d04c9dc3eba4fb2197916481f Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Sat, 24 Aug 2024 19:09:36 +0200 Subject: [PATCH] feat: Add sharing activity for teams This adds support for Teams (previously "Circles"), the user notification already work, but the notification for the owner and sharer need support in the `files_sharing` app. Signed-off-by: Ferdinand Thiessen --- lib/FilesHooks.php | 66 +++++++++++++++++++++++++++++++++++----- tests/FilesHooksTest.php | 55 +++++++++++++++------------------ tests/psalm-baseline.xml | 23 ++++++++------ 3 files changed, 96 insertions(+), 48 deletions(-) diff --git a/lib/FilesHooks.php b/lib/FilesHooks.php index 5d58deb24..758840a8d 100755 --- a/lib/FilesHooks.php +++ b/lib/FilesHooks.php @@ -11,6 +11,8 @@ use OCA\Activity\BackgroundJob\RemoteActivity; use OCA\Activity\Extension\Files; use OCA\Activity\Extension\Files_Sharing; +use OCA\Circles\CirclesManager; +use OCA\Circles\Model\Member; use OCP\Activity\IManager; use OCP\Constants; use OCP\Files\Config\IUserMountCache; @@ -60,7 +62,8 @@ public function __construct( protected IUserMountCache $userMountCache, protected IConfig $config, protected NotificationGenerator $notificationGenerator, - protected ITagManager $tagManager + protected ITagManager $tagManager, + protected ?CirclesManager $teamManager, ) { } @@ -664,6 +667,16 @@ public function share($share) { (int)$share->getId() ); break; + case IShare::TYPE_CIRCLE: + $this->shareWithTeam( + $share->getSharedWith(), + $share->getNodeId(), + $share->getNodeType(), + $share->getTarget(), + (int)$share->getId(), + $share->getSharedBy(), + ); + break; case IShare::TYPE_LINK: $this->shareByLink( $share->getNodeId(), @@ -726,7 +739,8 @@ protected function shareWithGroup($shareWith, $fileSource, $itemType, $fileTarge $offset = 0; $users = $group->searchUsers('', self::USER_BATCH_SIZE, $offset); while (!empty($users)) { - $this->addNotificationsForGroupUsers($users, 'shared_with_by', $fileSource, $itemType, $fileTarget, $shareId); + $users = array_map(fn (IUser $user) => $user->getUID(), $users); + $this->addNotificationsForUsers($users, 'shared_with_by', $fileSource, $itemType, $fileTarget, $shareId); $offset += self::USER_BATCH_SIZE; $users = $group->searchUsers('', self::USER_BATCH_SIZE, $offset); } @@ -758,6 +772,42 @@ protected function shareByLink($fileSource, $itemType, $linkOwner) { ); } + /** + * Sharing a file or folder with a team + * + * @param string $shareWith + * @param int $fileSource File ID that is being shared + * @param string $itemType File type that is being shared (file or folder) + * @param string $fileTarget File path + * @param int $shareId The Share ID of this share + */ + protected function shareWithTeam(string $shareWith, int $fileSource, string $itemType, string $fileTarget, int $shareId, string $sharer): void { + if ($this->teamManager === null) { + return; + } + + try { + $this->teamManager->startSuperSession(); + $team = $this->teamManager->getCircle($shareWith); + $members = $team->getInheritedMembers(); + $members = array_filter($members, fn ($member) => $member->getUserType() === Member::TYPE_USER); + $users = array_map(fn ($member) => $member->getUserId(), $members); + } catch (\Throwable $e) { + $this->logger->debug('Fetching team members for share activity failed', ['exception' => $e]); + // error in teams app - setting users list to empty + $users = []; + } + + // Activity for user performing the share + $this->shareNotificationForSharer('shared_team_self', $shareWith, $fileSource, $itemType); + // Activity for original owner of the file (re-sharing) + if ($this->currentUser->getUID() !== null) { + $this->shareNotificationForOriginalOwners($this->currentUser->getUID(), 're-shared_team_by', $shareWith, $fileSource, $itemType); + } + // Activity for all affected users + $this->addNotificationsForUsers($users, 'shared_with_by', $fileSource, $itemType, $fileTarget, $shareId); + } + /** * Manage unsharing events * @@ -878,9 +928,11 @@ protected function unshareFromGroup(IShare $share) { $offset = 0; $users = $group->searchUsers('', self::USER_BATCH_SIZE, $offset); + $users = array_map(fn (IUser $user) => $user->getUID(), $users); $shouldFlush = $this->startActivityTransaction(); while (!empty($users)) { - $this->addNotificationsForGroupUsers($users, $actionUser, $share->getNodeId(), $share->getNodeType(), $share->getTarget(), (int) $share->getId()); + $userIds = array_map(fn (IUser $user) => $user->getUID(), $users); + $this->addNotificationsForUsers($userIds, $actionUser, $share->getNodeId(), $share->getNodeType(), $share->getTarget(), (int)$share->getId()); $offset += self::USER_BATCH_SIZE; $users = $group->searchUsers('', self::USER_BATCH_SIZE, $offset); } @@ -940,18 +992,18 @@ protected function unshareLink(IShare $share) { } /** - * @param IUser[] $usersInGroup + * @param string[] $usersIds * @param string $actionUser * @param int $fileSource File ID that is being shared * @param string $itemType File type that is being shared (file or folder) * @param string $fileTarget File path * @param int $shareId The Share ID of this share */ - protected function addNotificationsForGroupUsers(array $usersInGroup, $actionUser, $fileSource, $itemType, $fileTarget, $shareId) { + protected function addNotificationsForUsers(array $usersIds, $actionUser, $fileSource, $itemType, $fileTarget, $shareId) { $affectedUsers = []; - foreach ($usersInGroup as $user) { - $affectedUsers[$user->getUID()] = $fileTarget; + foreach ($usersIds as $user) { + $affectedUsers[$user] = $fileTarget; } // Remove the triggering user, we already managed his notifications diff --git a/tests/FilesHooksTest.php b/tests/FilesHooksTest.php index b307921ae..d83f40f2b 100755 --- a/tests/FilesHooksTest.php +++ b/tests/FilesHooksTest.php @@ -31,6 +31,7 @@ use OCA\Activity\Extension\Files; use OCA\Activity\Extension\Files_Sharing; use OCA\Activity\Tests\TestCase; +use OCA\Circles\CirclesManager; use OCA\Files_Sharing\SharedStorage; use OCP\Activity\IEvent; use OCP\Activity\IManager; @@ -39,6 +40,7 @@ use OCP\Files\Mount\IMountPoint; use OCP\Files\NotFoundException; use OCP\IConfig; +use OCP\IDBConnection; use OCP\IGroup; use OCP\IGroupManager; use OCP\IURLGenerator; @@ -57,33 +59,21 @@ * @package OCA\Activity */ class FilesHooksTest extends TestCase { - /** @var FilesHooks */ - protected $filesHooks; - /** @var IManager|MockObject */ - protected $activityManager; - /** @var Data|MockObject */ - protected $data; - /** @var UserSettings|MockObject */ - protected $settings; - /** @var IGroupManager|MockObject */ - protected $groupManager; - /** @var View|MockObject */ - protected $view; - /** @var IRootFolder|MockObject */ - protected $rootFolder; - /** @var IShareHelper|MockObject */ - protected $shareHelper; - /** @var IURLGenerator|MockObject */ - protected $urlGenerator; - /** @var IUserMountCache|MockObject */ - protected $userMountCache; - /** @var IConfig|MockObject */ - protected $config; - /** @var NotificationGenerator|MockObject */ - protected $notificationGenerator; - /** @var TagManager|MockObject */ - protected $tagManager; + protected FilesHooks $filesHooks; + protected IManager&MockObject $activityManager; + protected Data&MockObject $data; + protected UserSettings&MockObject $settings; + protected IGroupManager&MockObject $groupManager; + protected View&MockObject $view; + protected IRootFolder&MockObject $rootFolder; + protected IShareHelper&MockObject $shareHelper; + protected IURLGenerator&MockObject $urlGenerator; + protected IUserMountCache&MockObject $userMountCache; + protected IConfig&MockObject $config; + protected NotificationGenerator&MockObject $notificationGenerator; + protected TagManager&MockObject $tagManager; protected $tags; + protected (CirclesManager&MockObject)|null $teamManager; protected function setUp(): void { parent::setUp(); @@ -103,6 +93,7 @@ protected function setUp(): void { $this->tags = $this->createMock(Tags::class); $this->tagManager->method('getUsersFavoritingObject') ->willReturn([]); + $this->teamManager = null; $this->tagManager->method('load') ->willReturn($this->tags); @@ -113,7 +104,7 @@ protected function setUp(): void { /** * @param array $mockedMethods * @param string $user - * @return FilesHooks|MockObject + * @return FilesHooks&MockObject */ protected function getFilesHooks(array $mockedMethods = [], string $user = 'user'): FilesHooks { $currentUser = $this->createMock(CurrentUser::class); @@ -136,14 +127,15 @@ protected function getFilesHooks(array $mockedMethods = [], string $user = 'user $this->view, $this->rootFolder, $this->shareHelper, - \OC::$server->getDatabaseConnection(), + \OCP\Server::get(IDBConnection::class), $this->urlGenerator, $logger, $currentUser, $this->userMountCache, $this->config, $this->notificationGenerator, - $this->tagManager + $this->tagManager, + $this->teamManager, ]) ->onlyMethods($mockedMethods) ->getMock(); @@ -157,14 +149,15 @@ protected function getFilesHooks(array $mockedMethods = [], string $user = 'user $this->view, $this->rootFolder, $this->shareHelper, - \OC::$server->getDatabaseConnection(), + \OCP\Server::get(IDBConnection::class), $this->urlGenerator, $logger, $currentUser, $this->userMountCache, $this->config, $this->notificationGenerator, - $this->tagManager + $this->tagManager, + $this->teamManager, ); } diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index cb384dac0..adc7e0b33 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -1,8 +1,8 @@ - + - ClientException + @@ -21,27 +21,30 @@ - - getId()]]> - + + teamManager]]> + teamManager]]> + + + - $shareOwner - $storage + + - SchemaException + - Type + - Type +