diff --git a/lib/Db/AclMapper.php b/lib/Db/AclMapper.php index 42aec5c4a..d34738d82 100644 --- a/lib/Db/AclMapper.php +++ b/lib/Db/AclMapper.php @@ -64,33 +64,33 @@ class AclMapper extends DeckMapper implements IPermissionMapper { * @return void */ public function transferOwnership($ownerId, $newOwnerId) { - $params = [ - 'owner' => $ownerId, - 'newOwner' => $newOwnerId, - 'type' => Acl::PERMISSION_TYPE_USER - ]; + $params = [ + 'owner' => $ownerId, + 'newOwner' => $newOwnerId, + 'type' => Acl::PERMISSION_TYPE_USER + ]; //We want preserve permissions from both users - $sql = "UPDATE `{$this->tableName}` AS `source` + $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}` + $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}` + $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(); + $stmt = $this->execute($sqlUpdate, $params); + $stmt->closeCursor(); } } diff --git a/lib/Db/AssignmentMapper.php b/lib/Db/AssignmentMapper.php index d783cdedb..1c5e43819 100644 --- a/lib/Db/AssignmentMapper.php +++ b/lib/Db/AssignmentMapper.php @@ -155,16 +155,16 @@ class AssignmentMapper extends QBMapper implements IPermissionMapper { public function transferOwnership($ownerId, $newOwnerId) { $params = [ 'newOwner' => $newOwnerId, - 'type' => AssignedUsers::TYPE_USER + '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 = "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/Service/BoardService.php b/lib/Service/BoardService.php index bbcc351e8..89652f286 100644 --- a/lib/Service/BoardService.php +++ b/lib/Service/BoardService.php @@ -685,9 +685,9 @@ class BoardService { */ 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); + $this->aclMapper->transferOwnership($owner, $newOwner); + $this->assignedUsersMapper->transferOwnership($owner, $newOwner); + $this->cardMapper->transferOwnership($owner, $newOwner); } private function enrichWithStacks($board, $since = -1) { diff --git a/tests/integration/database/TransferOwnershipTest.php b/tests/integration/database/TransferOwnershipTest.php index 459615860..582fca85d 100644 --- a/tests/integration/database/TransferOwnershipTest.php +++ b/tests/integration/database/TransferOwnershipTest.php @@ -66,7 +66,7 @@ class TransferOwnershipTest extends \Test\TestCase { $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 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); @@ -136,68 +136,68 @@ class TransferOwnershipTest extends \Test\TestCase { $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 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 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 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 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); - } + /** + * @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()); diff --git a/tests/unit/Service/BoardServiceTest.php b/tests/unit/Service/BoardServiceTest.php index c64422ff5..600f6a37c 100644 --- a/tests/unit/Service/BoardServiceTest.php +++ b/tests/unit/Service/BoardServiceTest.php @@ -59,8 +59,8 @@ class BoardServiceTest extends TestCase { private $boardMapper; /** @var StackMapper */ private $stackMapper; - /** @var CardMapper */ - private $cardMapper; + /** @var CardMapper */ + private $cardMapper; /** @var PermissionService */ private $permissionService; /** @var NotificationHelper */