ahtlon/deck
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

Transfer deck ownership even if target user already participant of a board

Browse Source 
https://github.com/nextcloud/deck/pull/1955#issuecomment-640392715
Signed-off-by: Sergey Shliakhov <husband.sergey@gmail.com>
This commit is contained in:
Sergey Shliakhov
2020-07-18 07:40:47 +03:00
committed by Julius Härtl
parent a0f93a81d2
commit bdf4631504
4 changed files with 113 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files

36
lib/Db/AclMapper.php
View File

@@ -64,13 +64,33 @@ class AclMapper extends DeckMapper implements IPermissionMapper {
* @return void * @return void
*/ */
public function transferOwnership($ownerId, $newOwnerId) { public function transferOwnership($ownerId, $newOwnerId) {
$params = [ $params = [
'owner' => $ownerId, 'owner' => $ownerId,
'newOwner' => $newOwnerId, 'newOwner' => $newOwnerId,
'type' => Acl::PERMISSION_TYPE_USER 'type' => Acl::PERMISSION_TYPE_USER
]; ];
$sql = "UPDATE `{$this->tableName}` SET `participant` = :newOwner WHERE `participant` = :owner AND `type` = :type"; //We want preserve permissions from both users
$stmt = $this->execute($sql, $params); $sql = "UPDATE `{$this->tableName}` AS `source`
$stmt->closeCursor(); 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();
} }
} }

9
lib/Db/AssignmentMapper.php
View File

