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 <opensource@fthiessen.de>
nextcloud · Aug 31, 2024 · 2295c86 · 2295c86
1 parent d952847
commit 2295c86
Show file tree

Hide file tree

Showing 3 changed files with 99 additions and 48 deletions.
diff --git 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
@@ -39,6 +39,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 +58,25 @@
  * @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;
+	/**
+	 * @todo With PHP 8.2 we can put this directly on the declaration instead of a comment but 8.1 does not allow the parenthesis
+	 * @var (OCA\Circles\CirclesManager&MockObject)|null
+	 */
+	protected $teamManager;
 
 	protected function setUp(): void {
 		parent::setUp();
@@ -103,6 +96,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 +107,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 +130,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 +152,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
@@ -1,8 +1,8 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<files psalm-version="5.19.0@06b71be009a6bd6d81b9811855d6629b9fe90e1b">
+<files psalm-version="5.25.0@01a8eb06b9e9cc6cfb6a320bf9fb14331919d505">
   <file src="lib/BackgroundJob/RemoteActivity.php">
     <UndefinedClass>
-      <code>ClientException</code>
+      <code><![CDATA[ClientException]]></code>
     </UndefinedClass>
   </file>
   <file src="lib/CurrentUser.php">
@@ -21,27 +21,30 @@
     </InvalidArrayOffset>
   </file>
   <file src="lib/FilesHooks.php">
-    <InvalidArgument>
-      <code><![CDATA[$share->getId()]]></code>
-    </InvalidArgument>
+    <UndefinedClass>
+      <code><![CDATA[$this->teamManager]]></code>
+      <code><![CDATA[$this->teamManager]]></code>
+      <code><![CDATA[Member]]></code>
+      <code><![CDATA[protected]]></code>
+    </UndefinedClass>
     <UndefinedDocblockClass>
-      <code>$shareOwner</code>
-      <code>$storage</code>
+      <code><![CDATA[$shareOwner]]></code>
+      <code><![CDATA[$storage]]></code>
     </UndefinedDocblockClass>
   </file>
   <file src="lib/Migration/Version2008Date20181011095117.php">
     <UndefinedClass>
-      <code>SchemaException</code>
+      <code><![CDATA[SchemaException]]></code>
     </UndefinedClass>
   </file>
   <file src="lib/Migration/Version2011Date20201006132544.php">
     <UndefinedClass>
-      <code>Type</code>
+      <code><![CDATA[Type]]></code>
     </UndefinedClass>
   </file>
   <file src="lib/Migration/Version2011Date20201006132545.php">
     <UndefinedClass>
-      <code>Type</code>
+      <code><![CDATA[Type]]></code>
     </UndefinedClass>
   </file>
 </files>