diff --git a/lib/Command/TransferOwnership.php b/lib/Command/TransferOwnership.php index 9d3ae1b48..3b42132ca 100644 --- a/lib/Command/TransferOwnership.php +++ b/lib/Command/TransferOwnership.php @@ -4,17 +4,22 @@ namespace OCA\Deck\Command; use OCA\Deck\Service\BoardService; use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Helper\QuestionHelper; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; +use Symfony\Component\Console\Question\ConfirmationQuestion; final class TransferOwnership extends Command { protected $boardService; + protected $questionHelper; - public function __construct(BoardService $boardService) { + public function __construct(BoardService $boardService, QuestionHelper $questionHelper) { parent::__construct(); $this->boardService = $boardService; + $this->questionHelper = $questionHelper; } protected function configure() { @@ -30,18 +35,54 @@ final class TransferOwnership extends Command { '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'); - $output->writeln("Transfer deck boards from $owner to $newOwner"); + $remapAssignment = $input->getOption('remap'); - $this->boardService->transferOwnership($owner, $newOwner); + $board = $boardId ? $this->boardService->find($boardId) : null; - $output->writeln("Transfer deck boards from $owner to $newOwner completed"); + 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; + } + + $this->boardService->transferOwnership($owner, $newOwner, $remapAssignment); + $output->writeln("All boards from $owner to $newOwner transferred"); return 0; } diff --git a/lib/Db/AclMapper.php b/lib/Db/AclMapper.php index c45a2d421..bc7784bdc 100644 --- a/lib/Db/AclMapper.php +++ b/lib/Db/AclMapper.php @@ -63,7 +63,7 @@ class AclMapper extends DeckMapper implements IPermissionMapper { * @param $newOwnerId * @return void */ - public function transferOwnership($boardId, $ownerId, $newOwnerId) { + public function transferOwnership($boardId, $newOwnerId) { $params = [ 'newOwner' => $newOwnerId, 'type' => Acl::PERMISSION_TYPE_USER, diff --git a/lib/Db/AssignmentMapper.php b/lib/Db/AssignmentMapper.php index 222e556cc..06c37e24d 100644 --- a/lib/Db/AssignmentMapper.php +++ b/lib/Db/AssignmentMapper.php @@ -153,7 +153,7 @@ class AssignmentMapper extends QBMapper implements IPermissionMapper { * @param $newOwnerId * @return void */ - public function transferOwnership(string $ownerId, string $newOwnerId) { + public function transferOwnership(string $ownerId, string $newOwnerId, int $boardId = null) { $params = [ 'owner' => $ownerId, 'newOwner' => $newOwnerId, @@ -164,7 +164,7 @@ class AssignmentMapper extends QBMapper implements IPermissionMapper { (SELECT id FROM `*PREFIX*{$this->tableName}` WHERE `participant` = :owner)"; $stmt = $this->db->executeQuery($sql, $params); $stmt->closeCursor(); - + $sql = "UPDATE `*PREFIX*{$this->tableName}` SET `participant` = :newOwner WHERE `participant` = :owner AND `type`= :type"; $stmt = $this->db->executeQuery($sql, $params); $stmt->closeCursor(); diff --git a/lib/Db/BoardMapper.php b/lib/Db/BoardMapper.php index 81dc648bf..93be76d7e 100644 --- a/lib/Db/BoardMapper.php +++ b/lib/Db/BoardMapper.php @@ -477,25 +477,23 @@ class BoardMapper extends QBMapper implements IPermissionMapper { } /** - * @param $ownerId - * @param $newOwnerId - * @return void + * @throws \OCP\DB\Exception */ - public function transferOwnership($ownerId, $newOwnerId) { - $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 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) - { + public function flushCache(?int $boardId = null, ?string $userId = null) { if ($boardId) { unset($this->boardCache[$boardId]); } else { diff --git a/lib/Db/CardMapper.php b/lib/Db/CardMapper.php index 973a1c736..471e2f9d7 100644 --- a/lib/Db/CardMapper.php +++ b/lib/Db/CardMapper.php @@ -587,12 +587,7 @@ class CardMapper extends QBMapper implements IPermissionMapper { }); } - /** - * @param $ownerId - * @param $newOwnerId - * @return void - */ - public function transferOwnership($ownerId, $newOwnerId) { + public function transferOwnership(string $ownerId, string $newOwnerId, int $boardId = null): void { $params = [ 'owner' => $ownerId, 'newOwner' => $newOwnerId diff --git a/lib/Service/BoardService.php b/lib/Service/BoardService.php index ceea63519..7ac663db7 100644 --- a/lib/Service/BoardService.php +++ b/lib/Service/BoardService.php @@ -681,15 +681,33 @@ class BoardService { return $newBoard; } - public function transferOwnership(string $owner, string $newOwner): void { + public function transferBoardOwnership(int $boardId, string $newOwner, $changeContent = false): void { + $board = $this->boardMapper->find($boardId); + $previousOwner = $board->getOwner(); + $this->clearBoardFromCache($board); + $this->aclMapper->transferOwnership($boardId, $newOwner); + $this->boardMapper->transferOwnership($previousOwner, $newOwner, $boardId); + + // Optionally also change user assignments and card owner information + if ($changeContent) { + $this->assignedUsersMapper->transferOwnership($previousOwner, $newOwner, $boardId); + $this->cardMapper->transferOwnership($previousOwner, $newOwner, $boardId); + } + } + + public function transferOwnership(string $owner, string $newOwner, $changeContent = false): void { $boards = $this->boardMapper->findAllByUser($owner); foreach ($boards as $board) { $this->clearBoardFromCache($board); - $this->aclMapper->transferOwnership($board->getId(), $owner, $newOwner); + $this->aclMapper->transferOwnership($board->getId(), $newOwner); } $this->boardMapper->transferOwnership($owner, $newOwner); - $this->assignedUsersMapper->transferOwnership($owner, $newOwner); - $this->cardMapper->transferOwnership($owner, $newOwner); + + // Optionally also change user assignments and card owner information + if ($changeContent) { + $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 7ffb4d135..8b55642d3 100644 --- a/tests/integration/database/TransferOwnershipTest.php +++ b/tests/integration/database/TransferOwnershipTest.php @@ -10,7 +10,7 @@ use OCA\Deck\Db\Card; /** * @group DB - * @coversDefaultClass OCA\Deck\Service\BoardService + * @coversDefaultClass \OCA\Deck\Service\BoardService */ class TransferOwnershipTest extends \Test\TestCase { private const TEST_USER_1 = 'test-share-user1'; @@ -92,8 +92,7 @@ class TransferOwnershipTest extends \Test\TestCase { /** * @covers ::transferOwnership */ - public function testTransferBoardOwnershipWithData() - { + public function testTransferBoardOwnershipWithData() { $this->boardService->transferOwnership(self::TEST_USER_1, self::TEST_USER_2); $board = $this->boardService->find($this->board->getId()); @@ -105,7 +104,7 @@ class TransferOwnershipTest extends \Test\TestCase { $cardUpdated = $this->cardService->find($card->getId()); return $cardUpdated->getOwner() === self::TEST_USER_2; })); - $this->assertTrue($newOwnerOwnsTheCards); + $this->assertTrue($newOwnerOwnsTheCards); } /** @@ -137,17 +136,27 @@ class TransferOwnershipTest extends \Test\TestCase { * @covers ::transferOwnership */ public function testTransferCardOwnership() { - $this->boardService->transferOwnership(self::TEST_USER_1, self::TEST_USER_2); + $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() { + $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() { - $this->boardService->transferOwnership(self::TEST_USER_1, self::TEST_USER_2); + $this->boardService->transferOwnership(self::TEST_USER_1, self::TEST_USER_2, true); $users = $this->assignmentMapper->findAll($this->cards[0]->getId()); $participantsUIDs = []; foreach ($users as $user) { @@ -157,6 +166,20 @@ class TransferOwnershipTest extends \Test\TestCase { $this->assertNotContains(self::TEST_USER_1, $participantsUIDs); } + /** + * @covers ::transferOwnership + */ + public function testNoReassignCardToNewOwner() { + $this->boardService->transferOwnership(self::TEST_USER_1, self::TEST_USER_2, false); + $users = $this->assignmentMapper->findAll($this->cards[0]->getId()); + $participantsUIDs = []; + foreach ($users as $user) { + $participantsUIDs[] = $user->getParticipant(); + } + $this->assertContains(self::TEST_USER_1, $participantsUIDs); + $this->assertNotContains(self::TEST_USER_2, $participantsUIDs); + } + /** * @covers ::transferOwnership */ diff --git a/tests/unit/Service/AttachmentServiceTest.php b/tests/unit/Service/AttachmentServiceTest.php index b236c55d2..9d12d1ab3 100644 --- a/tests/unit/Service/AttachmentServiceTest.php +++ b/tests/unit/Service/AttachmentServiceTest.php @@ -240,7 +240,7 @@ class AttachmentServiceTest extends TestCase { $this->attachmentServiceImpl->expects($this->exactly(2)) ->method('extendData') ->withConsecutive( - [$attachments[0]], + [$attachments[0]], [$attachments[1]] ); @@ -277,7 +277,7 @@ class AttachmentServiceTest extends TestCase { $this->attachmentServiceImpl->expects($this->exactly(4)) ->method('extendData') ->withConsecutive( - [$attachments[0]], + [$attachments[0]], [$attachments[1]], [$attachmentsDeleted[0]], [$attachmentsDeleted[1]]