diff --git a/.github/workflows/cypress-e2e.yml b/.github/workflows/cypress-e2e.yml index 12db70aac6..e8f6fa69f1 100644 --- a/.github/workflows/cypress-e2e.yml +++ b/.github/workflows/cypress-e2e.yml @@ -22,7 +22,6 @@ jobs: strategy: fail-fast: false matrix: - node-version: [20.x] containers: [1, 2, 3] php-versions: [ '8.2' ] server-versions: [ 'master' ] @@ -41,11 +40,6 @@ jobs: options: --health-cmd pg_isready --health-interval 5s --health-timeout 2s --health-retries 5 steps: - - name: Use Node.js ${{ matrix.node-version }} - uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 - with: - node-version: ${{ matrix.node-version }} - - name: Register text Git reference run: | text_app_ref="$(if [ "${{ matrix.server-versions }}" = "master" ]; then echo -n "main"; else echo -n "${{ matrix.server-versions }}"; fi)" @@ -65,6 +59,22 @@ jobs: persist-credentials: false path: apps/${{ env.APP_NAME }} + - name: Read package.json node and npm engines version + uses: skjnldsv/read-package-engines-version-actions@06d6baf7d8f41934ab630e97d9e6c0bc9c9ac5e4 # v3 + id: versions + with: + fallbackNode: '^24' + fallbackNpm: '^11.3' + path: apps/${{ env.APP_NAME }} + + - name: Set up node ${{ steps.versions.outputs.nodeVersion }} + uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 + with: + node-version: ${{ steps.versions.outputs.nodeVersion }} + + - name: Set up npm ${{ steps.versions.outputs.npmVersion }} + run: npm i -g 'npm@${{ steps.versions.outputs.npmVersion }}' + - name: Checkout text uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: diff --git a/.gitignore b/.gitignore index e8034c743f..f028d5b32e 100644 --- a/.gitignore +++ b/.gitignore @@ -9,5 +9,7 @@ tests/integration/composer.lock tests/.phpunit.result.cache vendor/ .php_cs.cache +.php-cs-fixer.cache \.idea/ settings.json +runtests.sh 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..e8b4966044 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() { @@ -39,12 +46,12 @@ protected function configure() { ->addArgument( 'owner', InputArgument::REQUIRED, - 'Owner uid' + 'Owner uid or Team (circle) ID to transfer from' ) ->addArgument( 'newOwner', InputArgument::REQUIRED, - 'New owner uid' + 'New owner uid or Team (circle) ID to transfer to' ) ->addArgument( 'boardId', @@ -57,6 +64,18 @@ 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' + ) + ->addOption( + 'from-team', + null, + InputOption::VALUE_NONE, + 'Treat as a team ID (internally stored as a circle ID) instead of a user UID' + ) ; } @@ -64,8 +83,77 @@ 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'); + $fromTeam = $input->getOption('from-team'); + $ownerType = Acl::PERMISSION_TYPE_USER; + $newOwnerType = Acl::PERMISSION_TYPE_USER; + $teamDisplayName = null; + if ($fromTeam) { + $ownerType = Acl::PERMISSION_TYPE_CIRCLE; + } else { + $ownerUserExists = $this->userManager->userExists($owner); + $ownerCircleExists = false; + if ($this->circlesService->isCirclesEnabled()) { + try { + $ownerCircleExists = $this->circlesService->getCircle($owner) !== null; + } catch (\Throwable $e) { + $ownerCircleExists = false; + } + } + + if ($ownerUserExists && $ownerCircleExists) { + $output->writeln('Ambiguous source owner: ' . $owner . ' matches both a user and a team (circle ID). Use --from-team if you mean the team.'); + return 1; + } + + /** @psalm-suppress RedundantCondition -- psalm traces probeCircle() stub as non-nullable and misses the exception path */ + if ($ownerCircleExists && !$ownerUserExists) { + $ownerType = Acl::PERMISSION_TYPE_CIRCLE; + } + } + 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); + if ($circle !== null) { + $circleExists = true; + $teamDisplayName = $circle->getDisplayName(); + } else { + $circleExists = false; + } + } 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; + } + + /** @psalm-suppress RedundantCondition -- psalm traces probeCircle() stub as non-nullable and misses the exception path */ + 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 +171,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 +181,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, $ownerType) 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/Acl.php b/lib/Db/Acl.php index 735f9a19fc..7fb1d7a986 100644 --- a/lib/Db/Acl.php +++ b/lib/Db/Acl.php @@ -21,6 +21,8 @@ * @method void setOwner(int $owner) * @method void setToken(string $token) * @method string getToken() + * @method bool isRetainsAccessViaMembership() + * @method void setRetainsAccessViaMembership(bool $retainsAccessViaMembership) * */ class Acl extends RelationalEntity { @@ -41,6 +43,7 @@ class Acl extends RelationalEntity { protected $permissionShare = false; protected $permissionManage = false; protected $owner = false; + protected $retainsAccessViaMembership = false; protected $token = null; public function __construct() { @@ -51,8 +54,10 @@ public function __construct() { $this->addType('permissionManage', 'boolean'); $this->addType('type', 'integer'); $this->addType('owner', 'boolean'); + $this->addType('retainsAccessViaMembership', 'boolean'); $this->addType('token', 'string'); $this->addRelation('owner'); + $this->addRelation('retainsAccessViaMembership'); $this->addResolvable('participant'); } diff --git a/lib/Db/Board.php b/lib/Db/Board.php index a0663dbd3d..ee84880fae 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) @@ -30,8 +32,11 @@ * @method int | null getExternalId() */ class Board extends RelationalEntity { + public const PERMISSION_OWNER = 4; + protected $title; protected $owner; + protected $ownerType = 0; protected $color; protected $archived = false; /** @var Label[]|null */ @@ -53,6 +58,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..d912cb2b51 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')) @@ -276,11 +295,12 @@ public function findAllByUser(string $userId, ?int $limit = null, ?int $offset = return $entries; } - public function findAllByOwner(string $userId, ?int $limit = null, ?int $offset = null) { + public function findAllByOwner(string $ownerId, int $ownerType = Acl::PERMISSION_TYPE_USER, ?int $limit = null, ?int $offset = null): array { $qb = $this->db->getQueryBuilder(); $qb->select('*') ->from('deck_boards') - ->where($qb->expr()->eq('owner', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR))) + ->where($qb->expr()->eq('owner', $qb->createNamedParameter($ownerId, IQueryBuilder::PARAM_STR))) + ->andWhere($qb->expr()->eq('owner_type', $qb->createNamedParameter($ownerType, IQueryBuilder::PARAM_INT))) ->orderBy('id'); if ($limit !== null) { $qb->setMaxResults($limit); @@ -300,7 +320,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 +376,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 +424,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 +508,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 +534,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 +614,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 +650,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/Listeners/ParticipantCleanupListener.php b/lib/Listeners/ParticipantCleanupListener.php index 3ba4e27c78..1cc837040e 100644 --- a/lib/Listeners/ParticipantCleanupListener.php +++ b/lib/Listeners/ParticipantCleanupListener.php @@ -30,7 +30,7 @@ public function __construct(AclMapper $aclMapper, AssignmentMapper $assignmentMa public function handle(Event $event): void { if ($event instanceof UserDeletedEvent) { - $boards = $this->boardMapper->findAllByOwner($event->getUser()->getUID()); + $boards = $this->boardMapper->findAllByOwner($event->getUser()->getUID(), Acl::PERMISSION_TYPE_USER); foreach ($boards as $board) { $this->boardMapper->delete($board); } @@ -44,6 +44,11 @@ public function handle(Event $event): void { if ($event instanceof CircleDestroyedEvent) { $circleId = $event->getCircle()->getSingleId(); + $boards = $this->boardMapper->findAllByOwner($circleId, Acl::PERMISSION_TYPE_CIRCLE); + foreach ($boards as $board) { + $this->boardMapper->delete($board); + } + $this->cleanupByParticipant(Acl::PERMISSION_TYPE_CIRCLE, $circleId); } } 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..07c23d8c0c 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; @@ -19,6 +20,7 @@ use OCA\Deck\Db\Card; use OCA\Deck\Db\CardMapper; use OCA\Deck\Db\User; +use OCA\Deck\Service\CirclesService; use OCA\Deck\Service\ConfigService; use OCA\Deck\Service\PermissionService; use OCP\AppFramework\Db\DoesNotExistException; @@ -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..3e43faaa1d 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..40912e13fb 100644 --- a/lib/Service/BoardService.php +++ b/lib/Service/BoardService.php @@ -8,6 +8,7 @@ namespace OCA\Deck\Service; use OC\User\LazyUser; +use OCA\Circles\Model\Member; use OCA\Deck\Activity\ActivityManager; use OCA\Deck\Activity\ChangeSet; use OCA\Deck\AppInfo\Application; @@ -82,6 +83,7 @@ public function __construct( private IUserManager $userManager, private ISecureRandom $random, private ConfigService $configService, + private CirclesService $circlesService, private ?string $userId, ) { } @@ -221,7 +223,8 @@ public function create(string $title, string $userId, string $color): Board { 'PERMISSION_READ' => $permissions[Acl::PERMISSION_READ] ?? false, 'PERMISSION_EDIT' => $permissions[Acl::PERMISSION_EDIT] ?? false, 'PERMISSION_MANAGE' => $permissions[Acl::PERMISSION_MANAGE] ?? false, - 'PERMISSION_SHARE' => $permissions[Acl::PERMISSION_SHARE] ?? false + 'PERMISSION_SHARE' => $permissions[Acl::PERMISSION_SHARE] ?? false, + 'PERMISSION_OWNER' => $permissions[Board::PERMISSION_OWNER] ?? false, ]); $this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_BOARD, $board, ActivityManager::SUBJECT_BOARD_CREATE, [], $userId); $this->changeHelper->boardChanged($board->getId()); @@ -410,6 +413,7 @@ public function addAcl(int $boardId, int $type, $participant, bool $edit, bool $ $board = $this->boardMapper->find($boardId); $this->clearBoardFromCache($board); + $this->invalidateAclCaches($boardId, $type, (string)$participant); // TODO: use the dispatched event for this try { @@ -472,6 +476,7 @@ public function updateAcl(int $id, bool $edit, bool $share, bool $manage): Acl { } $this->eventDispatcher->dispatchTyped(new AclUpdatedEvent($acl)); + $this->invalidateAclCaches($acl->getBoardId(), $acl->getType(), (string)$acl->getParticipant()); return $acl; } @@ -508,6 +513,7 @@ public function deleteAcl(int $id): ?Acl { $deletedAcl = $this->aclMapper->delete($acl); $this->eventDispatcher->dispatchTyped(new AclDeletedEvent($acl)); + $this->invalidateAclCaches($acl->getBoardId(), $acl->getType(), (string)$acl->getParticipant()); return $deletedAcl; } @@ -572,7 +578,8 @@ public function clone( 'PERMISSION_READ' => $permissions[Acl::PERMISSION_READ] ?? false, 'PERMISSION_EDIT' => $permissions[Acl::PERMISSION_EDIT] ?? false, 'PERMISSION_MANAGE' => $permissions[Acl::PERMISSION_MANAGE] ?? false, - 'PERMISSION_SHARE' => $permissions[Acl::PERMISSION_SHARE] ?? false + 'PERMISSION_SHARE' => $permissions[Acl::PERMISSION_SHARE] ?? false, + 'PERMISSION_OWNER' => $permissions[Board::PERMISSION_OWNER] ?? false, ]); $this->boardMapper->insert($newBoard); @@ -615,14 +622,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 +647,28 @@ 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); + + // When transferring ownership to a circle, re-add the circle as an explicit ACL + // sharee with full rights so it remains visible in the sharing sidebar. + // (It was removed above to prevent a duplicate DB entry.) + if ($newOwnerType === Acl::PERMISSION_TYPE_CIRCLE) { + try { + $this->addAcl($boardId, Acl::PERMISSION_TYPE_CIRCLE, $newOwner, true, true, true); + } catch (DbException $e) { + if ($e->getReason() !== DbException::REASON_UNIQUE_CONSTRAINT_VIOLATION) { + throw $e; + } + } + } + + // 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 +677,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, int $ownerType = 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, $ownerType); 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); } } @@ -682,6 +730,7 @@ private function enrichBoards(array $boards, bool $fullDetails = true): array { foreach ($board->getAcl() as &$acl) { $this->boardMapper->mapAcl($acl); } + $this->annotateAclRetainedAccess($board); } $permissions = $this->permissionService->matchPermissions($board); @@ -689,7 +738,8 @@ private function enrichBoards(array $boards, bool $fullDetails = true): array { 'PERMISSION_READ' => $permissions[Acl::PERMISSION_READ] ?? false, 'PERMISSION_EDIT' => $permissions[Acl::PERMISSION_EDIT] ?? false, 'PERMISSION_MANAGE' => $permissions[Acl::PERMISSION_MANAGE] ?? false, - 'PERMISSION_SHARE' => $permissions[Acl::PERMISSION_SHARE] ?? false + 'PERMISSION_SHARE' => $permissions[Acl::PERMISSION_SHARE] ?? false, + 'PERMISSION_OWNER' => $permissions[Board::PERMISSION_OWNER] ?? false, ]); if ($fullDetails) { @@ -711,6 +761,55 @@ private function enrichBoards(array $boards, bool $fullDetails = true): array { return $boards; } + private function annotateAclRetainedAccess(Board $board): void { + $acls = $board->getAcl() ?? []; + + // For circle-owned boards fetch members once instead of N isUserInCircle() calls + $circleMemberIds = null; + if ($board->getOwnerType() === Acl::PERMISSION_TYPE_CIRCLE + && $this->circlesService->isCirclesEnabled() + ) { + $circle = $this->circlesService->getCircle($board->getOwner()); + if ($circle !== null) { + $circleMemberIds = []; + foreach ($circle->getInheritedMembers() as $member) { + if ($member->getUserType() === Member::TYPE_USER + && $member->getLevel() >= Member::LEVEL_MEMBER + ) { + $circleMemberIds[$member->getUserId()] = true; + } + } + } + } + + foreach ($acls as $acl) { + if ($acl->getType() !== Acl::PERMISSION_TYPE_USER) { + $acl->setRetainsAccessViaMembership(false); + continue; + } + + $participant = (string)$acl->getParticipant(); + if ($participant === '') { + $acl->setRetainsAccessViaMembership(false); + continue; + } + + if ($board->getOwnerType() === Acl::PERMISSION_TYPE_USER && $board->getOwner() === $participant) { + $acl->setRetainsAccessViaMembership(true); + continue; + } + + // Circle-owned board: check prebuilt membership set (1 Circles call total) + if ($circleMemberIds !== null) { + $acl->setRetainsAccessViaMembership(isset($circleMemberIds[$participant])); + continue; + } + + $otherAcls = array_filter($acls, static fn (Acl $candidate): bool => $candidate->getId() !== $acl->getId()); + $acl->setRetainsAccessViaMembership($this->permissionService->userCan($otherAcls, Acl::PERMISSION_READ, $participant)); + } + } + private function cloneCards(Board $board, Board $newBoard, bool $withAssignments = false, bool $withLabels = false, bool $withDueDate = false, bool $moveCardsToLeftStack = false, bool $restoreArchivedCards = false): void { $stacks = $this->stackMapper->findAll($board->getId()); $newStacks = $this->stackMapper->findAll($newBoard->getId()); @@ -829,6 +928,14 @@ private function clearBoardFromCache(Board $board): void { unset($this->boardsCachePartial[$boardId]); } + private function invalidateAclCaches(int $boardId, int $aclType, string $participant): void { + $this->permissionService->clearUsersCache($boardId); + + if ($aclType === Acl::PERMISSION_TYPE_CIRCLE && $participant !== '') { + $this->circlesService->clearUserCircleCache($participant); + } + } + private function enrichWithCards(Board $board): void { $stacks = $this->stackMapper->findAll($board->getId()); if (\count($stacks) === 0) { diff --git a/lib/Service/CirclesService.php b/lib/Service/CirclesService.php index 96af9c155c..8883b0890d 100644 --- a/lib/Service/CirclesService.php +++ b/lib/Service/CirclesService.php @@ -17,6 +17,7 @@ use OCA\Circles\Model\Probes\DataProbe; use OCP\App\IAppManager; use OCP\Server; +use Psr\Log\LoggerInterface; use Throwable; /** @@ -27,8 +28,13 @@ class CirclesService { private bool $circlesEnabled; private $userCircleCache = []; + /** @var array */ + private array $userCirclesCache = []; - public function __construct(IAppManager $appManager) { + public function __construct( + IAppManager $appManager, + private ?LoggerInterface $logger = null, + ) { $this->circlesEnabled = $appManager->isEnabledForUser('circles'); } @@ -42,14 +48,21 @@ public function getCircle(string $circleId): ?Circle { } try { - // Enforce current user condition since we always want the full list of members - $circlesManager = Server::get(CirclesManager::class); + $circlesManager = $this->getCirclesManager(); $circlesManager->startSuperSession(); - $dataProbe = new DataProbe(); - $dataProbe->add(DataProbe::OWNER); - return $circlesManager->probeCircle($circleId, null, $dataProbe); + try { + $dataProbe = new DataProbe(); + $dataProbe->add(DataProbe::OWNER); + return $circlesManager->probeCircle($circleId, null, $dataProbe); + } finally { + $this->stopSessionSafely($circlesManager, 'getCircle'); + } } catch (Throwable $e) { + $this->logger?->debug('CirclesService::getCircle failed', [ + 'circleId' => $circleId, + 'exception' => $e, + ]); } return null; } @@ -64,14 +77,18 @@ public function isUserInCircle(string $circleId, string $userId): bool { } try { - $circlesManager = Server::get(CirclesManager::class); + $circlesManager = $this->getCirclesManager(); $federatedUser = $circlesManager->getFederatedUser($userId, Member::TYPE_USER); $circlesManager->startSession($federatedUser); - $dataProbe = new DataProbe(); - $dataProbe->add(DataProbe::INITIATOR); - $circle = $circlesManager->probeCircle($circleId, null, $dataProbe); - $member = $circle->getInitiator(); - $isUserInCircle = $member->getLevel() >= Member::LEVEL_MEMBER; + try { + $dataProbe = new DataProbe(); + $dataProbe->add(DataProbe::INITIATOR); + $circle = $circlesManager->probeCircle($circleId, null, $dataProbe); + $member = $circle->getInitiator(); + $isUserInCircle = $member->getLevel() >= Member::LEVEL_MEMBER; + } finally { + $this->stopSessionSafely($circlesManager, 'isUserInCircle'); + } if (!isset($this->userCircleCache[$circleId])) { $this->userCircleCache[$circleId] = []; @@ -80,6 +97,11 @@ public function isUserInCircle(string $circleId, string $userId): bool { return $isUserInCircle; } catch (Throwable $e) { + $this->logger?->debug('CirclesService::isUserInCircle failed', [ + 'circleId' => $circleId, + 'userId' => $userId, + 'exception' => $e, + ]); } return false; } @@ -93,17 +115,93 @@ public function getUserCircles(string $userId): array { return []; } + if (isset($this->userCirclesCache[$userId])) { + return $this->userCirclesCache[$userId]; + } + try { - $circlesManager = Server::get(CirclesManager::class); + $circlesManager = $this->getCirclesManager(); $federatedUser = $circlesManager->getFederatedUser($userId, Member::TYPE_USER); $circlesManager->startSession($federatedUser); - $probe = new CircleProbe(); - $probe->mustBeMember(); - return array_map(function (Circle $circle) { - return $circle->getSingleId(); - }, $circlesManager->probeCircles($probe)); + try { + $probe = new CircleProbe(); + $probe->mustBeMember(); + $circles = array_map(function (Circle $circle) { + return $circle->getSingleId(); + }, $circlesManager->probeCircles($probe)); + } finally { + $this->stopSessionSafely($circlesManager, 'getUserCircles'); + } + $this->userCirclesCache[$userId] = $circles; + return $circles; } catch (Throwable $e) { + $this->logger?->debug('CirclesService::getUserCircles failed', [ + 'userId' => $userId, + 'exception' => $e, + ]); } return []; } + + public function clearUserCircleCache(?string $circleId = null, ?string $userId = null): void { + if ($circleId === null && $userId === null) { + $this->userCircleCache = []; + return; + } + + if ($circleId !== null && $userId === null) { + unset($this->userCircleCache[$circleId]); + return; + } + + /** @psalm-suppress RedundantCondition -- $userId !== null is implied by the prior guard but kept explicit for clarity */ + if ($circleId !== null && $userId !== null) { + unset($this->userCircleCache[$circleId][$userId]); + if (empty($this->userCircleCache[$circleId])) { + unset($this->userCircleCache[$circleId]); + } + return; + } + + foreach ($this->userCircleCache as $cachedCircleId => $users) { + unset($users[$userId]); + if (empty($users)) { + unset($this->userCircleCache[$cachedCircleId]); + continue; + } + $this->userCircleCache[$cachedCircleId] = $users; + } + } + + public function clearUserCirclesCache(?string $userId = null): void { + if ($userId === null) { + $this->userCirclesCache = []; + return; + } + + unset($this->userCirclesCache[$userId]); + } + + public function clearAllCaches(): void { + $this->clearUserCircleCache(); + $this->clearUserCirclesCache(); + } + + protected function getCirclesManager(): CirclesManager { + return Server::get(CirclesManager::class); + } + + private function stopSessionSafely(CirclesManager $circlesManager, string $method): void { + if (!method_exists($circlesManager, 'stopSession')) { + return; + } + + try { + $circlesManager->stopSession(); + } catch (Throwable $e) { + $this->logger?->debug('CirclesService::' . $method . ' stopSession failed', [ + 'exception' => $e, + ]); + } + } } diff --git a/lib/Service/PermissionService.php b/lib/Service/PermissionService.php index 4de5327367..f6ca56cc1c 100644 --- a/lib/Service/PermissionService.php +++ b/lib/Service/PermissionService.php @@ -116,7 +116,7 @@ public function getPermissions(int $boardId, ?string $userId = null, bool $allow * Get current user permissions for a board * * @param Board $board - * @return array|bool + * @return array * @internal param $boardId */ public function matchPermissions(Board $board) { @@ -135,7 +135,8 @@ public function matchPermissions(Board $board) { Acl::PERMISSION_EDIT => $owner || $this->userCan($acls, Acl::PERMISSION_EDIT), Acl::PERMISSION_MANAGE => $owner || $this->userCan($acls, Acl::PERMISSION_MANAGE), Acl::PERMISSION_SHARE => ($owner || $this->userCan($acls, Acl::PERMISSION_SHARE)) - && (!$this->shareManager->sharingDisabledForUser($this->userId)) + && (!$this->shareManager->sharingDisabledForUser($this->userId)), + Board::PERMISSION_OWNER => $owner, ]; } @@ -180,12 +181,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 +287,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 +350,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; } @@ -377,4 +405,13 @@ public function setUserId(string $userId): void { $this->userId = $userId; $this->permissionCache->clear(); } + + public function clearUsersCache(?int $boardId = null): void { + if ($boardId === null) { + $this->users = []; + return; + } + + unset($this->users[(string)$boardId]); + } } diff --git a/src/components/board/SharingTabSidebar.vue b/src/components/board/SharingTabSidebar.vue index 035b1f363b..963bd02aaf 100644 --- a/src/components/board/SharingTabSidebar.vue +++ b/src/components/board/SharingTabSidebar.vue @@ -12,13 +12,24 @@
  • - + +
    {{ board.owner.displayname }} - + {{ t('deck', 'Board owner') }} + + {{ t('deck', 'Team') }} + + + + {{ t('deck', 'Transfer ownership to myself') }} + +
  • @@ -51,17 +62,22 @@ @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') }} + - {{ t('deck', 'Delete') }} + {{ acl.retainsAccessViaMembership + ? t('deck', 'Remove extra permissions') + : t('deck', 'Delete') }}
    • @@ -85,14 +101,7 @@ import { getCurrentUser } from '@nextcloud/auth' import { showError, showSuccess } from '@nextcloud/dialogs' import { loadState } from '@nextcloud/initial-state' import debounce from 'lodash/debounce.js' -const SOURCE_TO_SHARE_TYPE = { - users: 0, - groups: 1, - emails: 4, - remotes: 6, - circles: 7, - teams: 7, -} +import { PERMISSION_TYPE_CIRCLE, PERMISSION_TYPE_USER, SOURCE_TO_SHARE_TYPE } from '../../helpers/constants.js' export default { name: 'SharingTabSidebar', @@ -119,6 +128,9 @@ export default { addAclForAPI: null, newOwner: null, projectsEnabled: loadState('core', 'projects_enabled', false), + // Expose ACL type constants to the template + PERMISSION_TYPE_CIRCLE, + PERMISSION_TYPE_USER, } }, computed: { @@ -129,10 +141,29 @@ export default { 'canEdit', 'canManage', 'canShare', + 'isBoardOwner', ]), isCurrentUser() { return (uid) => uid === getCurrentUser().uid }, + currentUserUid() { + return 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: only actual board owners (team members) can transfer + return this.isBoardOwner + } + }, formatedSharees() { const result = this.unallocatedSharees.map(item => { const res = { @@ -140,7 +171,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 +255,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 +275,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..9d20ce79ce 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' @@ -126,6 +127,9 @@ export default function storeFactory() { canShare: state => { return state.currentBoard ? state.currentBoard.permissions.PERMISSION_SHARE : false }, + isBoardOwner: state => { + return state.currentBoard ? state.currentBoard.permissions.PERMISSION_OWNER : false + }, isArchived: state => { return state.currentBoard && state.currentBoard.archived }, @@ -524,9 +528,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/bootstrap-unit.php b/tests/bootstrap-unit.php new file mode 100644 index 0000000000..adefd227cc --- /dev/null +++ b/tests/bootstrap-unit.php @@ -0,0 +1,16 @@ + $value) { @@ -279,7 +279,7 @@ public function assertDatabase(string $owner = 'admin') { $stackMapper = self::getFreshService(StackMapper::class); $cardMapper = self::getFreshService(CardMapper::class); - $boards = $boardMapper->findAllByOwner($owner); + $boards = $boardMapper->findAllByOwner($owner, Acl::PERMISSION_TYPE_USER); $boardNames = array_map(fn ($board) => $board->getTitle(), $boards); self::assertEquals(2, count($boards)); diff --git a/tests/phpunit.xml b/tests/phpunit.xml index af2ea49bd7..ecc9c93273 100644 --- a/tests/phpunit.xml +++ b/tests/phpunit.xml @@ -1,5 +1,5 @@ - + ./../lib diff --git a/tests/stub.phpstub b/tests/stub.phpstub index 8ca73d9dd8..163613fb77 100644 --- a/tests/stub.phpstub +++ b/tests/stub.phpstub @@ -72,7 +72,8 @@ namespace OCA\Circles\Model { public const APP_DEFAULT = 11000; public function getLevel(): int {} - public function getUserType(): int{} + public function getUserType(): int {} + public function getUserId(): string {} } class Circle { diff --git a/tests/stubs.php b/tests/stubs.php new file mode 100644 index 0000000000..3981517d32 --- /dev/null +++ b/tests/stubs.php @@ -0,0 +1,117 @@ +activityManager->getActivityFormat('cz', $value, [], false); if ($format !== '') { $this->assertStringContainsString('{user}', $format); - } else { - self::addWarning('No activity string found for ' . $constant); } $format = $this->activityManager->getActivityFormat('cz', $value, [], true); if ($format !== '') { $this->assertStringStartsWith('You', $format); - } else { - self::addWarning('No own activity string found for ' . $constant); } } } @@ -325,10 +321,6 @@ public function testSendToUser($objectType) { ->willReturn(1); $event->expects(self::exactly(2)) ->method('setAffectedUser') - ->withConsecutive( - ['user1'], - ['user2'], - ) ->willReturnSelf(); $mapper = null; diff --git a/tests/unit/Command/BoardImportTest.php b/tests/unit/Command/BoardImportTest.php index db63263083..3113cc16b3 100644 --- a/tests/unit/Command/BoardImportTest.php +++ b/tests/unit/Command/BoardImportTest.php @@ -54,14 +54,11 @@ public function testExecuteWithSuccess() { $input = $this->createMock(InputInterface::class); $input ->method('getOption') - ->withConsecutive( - ['system'], - ['config'] - ) - ->will($this->returnValueMap([ - ['system', 'trelloJson'], - ['config', null] - ])); + ->willReturnCallback(fn ($key) => match($key) { + 'system' => 'trelloJson', + 'config' => null, + default => null, + }); $output = $this->createMock(OutputInterface::class); diff --git a/tests/unit/Cron/DeleteCronTest.php b/tests/unit/Cron/DeleteCronTest.php index 73ab3a416a..1b401f4349 100644 --- a/tests/unit/Cron/DeleteCronTest.php +++ b/tests/unit/Cron/DeleteCronTest.php @@ -108,13 +108,7 @@ public function testDeleteCron() { ->method('findToDelete') ->willReturn($boards); $this->boardMapper->expects($this->exactly(count($boards))) - ->method('delete') - ->withConsecutive( - [$boards[0]], - [$boards[1]], - [$boards[2]], - [$boards[3]] - ); + ->method('delete'); $cards = [ $this->getCard(10) ]; $this->cardMapper->expects($this->once()) @@ -150,11 +144,7 @@ public function testDeleteCron() { ->method('findToDelete') ->willReturn($stacks); $this->stackMapper->expects($this->exactly(count($stacks))) - ->method('delete') - ->withConsecutive( - [$stacks[0]], - [$stacks[1]] - ); + ->method('delete'); $this->invokePrivate($this->deleteCron, 'run', [null]); } diff --git a/tests/unit/Db/AclTest.php b/tests/unit/Db/AclTest.php index 06953b9838..a89f57ef5c 100644 --- a/tests/unit/Db/AclTest.php +++ b/tests/unit/Db/AclTest.php @@ -59,7 +59,8 @@ public function testJsonSerialize() { 'permissionEdit' => true, 'permissionShare' => true, 'permissionManage' => true, - 'owner' => false + 'owner' => false, + 'retainsAccessViaMembership' => false, ], $acl->jsonSerialize()); $acl = $this->createAclGroup(); $this->assertEquals([ @@ -70,7 +71,8 @@ public function testJsonSerialize() { 'permissionEdit' => true, 'permissionShare' => true, 'permissionManage' => true, - 'owner' => false + 'owner' => false, + 'retainsAccessViaMembership' => false, ], $acl->jsonSerialize()); } @@ -85,7 +87,8 @@ public function testSetOwner() { 'permissionEdit' => true, 'permissionShare' => true, 'permissionManage' => true, - 'owner' => true + 'owner' => true, + 'retainsAccessViaMembership' => false, ], $acl->jsonSerialize()); } 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/Listeners/ParticipantCleanupListenerTest.php b/tests/unit/Listeners/ParticipantCleanupListenerTest.php new file mode 100644 index 0000000000..1b6fd70d55 --- /dev/null +++ b/tests/unit/Listeners/ParticipantCleanupListenerTest.php @@ -0,0 +1,78 @@ +aclMapper = $this->createMock(AclMapper::class); + $this->assignmentMapper = $this->createMock(AssignmentMapper::class); + $this->boardMapper = $this->createMock(BoardMapper::class); + $this->listener = new ParticipantCleanupListener($this->aclMapper, $this->assignmentMapper, $this->boardMapper); + } + + public function testCircleDestroyedDeletesCircleOwnedBoardsAndCleansParticipantData(): void { + $circleId = 'circle-123'; + + $circle = $this->createMock(Circle::class); + $circle->expects($this->once()) + ->method('getSingleId') + ->willReturn($circleId); + + $event = $this->createMock(CircleDestroyedEvent::class); + $event->expects($this->once()) + ->method('getCircle') + ->willReturn($circle); + + $boardOne = new Board(); + $boardTwo = new Board(); + $this->boardMapper->expects($this->once()) + ->method('findAllByOwner') + ->with($circleId, Acl::PERMISSION_TYPE_CIRCLE) + ->willReturn([$boardOne, $boardTwo]); + $this->boardMapper->expects($this->exactly(2)) + ->method('delete'); + + $acl = new Acl(); + $this->aclMapper->expects($this->once()) + ->method('findByParticipant') + ->with(Acl::PERMISSION_TYPE_CIRCLE, $circleId) + ->willReturn([$acl]); + $this->aclMapper->expects($this->once()) + ->method('delete') + ->with($acl); + + $assignment = new Assignment(); + $this->assignmentMapper->expects($this->once()) + ->method('findByParticipant') + ->with($circleId, Acl::PERMISSION_TYPE_CIRCLE) + ->willReturn([$assignment]); + $this->assignmentMapper->expects($this->once()) + ->method('delete') + ->with($assignment); + + $this->listener->handle($event); + } +} diff --git a/tests/unit/Notification/NotificationHelperTest.php b/tests/unit/Notification/NotificationHelperTest.php index 36cb9bb55e..427427debf 100644 --- a/tests/unit/Notification/NotificationHelperTest.php +++ b/tests/unit/Notification/NotificationHelperTest.php @@ -31,6 +31,7 @@ use OCA\Deck\Db\Card; use OCA\Deck\Db\CardMapper; use OCA\Deck\Db\User; +use OCA\Deck\Service\CirclesService; use OCA\Deck\Service\ConfigService; use OCA\Deck\Service\PermissionService; use OCP\Comments\IComment; @@ -65,6 +66,8 @@ class NotificationHelperTest extends \Test\TestCase { protected $assignedUsersMapper; /** @var PermissionService|MockObject */ protected $permissionService; + /** @var CirclesService|MockObject */ + protected $circlesService; /** @var IConfig|MockObject */ protected $config; /** @var IManager|MockObject */ @@ -82,6 +85,7 @@ public function setUp(): void { $this->boardMapper = $this->createMock(BoardMapper::class); $this->assignedUsersMapper = $this->createMock(AssignmentMapper::class); $this->permissionService = $this->createMock(PermissionService::class); + $this->circlesService = $this->createMock(CirclesService::class); $this->config = $this->createMock(IConfig::class); $this->notificationManager = $this->createMock(IManager::class); $this->groupManager = $this->createMock(IGroupManager::class); @@ -91,6 +95,7 @@ public function setUp(): void { $this->boardMapper, $this->assignedUsersMapper, $this->permissionService, + $this->circlesService, $this->config, $this->notificationManager, $this->groupManager, @@ -123,11 +128,6 @@ public function testSendCardDuedate() { $this->config->expects($this->exactly(3)) ->method('getUserValue') - ->withConsecutive( - [$param1[0], $param2, $param3, $DUE_ASSIGNED], - [$param1[1], $param2, $param3, $DUE_ASSIGNED], - [$param1[2], $param2, $param3, $DUE_ASSIGNED], - ) ->willReturn(ConfigService::SETTING_BOARD_NOTIFICATION_DUE_ALL); $card = Card::fromParams([ @@ -187,8 +187,7 @@ public function testSendCardDuedate() { ->method('createNotification') ->willReturnOnConsecutiveCalls($n1, $n2, $n3); $this->notificationManager->expects($this->exactly(3)) - ->method('notify') - ->withConsecutive([$n1], [$n2], [$n3]); + ->method('notify'); $this->cardMapper->expects($this->once()) ->method('markNotified') @@ -205,11 +204,6 @@ public function testSendCardDuedateAssigned() { $this->config->expects($this->exactly(3)) ->method('getUserValue') - ->withConsecutive( - [$param1[0], $param2, $param3, $DUE_ASSIGNED], - [$param1[1], $param2, $param3, $DUE_ASSIGNED], - [$param1[2], $param2, $param3, $DUE_ASSIGNED] - ) ->willReturn($DUE_ASSIGNED); $users = [ @@ -275,8 +269,7 @@ public function testSendCardDuedateAssigned() { ->method('createNotification') ->willReturnOnConsecutiveCalls($n1, $n2, $n3); $this->notificationManager->expects($this->exactly(3)) - ->method('notify') - ->withConsecutive([$n1], [$n2], [$n3]); + ->method('notify'); $this->cardMapper->expects($this->once()) ->method('markNotified') @@ -293,14 +286,13 @@ public function testSendCardDuedateNever() { $DUE_ASSIGNED = ConfigService::SETTING_BOARD_NOTIFICATION_DUE_ASSIGNED; $DUE_OFF = ConfigService::SETTING_BOARD_NOTIFICATION_DUE_OFF; + $dueCalls = [$DUE_ASSIGNED, $DUE_ASSIGNED, $DUE_OFF]; + $dueIndex = 0; $this->config->expects($this->exactly(3)) ->method('getUserValue') - ->withConsecutive( - [$param1[0], $param2, $param3, $DUE_ASSIGNED], - [$param1[1], $param2, $param3, $DUE_ASSIGNED], - [$param1[2], $param2, $param3, $DUE_ASSIGNED] - ) - ->willReturnOnConsecutiveCalls($DUE_ASSIGNED, $DUE_ASSIGNED, $DUE_OFF); + ->willReturnCallback(function () use (&$dueIndex, $dueCalls) { + return $dueCalls[$dueIndex++]; + }); $users = [ new DummyUser('foo'), new DummyUser('bar'), new DummyUser('asd') @@ -358,8 +350,7 @@ public function testSendCardDuedateNever() { ->method('createNotification') ->willReturnOnConsecutiveCalls($n1, $n2); $this->notificationManager->expects($this->exactly(2)) - ->method('notify') - ->withConsecutive([$n1], [$n2]); + ->method('notify'); $this->cardMapper->expects($this->once()) ->method('markNotified') @@ -522,8 +513,7 @@ public function testSendMention() { ->method('createNotification') ->willReturnOnConsecutiveCalls($notification1, $notification2); $this->notificationManager->expects($this->exactly(2)) - ->method('notify') - ->withConsecutive([$notification1], [$notification2]); + ->method('notify'); $this->notificationHelper->sendMention($comment); } diff --git a/tests/unit/Service/AttachmentServiceTest.php b/tests/unit/Service/AttachmentServiceTest.php index 638819073b..1de9949498 100644 --- a/tests/unit/Service/AttachmentServiceTest.php +++ b/tests/unit/Service/AttachmentServiceTest.php @@ -119,14 +119,11 @@ public function setUp(): void { $this->appContainer->expects($this->exactly(2)) ->method('get') - ->withConsecutive( - [FileService::class], - [FilesAppService::class] - ) - ->willReturnOnConsecutiveCalls( - $this->attachmentServiceImpl, - $this->filesAppServiceImpl - ); + ->willReturnCallback(fn ($class) => match($class) { + FileService::class => $this->attachmentServiceImpl, + FilesAppService::class => $this->filesAppServiceImpl, + default => null, + }); $this->application->expects($this->any()) ->method('getContainer') @@ -157,18 +154,15 @@ public function testRegisterAttachmentService() { $fileServiceMock = $this->createMock(FileService::class); $fileAppServiceMock = $this->createMock(FilesAppService::class); + $myAttachmentService = new MyAttachmentService(); $appContainer->expects($this->exactly(3)) ->method('get') - ->withConsecutive( - [FileService::class], - [FilesAppService::class], - [MyAttachmentService::class] - ) - ->willReturnOnConsecutiveCalls( - $fileServiceMock, - $fileAppServiceMock, - new MyAttachmentService() - ); + ->willReturnCallback(fn ($class) => match($class) { + FileService::class => $fileServiceMock, + FilesAppService::class => $fileAppServiceMock, + MyAttachmentService::class => $myAttachmentService, + default => null, + }); $application->expects($this->any()) ->method('getContainer') @@ -186,18 +180,15 @@ public function testRegisterAttachmentServiceNotExisting() { $fileServiceMock = $this->createMock(FileService::class); $fileAppServiceMock = $this->createMock(FilesAppService::class); + $myAttachmentService2 = new MyAttachmentService(); $appContainer->expects($this->exactly(3)) ->method('get') - ->withConsecutive( - [FileService::class], - [FilesAppService::class], - [MyAttachmentService::class] - ) - ->willReturnOnConsecutiveCalls( - $fileServiceMock, - $fileAppServiceMock, - new MyAttachmentService() - ); + ->willReturnCallback(fn ($class) => match($class) { + FileService::class => $fileServiceMock, + FilesAppService::class => $fileAppServiceMock, + MyAttachmentService::class => $myAttachmentService2, + default => null, + }); $application->expects($this->any()) ->method('getContainer') @@ -236,14 +227,7 @@ public function testFindAll() { $this->attachmentServiceImpl->expects($this->exactly(2)) ->method('extendData') - ->withConsecutive( - [$attachments[0]], - [$attachments[1]], - ) - ->willReturnOnConsecutiveCalls( - $attachments[0], - $attachments[1], - ); + ->willReturnCallback(fn ($a) => $a); $this->assertEquals($attachments, $this->attachmentService->findAll(123, false)); } @@ -271,12 +255,7 @@ public function testFindAllWithDeleted() { $this->attachmentServiceImpl->expects($this->exactly(4)) ->method('extendData') - ->withConsecutive( - [$attachments[0]], - [$attachments[1]], - [$attachmentsDeleted[0]], - [$attachmentsDeleted[1]] - ); + ->willReturnArgument(0); $this->assertEquals(array_merge($attachments, $attachmentsDeleted), $this->attachmentService->findAll(123, true)); } diff --git a/tests/unit/Service/BoardServiceTest.php b/tests/unit/Service/BoardServiceTest.php index 549021bf0c..0799807d98 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 ); @@ -308,11 +313,43 @@ public function testAddAcl() { ->willReturn([ 'admin' => 'admin', ]); + $this->permissionService->expects($this->once()) + ->method('clearUsersCache') + ->with(123); + $this->circlesService->expects($this->never()) + ->method('clearUserCircleCache'); $this->assertEquals($acl, $this->service->addAcl( 123, Acl::PERMISSION_TYPE_USER, 'admin', true, true, true )); } + public function testAddCircleAclClearsCircleMembershipCache(): void { + $acl = new Acl(); + $acl->setBoardId(123); + $acl->setType(Acl::PERMISSION_TYPE_CIRCLE); + $acl->setParticipant('circle-1'); + $acl->setPermissionEdit(true); + $acl->setPermissionShare(true); + $acl->setPermissionManage(true); + + $this->notificationHelper->expects($this->once()) + ->method('sendBoardShared'); + $this->aclMapper->expects($this->once()) + ->method('insert') + ->with($acl) + ->willReturn($acl); + $this->permissionService->expects($this->once()) + ->method('clearUsersCache') + ->with(123); + $this->circlesService->expects($this->once()) + ->method('clearUserCircleCache') + ->with('circle-1'); + + $this->assertEquals($acl, $this->service->addAcl( + 123, Acl::PERMISSION_TYPE_CIRCLE, 'circle-1', true, true, true + )); + } + public static function dataAddAclExtendPermission() { return [ [[false, false, false], [false, false, false], [false, false, false]], @@ -354,27 +391,22 @@ public function testAddAclExtendPermission($currentUserAcl, $providedAcl, $resul if ($currentUserAcl[2]) { $this->permissionService->expects($this->exactly(2)) ->method('checkPermission') - ->withConsecutive( - [$this->boardMapper, 123, Acl::PERMISSION_SHARE, null], - [$this->boardMapper, 123, Acl::PERMISSION_MANAGE, null] - ); + ->willReturn(true); } else { $this->aclMapper->expects($this->once()) ->method('findAll') ->willReturn([$existingAcl]); + $callCount = 0; $this->permissionService->expects($this->exactly(2)) ->method('checkPermission') - ->withConsecutive( - [$this->boardMapper, 123, Acl::PERMISSION_SHARE, null], - [$this->boardMapper, 123, Acl::PERMISSION_MANAGE, null] - ) - ->will( - $this->onConsecutiveCalls( - true, - $this->throwException(new NoPermissionException('No permission')) - ) - ); + ->willReturnCallback(function ($mapper, $boardId, $permission) use (&$callCount) { + $callCount++; + if ($callCount === 2) { + throw new NoPermissionException('No permission'); + } + return true; + }); $this->permissionService->expects($this->exactly(3)) ->method('userCan') @@ -434,6 +466,9 @@ public function testUpdateAcl() { ->method('update') ->with($acl) ->willReturn($acl); + $this->permissionService->expects($this->once()) + ->method('clearUsersCache') + ->with(123); $result = $this->service->updateAcl( 123, false, false, false @@ -469,9 +504,421 @@ public function testDeleteAcl() { ->method('delete') ->with($acl) ->willReturn($acl); + $this->permissionService->expects($this->once()) + ->method('clearUsersCache') + ->with(123); $this->eventDispatcher->expects(self::once()) ->method('dispatchTyped') ->with(new AclDeletedEvent($acl)); $this->assertEquals($acl, $this->service->deleteAcl(123)); } + + public function testFindMarksUserAclAsRetainedViaOwnerCircleMembership(): void { + $board = new Board(); + $board->setId(10); + $board->setOwner('circle-1'); + $board->setOwnerType(Acl::PERMISSION_TYPE_CIRCLE); + + $userAcl = new Acl(); + $userAcl->setId(501); + $userAcl->setBoardId(10); + $userAcl->setType(Acl::PERMISSION_TYPE_USER); + $userAcl->setParticipant('alice'); + $board->setAcl([$userAcl]); + + $this->permissionService->expects($this->once()) + ->method('checkPermission') + ->with($this->boardMapper, 10, Acl::PERMISSION_READ, null, false); + + $this->boardMapper->expects($this->once()) + ->method('find') + ->with(10, true, true, false) + ->willReturn($board); + + $this->boardMapper->expects($this->once()) + ->method('mapOwner') + ->with($board); + + $this->boardMapper->expects($this->once()) + ->method('mapAcl') + ->with($userAcl); + + $member = $this->createMock(\OCA\Circles\Model\Member::class); + $member->expects($this->once()) + ->method('getUserType') + ->willReturn(\OCA\Circles\Model\Member::TYPE_USER); + $member->expects($this->once()) + ->method('getLevel') + ->willReturn(\OCA\Circles\Model\Member::LEVEL_MEMBER); + $member->expects($this->once()) + ->method('getUserId') + ->willReturn('alice'); + + $circle = $this->createMock(\OCA\Circles\Model\Circle::class); + $circle->expects($this->once()) + ->method('getInheritedMembers') + ->willReturn([$member]); + + $this->circlesService->expects($this->once()) + ->method('isCirclesEnabled') + ->willReturn(true); + $this->circlesService->expects($this->once()) + ->method('getCircle') + ->with('circle-1') + ->willReturn($circle); + + $this->permissionService->expects($this->never()) + ->method('userCan'); + + $this->permissionService->expects($this->once()) + ->method('matchPermissions') + ->with($board) + ->willReturn([ + Acl::PERMISSION_READ => true, + Acl::PERMISSION_EDIT => true, + Acl::PERMISSION_MANAGE => true, + Acl::PERMISSION_SHARE => true, + Board::PERMISSION_OWNER => true, + ]); + + $result = $this->service->find(10, false); + $this->assertTrue($result->getAcl()[0]->isRetainsAccessViaMembership()); + } + + public function testFindMarksUserAclAsRetainedViaOtherAclMembership(): void { + $board = new Board(); + $board->setId(11); + $board->setOwner('bob'); + $board->setOwnerType(Acl::PERMISSION_TYPE_USER); + + $userAcl = new Acl(); + $userAcl->setId(601); + $userAcl->setBoardId(11); + $userAcl->setType(Acl::PERMISSION_TYPE_USER); + $userAcl->setParticipant('alice'); + + $groupAcl = new Acl(); + $groupAcl->setId(602); + $groupAcl->setBoardId(11); + $groupAcl->setType(Acl::PERMISSION_TYPE_GROUP); + $groupAcl->setParticipant('devs'); + + $board->setAcl([$userAcl, $groupAcl]); + + $this->permissionService->expects($this->once()) + ->method('checkPermission') + ->with($this->boardMapper, 11, Acl::PERMISSION_READ, null, false); + + $this->boardMapper->expects($this->once()) + ->method('find') + ->with(11, true, true, false) + ->willReturn($board); + + $this->boardMapper->expects($this->once()) + ->method('mapOwner') + ->with($board); + + $this->boardMapper->expects($this->exactly(2)) + ->method('mapAcl') + ->willReturnCallback(fn ($acl) => match(true) { + $acl === $userAcl => null, + $acl === $groupAcl => null, + default => null, + }); + + $this->circlesService->expects($this->never()) + ->method('isUserInCircle'); + + $this->permissionService->expects($this->once()) + ->method('userCan') + ->with( + $this->callback(static function (array $otherAcls): bool { + $otherAcls = array_values($otherAcls); + return count($otherAcls) === 1 && $otherAcls[0]->getId() === 602; + }), + Acl::PERMISSION_READ, + 'alice' + ) + ->willReturn(true); + + $this->permissionService->expects($this->once()) + ->method('matchPermissions') + ->with($board) + ->willReturn([ + Acl::PERMISSION_READ => true, + Acl::PERMISSION_EDIT => true, + Acl::PERMISSION_MANAGE => true, + Acl::PERMISSION_SHARE => true, + Board::PERMISSION_OWNER => false, + ]); + + $result = $this->service->find(11, false); + $this->assertTrue($result->getAcl()[0]->isRetainsAccessViaMembership()); + $this->assertFalse($result->getAcl()[1]->isRetainsAccessViaMembership()); + } + + public function testFindMarksUserAclAsNotRetainedWithoutInheritedAccess(): void { + $board = new Board(); + $board->setId(12); + $board->setOwner('bob'); + $board->setOwnerType(Acl::PERMISSION_TYPE_USER); + + $userAcl = new Acl(); + $userAcl->setId(701); + $userAcl->setBoardId(12); + $userAcl->setType(Acl::PERMISSION_TYPE_USER); + $userAcl->setParticipant('alice'); + $board->setAcl([$userAcl]); + + $this->permissionService->expects($this->once()) + ->method('checkPermission') + ->with($this->boardMapper, 12, Acl::PERMISSION_READ, null, false); + + $this->boardMapper->expects($this->once()) + ->method('find') + ->with(12, true, true, false) + ->willReturn($board); + + $this->boardMapper->expects($this->once()) + ->method('mapOwner') + ->with($board); + + $this->boardMapper->expects($this->once()) + ->method('mapAcl') + ->with($userAcl); + + $this->circlesService->expects($this->never()) + ->method('isUserInCircle'); + + $this->permissionService->expects($this->once()) + ->method('userCan') + ->with([], Acl::PERMISSION_READ, 'alice') + ->willReturn(false); + + $this->permissionService->expects($this->once()) + ->method('matchPermissions') + ->with($board) + ->willReturn([ + Acl::PERMISSION_READ => true, + Acl::PERMISSION_EDIT => true, + Acl::PERMISSION_MANAGE => true, + Acl::PERMISSION_SHARE => true, + Board::PERMISSION_OWNER => false, + ]); + + $result = $this->service->find(12, false); + $this->assertFalse($result->getAcl()[0]->isRetainsAccessViaMembership()); + } + + 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(4)) + ->method('find') + ->willReturnOnConsecutiveCalls($board, $board, $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->method('findAll')->willReturn([]); + $this->aclMapper->expects($this->exactly(2)) + ->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 testFindMarksCircleAclAsRetainedViaMembership(): void { + $board = new Board(); + $board->setId(13); + $board->setOwner('bob'); + $board->setOwnerType(Acl::PERMISSION_TYPE_USER); + + $circleAcl = new Acl(); + $circleAcl->setId(801); + $circleAcl->setBoardId(13); + $circleAcl->setType(Acl::PERMISSION_TYPE_CIRCLE); + $circleAcl->setParticipant('circle-2'); + $board->setAcl([$circleAcl]); + + $this->permissionService->expects($this->once()) + ->method('checkPermission') + ->with($this->boardMapper, 13, Acl::PERMISSION_READ, null, false); + + $this->boardMapper->expects($this->once()) + ->method('find') + ->with(13, true, true, false) + ->willReturn($board); + + $this->boardMapper->expects($this->once()) + ->method('mapOwner') + ->with($board); + + $this->boardMapper->expects($this->once()) + ->method('mapAcl') + ->with($circleAcl); + + $this->circlesService->expects($this->never()) + ->method('isUserInCircle'); + + $this->permissionService->expects($this->never()) + ->method('userCan'); + + $this->permissionService->expects($this->once()) + ->method('matchPermissions') + ->with($board) + ->willReturn([ + Acl::PERMISSION_READ => true, + Acl::PERMISSION_EDIT => true, + Acl::PERMISSION_MANAGE => true, + Acl::PERMISSION_SHARE => true, + Board::PERMISSION_OWNER => false, + ]); + + $result = $this->service->find(13, false); + $this->assertFalse($result->getAcl()[0]->isRetainsAccessViaMembership()); + } + + public function testFindMarksRemoteAclAsNotRetainedViaMembership(): void { + $board = new Board(); + $board->setId(14); + $board->setOwner('bob'); + $board->setOwnerType(Acl::PERMISSION_TYPE_USER); + + $remoteAcl = new Acl(); + $remoteAcl->setId(901); + $remoteAcl->setBoardId(14); + $remoteAcl->setType(Acl::PERMISSION_TYPE_REMOTE); + $remoteAcl->setParticipant('https://remote.example'); + $board->setAcl([$remoteAcl]); + + $this->permissionService->expects($this->once()) + ->method('checkPermission') + ->with($this->boardMapper, 14, Acl::PERMISSION_READ, null, false); + + $this->boardMapper->expects($this->once()) + ->method('find') + ->with(14, true, true, false) + ->willReturn($board); + + $this->boardMapper->expects($this->once()) + ->method('mapOwner') + ->with($board); + + $this->boardMapper->expects($this->once()) + ->method('mapAcl') + ->with($remoteAcl); + + $this->circlesService->expects($this->never()) + ->method('isUserInCircle'); + + $this->permissionService->expects($this->never()) + ->method('userCan'); + + $this->permissionService->expects($this->once()) + ->method('matchPermissions') + ->with($board) + ->willReturn([ + Acl::PERMISSION_READ => true, + Acl::PERMISSION_EDIT => true, + Acl::PERMISSION_MANAGE => true, + Acl::PERMISSION_SHARE => true, + Board::PERMISSION_OWNER => false, + ]); + + $result = $this->service->find(14, false); + $this->assertFalse($result->getAcl()[0]->isRetainsAccessViaMembership()); + } + + 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 testTransferOwnershipUsesSourceOwnerTypeWhenFetchingBoards(): void { + $this->userManager->expects($this->once()) + ->method('userExists') + ->with('bob') + ->willReturn(true); + + $this->boardMapper->expects($this->once()) + ->method('findAllByOwner') + ->with('circle-1', Acl::PERMISSION_TYPE_CIRCLE) + ->willReturn([]); + + $result = iterator_to_array($this->service->transferOwnership('circle-1', 'bob', false, Acl::PERMISSION_TYPE_USER, Acl::PERMISSION_TYPE_CIRCLE)); + $this->assertSame([], $result); + } + + 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/CirclesServiceTest.php b/tests/unit/Service/CirclesServiceTest.php new file mode 100644 index 0000000000..ab34ed1f77 --- /dev/null +++ b/tests/unit/Service/CirclesServiceTest.php @@ -0,0 +1,127 @@ +appManager = $this->createMock(IAppManager::class); + $this->appManager->method('isEnabledForUser')->with('circles')->willReturn(true); + } + + public function testGetUserCirclesStopsSessionOnSuccess(): void { + $circle = $this->createMock(Circle::class); + $circle->expects($this->once()) + ->method('getSingleId') + ->willReturn('circle-1'); + + $federatedUser = $this->createMock(FederatedUser::class); + $manager = $this->getMockBuilder(CirclesManager::class) + ->disableOriginalConstructor() + ->onlyMethods(['getFederatedUser', 'startSession', 'probeCircles', 'stopSession']) + ->getMock(); + + $manager->expects($this->once()) + ->method('getFederatedUser') + ->with('alice', Member::TYPE_USER) + ->willReturn($federatedUser); + $manager->expects($this->once()) + ->method('startSession') + ->with($federatedUser); + $manager->expects($this->once()) + ->method('probeCircles') + ->with($this->isInstanceOf(CircleProbe::class)) + ->willReturn([$circle]); + $manager->expects($this->once()) + ->method('stopSession'); + + $service = $this->getMockBuilder(CirclesService::class) + ->setConstructorArgs([$this->appManager]) + ->onlyMethods(['getCirclesManager']) + ->getMock(); + $service->expects($this->once()) + ->method('getCirclesManager') + ->willReturn($manager); + + $result = $service->getUserCircles('alice'); + $this->assertSame(['circle-1'], $result); + } + + public function testGetUserCirclesStopsSessionOnFailure(): void { + $federatedUser = $this->createMock(FederatedUser::class); + $manager = $this->getMockBuilder(CirclesManager::class) + ->disableOriginalConstructor() + ->onlyMethods(['getFederatedUser', 'startSession', 'probeCircles', 'stopSession']) + ->getMock(); + + $manager->expects($this->once()) + ->method('getFederatedUser') + ->with('alice', Member::TYPE_USER) + ->willReturn($federatedUser); + $manager->expects($this->once()) + ->method('startSession') + ->with($federatedUser); + $manager->expects($this->once()) + ->method('probeCircles') + ->with($this->isInstanceOf(CircleProbe::class)) + ->willThrowException(new \RuntimeException('Boom')); + $manager->expects($this->once()) + ->method('stopSession'); + + $service = $this->getMockBuilder(CirclesService::class) + ->setConstructorArgs([$this->appManager]) + ->onlyMethods(['getCirclesManager']) + ->getMock(); + $service->expects($this->once()) + ->method('getCirclesManager') + ->willReturn($manager); + + $this->assertSame([], $service->getUserCircles('alice')); + } + + public function testGetCircleStopsSuperSession(): void { + $circle = $this->createMock(Circle::class); + $manager = $this->getMockBuilder(CirclesManager::class) + ->disableOriginalConstructor() + ->onlyMethods(['startSuperSession', 'probeCircle', 'stopSession']) + ->getMock(); + + $manager->expects($this->once()) + ->method('startSuperSession'); + $manager->expects($this->once()) + ->method('probeCircle') + ->with('circle-1', null, $this->isInstanceOf(DataProbe::class)) + ->willReturn($circle); + $manager->expects($this->once()) + ->method('stopSession'); + + $service = $this->getMockBuilder(CirclesService::class) + ->setConstructorArgs([$this->appManager]) + ->onlyMethods(['getCirclesManager']) + ->getMock(); + $service->expects($this->once()) + ->method('getCirclesManager') + ->willReturn($manager); + + $this->assertSame($circle, $service->getCircle('circle-1')); + } +} diff --git a/tests/unit/Service/DefaultBoardServiceTest.php b/tests/unit/Service/DefaultBoardServiceTest.php index 78011637c3..4a03be9984 100644 --- a/tests/unit/Service/DefaultBoardServiceTest.php +++ b/tests/unit/Service/DefaultBoardServiceTest.php @@ -152,13 +152,12 @@ public function testCreateDefaultBoard() { $this->stackService->expects($this->exactly(4)) ->method('create') - ->withConsecutive( - [$this->l10n->t('Custom lists - click to rename!'), $boardId, 0], - [$this->l10n->t('To Do'), $boardId, 1], - [$this->l10n->t('In Progress'), $boardId, 2], - [$this->l10n->t('Done'), $boardId, 3] - ) - ->willReturnOnConsecutiveCalls($stackCustom, $stackToDo, $stackDoing, $stackDone); + ->willReturnCallback(fn ($title, $bid, $order) => match($order) { + 0 => $stackCustom, + 1 => $stackToDo, + 2 => $stackDoing, + 3 => $stackDone, + }); $cardExampleTask1 = $this->assembleTestCard( '1. Open to learn more about boards and cards', @@ -186,22 +185,13 @@ public function testCreateDefaultBoard() { $this->userId ); + $cardResults = [$cardExampleTask1, $cardExampleTask2, $cardExampleTask3, $cardExampleTask4, $cardExampleTask5]; + $cardIndex = 0; $this->cardService->expects($this->exactly(5)) ->method('create') - ->withConsecutive( - ['1. Open to learn more about boards and cards', $stackCustomId, 'text', 0, $this->userId], - ['2. Drag cards left and right, up and down', $stackToDoId, 'text', 0, $this->userId], - ['Create your first card!', $stackToDoId, 'text', 1, $this->userId], - ['3. Apply rich formatting and link content', $stackDoingId, 'text', 0, $this->userId], - ['4. Share, comment and collaborate!', $stackDoneId, 'text', 0, $this->userId] - ) - ->willReturnonConsecutiveCalls( - $cardExampleTask1, - $cardExampleTask2, - $cardExampleTask3, - $cardExampleTask4, - $cardExampleTask5 - ); + ->willReturnCallback(function () use (&$cardIndex, $cardResults) { + return $cardResults[$cardIndex++]; + }); $result = $this->service->createDefaultBoard($title, $this->userId, $color); diff --git a/tests/unit/Service/Importer/BoardImportServiceTest.php b/tests/unit/Service/Importer/BoardImportServiceTest.php index 4cbaeb9fce..1730f4fc28 100644 --- a/tests/unit/Service/Importer/BoardImportServiceTest.php +++ b/tests/unit/Service/Importer/BoardImportServiceTest.php @@ -142,14 +142,11 @@ public function setUp(): void { ->willReturn('johndoe'); $this->userManager ->method('get') - ->withConsecutive( - ['admin'], - ['johndoe'] - ) - ->willReturnonConsecutiveCalls( - $owner, - $johndoe - ); + ->willReturnCallback(fn ($uid) => match($uid) { + 'admin' => $owner, + 'johndoe' => $johndoe, + default => null, + }); } public function testImportSuccess() { diff --git a/tests/unit/Service/PermissionServiceTest.php b/tests/unit/Service/PermissionServiceTest.php index 3099b9edd2..e15596216c 100644 --- a/tests/unit/Service/PermissionServiceTest.php +++ b/tests/unit/Service/PermissionServiceTest.php @@ -144,8 +144,10 @@ public function testUserIsBoardOwner() { $this->boardMapper->expects($this->exactly(2)) ->method('find') - ->withConsecutive([123], [234]) - ->willReturnOnConsecutiveCalls($adminBoard, $userBoard); + ->willReturnCallback(fn ($id) => match($id) { + 123 => $adminBoard, + 234 => $userBoard, + }); $this->assertEquals(true, $this->service->userIsBoardOwner(123)); $this->assertEquals(false, $this->service->userIsBoardOwner(234)); @@ -156,6 +158,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->once()) + ->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 @@ -347,11 +370,8 @@ public function testFindUsers() { $aclGroup->setType(Acl::PERMISSION_TYPE_GROUP); $aclGroup->setParticipant('group1'); - $board = $this->createMock(Board::class); - $board->expects($this->any()) - ->method('__call') - ->with('getOwner', []) - ->willReturn('user1'); + $board = new Board(); + $board->setOwner('user1'); $this->aclMapper->expects($this->once()) ->method('findAll') ->with(123) @@ -360,10 +380,13 @@ public function testFindUsers() { ->method('find') ->with(123) ->willReturn($board); - $this->userManager->expects($this->any()) + $this->userManager->expects($this->exactly(2)) ->method('userExists') - ->withConsecutive(['user1'], ['user2']) - ->willReturnOnConsecutiveCalls($user1, $user2); + ->willReturnCallback(fn ($uid) => match($uid) { + 'user1' => true, + 'user2' => true, + default => false, + }); $group = $this->createMock(IGroup::class); $group->expects($this->once()) diff --git a/tests/unit/controller/CardApiControllerTest.php b/tests/unit/controller/CardApiControllerTest.php index c466b9614f..1b25150ed4 100644 --- a/tests/unit/controller/CardApiControllerTest.php +++ b/tests/unit/controller/CardApiControllerTest.php @@ -107,12 +107,11 @@ public function testUpdate() { $this->request->expects($this->exactly(2)) ->method('getParam') - ->withConsecutive( - ['cardId'], - ['stackId'] - )->willReturnonConsecutiveCalls( - $this->cardExample['id'], - $this->stackExample['id']); + ->willReturnCallback(fn ($key) => match($key) { + 'cardId' => $this->cardExample['id'], + 'stackId' => $this->stackExample['id'], + default => null, + }); $this->cardService->expects($this->once()) ->method('update') diff --git a/tests/unit/controller/StackApiControllerTest.php b/tests/unit/controller/StackApiControllerTest.php index 226b691f85..c5edb8f1a1 100644 --- a/tests/unit/controller/StackApiControllerTest.php +++ b/tests/unit/controller/StackApiControllerTest.php @@ -129,11 +129,11 @@ public function testCreate() { public function testUpdate() { $this->request->expects($this->exactly(2)) ->method('getParam') - ->withConsecutive( - ['stackId'], - ['boardId'] - ) - ->willReturnonConsecutiveCalls($this->exampleStack['id'], $this->exampleBoard['boardId']); + ->willReturnCallback(fn ($key) => match($key) { + 'stackId' => $this->exampleStack['id'], + 'boardId' => $this->exampleBoard['boardId'], + default => null, + }); $stack = new Stack(); $stack->setId($this->exampleStack['id']);