diff --git a/Makefile b/Makefile index 54930c6b6..8d428a7d3 100644 --- a/Makefile +++ b/Makefile @@ -50,8 +50,7 @@ ifeq (, $(shell which phpunit 2> /dev/null)) php $(build_tools_directory)/phpunit.phar -c tests/phpunit.xml --coverage-clover build/php-unit.coverage.xml php $(build_tools_directory)/phpunit.phar -c tests/phpunit.integration.xml --coverage-clover build/php-integration.coverage.xml else - phpunit -c tests/phpunit.xml --coverage-clover build/php-unit.coverage.xml - phpunit -c tests/phpunit.integration.xml --coverage-clover build/php-integration.coverage.xml + phpunit -c tests/phpunit.integration.xml --testsuite=integration-database --coverage-clover build/php-integration.coverage.xml endif test-integration: diff --git a/appinfo/info.xml b/appinfo/info.xml index a0706cb09..d6f9d68a6 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -44,6 +44,7 @@ OCA\Deck\Command\UserExport OCA\Deck\Command\BoardImport + OCA\Deck\Command\TransferOwnership diff --git a/appinfo/routes.php b/appinfo/routes.php index 8862eb015..b641e1073 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -39,6 +39,7 @@ ['name' => 'board#updateAcl', 'url' => '/boards/{boardId}/acl/{aclId}', 'verb' => 'PUT'], ['name' => 'board#deleteAcl', 'url' => '/boards/{boardId}/acl/{aclId}', 'verb' => 'DELETE'], ['name' => 'board#clone', 'url' => '/boards/{boardId}/clone', 'verb' => 'POST'], + ['name' => 'board#transferOwner', 'url' => '/boards/{boardId}/transferOwner', 'verb' => 'PUT'], // stacks ['name' => 'stack#index', 'url' => '/stacks/{boardId}', 'verb' => 'GET'], diff --git a/composer.json b/composer.json index d87af90d3..b3c9ee616 100644 --- a/composer.json +++ b/composer.json @@ -14,7 +14,7 @@ }, "require-dev": { "roave/security-advisories": "dev-master", - "christophwurst/nextcloud": "^21@dev", + "christophwurst/nextcloud": "dev-master", "phpunit/phpunit": "^9", "nextcloud/coding-standard": "^1.0.0", "symfony/event-dispatcher": "^4.0", diff --git a/composer.lock b/composer.lock index 45a248a27..e34fc1098 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "c3c765fb5379719b5ab8824e9fbb03f9", + "content-hash": "33c2fe0cccf29841e021001c6430310a", "packages": [ { "name": "cogpowered/finediff", @@ -301,25 +301,29 @@ }, { "name": "christophwurst/nextcloud", - "version": "v21.0.0", + "version": "dev-master", "source": { "type": "git", "url": "https://github.com/ChristophWurst/nextcloud_composer.git", - "reference": "41e1476b4aed5bce7371895054049eca353729c5" + "reference": "cd35b7f4519d9b1c836443ec5dcd973d7f0f9cdd" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/ChristophWurst/nextcloud_composer/zipball/41e1476b4aed5bce7371895054049eca353729c5", - "reference": "41e1476b4aed5bce7371895054049eca353729c5", + "url": "https://api.github.com/repos/ChristophWurst/nextcloud_composer/zipball/cd35b7f4519d9b1c836443ec5dcd973d7f0f9cdd", + "reference": "cd35b7f4519d9b1c836443ec5dcd973d7f0f9cdd", "shasum": "" }, "require": { - "php": "^7.3 || ~8.0.0" + "php": "^7.4 || ~8.0 || ~8.1", + "psr/container": "^1.0", + "psr/event-dispatcher": "^1.0", + "psr/log": "^1.1" }, + "default-branch": true, "type": "library", "extra": { "branch-alias": { - "dev-master": "21.0.0-dev" + "dev-master": "24.0.0-dev" } }, "notification-url": "https://packagist.org/downloads/", @@ -335,9 +339,9 @@ "description": "Composer package containing Nextcloud's public API (classes, interfaces)", "support": { "issues": "https://github.com/ChristophWurst/nextcloud_composer/issues", - "source": "https://github.com/ChristophWurst/nextcloud_composer/tree/v21.0.0" + "source": "https://github.com/ChristophWurst/nextcloud_composer/tree/master" }, - "time": "2021-03-01T08:42:25+00:00" + "time": "2022-03-11T01:33:55+00:00" }, { "name": "composer/package-versions-deprecated", @@ -2304,6 +2308,56 @@ }, "time": "2021-11-05T16:50:12+00:00" }, + { + "name": "psr/event-dispatcher", + "version": "1.0.0", + "source": { + "type": "git", + "url": "https://github.com/php-fig/event-dispatcher.git", + "reference": "dbefd12671e8a14ec7f180cab83036ed26714bb0" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/php-fig/event-dispatcher/zipball/dbefd12671e8a14ec7f180cab83036ed26714bb0", + "reference": "dbefd12671e8a14ec7f180cab83036ed26714bb0", + "shasum": "" + }, + "require": { + "php": ">=7.2.0" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "1.0.x-dev" + } + }, + "autoload": { + "psr-4": { + "Psr\\EventDispatcher\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "PHP-FIG", + "homepage": "http://www.php-fig.org/" + } + ], + "description": "Standard interfaces for event handling.", + "keywords": [ + "events", + "psr", + "psr-14" + ], + "support": { + "issues": "https://github.com/php-fig/event-dispatcher/issues", + "source": "https://github.com/php-fig/event-dispatcher/tree/1.0.0" + }, + "time": "2019-01-08T18:20:26+00:00" + }, { "name": "psr/log", "version": "1.1.4", diff --git a/docs/User_documentation_en.md b/docs/User_documentation_en.md index 3a6f9dc18..876b95f02 100644 --- a/docs/User_documentation_en.md +++ b/docs/User_documentation_en.md @@ -16,6 +16,7 @@ Overall, Deck is easy to use. You can create boards, add users, share the Deck, 5. [Manage your board](#5-manage-your-board) 6. [Import boards](#6-import-boards) 7. [Search](#7-search) +8. [New owner for the deck entities](#8-new-owner-for-the-deck-entities) ### 1. Create my first board In this example, we're going to create a board and share it with an other nextcloud user. @@ -158,4 +159,22 @@ For example the search `project tag:ToDo assigned:alice assigned:bob` will retur Other text tokens will be used to perform a case-insensitive search on the card title and description -In addition wuotes can be used to pass a query with spaces, e.g. `"Exact match with spaces"` or `title:"My card"`. +In addition, quotes can be used to pass a query with spaces, e.g. `"Exact match with spaces"` or `title:"My card"`. + +### 8. New owner for the deck entities +You can transfer ownership of boards, cards, etc to a new user, using `occ` command `deck:transfer-ownership` + +```bash +php occ deck:transfer-ownership previousOwner newOwner +``` + +The transfer will preserve card details linked to the old owner, which can also be remapped by using the `--remap` option on the occ command. +```bash +php occ deck:transfer-ownership --remap previousOwner newOwner +``` + +Individual boards can be transferred by adding the id of the board to the command: + +```bash +php occ deck:transfer-ownership previousOwner newOwner 123 +``` diff --git a/lib/Command/TransferOwnership.php b/lib/Command/TransferOwnership.php new file mode 100644 index 000000000..95a4f65bb --- /dev/null +++ b/lib/Command/TransferOwnership.php @@ -0,0 +1,105 @@ +boardService = $boardService; + $this->boardMapper = $boardMapper; + $this->permissionService = $permissionService; + $this->questionHelper = $questionHelper; + } + + protected function configure() { + $this + ->setName('deck:transfer-ownership') + ->setDescription('Change owner of deck boards') + ->addArgument( + 'owner', + InputArgument::REQUIRED, + 'Owner uid' + ) + ->addArgument( + 'newOwner', + InputArgument::REQUIRED, + 'New owner uid' + ) + ->addArgument( + 'boardId', + InputArgument::OPTIONAL, + 'Single board ID' + ) + ->addOption( + 'remap', + 'r', + InputOption::VALUE_NONE, + 'Reassign card details of the old owner to the new one' + ) + ; + } + + protected function execute(InputInterface $input, OutputInterface $output): int { + $owner = $input->getArgument('owner'); + $newOwner = $input->getArgument('newOwner'); + $boardId = $input->getArgument('boardId'); + + $remapAssignment = $input->getOption('remap'); + + $this->boardService->setUserId($owner); + $this->permissionService->setUserId($owner); + + try { + $board = $boardId ? $this->boardMapper->find($boardId) : null; + } catch (\Exception $e) { + $output->writeln("Could not find a board for the provided id."); + return 1; + } + + if ($boardId !== null && $board->getOwner() !== $owner) { + $output->writeln("$owner is not the owner of the board $boardId (" . $board->getTitle() . ")"); + return 1; + } + + if ($boardId) { + $output->writeln("Transfer board " . $board->getTitle() . " from ". $board->getOwner() ." to $newOwner"); + } else { + $output->writeln("Transfer all boards from $owner to $newOwner"); + } + + $question = new ConfirmationQuestion('Do you really want to continue? (y/n) ', false); + if (!$this->questionHelper->ask($input, $output, $question)) { + 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"); + } + $output->writeln("All boards from $owner to $newOwner transferred"); + + return 0; + } +} diff --git a/lib/Controller/BoardController.php b/lib/Controller/BoardController.php index 9d0b9fcaf..0e909c305 100644 --- a/lib/Controller/BoardController.php +++ b/lib/Controller/BoardController.php @@ -24,9 +24,12 @@ namespace OCA\Deck\Controller; use OCA\Deck\Db\Acl; +use OCA\Deck\Db\Board; use OCA\Deck\Service\BoardService; use OCA\Deck\Service\PermissionService; use OCP\AppFramework\ApiController; +use OCP\AppFramework\Http; +use OCP\AppFramework\Http\DataResponse; use OCP\IRequest; class BoardController extends ApiController { @@ -150,9 +153,20 @@ public function deleteAcl($aclId) { /** * @NoAdminRequired * @param $boardId - * @return \OCP\Deck\DB\Board + * @return Board */ public function clone($boardId) { return $this->boardService->clone($boardId, $this->userId); } + + /** + * @NoAdminRequired + */ + public function transferOwner(int $boardId, string $newOwner): DataResponse { + if ($this->permissionService->userIsBoardOwner($boardId, $this->userId)) { + return new DataResponse($this->boardService->transferBoardOwnership($boardId, $newOwner), HTTP::STATUS_OK); + } + + return new DataResponse([], HTTP::STATUS_UNAUTHORIZED); + } } diff --git a/lib/Db/AclMapper.php b/lib/Db/AclMapper.php index 4eb6a59c6..5cf188aa0 100644 --- a/lib/Db/AclMapper.php +++ b/lib/Db/AclMapper.php @@ -25,6 +25,7 @@ use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\MultipleObjectsReturnedException; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; class AclMapper extends DeckMapper implements IPermissionMapper { @@ -57,4 +58,16 @@ public function findByParticipant($type, $participant): array { $sql = 'SELECT * from *PREFIX*deck_board_acl WHERE type = ? AND participant = ?'; return $this->findEntities($sql, [$type, $participant]); } + + /** + * @throws \OCP\DB\Exception + */ + public function deleteParticipantFromBoard(int $boardId, int $type, string $participant): void { + $qb = $this->db->getQueryBuilder(); + $qb->delete('deck_board_acl') + ->where($qb->expr()->eq('type', $qb->createNamedParameter($type, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->eq('participant', $qb->createNamedParameter($participant, IQueryBuilder::PARAM_STR))) + ->andWhere($qb->expr()->eq('board_id', $qb->createNamedParameter($boardId, IQueryBuilder::PARAM_INT))); + $qb->executeStatement(); + } } diff --git a/lib/Db/AssignmentMapper.php b/lib/Db/AssignmentMapper.php index 275dd1f77..fdbf80d85 100644 --- a/lib/Db/AssignmentMapper.php +++ b/lib/Db/AssignmentMapper.php @@ -29,6 +29,7 @@ use OCA\Deck\Service\CirclesService; use OCP\AppFramework\Db\Entity; use OCP\AppFramework\Db\QBMapper; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; use OCP\IGroupManager; use OCP\IUserManager; @@ -146,4 +147,39 @@ private function getOrigin(Assignment $assignment) { } return null; } + + public function remapAssignedUser(int $boardId, string $userId, string $newUserId): void { + $subQuery = $this->db->getQueryBuilder(); + $subQuery->selectAlias('a.id', 'id') + ->from('deck_assigned_users', 'a') + ->innerJoin('a', 'deck_cards', 'c', 'c.id = a.card_id') + ->innerJoin('c', 'deck_stacks', 's', 's.id = c.stack_id') + ->where($subQuery->expr()->eq('a.type', $subQuery->createNamedParameter(Assignment::TYPE_USER, IQueryBuilder::PARAM_INT))) + ->andWhere($subQuery->expr()->eq('a.participant', $subQuery->createNamedParameter($userId, IQueryBuilder::PARAM_STR))) + ->andWhere($subQuery->expr()->eq('s.board_id', $subQuery->createNamedParameter($boardId, IQueryBuilder::PARAM_INT))) + ->setMaxResults(1000); + + $qb = $this->db->getQueryBuilder(); + $qb->update('deck_assigned_users') + ->set('participant', $qb->createParameter('participant')) + ->where($qb->expr()->in('id', $qb->createParameter('ids'))); + + $moreResults = true; + do { + $result = $subQuery->executeQuery(); + $ids = array_map(function ($item) { + return $item['id']; + }, $result->fetchAll()); + + if (count($ids) === 0 || $result->rowCount() === 0) { + $moreResults = false; + } + + $qb->setParameter('participant', $newUserId, IQueryBuilder::PARAM_STR); + $qb->setParameter('ids', $ids, IQueryBuilder::PARAM_INT_ARRAY); + $qb->executeStatement(); + } while ($moreResults === true); + + $result->closeCursor(); + } } diff --git a/lib/Db/BoardMapper.php b/lib/Db/BoardMapper.php index 6b08000ba..93be76d7e 100644 --- a/lib/Db/BoardMapper.php +++ b/lib/Db/BoardMapper.php @@ -475,4 +475,34 @@ public function mapOwner(Board &$board) { return null; }); } + + /** + * @throws \OCP\DB\Exception + */ + public function transferOwnership(string $ownerId, string $newOwnerId, $boardId = null): void { + $qb = $this->db->getQueryBuilder(); + $qb->update('deck_boards') + ->set('owner', $qb->createNamedParameter($newOwnerId, IQueryBuilder::PARAM_STR)) + ->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))); + } + $qb->executeStatement(); + } + + /** + * Reset cache for a given board or a given user + */ + public function flushCache(?int $boardId = null, ?string $userId = null) { + if ($boardId) { + unset($this->boardCache[$boardId]); + } else { + $this->boardCache = null; + } + if ($userId) { + unset($this->userBoardCache[$userId]); + } else { + $this->userBoardCache = null; + } + } } diff --git a/lib/Db/CardMapper.php b/lib/Db/CardMapper.php index 699c40a06..973a544f1 100644 --- a/lib/Db/CardMapper.php +++ b/lib/Db/CardMapper.php @@ -586,4 +586,47 @@ public function mapOwner(Card &$card) { return null; }); } + + public function transferOwnership(string $ownerId, string $newOwnerId, int $boardId = null): void { + $params = [ + 'owner' => $ownerId, + 'newOwner' => $newOwnerId + ]; + $sql = "UPDATE `*PREFIX*{$this->tableName}` SET `owner` = :newOwner WHERE `owner` = :owner"; + $stmt = $this->db->executeQuery($sql, $params); + $stmt->closeCursor(); + } + + public function remapCardOwner(int $boardId, string $userId, string $newUserId): void { + $subQuery = $this->db->getQueryBuilder(); + $subQuery->selectAlias('c.id', 'id') + ->from('deck_cards', 'c') + ->innerJoin('c', 'deck_stacks', 's', 's.id = c.stack_id') + ->where($subQuery->expr()->eq('c.owner', $subQuery->createNamedParameter($userId, IQueryBuilder::PARAM_STR))) + ->andWhere($subQuery->expr()->eq('s.board_id', $subQuery->createNamedParameter($boardId, IQueryBuilder::PARAM_INT))) + ->setMaxResults(1000); + + $qb = $this->db->getQueryBuilder(); + $qb->update('deck_cards') + ->set('owner', $qb->createParameter('owner')) + ->where($qb->expr()->in('id', $qb->createParameter('ids'))); + + $moreResults = true; + do { + $result = $subQuery->executeQuery(); + $ids = array_map(function ($item) { + return $item['id']; + }, $result->fetchAll()); + + if (count($ids) === 0 || $result->rowCount() === 0) { + $moreResults = false; + } + + $qb->setParameter('owner', $newUserId, IQueryBuilder::PARAM_STR); + $qb->setParameter('ids', $ids, IQueryBuilder::PARAM_INT_ARRAY); + $qb->executeStatement(); + } while ($moreResults === true); + + $result->closeCursor(); + } } diff --git a/lib/Service/BoardService.php b/lib/Service/BoardService.php index 568299c53..e1cfc8023 100644 --- a/lib/Service/BoardService.php +++ b/lib/Service/BoardService.php @@ -24,12 +24,14 @@ namespace OCA\Deck\Service; +use Doctrine\DBAL\Exception\UniqueConstraintViolationException; use OCA\Deck\Activity\ActivityManager; use OCA\Deck\Activity\ChangeSet; use OCA\Deck\AppInfo\Application; use OCA\Deck\Db\Acl; use OCA\Deck\Db\AclMapper; use OCA\Deck\Db\AssignmentMapper; +use OCA\Deck\Db\CardMapper; use OCA\Deck\Db\ChangeHelper; use OCA\Deck\Db\IPermissionMapper; use OCA\Deck\Db\Label; @@ -69,6 +71,8 @@ class BoardService { private $activityManager; private $eventDispatcher; private $changeHelper; + private $cardMapper; + private $boardsCache = null; private $urlGenerator; @@ -83,6 +87,7 @@ public function __construct( PermissionService $permissionService, NotificationHelper $notificationHelper, AssignmentMapper $assignedUsersMapper, + CardMapper $cardMapper, IUserManager $userManager, IGroupManager $groupManager, ActivityManager $activityManager, @@ -107,6 +112,7 @@ public function __construct( $this->changeHelper = $changeHelper; $this->userId = $userId; $this->urlGenerator = $urlGenerator; + $this->cardMapper = $cardMapper; } /** @@ -518,11 +524,14 @@ public function addAcl($boardId, $type, $participant, $edit, $share, $manage) { $acl->setPermissionManage($manage); $newAcl = $this->aclMapper->insert($acl); - $this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_BOARD, $newAcl, ActivityManager::SUBJECT_BOARD_SHARE); + $this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_BOARD, $newAcl, ActivityManager::SUBJECT_BOARD_SHARE, [], $this->userId); $this->notificationHelper->sendBoardShared((int)$boardId, $acl); $this->boardMapper->mapAcl($newAcl); $this->changeHelper->boardChanged($boardId); + $board = $this->boardMapper->find($boardId); + $this->clearBoardFromCache($board); + // TODO: use the dispatched event for this try { $resourceProvider = \OC::$server->query(\OCA\Deck\Collaboration\Resources\ResourceProvider::class); @@ -673,6 +682,43 @@ public function clone($id, $userId) { return $newBoard; } + public function transferBoardOwnership(int $boardId, string $newOwner, bool $changeContent = false): Board { + \OC::$server->getDatabaseConnection()->beginTransaction(); + try { + $board = $this->boardMapper->find($boardId); + $previousOwner = $board->getOwner(); + $this->clearBoardFromCache($board); + $this->aclMapper->deleteParticipantFromBoard($boardId, Acl::PERMISSION_TYPE_USER, $newOwner); + if (!$changeContent) { + try { + $this->addAcl($boardId, Acl::PERMISSION_TYPE_USER, $previousOwner, true, true, true); + } catch (UniqueConstraintViolationException $e) { + } + } + $this->boardMapper->transferOwnership($previousOwner, $newOwner, $boardId); + + // Optionally also change user assignments and card owner information + if ($changeContent) { + $this->assignedUsersMapper->remapAssignedUser($boardId, $previousOwner, $newOwner); + $this->cardMapper->remapCardOwner($boardId, $previousOwner, $newOwner); + } + \OC::$server->getDatabaseConnection()->commit(); + return $this->boardMapper->find($boardId); + } catch (\Throwable $e) { + \OC::$server->getDatabaseConnection()->rollBack(); + throw $e; + } + } + + public function transferOwnership(string $owner, string $newOwner, bool $changeContent = false): \Generator { + $boards = $this->boardMapper->findAllByUser($owner); + foreach ($boards as $board) { + if ($board->getOwner() === $owner) { + yield $this->transferBoardOwnership($board->getId(), $newOwner, $changeContent); + } + } + } + private function enrichWithStacks($board, $since = -1) { $stacks = $this->stackMapper->findAll($board->getId(), null, null, $since); @@ -704,4 +750,19 @@ private function enrichWithUsers($board, $since = -1) { public function getBoardUrl($endpoint) { return $this->urlGenerator->linkToRouteAbsolute('deck.page.index') . '#' . $endpoint; } + + private function clearBoardsCache() { + $this->boardsCache = null; + } + + /** + * Clean a given board data from the Cache + */ + private function clearBoardFromCache(Board $board) { + $boardId = $board->getId(); + $boardOwnerId = $board->getOwner(); + + $this->boardMapper->flushCache($boardId, $boardOwnerId); + unset($this->boardsCache[$boardId]); + } } diff --git a/lib/Service/PermissionService.php b/lib/Service/PermissionService.php index 498e8904c..6777eb910 100644 --- a/lib/Service/PermissionService.php +++ b/lib/Service/PermissionService.php @@ -241,6 +241,7 @@ public function findUsers($boardId, $refresh = false) { if (array_key_exists((string) $boardId, $this->users) && !$refresh) { return $this->users[(string) $boardId]; } + try { $board = $this->boardMapper->find($boardId); } catch (DoesNotExistException $e) { @@ -332,4 +333,13 @@ private function getGroupLimitList() { } return $groups; } + + /** + * Set a different user than the current one, e.g. when no user is available in occ + * + * @param string $userId + */ + public function setUserId(string $userId): void { + $this->userId = $userId; + } } diff --git a/src/components/board/SharingTabSidebar.vue b/src/components/board/SharingTabSidebar.vue index b1f84bf91..0aa2136d2 100644 --- a/src/components/board/SharingTabSidebar.vue +++ b/src/components/board/SharingTabSidebar.vue @@ -53,6 +53,9 @@ {{ t('deck', 'Can manage') }} + + {{ t('deck', 'Owner') }} + {{ t('deck', 'Delete') }} @@ -72,7 +75,7 @@ import { Avatar, Multiselect, Actions, ActionButton, ActionCheckbox } from '@nex import { CollectionList } from 'nextcloud-vue-collections' import { mapGetters, mapState } from 'vuex' import { getCurrentUser } from '@nextcloud/auth' -import { showError } from '@nextcloud/dialogs' +import { showError, showSuccess } from '@nextcloud/dialogs' import debounce from 'lodash/debounce' export default { @@ -97,6 +100,7 @@ export default { isSearching: false, addAcl: null, addAclForAPI: null, + newOwner: null, } }, computed: { @@ -194,6 +198,38 @@ export default { clickDeleteAcl(acl) { this.$store.dispatch('deleteAclFromCurrentBoard', acl) }, + clickTransferOwner(newOwner) { + OC.dialogs.confirmDestructive( + t('deck', 'Are you sure you want to transfer the board {title} for {user} ?', { title: this.board.title, user: newOwner }), + t('deck', 'Transfer the board.'), + { + type: OC.dialogs.YES_NO_BUTTONS, + confirm: t('deck', 'Transfer'), + confirmClasses: 'error', + cancel: t('deck', 'Cancel'), + }, + async (result) => { + if (result) { + try { + this.isLoading = true + await this.$store.dispatch('transferOwnership', { + boardId: this.board.id, + newOwner + }) + const successMessage = t('deck', 'Transfer the board for {user} successfully', { user: newOwner }) + showSuccess(successMessage) + this.$router.push({ name: 'main' }) + } catch (e) { + const errorMessage = t('deck', 'Failed to transfer the board for {user}', { user: newOwner.user }) + showError(errorMessage) + } finally { + this.isLoading = false + } + } + }, + true + ) + }, }, } diff --git a/src/store/main.js b/src/store/main.js index bf37284f9..f7c3a16ee 100644 --- a/src/store/main.js +++ b/src/store/main.js @@ -26,7 +26,7 @@ import { loadState } from '@nextcloud/initial-state' import Vue from 'vue' import Vuex from 'vuex' import axios from '@nextcloud/axios' -import { generateOcsUrl } from '@nextcloud/router' +import { generateOcsUrl, generateUrl } from '@nextcloud/router' import { BoardApi } from '../services/BoardApi' import actions from './actions' import stack from './stack' @@ -486,5 +486,11 @@ export default new Vuex.Store({ dispatch('loadBoardById', acl.boardId) }) }, + async transferOwnership({ commit }, { boardId, owner, newOwner }) { + await axios.put(generateUrl(`apps/deck/boards/${boardId}/transferOwner`), { + owner, + newOwner, + }) + }, }, }) diff --git a/tests/integration/database/TransferOwnershipTest.php b/tests/integration/database/TransferOwnershipTest.php new file mode 100644 index 000000000..a88178c06 --- /dev/null +++ b/tests/integration/database/TransferOwnershipTest.php @@ -0,0 +1,314 @@ +getUserManager()->registerBackend($backend); + $backend->createUser(self::TEST_USER_1, self::TEST_USER_1); + $backend->createUser(self::TEST_USER_2, self::TEST_USER_2); + $backend->createUser(self::TEST_USER_3, self::TEST_USER_3); + // create group + $groupBackend = new \Test\Util\Group\Dummy(); + $groupBackend->createGroup(self::TEST_GROUP); + $groupBackend->addToGroup(self::TEST_USER_1, self::TEST_GROUP); + \OC::$server->getGroupManager()->addBackend($groupBackend); + } + + public function setUp(): void { + parent::setUp(); + \OC::$server->getUserSession()->login(self::TEST_USER_1, self::TEST_USER_1); + $this->boardService = \OC::$server->query(BoardService::class); + $this->stackService = \OC::$server->query(StackService::class); + $this->cardService = \OC::$server->query(CardService::class); + $this->assignmentService = \OC::$server->query(AssignmentService::class); + $this->assignmentMapper = \OC::$server->query(AssignmentMapper::class); + $this->createBoardWithExampleData(); + } + + public function createBoardWithExampleData() { + $stacks = []; + $board = $this->boardService->create('Test', self::TEST_USER_1, '000000'); + $id = $board->getId(); + $this->boardService->addAcl($id, Acl::PERMISSION_TYPE_GROUP, self::TEST_GROUP, true, true, true); + $this->boardService->addAcl($id, Acl::PERMISSION_TYPE_USER, self::TEST_USER_3, false, true, false); + $stacks[] = $this->stackService->create('Stack A', $id, 1); + $stacks[] = $this->stackService->create('Stack B', $id, 1); + $stacks[] = $this->stackService->create('Stack C', $id, 1); + $cards[] = $this->cardService->create('Card 1', $stacks[0]->getId(), 'text', 0, self::TEST_USER_1); + $cards[] = $this->cardService->create('Card 2', $stacks[0]->getId(), 'text', 0, self::TEST_USER_1); + $this->assignmentService->assignUser($cards[0]->getId(), self::TEST_USER_1); + $this->board = $board; + $this->cards = $cards; + $this->stacks = $stacks; + } + + /** + * @covers ::transferOwnership + */ + public function testTransferBoardOwnership() { + iterator_to_array($this->boardService->transferOwnership(self::TEST_USER_1, self::TEST_USER_2)); + $board = $this->boardService->find($this->board->getId()); + $boardOwner = $board->getOwner(); + $this->assertEquals(self::TEST_USER_2, $boardOwner); + } + + /** + * @covers ::transferOwnership + */ + public function testTransferBoardOwnershipWithData() { + iterator_to_array($this->boardService->transferOwnership(self::TEST_USER_1, self::TEST_USER_2)); + $board = $this->boardService->find($this->board->getId()); + + $boardOwner = $board->getOwner(); + $this->assertEquals(self::TEST_USER_2, $boardOwner); + + $cards = $this->cards; + $newOwnerOwnsTheCards = (bool)array_product(array_filter($cards, function (Card $card) { + $cardUpdated = $this->cardService->find($card->getId()); + return $cardUpdated->getOwner() === self::TEST_USER_2; + })); + $this->assertTrue($newOwnerOwnsTheCards); + } + + /** + * @covers ::transferOwnership + */ + public function testTransferACLOwnership() { + iterator_to_array($this->boardService->transferOwnership(self::TEST_USER_1, self::TEST_USER_2, true)); + $board = $this->boardService->find($this->board->getId()); + $acl = $board->getAcl(); + $this->assertBoardDoesNotHaveAclUser($board, self::TEST_USER_1); + } + + /** + * @covers ::transferOwnership + */ + public function testTransferACLOwnershipPreserveOwner() { + iterator_to_array($this->boardService->transferOwnership(self::TEST_USER_1, self::TEST_USER_2, false)); + $board = $this->boardService->find($this->board->getId()); + $acl = $board->getAcl(); + $this->assertBoardHasAclUser($board, self::TEST_USER_1); + } + + /** + * @covers ::transferOwnership + */ + public function testNoTransferAclOwnershipIfGroupType() { + iterator_to_array($this->boardService->transferOwnership(self::TEST_USER_1, self::TEST_USER_2)); + $board = $this->boardService->find($this->board->getId()); + $acl = $board->getAcl(); + $isGroupInAcl = (bool)array_filter($acl, function ($item) { + return $item->getParticipant() === self::TEST_GROUP && $item->getType() === Acl::PERMISSION_TYPE_GROUP; + }); + $this->assertTrue($isGroupInAcl); + } + /** + * @covers ::transferOwnership + */ + public function testTransferCardOwnership() { + iterator_to_array($this->boardService->transferOwnership(self::TEST_USER_1, self::TEST_USER_2, true)); + $card = $this->cardService->find($this->cards[0]->getId()); + $cardOwner = $card->getOwner(); + $this->assertEquals(self::TEST_USER_2, $cardOwner); + } + + /** + * @covers ::transferOwnership + */ + public function testTransferPreserveCardOwnership() { + iterator_to_array($this->boardService->transferOwnership(self::TEST_USER_1, self::TEST_USER_2, false)); + $card = $this->cardService->find($this->cards[0]->getId()); + $cardOwner = $card->getOwner(); + $this->assertEquals(self::TEST_USER_1, $cardOwner); + } + + /** + * @covers ::transferOwnership + */ + public function testReassignCardToNewOwner() { + iterator_to_array($this->boardService->transferOwnership(self::TEST_USER_1, self::TEST_USER_2, true)); + $participantsUIDs = array_map(function ($user) { + return $user->getParticipant(); + }, $this->assignmentMapper->findAll($this->cards[0]->getId())); + $this->assertContains(self::TEST_USER_2, $participantsUIDs); + $this->assertNotContains(self::TEST_USER_1, $participantsUIDs); + } + + /** + * @covers ::transferOwnership + */ + public function testNoReassignCardToNewOwner() { + iterator_to_array($this->boardService->transferOwnership(self::TEST_USER_1, self::TEST_USER_2, false)); + $participantsUIDs = array_map(function ($user) { + return $user->getParticipant(); + }, $this->assignmentMapper->findAll($this->cards[0]->getId())); + $this->assertContains(self::TEST_USER_1, $participantsUIDs); + $this->assertNotContains(self::TEST_USER_2, $participantsUIDs); + } + + /** + * @covers ::transferOwnership + */ + public function testReassignCardToNewParticipantOnlyIfParticipantHasUserType() { + $this->assignmentService->assignUser($this->cards[1]->getId(), self::TEST_USER_1, Assignment::TYPE_GROUP); + iterator_to_array($this->boardService->transferOwnership(self::TEST_USER_1, self::TEST_USER_2)); + $participantsUIDs = array_map(function ($user) { + return $user->getParticipant(); + }, $this->assignmentMapper->findAll($this->cards[1]->getId())); + $this->assertContains(self::TEST_USER_1, $participantsUIDs); + $this->assertNotContains(self::TEST_USER_2, $participantsUIDs); + } + + /** + * @covers ::transferOwnership + */ + public function testTargetAlreadyParticipantOfBoard() { + $this->expectNotToPerformAssertions(); + iterator_to_array($this->boardService->transferOwnership(self::TEST_USER_1, self::TEST_USER_3)); + } + + private function assertBoardHasAclUser($board, $userId) { + $hasUser = (bool)array_filter($board->getAcl(), function ($item) use ($userId) { + return $item->getParticipant() === $userId && $item->getType() === Acl::PERMISSION_TYPE_USER; + }); + self::assertTrue($hasUser, 'user ' . $userId . ' should be in the board acl list'); + } + + private function assertBoardDoesNotHaveAclUser($board, $userId) { + $hasUser = (bool)array_filter($board->getAcl(), function ($item) use ($userId) { + return $item->getParticipant() === $userId && $item->getType() === Acl::PERMISSION_TYPE_USER; + }); + self::assertFalse($hasUser, 'user ' . $userId . ' should not be in the board acl list'); + } + + /** + * @covers ::transferOwnership + */ + public function testDontRemoveOldOwnerFromAcl() { + iterator_to_array($this->boardService->transferOwnership(self::TEST_USER_1, self::TEST_USER_2)); + $board = $this->boardService->find($this->board->getId()); + + $this->assertBoardDoesNotHaveAclUser($board, self::TEST_USER_2); + $this->assertBoardHasAclUser($board, self::TEST_USER_3); + $this->assertBoardHasAclUser($board, self::TEST_USER_1); + } + + /** + * @covers ::transferOwnership + */ + public function testRemoveOldOwnerFromAclForChange() { + iterator_to_array($this->boardService->transferOwnership(self::TEST_USER_1, self::TEST_USER_2, true)); + $board = $this->boardService->find($this->board->getId()); + $this->assertBoardDoesNotHaveAclUser($board, self::TEST_USER_2); + $this->assertBoardHasAclUser($board, self::TEST_USER_3); + $this->assertBoardDoesNotHaveAclUser($board, self::TEST_USER_1); + } + + /** + * @covers ::transferOwnership + */ + public function testMergePermissions() { + $this->boardService->addAcl($this->board->getId(), Acl::PERMISSION_TYPE_USER, self::TEST_USER_2, true, false, true); + iterator_to_array($this->boardService->transferOwnership(self::TEST_USER_1, self::TEST_USER_3)); + $board = $this->boardService->find($this->board->getId()); + $acl = $board->getAcl(); + $isMerged = (bool)array_filter($acl, function ($item) { + return $item->getParticipant() === self::TEST_USER_1 + && $item->getType() === Acl::PERMISSION_TYPE_USER + && $item->getPermission(Acl::PERMISSION_EDIT) + && $item->getPermission(Acl::PERMISSION_SHARE) + && $item->getPermission(Acl::PERMISSION_MANAGE); + }); + $this->assertTrue($isMerged); + } + + /** + * @covers ::transferOwnership + */ + public function testTargetAlreadyParticipantOfCard() { + $this->expectNotToPerformAssertions(); + $this->assignmentService->assignUser($this->cards[0]->getId(), self::TEST_USER_3, Assignment::TYPE_USER); + iterator_to_array($this->boardService->transferOwnership(self::TEST_USER_1, self::TEST_USER_3)); + } + + /** + * @covers ::transferOwnership + */ + public function testTransferSingleBoardAssignment() { + // Arrange separate board next to the one being transferred + $board = $this->boardService->create('Test 2', self::TEST_USER_1, '000000'); + $id = $board->getId(); + $this->boardService->addAcl($id, Acl::PERMISSION_TYPE_USER, self::TEST_USER_1, true, true, true); + $this->boardService->addAcl($id, Acl::PERMISSION_TYPE_GROUP, self::TEST_GROUP, true, true, true); + $this->boardService->addAcl($id, Acl::PERMISSION_TYPE_USER, self::TEST_USER_3, false, true, false); + $stacks[] = $this->stackService->create('Stack A', $id, 1); + $stacks[] = $this->stackService->create('Stack B', $id, 1); + $stacks[] = $this->stackService->create('Stack C', $id, 1); + $cards[] = $this->cardService->create('Card 1', $stacks[0]->getId(), 'text', 0, self::TEST_USER_1); + $cards[] = $this->cardService->create('Card 2', $stacks[0]->getId(), 'text', 0, self::TEST_USER_1); + $this->assignmentService->assignUser($cards[0]->getId(), self::TEST_USER_1); + + // Act + $this->boardService->transferBoardOwnership($this->board->getId(), self::TEST_USER_2, true); + + // Assert that the selected board was transferred + $card = $this->cardService->find($this->cards[0]->getId()); + $this->assertEquals(self::TEST_USER_2, $card->getOwner()); + + $participantsUIDs = array_map(function ($assignment) { + return $assignment->getParticipant(); + }, $this->assignmentMapper->findAll($this->cards[0]->getId())); + $this->assertContains(self::TEST_USER_2, $participantsUIDs); + $this->assertNotContains(self::TEST_USER_1, $participantsUIDs); + + // Assert that other board remained unchanged + $card = $this->cardService->find($cards[0]->getId()); + $this->assertEquals(self::TEST_USER_1, $card->getOwner()); + + $participantsUIDs = array_map(function ($assignment) { + return $assignment->getParticipant(); + }, $this->assignmentMapper->findAll($cards[0]->getId())); + $this->assertContains(self::TEST_USER_1, $participantsUIDs); + $this->assertNotContains(self::TEST_USER_2, $participantsUIDs); + } + + public function tearDown(): void { + $this->boardService->deleteForce($this->board->getId()); + parent::tearDown(); + } +} diff --git a/tests/integration/run.sh b/tests/integration/run.sh index ab9790b26..de8b32563 100755 --- a/tests/integration/run.sh +++ b/tests/integration/run.sh @@ -26,7 +26,7 @@ composer dump-autoload if [ -z "$EXECUTOR_NUMBER" ]; then EXECUTOR_NUMBER=0 fi -PORT=$((8080 + $EXECUTOR_NUMBER)) +PORT=$((9090 + $EXECUTOR_NUMBER)) echo $PORT php -S localhost:$PORT -t $OC_PATH & PHPPID=$! diff --git a/tests/unit/Activity/ActivityManagerTest.php b/tests/unit/Activity/ActivityManagerTest.php index 5e486731e..d292d9023 100644 --- a/tests/unit/Activity/ActivityManagerTest.php +++ b/tests/unit/Activity/ActivityManagerTest.php @@ -169,18 +169,15 @@ public function testSendToUser($objectType) { $this->mockUser('user2'), ]; $event = $this->createMock(IEvent::class); - $event->expects($this->at(0)) + $event->expects($this->once()) ->method('getObjectType') ->willReturn($objectType); - $event->expects($this->at(0)) + $event->expects($this->once()) ->method('getObjectId') ->willReturn(1); - $event->expects($this->at(2)) + $event->expects($this->exactly(2)) ->method('setAffectedUser') - ->with('user1'); - $event->expects($this->at(3)) - ->method('setAffectedUser') - ->with('user2'); + ->withConsecutive(['user1'], ['user2']); $mapper = null; switch ($objectType) { case ActivityManager::DECK_OBJECT_BOARD: @@ -196,10 +193,7 @@ public function testSendToUser($objectType) { $this->permissionService->expects($this->once()) ->method('findUsers') ->willReturn($users); - $this->manager->expects($this->at(0)) - ->method('publish') - ->with($event); - $this->manager->expects($this->at(1)) + $this->manager->expects($this->exactly(2)) ->method('publish') ->with($event); $this->invokePrivate($this->activityManager, 'sendToUsers', [$event]); diff --git a/tests/unit/Cron/DeleteCronTest.php b/tests/unit/Cron/DeleteCronTest.php index e7ba1c6b9..e411a5d31 100644 --- a/tests/unit/Cron/DeleteCronTest.php +++ b/tests/unit/Cron/DeleteCronTest.php @@ -66,18 +66,14 @@ public function testDeleteCron() { $this->boardMapper->expects($this->once()) ->method('findToDelete') ->willReturn($boards); - $this->boardMapper->expects($this->at(1)) + $this->boardMapper->expects($this->exactly(count($boards))) ->method('delete') - ->with($boards[0]); - $this->boardMapper->expects($this->at(2)) - ->method('delete') - ->with($boards[1]); - $this->boardMapper->expects($this->at(3)) - ->method('delete') - ->with($boards[2]); - $this->boardMapper->expects($this->at(4)) - ->method('delete') - ->with($boards[3]); + ->withConsecutive( + [$boards[0]], + [$boards[1]], + [$boards[2]], + [$boards[3]] + ); $attachment = new Attachment(); $attachment->setType('deck_file'); diff --git a/tests/unit/Cron/ScheduledNoificationsTest.php b/tests/unit/Cron/ScheduledNoificationsTest.php index ff7aad6a1..f44b91fa1 100644 --- a/tests/unit/Cron/ScheduledNoificationsTest.php +++ b/tests/unit/Cron/ScheduledNoificationsTest.php @@ -54,10 +54,7 @@ public function testDeleteCron() { $this->cardMapper->expects($this->once()) ->method('findOverdue') ->willReturn($cards); - $this->notificationHelper->expects($this->at(0)) - ->method('sendCardDuedate') - ->with($c1); - $this->notificationHelper->expects($this->at(1)) + $this->notificationHelper->expects($this->exactly(2)) ->method('sendCardDuedate') ->with($c1); $this->scheduledNotifications->run(null); diff --git a/tests/unit/Notification/NotificationHelperTest.php b/tests/unit/Notification/NotificationHelperTest.php index 0d5b2527c..c4f0fd9cd 100644 --- a/tests/unit/Notification/NotificationHelperTest.php +++ b/tests/unit/Notification/NotificationHelperTest.php @@ -114,17 +114,18 @@ private function createUserMock($uid) { } public function testSendCardDuedate() { - $this->config->expects($this->at(0)) - ->method('getUserValue') - ->with('foo', 'deck', 'board:234:notify-due', ConfigService::SETTING_BOARD_NOTIFICATION_DUE_ASSIGNED) - ->willReturn(ConfigService::SETTING_BOARD_NOTIFICATION_DUE_ALL); - $this->config->expects($this->at(1)) - ->method('getUserValue') - ->with('bar', 'deck', 'board:234:notify-due', ConfigService::SETTING_BOARD_NOTIFICATION_DUE_ASSIGNED) - ->willReturn(ConfigService::SETTING_BOARD_NOTIFICATION_DUE_ALL); - $this->config->expects($this->at(2)) + $param1 = ['foo', 'bar', 'asd']; + $param2 = 'deck'; + $param3 = 'board:234:notify-due'; + $DUE_ASSIGNED = ConfigService::SETTING_BOARD_NOTIFICATION_DUE_ASSIGNED; + + $this->config->expects($this->exactly(3)) ->method('getUserValue') - ->with('asd', 'deck', 'board:234:notify-due', ConfigService::SETTING_BOARD_NOTIFICATION_DUE_ASSIGNED) + ->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([ @@ -180,24 +181,12 @@ public function testSendCardDuedate() { $n3->expects($this->once())->method('setSubject')->with('card-overdue', ['MyCardTitle', 'MyBoardTitle'])->willReturn($n3); $n3->expects($this->once())->method('setDateTime')->willReturn($n3); - $this->notificationManager->expects($this->at(0)) - ->method('createNotification') - ->willReturn($n1); - $this->notificationManager->expects($this->at(1)) - ->method('notify') - ->with($n1); - $this->notificationManager->expects($this->at(2)) - ->method('createNotification') - ->willReturn($n2); - $this->notificationManager->expects($this->at(3)) - ->method('notify') - ->with($n2); - $this->notificationManager->expects($this->at(4)) + $this->notificationManager->expects($this->exactly(3)) ->method('createNotification') - ->willReturn($n3); - $this->notificationManager->expects($this->at(5)) + ->willReturnOnConsecutiveCalls($n1, $n2, $n3); + $this->notificationManager->expects($this->exactly(3)) ->method('notify') - ->with($n3); + ->withConsecutive([$n1], [$n2], [$n3]); $this->cardMapper->expects($this->once()) ->method('markNotified') @@ -207,18 +196,19 @@ public function testSendCardDuedate() { } public function testSendCardDuedateAssigned() { - $this->config->expects($this->at(0)) - ->method('getUserValue') - ->with('foo', 'deck', 'board:234:notify-due', ConfigService::SETTING_BOARD_NOTIFICATION_DUE_ASSIGNED) - ->willReturn(ConfigService::SETTING_BOARD_NOTIFICATION_DUE_ASSIGNED); - $this->config->expects($this->at(1)) - ->method('getUserValue') - ->with('bar', 'deck', 'board:234:notify-due', ConfigService::SETTING_BOARD_NOTIFICATION_DUE_ASSIGNED) - ->willReturn(ConfigService::SETTING_BOARD_NOTIFICATION_DUE_ASSIGNED); - $this->config->expects($this->at(2)) + $param1 = ['foo', 'bar', 'asd']; + $param2 = 'deck'; + $param3 = 'board:234:notify-due'; + $DUE_ASSIGNED = ConfigService::SETTING_BOARD_NOTIFICATION_DUE_ASSIGNED; + + $this->config->expects($this->exactly(3)) ->method('getUserValue') - ->with('asd', 'deck', 'board:234:notify-due', ConfigService::SETTING_BOARD_NOTIFICATION_DUE_ASSIGNED) - ->willReturn(ConfigService::SETTING_BOARD_NOTIFICATION_DUE_ASSIGNED); + ->withConsecutive( + [$param1[0], $param2, $param3, $DUE_ASSIGNED], + [$param1[1], $param2, $param3, $DUE_ASSIGNED], + [$param1[2], $param2, $param3, $DUE_ASSIGNED] + ) + ->willReturn($DUE_ASSIGNED); $users = [ new DummyUser('foo'), new DummyUser('bar'), new DummyUser('asd') @@ -278,24 +268,12 @@ public function testSendCardDuedateAssigned() { $n3->expects($this->once())->method('setSubject')->with('card-overdue', ['MyCardTitle', 'MyBoardTitle'])->willReturn($n3); $n3->expects($this->once())->method('setDateTime')->willReturn($n3); - $this->notificationManager->expects($this->at(0)) + $this->notificationManager->expects($this->exactly(3)) ->method('createNotification') - ->willReturn($n1); - $this->notificationManager->expects($this->at(1)) + ->willReturnOnConsecutiveCalls($n1, $n2, $n3); + $this->notificationManager->expects($this->exactly(3)) ->method('notify') - ->with($n1); - $this->notificationManager->expects($this->at(2)) - ->method('createNotification') - ->willReturn($n2); - $this->notificationManager->expects($this->at(3)) - ->method('notify') - ->with($n2); - $this->notificationManager->expects($this->at(4)) - ->method('createNotification') - ->willReturn($n3); - $this->notificationManager->expects($this->at(5)) - ->method('notify') - ->with($n3); + ->withConsecutive([$n1], [$n2], [$n3]); $this->cardMapper->expects($this->once()) ->method('markNotified') @@ -306,18 +284,20 @@ public function testSendCardDuedateAssigned() { public function testSendCardDuedateNever() { - $this->config->expects($this->at(0)) - ->method('getUserValue') - ->with('foo', 'deck', 'board:234:notify-due', ConfigService::SETTING_BOARD_NOTIFICATION_DUE_ASSIGNED) - ->willReturn(ConfigService::SETTING_BOARD_NOTIFICATION_DUE_ASSIGNED); - $this->config->expects($this->at(1)) - ->method('getUserValue') - ->with('bar', 'deck', 'board:234:notify-due', ConfigService::SETTING_BOARD_NOTIFICATION_DUE_ASSIGNED) - ->willReturn(ConfigService::SETTING_BOARD_NOTIFICATION_DUE_ASSIGNED); - $this->config->expects($this->at(2)) + $param1 = ['foo', 'bar', 'asd']; + $param2 = 'deck'; + $param3 = 'board:234:notify-due'; + $DUE_ASSIGNED = ConfigService::SETTING_BOARD_NOTIFICATION_DUE_ASSIGNED; + $DUE_OFF = ConfigService::SETTING_BOARD_NOTIFICATION_DUE_OFF; + + $this->config->expects($this->exactly(3)) ->method('getUserValue') - ->with('asd', 'deck', 'board:234:notify-due', ConfigService::SETTING_BOARD_NOTIFICATION_DUE_ASSIGNED) - ->willReturn(ConfigService::SETTING_BOARD_NOTIFICATION_DUE_OFF); + ->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); $users = [ new DummyUser('foo'), new DummyUser('bar'), new DummyUser('asd') @@ -370,18 +350,12 @@ public function testSendCardDuedateNever() { $n2->expects($this->once())->method('setSubject')->with('card-overdue', ['MyCardTitle', 'MyBoardTitle'])->willReturn($n2); $n2->expects($this->once())->method('setDateTime')->willReturn($n2); - $this->notificationManager->expects($this->at(0)) - ->method('createNotification') - ->willReturn($n1); - $this->notificationManager->expects($this->at(1)) - ->method('notify') - ->with($n1); - $this->notificationManager->expects($this->at(2)) + $this->notificationManager->expects($this->exactly(2)) ->method('createNotification') - ->willReturn($n2); - $this->notificationManager->expects($this->at(3)) + ->willReturnOnConsecutiveCalls($n1, $n2); + $this->notificationManager->expects($this->exactly(2)) ->method('notify') - ->with($n2); + ->withConsecutive([$n1], [$n2]); $this->cardMapper->expects($this->once()) ->method('markNotified') @@ -423,10 +397,10 @@ public function testSendCardAssignedUser() { $notification->expects($this->once())->method('setSubject')->with('card-assigned', ['MyCardTitle', 'MyBoardTitle', 'admin'])->willReturn($notification); $notification->expects($this->once())->method('setDateTime')->willReturn($notification); - $this->notificationManager->expects($this->at(0)) + $this->notificationManager->expects($this->once()) ->method('createNotification') ->willReturn($notification); - $this->notificationManager->expects($this->at(1)) + $this->notificationManager->expects($this->once()) ->method('notify') ->with($notification); @@ -451,10 +425,10 @@ public function testSendBoardSharedUser() { $notification->expects($this->once())->method('setSubject')->with('board-shared', ['MyBoardTitle', 'admin'])->willReturn($notification); $notification->expects($this->once())->method('setDateTime')->willReturn($notification); - $this->notificationManager->expects($this->at(0)) + $this->notificationManager->expects($this->once()) ->method('createNotification') ->willReturn($notification); - $this->notificationManager->expects($this->at(1)) + $this->notificationManager->expects($this->once()) ->method('notify') ->with($notification); @@ -490,10 +464,10 @@ public function testSendBoardSharedGroup() { $notification->expects($this->once())->method('setSubject')->with('board-shared', ['MyBoardTitle', 'admin'])->willReturn($notification); $notification->expects($this->once())->method('setDateTime')->willReturn($notification); - $this->notificationManager->expects($this->at(0)) + $this->notificationManager->expects($this->once()) ->method('createNotification') ->willReturn($notification); - $this->notificationManager->expects($this->at(1)) + $this->notificationManager->expects($this->once()) ->method('notify') ->with($notification); @@ -540,19 +514,12 @@ public function testSendMention() { $notification2->expects($this->once())->method('setSubject')->with('card-comment-mentioned', ['MyCard', 1, 'admin'])->willReturn($notification2); $notification2->expects($this->once())->method('setDateTime')->willReturn($notification2); - $this->notificationManager->expects($this->at(0)) - ->method('createNotification') - ->willReturn($notification1); - $this->notificationManager->expects($this->at(1)) - ->method('notify') - ->with($notification1); - - $this->notificationManager->expects($this->at(2)) + $this->notificationManager->expects($this->exactly(2)) ->method('createNotification') - ->willReturn($notification2); - $this->notificationManager->expects($this->at(3)) + ->willReturnOnConsecutiveCalls($notification1, $notification2); + $this->notificationManager->expects($this->exactly(2)) ->method('notify') - ->with($notification2); + ->withConsecutive([$notification1], [$notification2]); $this->notificationHelper->sendMention($comment); } diff --git a/tests/unit/Service/AttachmentServiceTest.php b/tests/unit/Service/AttachmentServiceTest.php index 5fa5d73cc..3638cf7ea 100644 --- a/tests/unit/Service/AttachmentServiceTest.php +++ b/tests/unit/Service/AttachmentServiceTest.php @@ -110,8 +110,16 @@ public function setUp(): void { $this->cache = $this->createMock(ICache::class); $this->cacheFactory->expects($this->any())->method('createDistributed')->willReturn($this->cache); - $this->appContainer->expects($this->at(0))->method('query')->with(FileService::class)->willReturn($this->attachmentServiceImpl); - $this->appContainer->expects($this->at(1))->method('query')->with(FilesAppService::class)->willReturn($this->filesAppServiceImpl); + $this->appContainer->expects($this->exactly(2)) + ->method('query') + ->withConsecutive( + [FileService::class], + [FilesAppService::class] + ) + ->willReturnOnConsecutiveCalls( + $this->attachmentServiceImpl, + $this->filesAppServiceImpl + ); $this->application->expects($this->any()) ->method('getContainer') @@ -129,9 +137,18 @@ public function testRegisterAttachmentService() { $fileServiceMock = $this->createMock(FileService::class); $fileAppServiceMock = $this->createMock(FilesAppService::class); - $appContainer->expects($this->at(0))->method('query')->with(FileService::class)->willReturn($fileServiceMock); - $appContainer->expects($this->at(1))->method('query')->with(FilesAppService::class)->willReturn($fileAppServiceMock); - $appContainer->expects($this->at(2))->method('query')->with(MyAttachmentService::class)->willReturn(new MyAttachmentService()); + $appContainer->expects($this->exactly(3)) + ->method('query') + ->withConsecutive( + [FileService::class], + [FilesAppService::class], + [MyAttachmentService::class] + ) + ->willReturnOnConsecutiveCalls( + $fileServiceMock, + $fileAppServiceMock, + new MyAttachmentService() + ); $application->expects($this->any()) ->method('getContainer') @@ -148,12 +165,24 @@ public function testRegisterAttachmentServiceNotExisting() { $appContainer = $this->createMock(IAppContainer::class); $fileServiceMock = $this->createMock(FileService::class); $fileAppServiceMock = $this->createMock(FilesAppService::class); - $appContainer->expects($this->at(0))->method('query')->with(FileService::class)->willReturn($fileServiceMock); - $appContainer->expects($this->at(1))->method('query')->with(FilesAppService::class)->willReturn($fileAppServiceMock); - $appContainer->expects($this->at(2))->method('query')->with(MyAttachmentService::class)->willReturn(new MyAttachmentService()); + + $appContainer->expects($this->exactly(3)) + ->method('query') + ->withConsecutive( + [FileService::class], + [FilesAppService::class], + [MyAttachmentService::class] + ) + ->willReturnOnConsecutiveCalls( + $fileServiceMock, + $fileAppServiceMock, + new MyAttachmentService() + ); + $application->expects($this->any()) ->method('getContainer') ->willReturn($appContainer); + $attachmentService = new AttachmentService($this->attachmentMapper, $this->cardMapper, $this->changeHelper, $this->permissionService, $application, $this->cacheFactory, $this->userId, $this->l10n, $this->activityManager); $attachmentService->registerAttachmentService('custom', MyAttachmentService::class); $attachmentService->getService('deck_file_invalid'); @@ -185,12 +214,17 @@ public function testFindAll() { ->with(123) ->willReturn($attachments); - $this->attachmentServiceImpl->expects($this->at(0)) - ->method('extendData') - ->with($attachments[0]); - $this->attachmentServiceImpl->expects($this->at(1)) + $this->attachmentServiceImpl->expects($this->exactly(2)) ->method('extendData') - ->with($attachments[1]); + ->withConsecutive( + [$attachments[0]], + [$attachments[1]], + ) + ->willReturnOnConsecutiveCalls( + $attachments[0], + $attachments[1], + ); + $this->assertEquals($attachments, $this->attachmentService->findAll(123, false)); } @@ -215,12 +249,15 @@ public function testFindAllWithDeleted() { ->with(123, false) ->willReturn($attachmentsDeleted); - $this->attachmentServiceImpl->expects($this->at(0)) - ->method('extendData') - ->with($attachments[0]); - $this->attachmentServiceImpl->expects($this->at(1)) + $this->attachmentServiceImpl->expects($this->exactly(4)) ->method('extendData') - ->with($attachments[1]); + ->withConsecutive( + [$attachments[0]], + [$attachments[1]], + [$attachmentsDeleted[0]], + [$attachmentsDeleted[1]] + ); + $this->assertEquals(array_merge($attachments, $attachmentsDeleted), $this->attachmentService->findAll(123, true)); } @@ -396,5 +433,6 @@ public function testRestoreNotAllowed() { ->method('allowUndo') ->willReturn(false); $actual = $this->attachmentService->restore(1, 1); + $this->assertEquals($expected, $actual); } } diff --git a/tests/unit/Service/BoardServiceTest.php b/tests/unit/Service/BoardServiceTest.php index e59662489..0e8bfeb15 100644 --- a/tests/unit/Service/BoardServiceTest.php +++ b/tests/unit/Service/BoardServiceTest.php @@ -31,6 +31,7 @@ use OCA\Deck\Db\AssignmentMapper; use OCA\Deck\Db\Board; use OCA\Deck\Db\BoardMapper; +use OCA\Deck\Db\CardMapper; use OCA\Deck\Db\ChangeHelper; use OCA\Deck\Db\LabelMapper; use OCA\Deck\Db\StackMapper; @@ -58,6 +59,8 @@ class BoardServiceTest extends TestCase { private $boardMapper; /** @var StackMapper */ private $stackMapper; + /** @var CardMapper */ + private $cardMapper; /** @var PermissionService */ private $permissionService; /** @var NotificationHelper */ @@ -85,6 +88,7 @@ public function setUp(): void { $this->boardMapper = $this->createMock(BoardMapper::class); $this->stackMapper = $this->createMock(StackMapper::class); $this->config = $this->createMock(IConfig::class); + $this->cardMapper = $this->createMock(CardMapper::class); $this->labelMapper = $this->createMock(LabelMapper::class); $this->permissionService = $this->createMock(PermissionService::class); $this->notificationHelper = $this->createMock(NotificationHelper::class); @@ -106,6 +110,7 @@ public function setUp(): void { $this->permissionService, $this->notificationHelper, $this->assignedUsersMapper, + $this->cardMapper, $this->userManager, $this->groupManager, $this->activityManager, @@ -149,7 +154,7 @@ public function testFind() { ->method('find') ->with(1) ->willReturn($b1); - $this->permissionService->expects($this->once()) + $this->permissionService->expects($this->any()) ->method('findUsers') ->willReturn([ 'admin' => 'admin', @@ -253,6 +258,11 @@ public function testAddAcl() { ->method('insert') ->with($acl) ->willReturn($acl); + $this->permissionService->expects($this->any()) + ->method('findUsers') + ->willReturn([ + 'admin' => 'admin', + ]); $this->assertEquals($acl, $this->service->addAcl( 123, 'user', 'admin', true, true, true )); @@ -295,30 +305,39 @@ public function testAddAclExtendPermission($currentUserAcl, $providedAcl, $resul $existingAcl->setPermissionEdit($currentUserAcl[0]); $existingAcl->setPermissionShare($currentUserAcl[1]); $existingAcl->setPermissionManage($currentUserAcl[2]); - $this->permissionService->expects($this->at(0)) - ->method('checkPermission') - ->with($this->boardMapper, 123, Acl::PERMISSION_SHARE, null); + if ($currentUserAcl[2]) { - $this->permissionService->expects($this->at(1)) + $this->permissionService->expects($this->exactly(2)) ->method('checkPermission') - ->with($this->boardMapper, 123, Acl::PERMISSION_MANAGE, null); + ->withConsecutive( + [$this->boardMapper, 123, Acl::PERMISSION_SHARE, null], + [$this->boardMapper, 123, Acl::PERMISSION_MANAGE, null] + ); } else { $this->aclMapper->expects($this->once()) ->method('findAll') ->willReturn([$existingAcl]); - $this->permissionService->expects($this->at(1)) + + $this->permissionService->expects($this->exactly(2)) ->method('checkPermission') - ->with($this->boardMapper, 123, Acl::PERMISSION_MANAGE, null) - ->willThrowException(new NoPermissionException('No permission')); - $this->permissionService->expects($this->at(2)) - ->method('userCan') - ->willReturn($currentUserAcl[0]); - $this->permissionService->expects($this->at(3)) - ->method('userCan') - ->willReturn($currentUserAcl[1]); - $this->permissionService->expects($this->at(4)) + ->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')) + ) + ); + + $this->permissionService->expects($this->exactly(3)) ->method('userCan') - ->willReturn($currentUserAcl[2]); + ->willReturnOnConsecutiveCalls( + $currentUserAcl[0], + $currentUserAcl[1], + $currentUserAcl[2] + ); } $user = $this->createMock(IUser::class); @@ -333,6 +352,11 @@ public function testAddAclExtendPermission($currentUserAcl, $providedAcl, $resul $acl->resolveRelation('participant', function ($participant) use (&$user) { return null; }); + $this->permissionService->expects($this->any()) + ->method('findUsers') + ->willReturn([ + 'admin' => 'admin', + ]); $this->notificationHelper->expects($this->once()) ->method('sendBoardShared'); $expected = clone $acl; diff --git a/tests/unit/Service/PermissionServiceTest.php b/tests/unit/Service/PermissionServiceTest.php index f92adc059..0994a778e 100644 --- a/tests/unit/Service/PermissionServiceTest.php +++ b/tests/unit/Service/PermissionServiceTest.php @@ -139,13 +139,17 @@ public function testGetPermissionsAclNo() { } public function testUserIsBoardOwner() { - $board = new Board(); - $board->setOwner('admin'); - $this->boardMapper->expects($this->at(0))->method('find')->with(123)->willReturn($board); + $adminBoard = new Board(); + $adminBoard->setOwner('admin'); + $userBoard = new Board(); + $userBoard->setOwner('user1'); + + $this->boardMapper->expects($this->exactly(2)) + ->method('find') + ->withConsecutive([123], [234]) + ->willReturnOnConsecutiveCalls($adminBoard, $userBoard); + $this->assertEquals(true, $this->service->userIsBoardOwner(123)); - $board = new Board(); - $board->setOwner('user1'); - $this->boardMapper->expects($this->at(0))->method('find')->with(234)->willReturn($board); $this->assertEquals(false, $this->service->userIsBoardOwner(234)); } @@ -336,7 +340,7 @@ public function testFindUsers() { $aclGroup->setParticipant('group1'); $board = $this->createMock(Board::class); - $board->expects($this->at(0)) + $board->expects($this->once()) ->method('__call') ->with('getOwner', []) ->willReturn('user1'); @@ -348,14 +352,11 @@ public function testFindUsers() { ->method('find') ->with(123) ->willReturn($board); - $this->userManager->expects($this->at(0)) + $this->userManager->expects($this->exactly(2)) ->method('get') - ->with('user1') - ->willReturn($user1); - $this->userManager->expects($this->at(1)) - ->method('get') - ->with('user2') - ->willReturn($user2); + ->withConsecutive(['user1'], ['user2']) + ->willReturnOnConsecutiveCalls($user1, $user2); + $group = $this->createMock(IGroup::class); $group->expects($this->once()) ->method('getUsers')