From a3336cb0a0d8b458308b0967c7e6f623e56f149c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Tue, 22 Mar 2022 08:21:18 +0100 Subject: [PATCH] Handle board exceptions more gracefully MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/Command/TransferOwnership.php | 18 ++++++++++++++++-- lib/Controller/BoardController.php | 4 ---- lib/Service/BoardService.php | 13 +++++++------ lib/Service/PermissionService.php | 9 +++++++++ 4 files changed, 32 insertions(+), 12 deletions(-) diff --git a/lib/Command/TransferOwnership.php b/lib/Command/TransferOwnership.php index 715200f75..95a4f65bb 100644 --- a/lib/Command/TransferOwnership.php +++ b/lib/Command/TransferOwnership.php @@ -2,7 +2,9 @@ namespace OCA\Deck\Command; +use OCA\Deck\Db\BoardMapper; use OCA\Deck\Service\BoardService; +use OCA\Deck\Service\PermissionService; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Helper\QuestionHelper; use Symfony\Component\Console\Input\InputArgument; @@ -13,12 +15,16 @@ use Symfony\Component\Console\Question\ConfirmationQuestion; final class TransferOwnership extends Command { protected $boardService; + protected $boardMapper; + protected $permissionService; protected $questionHelper; - public function __construct(BoardService $boardService, QuestionHelper $questionHelper) { + public function __construct(BoardService $boardService, BoardMapper $boardMapper, PermissionService $permissionService, QuestionHelper $questionHelper) { parent::__construct(); $this->boardService = $boardService; + $this->boardMapper = $boardMapper; + $this->permissionService = $permissionService; $this->questionHelper = $questionHelper; } @@ -57,7 +63,15 @@ final class TransferOwnership extends Command { $remapAssignment = $input->getOption('remap'); - $board = $boardId ? $this->boardService->find($boardId) : null; + $this->boardService->setUserId($owner); + $this->permissionService->setUserId($owner); + + try { + $board = $boardId ? $this->boardMapper->find($boardId) : null; + } catch (\Exception $e) { + $output->writeln("Could not find a board for the provided id."); + return 1; + } if ($boardId !== null && $board->getOwner() !== $owner) { $output->writeln("$owner is not the owner of the board $boardId (" . $board->getTitle() . ")"); diff --git a/lib/Controller/BoardController.php b/lib/Controller/BoardController.php index c5e2c62a0..0e909c305 100644 --- a/lib/Controller/BoardController.php +++ b/lib/Controller/BoardController.php @@ -161,10 +161,6 @@ class BoardController extends ApiController { /** * @NoAdminRequired - * @param $boardId - * @param $owner - * @param $newOwner - * * @return null|void */ public function transferOwner(int $boardId, string $newOwner): DataResponse { if ($this->permissionService->userIsBoardOwner($boardId, $this->userId)) { diff --git a/lib/Service/BoardService.php b/lib/Service/BoardService.php index ee0da6720..e1cfc8023 100644 --- a/lib/Service/BoardService.php +++ b/lib/Service/BoardService.php @@ -524,7 +524,7 @@ class BoardService { $acl->setPermissionManage($manage); $newAcl = $this->aclMapper->insert($acl); - $this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_BOARD, $newAcl, ActivityManager::SUBJECT_BOARD_SHARE); + $this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_BOARD, $newAcl, ActivityManager::SUBJECT_BOARD_SHARE, [], $this->userId); $this->notificationHelper->sendBoardShared((int)$boardId, $acl); $this->boardMapper->mapAcl($newAcl); $this->changeHelper->boardChanged($boardId); @@ -689,17 +689,18 @@ class BoardService { $previousOwner = $board->getOwner(); $this->clearBoardFromCache($board); $this->aclMapper->deleteParticipantFromBoard($boardId, Acl::PERMISSION_TYPE_USER, $newOwner); + if (!$changeContent) { + try { + $this->addAcl($boardId, Acl::PERMISSION_TYPE_USER, $previousOwner, true, true, true); + } catch (UniqueConstraintViolationException $e) { + } + } $this->boardMapper->transferOwnership($previousOwner, $newOwner, $boardId); // Optionally also change user assignments and card owner information if ($changeContent) { $this->assignedUsersMapper->remapAssignedUser($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(); return $this->boardMapper->find($boardId); diff --git a/lib/Service/PermissionService.php b/lib/Service/PermissionService.php index e94d073df..6777eb910 100644 --- a/lib/Service/PermissionService.php +++ b/lib/Service/PermissionService.php @@ -333,4 +333,13 @@ class PermissionService { } return $groups; } + + /** + * Set a different user than the current one, e.g. when no user is available in occ + * + * @param string $userId + */ + public function setUserId(string $userId): void { + $this->userId = $userId; + } }