diff --git a/.gitignore b/.gitignore index e8034c743f..087b37fd2b 100644 --- a/.gitignore +++ b/.gitignore @@ -9,5 +9,6 @@ tests/integration/composer.lock tests/.phpunit.result.cache vendor/ .php_cs.cache +.php-cs-fixer.cache \.idea/ settings.json diff --git a/CHANGELOG.md b/CHANGELOG.md index 4be848709e..5e3c0e912a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,13 @@ All notable changes to this project will be documented in this file. ## 1.16.0-beta.1 ### Added +- feat: add owner_type to boards for circle ownership support @jospoortvliet +- feat: resolve circle board owners in BoardMapper and include circle-owned boards in user board queries @jospoortvliet +- feat: grant full owner permissions to circle members on circle-owned boards @jospoortvliet +- feat: support transferring board ownership to a circle in BoardService @jospoortvliet +- feat: accept newOwnerType in the transfer-ownership REST endpoint @jospoortvliet +- feat: add --to-circle flag to deck:transfer-ownership OCC command @jospoortvliet +- feat: show team icon for circle-owned boards and add Transfer ownership button in sharing sidebar @jospoortvliet - feat: update default content @luka-nextcloud [#6740](https://github.com/nextcloud/deck/pull/6740) - feat: add board import and export @luka-nextcloud [#6872](https://github.com/nextcloud/deck/pull/6872) - feat: use outline icons @luka-nextcloud [#7114](https://github.com/nextcloud/deck/pull/7114) diff --git a/lib/Command/TransferOwnership.php b/lib/Command/TransferOwnership.php index dfa9a01c4b..a7da2c09d6 100644 --- a/lib/Command/TransferOwnership.php +++ b/lib/Command/TransferOwnership.php @@ -6,9 +6,12 @@ */ namespace OCA\Deck\Command; +use OCA\Deck\Db\Acl; use OCA\Deck\Db\BoardMapper; use OCA\Deck\Service\BoardService; +use OCA\Deck\Service\CirclesService; use OCA\Deck\Service\PermissionService; +use OCP\IUserManager; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Helper\QuestionHelper; use Symfony\Component\Console\Input\InputArgument; @@ -22,14 +25,18 @@ final class TransferOwnership extends Command { protected $boardMapper; protected $permissionService; protected $questionHelper; + protected $userManager; + protected $circlesService; - public function __construct(BoardService $boardService, BoardMapper $boardMapper, PermissionService $permissionService, QuestionHelper $questionHelper) { + public function __construct(BoardService $boardService, BoardMapper $boardMapper, PermissionService $permissionService, QuestionHelper $questionHelper, IUserManager $userManager, CirclesService $circlesService) { parent::__construct(); $this->boardService = $boardService; $this->boardMapper = $boardMapper; $this->permissionService = $permissionService; $this->questionHelper = $questionHelper; + $this->userManager = $userManager; + $this->circlesService = $circlesService; } protected function configure() { @@ -57,6 +64,12 @@ protected function configure() { InputOption::VALUE_NONE, 'Reassign card details of the old owner to the new one' ) + ->addOption( + 'to-team', + null, + InputOption::VALUE_NONE, + 'Treat as a team ID (internally stored as a circle ID) instead of a user UID' + ) ; } @@ -64,8 +77,49 @@ protected function execute(InputInterface $input, OutputInterface $output): int $owner = $input->getArgument('owner'); $newOwner = $input->getArgument('newOwner'); $boardId = $input->getArgument('boardId'); - $remapAssignment = $input->getOption('remap'); + $toTeam = $input->getOption('to-team'); + $newOwnerType = Acl::PERMISSION_TYPE_USER; + $teamDisplayName = null; + if ($toTeam) { + $newOwnerType = Acl::PERMISSION_TYPE_CIRCLE; + if ($this->circlesService->isCirclesEnabled()) { + try { + $circle = $this->circlesService->getCircle($newOwner); + if ($circle !== null) { + $teamDisplayName = $circle->getDisplayName(); + } + } catch (\Throwable $e) { + $teamDisplayName = null; + } + } + } else { + $userExists = $this->userManager->userExists($newOwner); + $circleExists = false; + $circle = null; + if ($this->circlesService->isCirclesEnabled()) { + try { + $circle = $this->circlesService->getCircle($newOwner); + $circleExists = $circle !== null; + if ($circle !== null) { + $teamDisplayName = $circle->getDisplayName(); + } + } catch (\Throwable $e) { + $circleExists = false; + } + } + + if ($userExists && $circleExists) { + $output->writeln('Ambiguous target: ' . $newOwner . ' matches both a user and a team (circle ID). Use --to-team to transfer to the team.'); + return 1; + } + + if ($circleExists && !$userExists) { + $newOwnerType = Acl::PERMISSION_TYPE_CIRCLE; + $output->writeln('Detected team target: treating ' . $newOwner . ' as team ' . ($teamDisplayName ?: $newOwner) . '.'); + } + } + $newOwnerLabel = $newOwnerType === Acl::PERMISSION_TYPE_CIRCLE ? 'team ' . ($teamDisplayName ?: $newOwner) : $newOwner; $this->boardService->setUserId($owner); $this->permissionService->setUserId($owner); @@ -83,9 +137,9 @@ protected function execute(InputInterface $input, OutputInterface $output): int } if ($boardId) { - $output->writeln('Transfer board ' . $board->getTitle() . ' from ' . $board->getOwner() . " to $newOwner"); + $output->writeln('Transfer board ' . $board->getTitle() . ' from ' . $board->getOwner() . " to $newOwnerLabel"); } else { - $output->writeln("Transfer all boards from $owner to $newOwner"); + $output->writeln("Transfer all boards from $owner to $newOwnerLabel"); } $question = new ConfirmationQuestion('Do you really want to continue? (y/n) ', false); @@ -93,16 +147,21 @@ protected function execute(InputInterface $input, OutputInterface $output): int return 1; } - if ($boardId) { - $this->boardService->transferBoardOwnership($boardId, $newOwner, $remapAssignment); - $output->writeln('Board ' . $board->getTitle() . ' from ' . $board->getOwner() . " transferred to $newOwner completed"); - return 0; - } - - foreach ($this->boardService->transferOwnership($owner, $newOwner, $remapAssignment) as $board) { - $output->writeln(' - ' . $board->getTitle() . ' transferred'); + try { + if ($boardId) { + $this->boardService->transferBoardOwnership($boardId, $newOwner, $remapAssignment, $newOwnerType); + $output->writeln('Board ' . $board->getTitle() . " transferred to $newOwnerLabel"); + return 0; + } + + foreach ($this->boardService->transferOwnership($owner, $newOwner, $remapAssignment, $newOwnerType) as $board) { + $output->writeln(' - ' . $board->getTitle() . ' transferred'); + } + $output->writeln("All boards from $owner transferred to $newOwnerLabel"); + } catch (\Exception $e) { + $output->writeln('' . $e->getMessage() . ''); + return 1; } - $output->writeln("All boards from $owner to $newOwner transferred"); return 0; } diff --git a/lib/Controller/BoardController.php b/lib/Controller/BoardController.php index 5c43ab7b12..3a3ae7e7c1 100644 --- a/lib/Controller/BoardController.php +++ b/lib/Controller/BoardController.php @@ -122,11 +122,13 @@ public function clone(int $boardId, bool $withCards = false, bool $withAssignmen /** * @NoAdminRequired */ - public function transferOwner(int $boardId, string $newOwner): DataResponse { + public function transferOwner(int $boardId, string $newOwner, int $newOwnerType = Acl::PERMISSION_TYPE_USER): DataResponse { + if ($newOwnerType !== Acl::PERMISSION_TYPE_USER && $newOwnerType !== Acl::PERMISSION_TYPE_CIRCLE) { + return new DataResponse(['message' => 'Invalid owner type'], HTTP::STATUS_BAD_REQUEST); + } if ($this->permissionService->userIsBoardOwner($boardId, $this->userId)) { - return new DataResponse($this->boardService->transferBoardOwnership($boardId, $newOwner), HTTP::STATUS_OK); + return new DataResponse($this->boardService->transferBoardOwnership($boardId, $newOwner, false, $newOwnerType), HTTP::STATUS_OK); } - return new DataResponse([], HTTP::STATUS_UNAUTHORIZED); } diff --git a/lib/Db/Board.php b/lib/Db/Board.php index a0663dbd3d..80865dcb67 100644 --- a/lib/Db/Board.php +++ b/lib/Db/Board.php @@ -22,6 +22,8 @@ * @method void setLastModified(int $lastModified) * @method string getOwner() * @method void setOwner(string $owner) + * @method int getOwnerType() + * @method void setOwnerType(int $ownerType) * @method string getColor() * @method void setColor(string $color) * @method void setShareToken(string $shareToken) @@ -32,6 +34,7 @@ class Board extends RelationalEntity { protected $title; protected $owner; + protected $ownerType = 0; protected $color; protected $archived = false; /** @var Label[]|null */ @@ -53,6 +56,7 @@ class Board extends RelationalEntity { public function __construct() { $this->addType('id', 'integer'); $this->addType('shared', 'integer'); + $this->addType('ownerType', 'integer'); $this->addType('archived', 'boolean'); $this->addType('deletedAt', 'integer'); $this->addType('lastModified', 'integer'); diff --git a/lib/Db/BoardMapper.php b/lib/Db/BoardMapper.php index 2583a117cd..41ade67f2a 100644 --- a/lib/Db/BoardMapper.php +++ b/lib/Db/BoardMapper.php @@ -86,6 +86,7 @@ public function findBoardIds(string $userId): array { ->from($this->getTableName(), 'b') ->where($qb->expr()->andX( $qb->expr()->eq('owner', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR)), + $qb->expr()->eq('b.owner_type', $qb->createNamedParameter(Acl::PERMISSION_TYPE_USER, IQueryBuilder::PARAM_INT)), )); $result = $qb->executeQuery(); $ownerBoards = array_map(function (string $id) { @@ -131,7 +132,23 @@ public function findBoardIds(string $userId): array { return (int)$id; }, $result->fetchAll(\PDO::FETCH_COLUMN)); $result->closeCursor(); - return array_unique(array_merge($ownerBoards, $sharedBoards)); + + // Owned by circles the user is in (reuse $circles already fetched above) + $circleOwnedBoards = []; + if (count($circles) !== 0) { + $qb = $this->db->getQueryBuilder(); + $qb->selectDistinct('b.id') + ->from($this->getTableName(), 'b') + ->where($qb->expr()->eq('owner_type', $qb->createNamedParameter(Acl::PERMISSION_TYPE_CIRCLE, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->in('owner', $qb->createNamedParameter($circles, IQueryBuilder::PARAM_STR_ARRAY))); + $result = $qb->executeQuery(); + $circleOwnedBoards = array_map(function (string $id) { + return (int)$id; + }, $result->fetchAll(\PDO::FETCH_COLUMN)); + $result->closeCursor(); + } + + return array_unique(array_merge($ownerBoards, $sharedBoards, $circleOwnedBoards)); } /** * @param int $externalId @@ -156,7 +173,8 @@ public function findAllForUser(string $userId, ?int $since = null, bool $include $userBoards = $this->findAllByUser($userId, null, null, $since, $includeArchived, $before, $term); $groupBoards = $this->findAllByGroups($userId, $groups, null, null, $since, $includeArchived, $before, $term); $circleBoards = $this->findAllByCircles($userId, null, null, $since, $includeArchived, $before, $term); - $allBoards = array_values(array_unique(array_merge($userBoards, $groupBoards, $circleBoards))); + $circleOwnedBoards = $this->findAllByCircleOwner($userId, null, null, $since, $includeArchived, $before, $term); + $allBoards = array_values(array_unique(array_merge($userBoards, $groupBoards, $circleBoards, $circleOwnedBoards))); // Could be moved outside $acls = $this->aclMapper->findIn(array_map(function ($board) { @@ -192,11 +210,12 @@ public function findAllByUser(string $userId, ?int $limit = null, ?int $offset = // FIXME this used to be a UNION to get boards owned by $userId and the user shares in one single query // Is it possible with the query builder? $qb = $this->db->getQueryBuilder(); - $qb->select('id', 'title', 'owner', 'color', 'archived', 'deleted_at', 'last_modified', 'external_id', 'share_token') + $qb->select('id', 'title', 'owner', 'owner_type', 'color', 'archived', 'deleted_at', 'last_modified', 'external_id', 'share_token') // this does not work in MySQL/PostgreSQL //->selectAlias('0', 'shared') ->from('deck_boards', 'b') - ->where($qb->expr()->eq('owner', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR))); + ->where($qb->expr()->eq('owner', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR))) + ->andWhere($qb->expr()->eq('b.owner_type', $qb->createNamedParameter(Acl::PERMISSION_TYPE_USER, IQueryBuilder::PARAM_INT))); if (!$includeArchived) { $qb->andWhere($qb->expr()->eq('archived', $qb->createNamedParameter(false, IQueryBuilder::PARAM_BOOL))) ->andWhere($qb->expr()->eq('deleted_at', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT))); @@ -232,7 +251,7 @@ public function findAllByUser(string $userId, ?int $limit = null, ?int $offset = // shared with user $qb = $this->db->getQueryBuilder(); - $qb->select('b.id', 'title', 'owner', 'color', 'archived', 'deleted_at', 'last_modified', 'external_id', 'share_token') + $qb->select('b.id', 'title', 'owner', 'owner_type', 'color', 'archived', 'deleted_at', 'last_modified', 'external_id', 'share_token') //->selectAlias('1', 'shared') ->from('deck_boards', 'b') ->innerJoin('b', 'deck_board_acl', 'acl', $qb->expr()->eq('b.id', 'acl.board_id')) @@ -300,7 +319,7 @@ public function findAllByGroups(string $userId, array $groups, ?int $limit = nul return []; } $qb = $this->db->getQueryBuilder(); - $qb->select('b.id', 'title', 'owner', 'color', 'archived', 'deleted_at', 'last_modified', 'external_id', 'share_token') + $qb->select('b.id', 'title', 'owner', 'owner_type', 'color', 'archived', 'deleted_at', 'last_modified', 'external_id', 'share_token') //->selectAlias('2', 'shared') ->from('deck_boards', 'b') ->innerJoin('b', 'deck_board_acl', 'acl', $qb->expr()->eq('b.id', 'acl.board_id')) @@ -356,7 +375,7 @@ public function findAllByCircles(string $userId, ?int $limit = null, ?int $offse } $qb = $this->db->getQueryBuilder(); - $qb->select('b.id', 'title', 'owner', 'color', 'archived', 'deleted_at', 'last_modified', 'external_id', 'share_token') + $qb->select('b.id', 'title', 'owner', 'owner_type', 'color', 'archived', 'deleted_at', 'last_modified', 'external_id', 'share_token') //->selectAlias('2', 'shared') ->from('deck_boards', 'b') ->innerJoin('b', 'deck_board_acl', 'acl', $qb->expr()->eq('b.id', 'acl.board_id')) @@ -404,9 +423,63 @@ public function findAllByCircles(string $userId, ?int $limit = null, ?int $offse return $entries; } + /** + * Find all boards that are owned by a circle the given user is a member of. + * + * These are boards where owner_type = PERMISSION_TYPE_CIRCLE and the owner field + * holds a circle ID that the user belongs to. They are distinct from boards that + * are merely *shared* with a circle via an ACL entry (handled by findAllByCircles). + */ + public function findAllByCircleOwner(string $userId, ?int $limit = null, ?int $offset = null, ?int $since = null, + bool $includeArchived = true, ?int $before = null, ?string $term = null): array { + $circles = $this->circlesService->getUserCircles($userId); + if (count($circles) === 0) { + return []; + } + + $qb = $this->db->getQueryBuilder(); + $qb->select('id', 'title', 'owner', 'owner_type', 'color', 'archived', 'deleted_at', 'last_modified', 'external_id', 'share_token') + ->from('deck_boards') + ->where($qb->expr()->eq('owner_type', $qb->createNamedParameter(Acl::PERMISSION_TYPE_CIRCLE, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->in('owner', $qb->createNamedParameter($circles, IQueryBuilder::PARAM_STR_ARRAY))); + if (!$includeArchived) { + $qb->andWhere($qb->expr()->eq('archived', $qb->createNamedParameter(false, IQueryBuilder::PARAM_BOOL))) + ->andWhere($qb->expr()->eq('deleted_at', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT))); + } + if ($since !== null) { + $qb->andWhere($qb->expr()->gt('last_modified', $qb->createNamedParameter($since, IQueryBuilder::PARAM_INT))); + } + if ($before !== null) { + $qb->andWhere($qb->expr()->lt('last_modified', $qb->createNamedParameter($before, IQueryBuilder::PARAM_INT))); + } + if ($term !== null) { + $qb->andWhere( + $qb->expr()->iLike( + 'title', + $qb->createNamedParameter( + '%' . $this->db->escapeLikeParameter($term) . '%', + IQueryBuilder::PARAM_STR + ) + ) + ); + } + $qb->orderBy('id'); + if ($limit !== null) { + $qb->setMaxResults($limit); + } + if ($offset !== null) { + $qb->setFirstResult($offset); + } + $entries = $this->findEntities($qb); + foreach ($entries as $entry) { + $entry->setShared(0); + } + return $entries; + } + public function findAllByTeam(string $teamId): array { $qb = $this->db->getQueryBuilder(); - $qb->select('b.id', 'title', 'owner', 'color', 'archived', 'deleted_at', 'last_modified', 'external_id', 'share_token') + $qb->select('b.id', 'title', 'owner', 'owner_type', 'color', 'archived', 'deleted_at', 'last_modified', 'external_id', 'share_token') ->from('deck_boards', 'b') ->innerJoin('b', 'deck_board_acl', 'acl', $qb->expr()->eq('b.id', 'acl.board_id')) ->where($qb->expr()->eq('acl.type', $qb->createNamedParameter(Acl::PERMISSION_TYPE_CIRCLE, IQueryBuilder::PARAM_INT))) @@ -434,7 +507,7 @@ public function findTeamsForBoard(int $boardId): array { public function isSharedWithTeam(int $boardId, string $teamId): bool { $qb = $this->db->getQueryBuilder(); - $qb->select('b.id', 'title', 'owner', 'color', 'archived', 'deleted_at', 'last_modified', 'external_id', 'share_token') + $qb->select('b.id', 'title', 'owner', 'owner_type', 'color', 'archived', 'deleted_at', 'last_modified', 'external_id', 'share_token') ->from('deck_boards', 'b') ->innerJoin('b', 'deck_board_acl', 'acl', $qb->expr()->eq('b.id', 'acl.board_id')) ->where($qb->expr()->eq('b.id', $qb->createNamedParameter($boardId, IQueryBuilder::PARAM_INT))) @@ -460,7 +533,7 @@ public function findToDelete() { // add buffer of 5 min $timeLimit = time() - (60 * 5); $qb = $this->db->getQueryBuilder(); - $qb->select('id', 'title', 'owner', 'color', 'archived', 'deleted_at', 'last_modified', 'external_id', 'share_token') + $qb->select('id', 'title', 'owner', 'owner_type', 'color', 'archived', 'deleted_at', 'last_modified', 'external_id', 'share_token') ->from('deck_boards') ->where($qb->expr()->gt('deleted_at', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT))) ->andWhere($qb->expr()->lt('deleted_at', $qb->createNamedParameter($timeLimit, IQueryBuilder::PARAM_INT))); @@ -540,11 +613,28 @@ public function mapAcl(Acl &$acl): void { /** * @param Board $board */ - public function mapOwner(Board &$board) { + public function mapOwner(Board &$board): void { $userManager = $this->userManager; $cloudIdManager = $this->cloudIdManager; + $circlesService = $this->circlesService; + $logger = $this->logger; $externalId = $board->getExternalId(); - $board->resolveRelation('owner', function ($owner) use (&$userManager, &$cloudIdManager, $externalId) { + $ownerType = $board->getOwnerType(); + $board->resolveRelation('owner', function ($owner) use ($userManager, $cloudIdManager, $circlesService, $logger, $externalId, $ownerType) { + if ($ownerType === Acl::PERMISSION_TYPE_CIRCLE) { + if (!$circlesService->isCirclesEnabled()) { + return null; + } + try { + $circle = $circlesService->getCircle($owner); + if ($circle !== null) { + return new Circle($circle); + } + } catch (\Throwable $e) { + $logger->error('Failed to get circle details when mapping board owner', ['exception' => $e]); + } + return null; + } if ($externalId !== null) { $cloudId = $cloudIdManager->resolveCloudId($owner); return new FederatedUser($cloudId); @@ -559,10 +649,11 @@ public function mapOwner(Board &$board) { /** * @throws \OCP\DB\Exception */ - public function transferOwnership(string $ownerId, string $newOwnerId, $boardId = null): void { + public function transferOwnership(string $ownerId, string $newOwnerId, ?int $boardId = null, int $newOwnerType = Acl::PERMISSION_TYPE_USER): void { $qb = $this->db->getQueryBuilder(); $qb->update('deck_boards') ->set('owner', $qb->createNamedParameter($newOwnerId, IQueryBuilder::PARAM_STR)) + ->set('owner_type', $qb->createNamedParameter($newOwnerType, IQueryBuilder::PARAM_INT)) ->where($qb->expr()->eq('owner', $qb->createNamedParameter($ownerId, IQueryBuilder::PARAM_STR))); if ($boardId !== null) { $qb->andWhere($qb->expr()->eq('id', $qb->createNamedParameter($boardId, IQueryBuilder::PARAM_INT))); diff --git a/lib/Migration/Version11002Date20260429000000.php b/lib/Migration/Version11002Date20260429000000.php new file mode 100644 index 0000000000..972687399a --- /dev/null +++ b/lib/Migration/Version11002Date20260429000000.php @@ -0,0 +1,41 @@ +hasTable('deck_boards')) { + $table = $schema->getTable('deck_boards'); + if (!$table->hasColumn('owner_type')) { + $table->addColumn('owner_type', 'smallint', [ + 'notnull' => true, + 'default' => 0, + 'unsigned' => false, + ]); + } + } + + return $schema; + } +} diff --git a/lib/Notification/NotificationHelper.php b/lib/Notification/NotificationHelper.php index 0813111334..b20e053b32 100644 --- a/lib/Notification/NotificationHelper.php +++ b/lib/Notification/NotificationHelper.php @@ -11,6 +11,7 @@ use DateTime; use Exception; +use OCA\Circles\Model\Member; use OCA\Deck\AppInfo\Application; use OCA\Deck\Db\Acl; use OCA\Deck\Db\AssignmentMapper; @@ -20,6 +21,7 @@ use OCA\Deck\Db\CardMapper; use OCA\Deck\Db\User; use OCA\Deck\Service\ConfigService; +use OCA\Deck\Service\CirclesService; use OCA\Deck\Service\PermissionService; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\MultipleObjectsReturnedException; @@ -39,6 +41,7 @@ public function __construct( protected readonly BoardMapper $boardMapper, protected readonly AssignmentMapper $assignmentMapper, protected readonly PermissionService $permissionService, + protected readonly CirclesService $circlesService, protected readonly IConfig $config, protected readonly IManager $notificationManager, protected readonly IGroupManager $groupManager, @@ -105,6 +108,45 @@ public function markDuedateAsRead(Card $card): void { } public function sendCardAssigned(Card $card, string $userId): void { + $this->sendCardAssignedByType($card, $userId, Acl::PERMISSION_TYPE_USER); + } + + public function sendCardAssignedByType(Card $card, string $participantId, int $type): void { + if ($type === Acl::PERMISSION_TYPE_CIRCLE) { + if (!$this->circlesService->isCirclesEnabled()) { + return; + } + + $circle = $this->circlesService->getCircle($participantId); + if ($circle === null) { + return; + } + + $teamName = $circle->getDisplayName() ?: $participantId; + $members = array_filter($circle->getInheritedMembers(), static function (Member $member) { + return $member->getUserType() === Member::TYPE_USER; + }); + foreach ($members as $member) { + $userId = $member->getUserId(); + if ($userId === $this->userId) { + continue; + } + + $this->sendCardAssignedToUser($card, $userId, $teamName); + } + return; + } + + foreach ($this->resolveParticipantUserIds($participantId, $type) as $userId) { + if ($userId === $this->userId) { + continue; + } + + $this->sendCardAssignedToUser($card, $userId); + } + } + + private function sendCardAssignedToUser(Card $card, string $userId, ?string $teamName = null): void { $boardId = $this->cardMapper->findBoardId($card->getId()); try { $board = $this->getBoard($boardId); @@ -112,21 +154,41 @@ public function sendCardAssigned(Card $card, string $userId): void { return; } + $subjectParams = [ + $card->getTitle(), + $board->getTitle(), + $this->userId, + ]; + if ($teamName !== null) { + $subjectParams[] = 'team'; + $subjectParams[] = $teamName; + } + $notification = $this->notificationManager->createNotification(); $notification ->setApp('deck') ->setUser($userId) ->setDateTime(new DateTime()) ->setObject('card', (string)$card->getId()) - ->setSubject('card-assigned', [ - $card->getTitle(), - $board->getTitle(), - $this->userId - ]); + ->setSubject('card-assigned', $subjectParams); $this->notificationManager->notify($notification); } public function markCardAssignedAsRead(Card $card, string $userId): void { + $this->markCardAssignedAsReadByType($card, $userId, Acl::PERMISSION_TYPE_USER); + } + + public function markCardAssignedAsReadByType(Card $card, string $participantId, int $type): void { + foreach ($this->resolveParticipantUserIds($participantId, $type) as $userId) { + if ($userId === $this->userId) { + continue; + } + + $this->markCardAssignedAsReadForUser($card, $userId); + } + } + + private function markCardAssignedAsReadForUser(Card $card, string $userId): void { $notification = $this->notificationManager->createNotification(); $notification ->setApp('deck') @@ -137,7 +199,45 @@ public function markCardAssignedAsRead(Card $card, string $userId): void { } /** - * Send notifications that a board was shared with a user/group + * Resolve assignment participant ids to concrete user ids for notifications. + * Teams are represented by circles internally. + * + * @return string[] + */ + private function resolveParticipantUserIds(string $participantId, int $type): array { + if ($type === Acl::PERMISSION_TYPE_USER) { + return [$participantId]; + } + + if ($type === Acl::PERMISSION_TYPE_GROUP) { + $group = $this->groupManager->get($participantId); + if ($group === null) { + return []; + } + return array_map(static fn ($user) => $user->getUID(), $group->getUsers()); + } + + if ($type === Acl::PERMISSION_TYPE_CIRCLE) { + if (!$this->circlesService->isCirclesEnabled()) { + return []; + } + + $circle = $this->circlesService->getCircle($participantId); + if ($circle === null) { + return []; + } + + $members = array_filter($circle->getInheritedMembers(), static function (Member $member) { + return $member->getUserType() === Member::TYPE_USER; + }); + return array_values(array_unique(array_map(static fn (Member $member) => $member->getUserId(), $members))); + } + + return []; + } + + /** + * Send notifications that a board was shared with a user/group/team. */ public function sendBoardShared(int $boardId, Acl $acl, bool $markAsRead = false): void { try { @@ -173,6 +273,33 @@ public function sendBoardShared(int $boardId, Acl $acl, bool $markAsRead = false } } } + if ($acl->getType() === Acl::PERMISSION_TYPE_CIRCLE) { + if (!$this->circlesService->isCirclesEnabled()) { + return; + } + + $circle = $this->circlesService->getCircle($acl->getParticipant()); + if ($circle === null) { + return; + } + + $members = array_filter($circle->getInheritedMembers(), static function (Member $member) { + return $member->getUserType() === Member::TYPE_USER; + }); + foreach ($members as $member) { + $userId = $member->getUserId(); + if ($userId === $this->userId) { + continue; + } + $notification = $this->generateBoardShared($board, $userId); + if ($markAsRead) { + $this->notificationManager->markProcessed($notification); + } else { + $notification->setDateTime(new DateTime()); + $this->notificationManager->notify($notification); + } + } + } } public function sendMention(IComment $comment): void { diff --git a/lib/Notification/Notifier.php b/lib/Notification/Notifier.php index 87ee25941e..5dc6a05898 100644 --- a/lib/Notification/Notifier.php +++ b/lib/Notification/Notifier.php @@ -79,33 +79,65 @@ public function prepare(INotification $notification, string $languageCode): INot } else { $dn = $params[2]; } - $notification->setParsedSubject( - $l->t('The card "%s" on "%s" has been assigned to you by %s.', [$params[0], $params[1], $dn]) - ); - $notification->setRichSubject( - $l->t('{user} has assigned the card {deck-card} on {deck-board} to you.'), - [ - 'deck-card' => [ - 'type' => 'deck-card', - 'id' => (string)$cardId, - 'name' => $params[0], - 'boardname' => (string)$params[1], - 'stackname' => $stack->getTitle(), - 'link' => $this->getCardUrl($boardId, $cardId), - ], - 'deck-board' => [ - 'type' => 'deck-board', - 'id' => (string)$boardId, - 'name' => (string)$params[1], - 'link' => $this->getBoardUrl($boardId), - ], - 'user' => [ - 'type' => 'user', - 'id' => (string)$params[2], - 'name' => $dn, - ] - ] - ); + $isTeamAssignment = isset($params[3]) && $params[3] === 'team'; + if ($isTeamAssignment) { + $teamName = $params[4] ?? ''; + $notification->setParsedSubject( + $l->t('The card "%1$s" on "%2$s" has been assigned to team "%3$s" by %4$s. You are receiving this because you are a member of that team.', [$params[0], $params[1], $teamName, $dn]) + ); + $notification->setRichSubject( + $l->t('{user} has assigned the card {deck-card} on {deck-board} to a team you are a member of.'), + [ + 'deck-card' => [ + 'type' => 'deck-card', + 'id' => (string)$cardId, + 'name' => $params[0], + 'boardname' => (string)$params[1], + 'stackname' => $stack->getTitle(), + 'link' => $this->getCardUrl($boardId, $cardId), + ], + 'deck-board' => [ + 'type' => 'deck-board', + 'id' => (string)$boardId, + 'name' => (string)$params[1], + 'link' => $this->getBoardUrl($boardId), + ], + 'user' => [ + 'type' => 'user', + 'id' => (string)$params[2], + 'name' => $dn, + ] + ] + ); + } else { + $notification->setParsedSubject( + $l->t('The card "%s" on "%s" has been assigned to you by %s.', [$params[0], $params[1], $dn]) + ); + $notification->setRichSubject( + $l->t('{user} has assigned the card {deck-card} on {deck-board} to you.'), + [ + 'deck-card' => [ + 'type' => 'deck-card', + 'id' => (string)$cardId, + 'name' => $params[0], + 'boardname' => (string)$params[1], + 'stackname' => $stack->getTitle(), + 'link' => $this->getCardUrl($boardId, $cardId), + ], + 'deck-board' => [ + 'type' => 'deck-board', + 'id' => (string)$boardId, + 'name' => (string)$params[1], + 'link' => $this->getBoardUrl($boardId), + ], + 'user' => [ + 'type' => 'user', + 'id' => (string)$params[2], + 'name' => $dn, + ] + ] + ); + } $notification->setLink($this->getCardUrl($boardId, $cardId)); break; case 'card-overdue': diff --git a/lib/Service/AssignmentService.php b/lib/Service/AssignmentService.php index dd8d8f17a0..81be618ef8 100644 --- a/lib/Service/AssignmentService.php +++ b/lib/Service/AssignmentService.php @@ -75,8 +75,8 @@ public function assignUser(int $cardId, string $userId, int $type = Assignment:: } - if ($type === Assignment::TYPE_USER && $userId !== $this->userId) { - $this->notificationHelper->sendCardAssigned($card, $userId); + if ($userId !== $this->userId) { + $this->notificationHelper->sendCardAssignedByType($card, $userId, $type); } $assignment = new Assignment(); @@ -109,8 +109,8 @@ public function unassignUser(int $cardId, string $userId, int $type = 0): Assign $assignment = $this->assignedUsersMapper->delete($assignment); $card = $this->cardMapper->find($cardId); $this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_CARD, $card, ActivityManager::SUBJECT_CARD_USER_UNASSIGN, ['assigneduser' => $userId]); - if ($type === Assignment::TYPE_USER && $userId !== $this->userId) { - $this->notificationHelper->markCardAssignedAsRead($card, $userId); + if ($userId !== $this->userId) { + $this->notificationHelper->markCardAssignedAsReadByType($card, $userId, $type); } $this->changeHelper->cardChanged($cardId); diff --git a/lib/Service/BoardService.php b/lib/Service/BoardService.php index e08b34b8c9..d46abfb9d6 100644 --- a/lib/Service/BoardService.php +++ b/lib/Service/BoardService.php @@ -82,6 +82,7 @@ public function __construct( private IUserManager $userManager, private ISecureRandom $random, private ConfigService $configService, + private CirclesService $circlesService, private ?string $userId, ) { } @@ -615,14 +616,23 @@ public function clone( return $this->find($newBoard->getId()); } - public function transferBoardOwnership(int $boardId, string $newOwner, bool $changeContent = false): Board { + public function transferBoardOwnership(int $boardId, string $newOwner, bool $changeContent = false, int $newOwnerType = Acl::PERMISSION_TYPE_USER): Board { $this->connection->beginTransaction(); try { $board = $this->boardMapper->find($boardId); $previousOwner = $board->getOwner(); + + // Validate the new owner before touching anything + $this->validateTransferTarget($newOwner, $newOwnerType); + $this->clearBoardFromCache($board); - $this->aclMapper->deleteParticipantFromBoard($boardId, Acl::PERMISSION_TYPE_USER, $newOwner); - if (!$changeContent) { + // Remove new owner from ACL (avoids a duplicate entry once they become the owner) + $this->aclMapper->deleteParticipantFromBoard($boardId, $newOwnerType, $newOwner); + + // Preserve the previous owner's access via an explicit ACL entry unless the + // caller opted into a full content transfer. Skip for circle previous-owners + // because a circle ID is not a valid user participant. + if (!$changeContent && $board->getOwnerType() === Acl::PERMISSION_TYPE_USER) { try { $this->addAcl($boardId, Acl::PERMISSION_TYPE_USER, $previousOwner, true, true, true); } catch (DbException $e) { @@ -631,13 +641,15 @@ public function transferBoardOwnership(int $boardId, string $newOwner, bool $cha } } } - $this->boardMapper->transferOwnership($previousOwner, $newOwner, $boardId); - // Optionally also change user assignments and card owner information - if ($changeContent) { + $this->boardMapper->transferOwnership($previousOwner, $newOwner, $boardId, $newOwnerType); + + // Card-content remap is only meaningful when transferring to a user, not a circle + if ($changeContent && $newOwnerType === Acl::PERMISSION_TYPE_USER) { $this->assignedUsersMapper->remapAssignedUser($boardId, $previousOwner, $newOwner); $this->cardMapper->remapCardOwner($boardId, $previousOwner, $newOwner); } + $this->connection->commit(); return $this->boardMapper->find($boardId); } catch (\Throwable $e) { @@ -646,12 +658,29 @@ public function transferBoardOwnership(int $boardId, string $newOwner, bool $cha } } - public function transferOwnership(string $owner, string $newOwner, bool $changeContent = false): \Generator { - $boards = $this->boardMapper->findAllByUser($owner); + public function transferOwnership(string $owner, string $newOwner, bool $changeContent = false, int $newOwnerType = Acl::PERMISSION_TYPE_USER): \Generator { + // Validate once up front so invalid targets fail even if no boards match. + $this->validateTransferTarget($newOwner, $newOwnerType); + $boards = $this->boardMapper->findAllByOwner($owner); foreach ($boards as $board) { - if ($board->getOwner() === $owner) { - yield $this->transferBoardOwnership($board->getId(), $newOwner, $changeContent); + yield $this->transferBoardOwnership($board->getId(), $newOwner, $changeContent, $newOwnerType); + } + } + + private function validateTransferTarget(string $newOwner, int $newOwnerType): void { + // Teams are represented by Circles internally (PERMISSION_TYPE_CIRCLE + circle ID). + if ($newOwnerType === Acl::PERMISSION_TYPE_CIRCLE) { + if (!$this->circlesService->isCirclesEnabled()) { + throw new BadRequestException('The Circles app is not enabled'); + } + if ($this->circlesService->getCircle($newOwner) === null) { + throw new BadRequestException('Circle not found: ' . $newOwner); } + return; + } + + if (!$this->userManager->userExists($newOwner)) { + throw new BadRequestException('User not found: ' . $newOwner); } } diff --git a/lib/Service/CirclesService.php b/lib/Service/CirclesService.php index e604a78698..bafd9949ea 100644 --- a/lib/Service/CirclesService.php +++ b/lib/Service/CirclesService.php @@ -26,6 +26,8 @@ class CirclesService { private bool $circlesEnabled; private $userCircleCache = []; + /** @var array */ + private array $userCirclesCache = []; public function __construct(IAppManager $appManager) { $this->circlesEnabled = $appManager->isEnabledForUser('circles'); @@ -88,15 +90,21 @@ public function getUserCircles(string $userId): array { return []; } + if (isset($this->userCirclesCache[$userId])) { + return $this->userCirclesCache[$userId]; + } + try { $circlesManager = Server::get(CirclesManager::class); $federatedUser = $circlesManager->getFederatedUser($userId, Member::TYPE_USER); $circlesManager->startSession($federatedUser); $probe = new CircleProbe(); $probe->mustBeMember(); - return array_map(function (Circle $circle) { + $circles = array_map(function (Circle $circle) { return $circle->getSingleId(); }, $circlesManager->getCircles($probe)); + $this->userCirclesCache[$userId] = $circles; + return $circles; } catch (Throwable $e) { } return []; diff --git a/lib/Service/PermissionService.php b/lib/Service/PermissionService.php index 4de5327367..81779a54df 100644 --- a/lib/Service/PermissionService.php +++ b/lib/Service/PermissionService.php @@ -180,12 +180,15 @@ public function checkPermission(?IPermissionMapper $mapper, $id, int $permission * @param $boardId * @return bool */ - public function userIsBoardOwner($boardId, $userId = null, bool $allowDeleted = false) { + public function userIsBoardOwner($boardId, $userId = null, bool $allowDeleted = false): bool { if ($userId === null) { $userId = $this->userId; } try { $board = $this->getBoard($boardId, $allowDeleted); + if ($board->getOwnerType() === Acl::PERMISSION_TYPE_CIRCLE) { + return $this->circlesService->isUserInCircle($board->getOwner(), $userId); + } return $userId === $board->getOwner(); } catch (DoesNotExistException|MultipleObjectsReturnedException $e) { } @@ -283,7 +286,31 @@ public function findUsers($boardId, $refresh = false) { } /** @var array */ $users = []; - if (!$this->userManager->userExists($board->getOwner())) { + if ($board->getOwnerType() === Acl::PERMISSION_TYPE_CIRCLE) { + // Board is owned by a circle: expand its members just like a circle ACL entry below. + if ($this->circlesService->isCirclesEnabled()) { + try { + $owningCircle = $this->circlesService->getCircle($board->getOwner()); + if ($owningCircle !== null) { + foreach ($owningCircle->getInheritedMembers() as $member) { + if ($member->getUserType() !== Member::TYPE_USER || $member->getLevel() < Member::LEVEL_MEMBER) { + continue; + } + $user = $this->userManager->get($member->getUserId()); + if ($user === null) { + $this->logger->info('No user found for circle owner member ' . $member->getUserId()); + } else { + $users[(string)$member->getUserId()] = new User($member->getUserId(), $this->userManager); + } + } + } else { + $this->logger->info('No circle found for board owner ' . $board->getOwner()); + } + } catch (\Exception $e) { + $this->logger->error('Failed to expand circle owner members for board ' . $board->getId(), ['exception' => $e]); + } + } + } elseif (!$this->userManager->userExists($board->getOwner())) { $this->logger->info('No owner found for board ' . $board->getId()); } else { $users[(string)$board->getOwner()] = new User($board->getOwner(), $this->userManager); @@ -322,7 +349,7 @@ public function findUsers($boardId, $refresh = false) { } foreach ($circle->getInheritedMembers() as $member) { - if ($member->getUserType() !== 1 || $member->getLevel() < Member::LEVEL_MEMBER) { + if ($member->getUserType() !== Member::TYPE_USER || $member->getLevel() < Member::LEVEL_MEMBER) { // deck currently only supports user members in circles continue; } diff --git a/package-lock.json b/package-lock.json index 271b222139..a9421bae5b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4750,34 +4750,6 @@ "@vue/shared": "3.5.30" } }, - "node_modules/@nextcloud/password-confirmation/node_modules/@vue/devtools-kit": { - "version": "7.7.9", - "resolved": "https://registry.npmjs.org/@vue/devtools-kit/-/devtools-kit-7.7.9.tgz", - "integrity": "sha512-PyQ6odHSgiDVd4hnTP+aDk2X4gl2HmLDfiyEnn3/oV+ckFDuswRs4IbBT7vacMuGdwY/XemxBoh302ctbsptuA==", - "license": "MIT", - "optional": true, - "peer": true, - "dependencies": { - "@vue/devtools-shared": "^7.7.9", - "birpc": "^2.3.0", - "hookable": "^5.5.3", - "mitt": "^3.0.1", - "perfect-debounce": "^1.0.0", - "speakingurl": "^14.0.1", - "superjson": "^2.2.2" - } - }, - "node_modules/@nextcloud/password-confirmation/node_modules/@vue/devtools-shared": { - "version": "7.7.9", - "resolved": "https://registry.npmjs.org/@vue/devtools-shared/-/devtools-shared-7.7.9.tgz", - "integrity": "sha512-iWAb0v2WYf0QWmxCGy0seZNDPdO3Sp5+u78ORnyeonS6MT4PC7VPrryX2BpMJrwlDeaZ6BD4vP4XKjK0SZqaeA==", - "license": "MIT", - "optional": true, - "peer": true, - "dependencies": { - "rfdc": "^1.4.1" - } - }, "node_modules/@nextcloud/password-confirmation/node_modules/@vue/server-renderer": { "version": "3.5.30", "resolved": "https://registry.npmjs.org/@vue/server-renderer/-/server-renderer-3.5.30.tgz", @@ -4973,14 +4945,6 @@ "url": "https://github.com/sponsors/sindresorhus" } }, - "node_modules/@nextcloud/password-confirmation/node_modules/perfect-debounce": { - "version": "1.0.0", - "resolved": "https://registry.npmjs.org/perfect-debounce/-/perfect-debounce-1.0.0.tgz", - "integrity": "sha512-xCy9V055GLEqoFaHoC1SoLIaLmWctgCUaBaWxDZ7/Zx4CTyX7cJQLJOok/orfjZAh9kEYpjJa4d0KcJmCbctZA==", - "license": "MIT", - "optional": true, - "peer": true - }, "node_modules/@nextcloud/password-confirmation/node_modules/picomatch": { "version": "4.0.4", "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.4.tgz", @@ -4993,40 +4957,6 @@ "url": "https://github.com/sponsors/jonschlinkert" } }, - "node_modules/@nextcloud/password-confirmation/node_modules/pinia": { - "version": "3.0.4", - "resolved": "https://registry.npmjs.org/pinia/-/pinia-3.0.4.tgz", - "integrity": "sha512-l7pqLUFTI/+ESXn6k3nu30ZIzW5E2WZF/LaHJEpoq6ElcLD+wduZoB2kBN19du6K/4FDpPMazY2wJr+IndBtQw==", - "license": "MIT", - "optional": true, - "peer": true, - "dependencies": { - "@vue/devtools-api": "^7.7.7" - }, - "funding": { - "url": "https://github.com/sponsors/posva" - }, - "peerDependencies": { - "typescript": ">=4.5.0", - "vue": "^3.5.11" - }, - "peerDependenciesMeta": { - "typescript": { - "optional": true - } - } - }, - "node_modules/@nextcloud/password-confirmation/node_modules/pinia/node_modules/@vue/devtools-api": { - "version": "7.7.9", - "resolved": "https://registry.npmjs.org/@vue/devtools-api/-/devtools-api-7.7.9.tgz", - "integrity": "sha512-kIE8wvwlcZ6TJTbNeU2HQNtaxLx3a84aotTITUuL/4bzfPxzajGBOoqjMhwZJ8L9qFYDU/lAYMEEm11dnZOD6g==", - "license": "MIT", - "optional": true, - "peer": true, - "dependencies": { - "@vue/devtools-kit": "^7.7.9" - } - }, "node_modules/@nextcloud/password-confirmation/node_modules/readdirp": { "version": "5.0.0", "resolved": "https://registry.npmjs.org/readdirp/-/readdirp-5.0.0.tgz", @@ -9845,23 +9775,6 @@ "dev": true, "peer": true }, - "node_modules/copy-anything": { - "version": "4.0.5", - "resolved": "https://registry.npmjs.org/copy-anything/-/copy-anything-4.0.5.tgz", - "integrity": "sha512-7Vv6asjS4gMOuILabD3l739tsaxFQmC+a7pLZm02zyvs8p977bL3zEgq3yDk5rn9B0PbYgIv++jmHcuUab4RhA==", - "license": "MIT", - "optional": true, - "peer": true, - "dependencies": { - "is-what": "^5.2.0" - }, - "engines": { - "node": ">=18" - }, - "funding": { - "url": "https://github.com/sponsors/mesqueeb" - } - }, "node_modules/core-js": { "version": "2.6.9", "hasInstallScript": true, @@ -15470,20 +15383,6 @@ "url": "https://github.com/sponsors/ljharb" } }, - "node_modules/is-what": { - "version": "5.5.0", - "resolved": "https://registry.npmjs.org/is-what/-/is-what-5.5.0.tgz", - "integrity": "sha512-oG7cgbmg5kLYae2N5IVd3jm2s+vldjxJzK1pcu9LfpGuQ93MQSzo0okvRna+7y5ifrD+20FE8FvjusyGaz14fw==", - "license": "MIT", - "optional": true, - "peer": true, - "engines": { - "node": ">=18" - }, - "funding": { - "url": "https://github.com/sponsors/mesqueeb" - } - }, "node_modules/is-whitespace": { "version": "0.3.0", "dev": true, @@ -19114,14 +19013,6 @@ "node": ">=16 || 14 >=14.17" } }, - "node_modules/mitt": { - "version": "3.0.1", - "resolved": "https://registry.npmjs.org/mitt/-/mitt-3.0.1.tgz", - "integrity": "sha512-vKivATfr97l2/QBCYAkXYDbrIWPM2IIKEl7YPhjCvKlG3kE2gm+uBo6nEXK3M5/Ffh/FLpKExzOQ3JJoJGFKBw==", - "license": "MIT", - "optional": true, - "peer": true - }, "node_modules/mkdirp-classic": { "version": "0.5.3", "resolved": "https://registry.npmjs.org/mkdirp-classic/-/mkdirp-classic-0.5.3.tgz", @@ -22694,17 +22585,6 @@ "wbuf": "^1.7.3" } }, - "node_modules/speakingurl": { - "version": "14.0.1", - "resolved": "https://registry.npmjs.org/speakingurl/-/speakingurl-14.0.1.tgz", - "integrity": "sha512-1POYv7uv2gXoyGFpBCmpDVSNV74IfsWlDW216UPjbWufNf+bSU6GdbDsxdcxtfwb4xlI3yxzOTKClUosxARYrQ==", - "license": "BSD-3-Clause", - "optional": true, - "peer": true, - "engines": { - "node": ">=0.10.0" - } - }, "node_modules/spec-change": { "version": "1.11.21", "resolved": "https://registry.npmjs.org/spec-change/-/spec-change-1.11.21.tgz", @@ -23865,20 +23745,6 @@ "node": ">=18" } }, - "node_modules/superjson": { - "version": "2.2.6", - "resolved": "https://registry.npmjs.org/superjson/-/superjson-2.2.6.tgz", - "integrity": "sha512-H+ue8Zo4vJmV2nRjpx86P35lzwDT3nItnIsocgumgr0hHMQ+ZGq5vrERg9kJBo5AWGmxZDhzDo+WVIJqkB0cGA==", - "license": "MIT", - "optional": true, - "peer": true, - "dependencies": { - "copy-anything": "^4" - }, - "engines": { - "node": ">=16" - } - }, "node_modules/superstruct": { "version": "2.0.2", "resolved": "https://registry.npmjs.org/superstruct/-/superstruct-2.0.2.tgz", diff --git a/src/components/board/SharingTabSidebar.vue b/src/components/board/SharingTabSidebar.vue index 035b1f363b..2649b2b67d 100644 --- a/src/components/board/SharingTabSidebar.vue +++ b/src/components/board/SharingTabSidebar.vue @@ -12,12 +12,16 @@
  • - + +
    {{ board.owner.displayname }} - + {{ t('deck', 'Board owner') }} + + {{ t('deck', 'Team') }} +
  • @@ -51,12 +55,12 @@ @change="clickManageAcl(acl)"> {{ t('deck', 'Can manage') }} - - {{ t('deck', 'Owner') }} - + @click="clickTransferOwner(acl.participant.uid || acl.participant.id, acl.type, acl.participant.displayname || acl.participant.id || acl.participant.uid)"> + {{ t('deck', 'Transfer ownership') }} + uid === getCurrentUser().uid }, + canTransferTo() { + return (acl) => { + const canBeOwnershipTarget = acl.type === PERMISSION_TYPE_USER || acl.type === PERMISSION_TYPE_CIRCLE + if (!canBeOwnershipTarget) { + return false + } + + // For user-owned boards: only the current owner can transfer + if (this.board.ownerType !== PERMISSION_TYPE_CIRCLE) { + return this.isCurrentUser(this.board.owner.uid) + } + // For circle-owned boards: any circle member with manage rights can transfer + return this.canManage + } + }, formatedSharees() { const result = this.unallocatedSharees.map(item => { const res = { @@ -140,7 +155,7 @@ export default { displayName: item.displayname || item.name || item.label || item.id, user: item.id, subname: item.shareWithDisplayNameUnique || item.subline || item.id, // NcSelectUser does its own pattern matching to filter things out - type: SOURCE_TO_SHARE_TYPE[item.source] + type: SOURCE_TO_SHARE_TYPE[item.source], } return res }).slice(0, 10) @@ -224,10 +239,13 @@ export default { clickDeleteAcl(acl) { this.$store.dispatch('deleteAclFromCurrentBoard', acl) }, - clickTransferOwner(newOwner) { + clickTransferOwner(newOwner, newOwnerType, newOwnerDisplayName = null) { + const targetLabel = newOwnerType === PERMISSION_TYPE_CIRCLE + ? t('deck', 'team {name}', { name: newOwnerDisplayName || newOwner }) + : newOwner OC.dialogs.confirmDestructive( - t('deck', 'Are you sure you want to transfer the board {title} to {user}?', { title: this.board.title, user: newOwner }), - t('deck', 'Transfer the board.'), + t('deck', 'Are you sure you want to transfer the board {title} to {target}?', { title: this.board.title, target: targetLabel }), + t('deck', 'Transfer the board'), { type: OC.dialogs.YES_NO_BUTTONS, confirm: t('deck', 'Transfer'), @@ -241,12 +259,13 @@ export default { await this.$store.dispatch('transferOwnership', { boardId: this.board.id, newOwner, + newOwnerType, }) - const successMessage = t('deck', 'The board has been transferred to {user}', { user: newOwner }) + const successMessage = t('deck', 'The board has been transferred to {target}', { target: targetLabel }) showSuccess(successMessage) this.$router.push({ name: 'main' }) } catch (e) { - const errorMessage = t('deck', 'Failed to transfer the board to {user}', { user: newOwner.user }) + const errorMessage = t('deck', 'Failed to transfer the board to {target}', { target: targetLabel }) showError(errorMessage) } finally { this.isLoading = false diff --git a/src/components/boards/BoardItem.vue b/src/components/boards/BoardItem.vue index 86c174eb4c..f355a28ef3 100644 --- a/src/components/boards/BoardItem.vue +++ b/src/components/boards/BoardItem.vue @@ -16,10 +16,14 @@ {{ board.title }}
    - +
    import { NcAvatar } from '@nextcloud/vue' +import { PERMISSION_TYPE_CIRCLE } from '../../helpers/constants.js' export default { name: 'BoardItem', @@ -46,6 +51,11 @@ export default { default: () => { return {} }, }, }, + data() { + return { + PERMISSION_TYPE_CIRCLE, + } + }, computed: { routeTo() { return { diff --git a/src/helpers/constants.js b/src/helpers/constants.js new file mode 100644 index 0000000000..620b51cf8d --- /dev/null +++ b/src/helpers/constants.js @@ -0,0 +1,25 @@ +/** + * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +/** + * ACL / owner permission types – mirror Acl::PERMISSION_TYPE_* PHP constants. + * Keep in sync with lib/Db/Acl.php. + */ +export const PERMISSION_TYPE_USER = 0 +export const PERMISSION_TYPE_GROUP = 1 +export const PERMISSION_TYPE_REMOTE = 6 +export const PERMISSION_TYPE_CIRCLE = 7 + +/** + * Map from the Nextcloud sharee-picker source identifier to the board ACL type. + */ +export const SOURCE_TO_SHARE_TYPE = { + users: PERMISSION_TYPE_USER, + groups: PERMISSION_TYPE_GROUP, + emails: 4, + remotes: PERMISSION_TYPE_REMOTE, + circles: PERMISSION_TYPE_CIRCLE, + teams: PERMISSION_TYPE_CIRCLE, +} diff --git a/src/store/main.js b/src/store/main.js index 8b7a55b6c7..b8963d24bf 100644 --- a/src/store/main.js +++ b/src/store/main.js @@ -11,6 +11,7 @@ import Vuex from 'vuex' import axios from '@nextcloud/axios' import { generateOcsUrl, generateUrl } from '@nextcloud/router' import { BoardApi } from '../services/BoardApi.js' +import { PERMISSION_TYPE_USER } from '../helpers/constants.js' import stackModuleFactory from './stack.js' import cardModuleFactory from './card.js' import comment from './comment.js' @@ -524,9 +525,10 @@ export default function storeFactory() { dispatch('loadBoardById', acl.boardId) }) }, - async transferOwnership({ commit }, { boardId, newOwner }) { + async transferOwnership({ commit }, { boardId, newOwner, newOwnerType = PERMISSION_TYPE_USER }) { await axios.put(generateUrl(`apps/deck/boards/${boardId}/transferOwner`), { newOwner, + newOwnerType, }) }, toggleShortcutLock({ commit }, lock) { diff --git a/tests/unit/Db/BoardTest.php b/tests/unit/Db/BoardTest.php index 5d729b518e..0fff6620b0 100644 --- a/tests/unit/Db/BoardTest.php +++ b/tests/unit/Db/BoardTest.php @@ -22,6 +22,7 @@ public function testJsonSerialize() { 'id' => 1, 'title' => 'My Board', 'owner' => 'admin', + 'ownerType' => 0, 'color' => '000000', 'labels' => [], 'permissions' => [], @@ -48,6 +49,7 @@ public function testUnfetchedValues() { 'id' => 1, 'title' => 'My Board', 'owner' => 'admin', + 'ownerType' => 0, 'color' => '000000', 'labels' => [], 'permissions' => [], @@ -72,6 +74,7 @@ public function testSetLabels() { 'id' => 1, 'title' => 'My Board', 'owner' => 'admin', + 'ownerType' => 0, 'color' => '000000', 'labels' => ['foo', 'bar'], 'permissions' => [], @@ -103,6 +106,7 @@ public function testSetShared() { 'id' => 1, 'title' => 'My Board', 'owner' => 'admin', + 'ownerType' => 0, 'color' => '000000', 'labels' => [], 'permissions' => [], diff --git a/tests/unit/Service/BoardServiceTest.php b/tests/unit/Service/BoardServiceTest.php index 549021bf0c..faea13853d 100644 --- a/tests/unit/Service/BoardServiceTest.php +++ b/tests/unit/Service/BoardServiceTest.php @@ -32,6 +32,7 @@ use OC\L10N\L10N; use OC\Security\SecureRandom; use OCA\Deck\Activity\ActivityManager; +use OCA\Deck\BadRequestException; use OCA\Deck\Db\Acl; use OCA\Deck\Db\AclMapper; use OCA\Deck\Db\Assignment; @@ -103,6 +104,8 @@ class BoardServiceTest extends TestCase { /** @var IUserManager */ private $userManager; + /** @var CirclesService|MockObject */ + private $circlesService; public function setUp(): void { parent::setUp(); @@ -125,6 +128,7 @@ public function setUp(): void { $this->boardServiceValidator = $this->createMock(BoardServiceValidator::class); $this->sessionMapper = $this->createMock(SessionMapper::class); $this->userManager = $this->createMock(IUserManager::class); + $this->circlesService = $this->createMock(CirclesService::class); $this->service = new BoardService( $this->boardMapper, @@ -151,6 +155,7 @@ public function setUp(): void { $this->userManager, $this->createMock(SecureRandom::class), $this->createMock(ConfigService::class), + $this->circlesService, $this->userId ); @@ -474,4 +479,101 @@ public function testDeleteAcl() { ->with(new AclDeletedEvent($acl)); $this->assertEquals($acl, $this->service->deleteAcl(123)); } + + public function testTransferBoardOwnershipToCircle(): void { + $board = new Board(); + $board->setId(10); + $board->setOwner('alice'); + $board->setOwnerType(Acl::PERMISSION_TYPE_USER); + + $updatedBoard = new Board(); + $updatedBoard->setId(10); + $updatedBoard->setOwner('circle-id-xyz'); + $updatedBoard->setOwnerType(Acl::PERMISSION_TYPE_CIRCLE); + + $this->connection->expects($this->once())->method('beginTransaction'); + $this->connection->expects($this->once())->method('commit'); + + $this->boardMapper->expects($this->exactly(2)) + ->method('find') + ->willReturnOnConsecutiveCalls($board, $updatedBoard); + + $this->circlesService->expects($this->once()) + ->method('isCirclesEnabled') + ->willReturn(true); + $this->circlesService->expects($this->once()) + ->method('getCircle') + ->with('circle-id-xyz') + ->willReturn($this->createMock(\OCA\Circles\Model\Circle::class)); + + $this->aclMapper->expects($this->once()) + ->method('deleteParticipantFromBoard') + ->with(10, Acl::PERMISSION_TYPE_CIRCLE, 'circle-id-xyz'); + + // Previous user owner gets an ACL entry when changeContent = false + $this->aclMapper->expects($this->once()) + ->method('findAll') + ->willReturn([]); + $this->aclMapper->expects($this->once()) + ->method('insert') + ->willReturnCallback(fn ($acl) => $acl); + + $this->boardMapper->expects($this->once()) + ->method('transferOwnership') + ->with('alice', 'circle-id-xyz', 10, Acl::PERMISSION_TYPE_CIRCLE); + + // No content remap when target is a circle + $this->assignedUsersMapper->expects($this->never())->method('remapAssignedUser'); + $this->cardMapper->expects($this->never())->method('remapCardOwner'); + + $result = $this->service->transferBoardOwnership(10, 'circle-id-xyz', false, Acl::PERMISSION_TYPE_CIRCLE); + $this->assertSame($updatedBoard, $result); + } + + public function testTransferBoardOwnershipToNonExistentUserThrows(): void { + $board = new Board(); + $board->setId(10); + $board->setOwner('alice'); + $board->setOwnerType(Acl::PERMISSION_TYPE_USER); + + $this->connection->expects($this->once())->method('beginTransaction'); + $this->connection->expects($this->once())->method('rollBack'); + + $this->boardMapper->expects($this->once()) + ->method('find') + ->willReturn($board); + + $this->userManager->expects($this->once()) + ->method('userExists') + ->with('ghost') + ->willReturn(false); + + $this->expectException(BadRequestException::class); + $this->service->transferBoardOwnership(10, 'ghost', false, Acl::PERMISSION_TYPE_USER); + } + + public function testTransferBoardOwnershipToNonExistentCircleThrows(): void { + $board = new Board(); + $board->setId(10); + $board->setOwner('alice'); + $board->setOwnerType(Acl::PERMISSION_TYPE_USER); + + $this->connection->expects($this->once())->method('beginTransaction'); + $this->connection->expects($this->once())->method('rollBack'); + + $this->boardMapper->expects($this->once()) + ->method('find') + ->willReturn($board); + + $this->circlesService->expects($this->once()) + ->method('isCirclesEnabled') + ->willReturn(true); + $this->circlesService->expects($this->once()) + ->method('getCircle') + ->with('bad-circle') + ->willReturn(null); + + $this->expectException(BadRequestException::class); + $this->service->transferBoardOwnership(10, 'bad-circle', false, Acl::PERMISSION_TYPE_CIRCLE); + } } diff --git a/tests/unit/Service/PermissionServiceTest.php b/tests/unit/Service/PermissionServiceTest.php index 3099b9edd2..cc9257386b 100644 --- a/tests/unit/Service/PermissionServiceTest.php +++ b/tests/unit/Service/PermissionServiceTest.php @@ -156,6 +156,27 @@ public function testUserIsBoardOwnerNull() { $this->assertEquals(false, $this->service->userIsBoardOwner(123)); } + public function testUserIsBoardOwnerCircleMember() { + $board = new Board(); + $board->setOwner('circle-id-abc'); + $board->setOwnerType(Acl::PERMISSION_TYPE_CIRCLE); + + $this->boardMapper->expects($this->exactly(2)) + ->method('find') + ->willReturn($board); + + $this->circlesService->expects($this->exactly(2)) + ->method('isUserInCircle') + ->with('circle-id-abc', $this->anything()) + ->willReturnMap([ + ['circle-id-abc', 'admin', true], + ['circle-id-abc', 'user1', false], + ]); + + $this->assertEquals(true, $this->service->userIsBoardOwner(123, 'admin')); + $this->assertEquals(false, $this->service->userIsBoardOwner(123, 'user1')); + } + public static function dataTestUserCan() { return [ // participant permissions type