Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: Add sharing activity for teams #1754

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat: Add sharing activity for teams #1754

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 59 additions & 7 deletions lib/FilesHooks.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
) {
}

Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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
*
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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
Expand Down
58 changes: 27 additions & 31 deletions tests/FilesHooksTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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();
Expand All @@ -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,
);
}

Expand Down
23 changes: 13 additions & 10 deletions tests/psalm-baseline.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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">
Expand All @@ -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>
Loading