Allow transfer of single boards

Signed-off-by: Julius Härtl <jus@bitgrid.net>

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;
 	}

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,

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,

lib/Db/BoardMapper.php

@@ -477,25 +477,23 @@ class BoardMapper extends QBMapper implements IPermissionMapper {
 	}
 
 	/**
-	 * @param $ownerId
+	 * @throws \OCP\DB\Exception
-	 * @param $newOwnerId
-	 * @return void
 	 */
-	public function transferOwnership($ownerId, $newOwnerId) {
+	public function transferOwnership(string $ownerId, string $newOwnerId, $boardId = null): void {
-		$params = [
+		$qb = $this->db->getQueryBuilder();
-			'owner' => $ownerId,
+		$qb->update('deck_boards')
-			'newOwner' => $newOwnerId
+			->set('owner', $qb->createNamedParameter($newOwnerId, IQueryBuilder::PARAM_STR))
-		];
+			->where($qb->expr()->eq('owner', $qb->createNamedParameter($ownerId, IQueryBuilder::PARAM_STR)));
-		$sql = "UPDATE `*PREFIX*{$this->tableName}` SET `owner` = :newOwner WHERE `owner` = :owner";
+		if ($boardId !== null) {
-		$stmt = $this->db->executeQuery($sql, $params);
+			$qb->andWhere($qb->expr()->eq('id', $qb->createNamedParameter($boardId, IQueryBuilder::PARAM_INT)));
-		$stmt->closeCursor();
+		}
+		$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 {

lib/Db/CardMapper.php

@@ -587,12 +587,7 @@ class CardMapper extends QBMapper implements IPermissionMapper {
 		});
 	}
 
-	/**
+	public function transferOwnership(string $ownerId, string $newOwnerId, int $boardId = null): void {
-	 * @param $ownerId
-	 * @param $newOwnerId
-	 * @return void
-	 */
-	public function transferOwnership($ownerId, $newOwnerId) {
 		$params = [
 			'owner' => $ownerId,
 			'newOwner' => $newOwnerId

lib/Service/BoardService.php

@@ -681,16 +681,34 @@ 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);
+
+		// 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) {
 		$stacks = $this->stackMapper->findAll($board->getId(), null, null, $since);

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());
 
@@ -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
 	 */