@@ -154,10 +154,17 @@ class AssignmentMapper extends QBMapper implements IPermissionMapper {
*/ */
public function transferOwnership($ownerId, $newOwnerId) { public function transferOwnership($ownerId, $newOwnerId) {
$params = [ $params = [
'owner' => $ownerId,
'newOwner' => $newOwnerId, '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 = "UPDATE `{$this->tableName}` SET `participant` = :newOwner WHERE `participant` = :owner AND `type`= :type"; $sql = "UPDATE `{$this->tableName}` SET `participant` = :newOwner WHERE `participant` = :owner AND `type`= :type";
$stmt = $this->execute($sql, $params); $stmt = $this->execute($sql, $params);
$stmt->closeCursor(); $stmt->closeCursor();

6
lib/Service/BoardService.php
View File

@@ -682,9 +682,9 @@ class BoardService {
*/ */
public function transferOwnership($owner, $newOwner) { public function transferOwnership($owner, $newOwner) {
$this->boardMapper->transferOwnership($owner, $newOwner); $this->boardMapper->transferOwnership($owner, $newOwner);
$this->assignedUsersMapper->transferOwnership($owner, $newOwner); $this->aclMapper->transferOwnership($owner, $newOwner);
$this->aclMapper->transferOwnership($owner, $newOwner); $this->assignedUsersMapper->transferOwnership($owner, $newOwner);
$this->cardMapper->transferOwnership($owner, $newOwner); $this->cardMapper->transferOwnership($owner, $newOwner);
} }
private function enrichWithStacks($board, $since = -1) { private function enrichWithStacks($board, $since = -1) {

115
tests/integration/database/TransferOwnershipTest.php
View File

@@ -3,6 +3,7 @@
namespace OCA\Deck\Service; namespace OCA\Deck\Service;
use OCA\Deck\Db\Acl; use OCA\Deck\Db\Acl;
use OCA\Deck\Db\AssignedUsers;
use OCA\Deck\Db\AssignedUsersMapper; use OCA\Deck\Db\AssignedUsersMapper;
use OCA\Deck\Db\Board; use OCA\Deck\Db\Board;
@@ -11,10 +12,9 @@ use OCA\Deck\Db\Board;
* @coversDefaultClass OCA\Deck\Service\BoardService * @coversDefaultClass OCA\Deck\Service\BoardService
*/ */
class TransferOwnershipTest extends \Test\TestCase { class TransferOwnershipTest extends \Test\TestCase {
private const TEST_OWNER = 'test-share-user1'; private const TEST_USER_1 = 'test-share-user1';
private const TEST_NEW_OWNER = 'target'; private const TEST_USER_2 = 'test-user2';
private const TEST_NEW_OWNER_PARTICIPANT = 'target-participant'; private const TEST_USER_3 = 'test-user3';
private const TEST_NEW_OWNER_IN_ACL = 'target-in-acl';
private const TEST_GROUP = 'test-share-user1'; private const TEST_GROUP = 'test-share-user1';
/** @var BoardService */ /** @var BoardService */
@@ -38,19 +38,19 @@ class TransferOwnershipTest extends \Test\TestCase {
$backend = new \Test\Util\User\Dummy(); $backend = new \Test\Util\User\Dummy();
\OC_User::useBackend($backend); \OC_User::useBackend($backend);
\OC::$server->getUserManager()->registerBackend($backend); \OC::$server->getUserManager()->registerBackend($backend);
$backend->createUser(self::TEST_OWNER, self::TEST_OWNER); $backend->createUser(self::TEST_USER_1, self::TEST_USER_1);
$backend->createUser(self::TEST_NEW_OWNER, self::TEST_NEW_OWNER); $backend->createUser(self::TEST_USER_2, self::TEST_USER_2);
$backend->createUser(self::TEST_NEW_OWNER_PARTICIPANT, self::TEST_NEW_OWNER_PARTICIPANT); $backend->createUser(self::TEST_USER_3, self::TEST_USER_3);
// create group // create group
$groupBackend = new \Test\Util\Group\Dummy(); $groupBackend = new \Test\Util\Group\Dummy();
$groupBackend->createGroup(self::TEST_GROUP); $groupBackend->createGroup(self::TEST_GROUP);
$groupBackend->addToGroup(self::TEST_OWNER, self::TEST_GROUP); $groupBackend->addToGroup(self::TEST_USER_1, self::TEST_GROUP);
\OC::$server->getGroupManager()->addBackend($groupBackend); \OC::$server->getGroupManager()->addBackend($groupBackend);
} }
public function setUp(): void { public function setUp(): void {
parent::setUp(); parent::setUp();
\OC::$server->getUserSession()->login(self::TEST_OWNER, self::TEST_OWNER); \OC::$server->getUserSession()->login(self::TEST_USER_1, self::TEST_USER_1);
$this->boardService = \OC::$server->query(BoardService::class); $this->boardService = \OC::$server->query(BoardService::class);
$this->stackService = \OC::$server->query(StackService::class); $this->stackService = \OC::$server->query(StackService::class);
$this->cardService = \OC::$server->query(CardService::class); $this->cardService = \OC::$server->query(CardService::class);
@@ -61,17 +61,17 @@ class TransferOwnershipTest extends \Test\TestCase {
public function createBoardWithExampleData() { public function createBoardWithExampleData() {
$stacks = []; $stacks = [];
$board = $this->boardService->create('Test', self::TEST_OWNER, '000000'); $board = $this->boardService->create('Test', self::TEST_USER_1, '000000');
$id = $board->getId(); $id = $board->getId();
$this->boardService->addAcl($id, Acl::PERMISSION_TYPE_USER, self::TEST_OWNER, true, true, true); $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_GROUP, self::TEST_GROUP, true, true, true);
$stacks[] = $this->stackService->create('Stack A', $id, 1); $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 B', $id, 1);
$stacks[] = $this->stackService->create('Stack C', $id, 1); $stacks[] = $this->stackService->create('Stack C', $id, 1);
$cards[] = $this->cardService->create('Card 1', $stacks[0]->getId(), 'text', 0, self::TEST_OWNER); $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_OWNER); $cards[] = $this->cardService->create('Card 2', $stacks[0]->getId(), 'text', 0, self::TEST_USER_1);
$this->assignmentService->assignUser($cards[0]->getId(), self::TEST_OWNER); $this->assignmentService->assignUser($cards[0]->getId(), self::TEST_USER_1);
$this->assignmentService->assignUser($cards[0]->getId(), self::TEST_NEW_OWNER_PARTICIPANT);
$this->board = $board; $this->board = $board;
$this->cards = $cards; $this->cards = $cards;
$this->stacks = $stacks; $this->stacks = $stacks;
@@ -81,21 +81,21 @@ class TransferOwnershipTest extends \Test\TestCase {
* @covers ::transferOwnership * @covers ::transferOwnership
*/ */
public function testTransferBoardOwnership() { public function testTransferBoardOwnership() {
$this->boardService->transferOwnership(self::TEST_OWNER, self::TEST_NEW_OWNER); $this->boardService->transferOwnership(self::TEST_USER_1, self::TEST_USER_2);
$board = $this->boardService->find($this->board->getId()); $board = $this->boardService->find($this->board->getId());
$boardOwner = $board->getOwner(); $boardOwner = $board->getOwner();
$this->assertEquals(self::TEST_NEW_OWNER, $boardOwner); $this->assertEquals(self::TEST_USER_2, $boardOwner);
} }
/** /**
* @covers ::transferOwnership * @covers ::transferOwnership
*/ */
public function testTransferACLOwnership() { public function testTransferACLOwnership() {
$this->boardService->transferOwnership(self::TEST_OWNER, self::TEST_NEW_OWNER); $this->boardService->transferOwnership(self::TEST_USER_1, self::TEST_USER_2);
$board = $this->boardService->find($this->board->getId()); $board = $this->boardService->find($this->board->getId());
$acl = $board->getAcl(); $acl = $board->getAcl();
$isTargetInAcl = (bool)array_filter($acl, function ($item) { $isTargetInAcl = (bool)array_filter($acl, function ($item) {
return $item->getParticipant() === self::TEST_NEW_OWNER && $item->getType() === Acl::PERMISSION_TYPE_USER; return $item->getParticipant() === self::TEST_USER_2 && $item->getType() === Acl::PERMISSION_TYPE_USER;
}); });
$this->assertTrue($isTargetInAcl); $this->assertTrue($isTargetInAcl);
} }
@@ -104,7 +104,7 @@ class TransferOwnershipTest extends \Test\TestCase {
* @covers ::transferOwnership * @covers ::transferOwnership
*/ */
public function testNoTransferAclOwnershipIfGroupType() { public function testNoTransferAclOwnershipIfGroupType() {
$this->boardService->transferOwnership(self::TEST_OWNER, self::TEST_NEW_OWNER); $this->boardService->transferOwnership(self::TEST_USER_1, self::TEST_USER_2);
$board = $this->boardService->find($this->board->getId()); $board = $this->boardService->find($this->board->getId());
$acl = $board->getAcl(); $acl = $board->getAcl();
$isGroupInAcl = (bool)array_filter($acl, function ($item) { $isGroupInAcl = (bool)array_filter($acl, function ($item) {
@@ -116,54 +116,87 @@ class TransferOwnershipTest extends \Test\TestCase {
* @covers ::transferOwnership * @covers ::transferOwnership
*/ */
public function testTransferCardOwnership() { public function testTransferCardOwnership() {
$this->boardService->transferOwnership(self::TEST_OWNER, self::TEST_NEW_OWNER); $this->boardService->transferOwnership(self::TEST_USER_1, self::TEST_USER_2);
$card = $this->cardService->find($this->cards[0]->getId()); $card = $this->cardService->find($this->cards[0]->getId());
$cardOwner = $card->getOwner(); $cardOwner = $card->getOwner();
$this->assertEquals(self::TEST_NEW_OWNER, $cardOwner); $this->assertEquals(self::TEST_USER_2, $cardOwner);
} }
/** /**
* @covers ::transferOwnership * @covers ::transferOwnership
*/ */
public function testReassignCardToNewOwner() { public function testReassignCardToNewOwner() {
$this->boardService->transferOwnership(self::TEST_OWNER, self::TEST_NEW_OWNER); $this->boardService->transferOwnership(self::TEST_USER_1, self::TEST_USER_2);
$assignedUsers = $this->assignedUsersMapper->find($this->cards[0]->getId()); $assignedUsers = $this->assignedUsersMapper->find($this->cards[0]->getId());
$participantsUIDs = []; $participantsUIDs = [];
foreach ($assignedUsers as $user) { foreach ($assignedUsers as $user) {
$participantsUIDs[] = $user->getParticipant(); $participantsUIDs[] = $user->getParticipant();
} }
$this->assertContains(self::TEST_NEW_OWNER, $participantsUIDs); $this->assertContains(self::TEST_USER_2, $participantsUIDs);
$this->assertNotContains(self::TEST_OWNER, $participantsUIDs); $this->assertNotContains(self::TEST_USER_1, $participantsUIDs);
} }
/** /**
* @covers ::transferOwnership * @covers ::transferOwnership
*/ */
public function testReassignCardToNewParticipantOnlyIfParticipantHasUserType() { public function testReassignCardToNewParticipantOnlyIfParticipantHasUserType() {
$this->boardService->transferOwnership(self::TEST_OWNER, self::TEST_NEW_OWNER); $this->assignmentService->assignUser($this->cards[1]->getId(), self::TEST_USER_1, AssignedUsers::TYPE_GROUP);
$this->assignmentService->ass($this->cards[0]->getId(), self::TEST_OWNER); $this->boardService->transferOwnership(self::TEST_USER_1, self::TEST_USER_2);
$assignedUsers = $this->assignedUsersMapper->find($this->cards[0]->getId()); $assignedUsers = $this->assignedUsersMapper->find($this->cards[1]->getId());
$participantsUIDs = []; $participantsUIDs = [];
foreach ($assignedUsers as $user) { foreach ($assignedUsers as $user) {
$participantsUIDs[] = $user->getParticipant(); $participantsUIDs[] = $user->getParticipant();
} }
$this->assertContains(self::TEST_NEW_OWNER, $participantsUIDs); $this->assertContains(self::TEST_USER_1, $participantsUIDs);
$this->assertNotContains(self::TEST_OWNER, $participantsUIDs); $this->assertNotContains(self::TEST_USER_2, $participantsUIDs);
} }
/** /**
* @covers ::transferOwnership * @covers ::transferOwnership
*/ */
public function testTargetAlreadyParticipantOfTransferedCard() { public function testTargetAlreadyParticipantOfBoard() {
$this->boardService->transferOwnership(self::TEST_OWNER, self::TEST_NEW_OWNER_PARTICIPANT); $this->expectNotToPerformAssertions();
$this->assignmentService->assignUser($this->cards[0]->getId(), self::TEST_OWNER); $this->boardService->transferOwnership(self::TEST_USER_1, self::TEST_USER_3);
$assignedUsers = $this->assignedUsersMapper->find($this->cards[0]->getId()); }
$participantsUIDs = [];
foreach ($assignedUsers as $user) { /**
$participantsUIDs[] = $user->getParticipant(); * @covers ::transferOwnership
} */
$this->assertContains(self::TEST_NEW_OWNER, $participantsUIDs); public function testDontRemoveTargetFromAcl() {
$this->assertNotContains(self::TEST_OWNER, $participantsUIDs); $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 { public function tearDown(): void {