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 05755a365..a40f79e99 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -50,6 +50,7 @@ OCA\Deck\Command\UserExport + OCA\Deck\Command\TransferOwnership diff --git a/docs/User_documentation_en.md b/docs/User_documentation_en.md index 75b0e57a5..40f137d5c 100644 --- a/docs/User_documentation_en.md +++ b/docs/User_documentation_en.md @@ -14,6 +14,7 @@ Overall, Deck is easy to use. You can create boards, add users, share the Deck, 3. [Handle cards options](#3-handle-cards-options) 4. [Archive old tasks](#4-archive-old-tasks) 5. [Manage your board](#5-manage-your-board) +6. [New owner for the deck entities](#6-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. @@ -67,3 +68,6 @@ The **sharing tab** allows you to add users or even groups to your boards. **Deleted objects** allows you to return previously deleted stacks or cards. The **Timeline** allows you to see everything that happened in your boards. Everything! +### 6. New owner for the deck entities +You can transfer ownership of boards, cards, etc to a new user, using `occ` command `deck:transfer-ownership` +`$ php occ deck:transfer-ownership owner newOwner` diff --git a/lib/Command/TransferOwnership.php b/lib/Command/TransferOwnership.php new file mode 100644 index 000000000..c3175477e --- /dev/null +++ b/lib/Command/TransferOwnership.php @@ -0,0 +1,46 @@ +boardService = $boardService; + } + + protected function configure() { + $this + ->setName('deck:transfer-ownership') + ->setDescription('Change owner of deck entities') + ->addArgument( + 'owner', + InputArgument::REQUIRED, + 'Owner uid' + ) + ->addArgument( + 'newOwner', + InputArgument::REQUIRED, + 'New owner uid' + ); + } + + protected function execute(InputInterface $input, OutputInterface $output) { + $owner = $input->getArgument('owner'); + $newOwner = $input->getArgument('newOwner'); + + $output->writeln("Transfer deck entities from $owner to $newOwner"); + + $this->boardService->transferOwnership($owner, $newOwner); + + $output->writeln("Transfer deck entities from $owner to $newOwner completed"); + } +} diff --git a/lib/Db/AclMapper.php b/lib/Db/AclMapper.php index e0ed3c838..6e9ddb6aa 100644 --- a/lib/Db/AclMapper.php +++ b/lib/Db/AclMapper.php @@ -51,4 +51,40 @@ public function findByParticipant($type, $participant) { $sql = 'SELECT * from *PREFIX*deck_board_acl WHERE type = ? AND participant = ?'; return $this->findEntities($sql, [$type, $participant]); } + + /** + * @param $ownerId + * @param $newOwnerId + * @return void + */ + public function transferOwnership($ownerId, $newOwnerId) { + $params = [ + 'owner' => $ownerId, + 'newOwner' => $newOwnerId, + 'type' => Acl::PERMISSION_TYPE_USER + ]; + //We want preserve permissions from both users + $sql = "UPDATE `{$this->tableName}` AS `source` + LEFT JOIN `{$this->tableName}` AS `target` + ON `target`.`participant` = :newOwner AND `target`.`type` = :type + SET `source`.`permission_edit` =(`source`.`permission_edit` || `target`.`permission_edit`), + `source`.`permission_share` =(`source`.`permission_share` || `target`.`permission_share`), + `source`.`permission_manage` =(`source`.`permission_manage` || `target`.`permission_manage`) + WHERE `source`.`participant` = :owner AND `source`.`type` = :type"; + $stmt = $this->execute($sql, $params); + $stmt->closeCursor(); + //We can't transfer acl if target already in acl + $sql = "DELETE FROM `{$this->tableName}` + WHERE `participant` = :newOwner + AND `type` = :type + AND EXISTS (SELECT `id` FROM (SELECT `id` FROM `{$this->tableName}` + WHERE `participant` = :owner AND `type` = :type) as tmp)"; + $stmt = $this->execute($sql, $params); + $stmt->closeCursor(); + //Now we can transfer without errors + $sqlUpdate = "UPDATE `{$this->tableName}` + SET `participant` = :newOwner WHERE `participant` = :owner AND `type` = :type"; + $stmt = $this->execute($sqlUpdate, $params); + $stmt->closeCursor(); + } } diff --git a/lib/Db/AssignedUsersMapper.php b/lib/Db/AssignedUsersMapper.php index bd18f46cd..d3c9d6670 100644 --- a/lib/Db/AssignedUsersMapper.php +++ b/lib/Db/AssignedUsersMapper.php @@ -114,4 +114,27 @@ private function getOrigin(AssignedUsers $assignment) { } return null; } + + /** + * @param $ownerId + * @param $newOwnerId + * @return void + */ + public function transferOwnership($ownerId, $newOwnerId) { + $params = [ + 'newOwner' => $newOwnerId, + 'type' => AssignedUsers::TYPE_USER + ]; + $sql = "DELETE FROM `{$this->tableName}` WHERE `participant` = :newOwner AND `type`= :type"; + $stmt = $this->execute($sql, $params); + $stmt->closeCursor(); + $params = [ + 'owner' => $ownerId, + 'newOwner' => $newOwnerId, + 'type' => AssignedUsers::TYPE_USER + ]; + $sql = "UPDATE `{$this->tableName}` SET `participant` = :newOwner WHERE `participant` = :owner AND `type`= :type"; + $stmt = $this->execute($sql, $params); + $stmt->closeCursor(); + } } diff --git a/lib/Db/BoardMapper.php b/lib/Db/BoardMapper.php index 43e070a6b..7d07de92b 100644 --- a/lib/Db/BoardMapper.php +++ b/lib/Db/BoardMapper.php @@ -261,4 +261,19 @@ public function mapOwner(Board &$board) { return null; }); } + + /** + * @param $ownerId + * @param $newOwnerId + * @return void + */ + public function transferOwnership($ownerId, $newOwnerId) { + $params = [ + 'owner' => $ownerId, + 'newOwner' => $newOwnerId + ]; + $sql = "UPDATE `{$this->tableName}` SET `owner` = :newOwner WHERE `owner` = :owner"; + $stmt = $this->execute($sql, $params); + $stmt->closeCursor(); + } } diff --git a/lib/Db/CardMapper.php b/lib/Db/CardMapper.php index ad2c97724..2ec63dcc1 100644 --- a/lib/Db/CardMapper.php +++ b/lib/Db/CardMapper.php @@ -205,4 +205,19 @@ public function mapOwner(Card &$card) { return null; }); } + + /** + * @param $ownerId + * @param $newOwnerId + * @return void + */ + public function transferOwnership($ownerId, $newOwnerId) { + $params = [ + 'owner' => $ownerId, + 'newOwner' => $newOwnerId + ]; + $sql = "UPDATE `{$this->tableName}` SET `owner` = :newOwner WHERE `owner` = :owner"; + $stmt = $this->execute($sql, $params); + $stmt->closeCursor(); + } } diff --git a/lib/Service/BoardService.php b/lib/Service/BoardService.php index 868164b91..e9c01d680 100644 --- a/lib/Service/BoardService.php +++ b/lib/Service/BoardService.php @@ -29,6 +29,7 @@ use OCA\Deck\Db\Acl; use OCA\Deck\Db\AclMapper; use OCA\Deck\Db\AssignedUsersMapper; +use OCA\Deck\Db\CardMapper; use OCA\Deck\Db\ChangeHelper; use OCA\Deck\Db\IPermissionMapper; use OCA\Deck\Db\Label; @@ -63,6 +64,7 @@ class BoardService { /** @var EventDispatcherInterface */ private $eventDispatcher; private $changeHelper; + private $cardMapper; public function __construct( BoardMapper $boardMapper, @@ -73,6 +75,7 @@ public function __construct( PermissionService $permissionService, NotificationHelper $notificationHelper, AssignedUsersMapper $assignedUsersMapper, + CardMapper $cardMapper, IUserManager $userManager, IGroupManager $groupManager, ActivityManager $activityManager, @@ -94,6 +97,7 @@ public function __construct( $this->eventDispatcher = $eventDispatcher; $this->changeHelper = $changeHelper; $this->userId = $userId; + $this->cardMapper = $cardMapper; } /** @@ -669,6 +673,18 @@ public function clone($id, $userId) { return $newBoard; } + /** + * @param $ownerId + * @param $newOwnerId + * @return void + */ + public function transferOwnership($owner, $newOwner) { + $this->boardMapper->transferOwnership($owner, $newOwner); + $this->aclMapper->transferOwnership($owner, $newOwner); + $this->assignedUsersMapper->transferOwnership($owner, $newOwner); + $this->cardMapper->transferOwnership($owner, $newOwner); + } + private function enrichWithStacks($board, $since = -1) { $stacks = $this->stackMapper->findAll($board->getId(), null, null, $since); diff --git a/tests/integration/config/behat.yml b/tests/integration/config/behat.yml index f557adde8..5006985ec 100644 --- a/tests/integration/config/behat.yml +++ b/tests/integration/config/behat.yml @@ -5,8 +5,8 @@ default: - '%paths.base%/../features/' contexts: - FeatureContext: - baseUrl: http://localhost:8080/index.php/ocs/ + baseUrl: http://localhost:9090/index.php/ocs/ admin: - admin - admin - regular_user_password: 123456 \ No newline at end of file + regular_user_password: 123456 diff --git a/tests/integration/database/TransferOwnershipTest.php b/tests/integration/database/TransferOwnershipTest.php new file mode 100644 index 000000000..582fca85d --- /dev/null +++ b/tests/integration/database/TransferOwnershipTest.php @@ -0,0 +1,206 @@ +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->assignedUsersMapper = \OC::$server->query(AssignedUsersMapper::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_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); + $this->board = $board; + $this->cards = $cards; + $this->stacks = $stacks; + } + + /** + * @covers ::transferOwnership + */ + public function testTransferBoardOwnership() { + $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 testTransferACLOwnership() { + $this->boardService->transferOwnership(self::TEST_USER_1, self::TEST_USER_2); + $board = $this->boardService->find($this->board->getId()); + $acl = $board->getAcl(); + $isTargetInAcl = (bool)array_filter($acl, function ($item) { + return $item->getParticipant() === self::TEST_USER_2 && $item->getType() === Acl::PERMISSION_TYPE_USER; + }); + $this->assertTrue($isTargetInAcl); + } + + /** + * @covers ::transferOwnership + */ + public function testNoTransferAclOwnershipIfGroupType() { + $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() { + $this->boardService->transferOwnership(self::TEST_USER_1, self::TEST_USER_2); + $card = $this->cardService->find($this->cards[0]->getId()); + $cardOwner = $card->getOwner(); + $this->assertEquals(self::TEST_USER_2, $cardOwner); + } + + /** + * @covers ::transferOwnership + */ + public function testReassignCardToNewOwner() { + $this->boardService->transferOwnership(self::TEST_USER_1, self::TEST_USER_2); + $assignedUsers = $this->assignedUsersMapper->find($this->cards[0]->getId()); + $participantsUIDs = []; + foreach ($assignedUsers as $user) { + $participantsUIDs[] = $user->getParticipant(); + } + $this->assertContains(self::TEST_USER_2, $participantsUIDs); + $this->assertNotContains(self::TEST_USER_1, $participantsUIDs); + } + + /** + * @covers ::transferOwnership + */ + public function testReassignCardToNewParticipantOnlyIfParticipantHasUserType() { + $this->assignmentService->assignUser($this->cards[1]->getId(), self::TEST_USER_1, AssignedUsers::TYPE_GROUP); + $this->boardService->transferOwnership(self::TEST_USER_1, self::TEST_USER_2); + $assignedUsers = $this->assignedUsersMapper->find($this->cards[1]->getId()); + $participantsUIDs = []; + foreach ($assignedUsers as $user) { + $participantsUIDs[] = $user->getParticipant(); + } + $this->assertContains(self::TEST_USER_1, $participantsUIDs); + $this->assertNotContains(self::TEST_USER_2, $participantsUIDs); + } + + /** + * @covers ::transferOwnership + */ + public function testTargetAlreadyParticipantOfBoard() { + $this->expectNotToPerformAssertions(); + $this->boardService->transferOwnership(self::TEST_USER_1, self::TEST_USER_3); + } + + /** + * @covers ::transferOwnership + */ + public function testDontRemoveTargetFromAcl() { + $this->boardService->transferOwnership(self::TEST_USER_2, self::TEST_USER_3); + $board = $this->boardService->find($this->board->getId()); + $acl = $board->getAcl(); + $isOwnerInAcl = (bool)array_filter($acl, function ($item) { + return $item->getParticipant() === self::TEST_USER_3 && $item->getType() === Acl::PERMISSION_TYPE_USER; + }); + $this->assertTrue($isOwnerInAcl); + } + + /** + * @covers ::transferOwnership + */ + public function testMergePermissions() { + $this->boardService->addAcl($this->board->getId(), Acl::PERMISSION_TYPE_USER, self::TEST_USER_2, true, false, true); + $this->boardService->transferOwnership(self::TEST_USER_2, 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, AssignedUsers::TYPE_USER); + $this->boardService->transferOwnership(self::TEST_USER_1, self::TEST_USER_3); + } + + 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 63a733723..17ad85028 100755 --- a/tests/integration/run.sh +++ b/tests/integration/run.sh @@ -28,7 +28,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/Service/BoardServiceTest.php b/tests/unit/Service/BoardServiceTest.php index 6553a6245..b6f22f7fd 100644 --- a/tests/unit/Service/BoardServiceTest.php +++ b/tests/unit/Service/BoardServiceTest.php @@ -31,6 +31,7 @@ use OCA\Deck\Db\AssignedUsersMapper; 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; @@ -56,6 +57,8 @@ class BoardServiceTest extends TestCase { private $boardMapper; /** @var StackMapper */ private $stackMapper; + /** @var CardMapper */ + private $cardMapper; /** @var PermissionService */ private $permissionService; /** @var NotificationHelper */ @@ -80,6 +83,7 @@ public function setUp(): void { $this->aclMapper = $this->createMock(AclMapper::class); $this->boardMapper = $this->createMock(BoardMapper::class); $this->stackMapper = $this->createMock(StackMapper::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); @@ -99,6 +103,7 @@ public function setUp(): void { $this->permissionService, $this->notificationHelper, $this->assignedUsersMapper, + $this->cardMapper, $this->userManager, $this->groupManager, $this->activityManager,