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

Cover case where the owner is preserved

Browse Source 
Signed-off-by: Julius Härtl <jus@bitgrid.net>
This commit is contained in:
Julius Härtl
2022-03-21 13:42:15 +01:00
parent eb69512b5f
commit 379f1144b3
4 changed files with 56 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
lib/Controller/BoardController.php
View File

@@ -24,6 +24,7 @@
namespace OCA\Deck\Controller; namespace OCA\Deck\Controller;
use OCA\Deck\Db\Acl; use OCA\Deck\Db\Acl;
use OCA\Deck\Db\Board;
use OCA\Deck\Service\BoardService; use OCA\Deck\Service\BoardService;
use OCA\Deck\Service\PermissionService; use OCA\Deck\Service\PermissionService;
use OCP\AppFramework\ApiController; use OCP\AppFramework\ApiController;
@@ -152,7 +153,7 @@ class BoardController extends ApiController {
/** /**
* @NoAdminRequired * @NoAdminRequired
* @param $boardId * @param $boardId
* @return \OCP\Deck\DB\Board * @return Board
*/ */
public function clone($boardId) { public function clone($boardId) {
return $this->boardService->clone($boardId, $this->userId); return $this->boardService->clone($boardId, $this->userId);

8
lib/Service/BoardService.php
View File

@@ -24,6 +24,7 @@
namespace OCA\Deck\Service; namespace OCA\Deck\Service;
use Doctrine\DBAL\Exception\UniqueConstraintViolationException;
use OCA\Deck\Activity\ActivityManager; use OCA\Deck\Activity\ActivityManager;
use OCA\Deck\Activity\ChangeSet; use OCA\Deck\Activity\ChangeSet;
use OCA\Deck\AppInfo\Application; use OCA\Deck\AppInfo\Application;
@@ -694,6 +695,11 @@ class BoardService {
if ($changeContent) { if ($changeContent) {
$this->assignedUsersMapper->remapAssignedUser($boardId, $previousOwner, $newOwner); $this->assignedUsersMapper->remapAssignedUser($boardId, $previousOwner, $newOwner);
$this->cardMapper->remapCardOwner($boardId, $previousOwner, $newOwner); $this->cardMapper->remapCardOwner($boardId, $previousOwner, $newOwner);
} else {
try {
$this->addAcl($boardId, Acl::PERMISSION_TYPE_USER, $previousOwner, true, true, true);
} catch (UniqueConstraintViolationException $e) {
}
} }
\OC::$server->getDatabaseConnection()->commit(); \OC::$server->getDatabaseConnection()->commit();
return $this->boardMapper->find($boardId); return $this->boardMapper->find($boardId);
@@ -706,9 +712,11 @@ class BoardService {
public function transferOwnership(string $owner, string $newOwner, bool $changeContent = false): \Generator { public function transferOwnership(string $owner, string $newOwner, bool $changeContent = false): \Generator {
$boards = $this->boardMapper->findAllByUser($owner); $boards = $this->boardMapper->findAllByUser($owner);
foreach ($boards as $board) { foreach ($boards as $board) {
if ($board->getOwner() === $owner) {
yield $this->transferBoardOwnership($board->getId(), $newOwner, $changeContent); yield $this->transferBoardOwnership($board->getId(), $newOwner, $changeContent);
} }
} }
}
private function enrichWithStacks($board, $since = -1) { private function enrichWithStacks($board, $since = -1) {
$stacks = $this->stackMapper->findAll($board->getId(), null, null, $since); $stacks = $this->stackMapper->findAll($board->getId(), null, null, $since);

2
src/components/board/SharingTabSidebar.vue
View File

@@ -53,7 +53,7 @@
<ActionCheckbox v-if="canManage" :checked="acl.permissionManage" @change="clickManageAcl(acl)"> <ActionCheckbox v-if="canManage" :checked="acl.permissionManage" @change="clickManageAcl(acl)">
{{ t('deck', 'Can manage') }} {{ t('deck', 'Can manage') }}
</ActionCheckbox> </ActionCheckbox>
<ActionCheckbox v-if="isCurrentUser(board.owner.uid)" :checked="acl.owner" @change="clickTransferOwner(acl.participant.uid)"> <ActionCheckbox v-if="acl.type === 0 && isCurrentUser(board.owner.uid)" :checked="acl.owner" @change="clickTransferOwner(acl.participant.uid)">
{{ t('deck', 'Owner') }} {{ t('deck', 'Owner') }}
</ActionCheckbox> </ActionCheckbox>
<ActionButton v-if="canManage" icon="icon-delete" @click="clickDeleteAcl(acl)"> <ActionButton v-if="canManage" icon="icon-delete" @click="clickDeleteAcl(acl)">

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

@@ -64,7 +64,6 @@ class TransferOwnershipTest extends \Test\TestCase {
$stacks = []; $stacks = [];
$board = $this->boardService->create('Test', self::TEST_USER_1, '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_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);
$this->boardService->addAcl($id, Acl::PERMISSION_TYPE_USER, self::TEST_USER_3, false, true, false); $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);
@@ -110,13 +109,20 @@ class TransferOwnershipTest extends \Test\TestCase {
* @covers ::transferOwnership * @covers ::transferOwnership
*/ */
public function testTransferACLOwnership() { public function testTransferACLOwnership() {
iterator_to_array($this->boardService->transferOwnership(self::TEST_USER_1, self::TEST_USER_2)); iterator_to_array($this->boardService->transferOwnership(self::TEST_USER_1, self::TEST_USER_2, true));
$board = $this->boardService->find($this->board->getId()); $board = $this->boardService->find($this->board->getId());
$acl = $board->getAcl(); $acl = $board->getAcl();
// Check if old owner is no longer in ACL $this->assertBoardDoesNotHaveAclUser($board, self::TEST_USER_1);
$this->assertTrue((bool)array_filter($acl, function ($item) { }
return $item->getParticipant() === self::TEST_USER_1 && $item->getType() === Acl::PERMISSION_TYPE_USER;
})); /**
* @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);
} }
/** /**
@@ -196,17 +202,41 @@ class TransferOwnershipTest extends \Test\TestCase {
iterator_to_array($this->boardService->transferOwnership(self::TEST_USER_1, self::TEST_USER_3)); 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 * @covers ::transferOwnership
*/ */
public function testDontRemoveTargetFromAcl() { public function testDontRemoveOldOwnerFromAcl() {
iterator_to_array($this->boardService->transferOwnership(self::TEST_USER_2, self::TEST_USER_3)); iterator_to_array($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();
$isOwnerInAcl = (bool)array_filter($acl, function ($item) { $this->assertBoardDoesNotHaveAclUser($board, self::TEST_USER_2);
return $item->getParticipant() === self::TEST_USER_3 && $item->getType() === Acl::PERMISSION_TYPE_USER; $this->assertBoardHasAclUser($board, self::TEST_USER_3);
}); $this->assertBoardHasAclUser($board, self::TEST_USER_1);
$this->assertTrue($isOwnerInAcl); }
/**
* @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);
} }
/** /**
@@ -214,7 +244,7 @@ class TransferOwnershipTest extends \Test\TestCase {
*/ */
public function testMergePermissions() { public function testMergePermissions() {
$this->boardService->addAcl($this->board->getId(), Acl::PERMISSION_TYPE_USER, self::TEST_USER_2, true, false, true); $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_2, self::TEST_USER_3)); iterator_to_array($this->boardService->transferOwnership(self::TEST_USER_1, self::TEST_USER_3));
$board = $this->boardService->find($this->board->getId()); $board = $this->boardService->find($this->board->getId());
$acl = $board->getAcl(); $acl = $board->getAcl();
$isMerged = (bool)array_filter($acl, function ($item) { $isMerged = (bool)array_filter($acl, function ($item) {