diff --git a/lib/Collaboration/Resources/ResourceProvider.php b/lib/Collaboration/Resources/ResourceProvider.php index 10daab298..f00a8c9ae 100644 --- a/lib/Collaboration/Resources/ResourceProvider.php +++ b/lib/Collaboration/Resources/ResourceProvider.php @@ -111,7 +111,7 @@ public function canAccessResource(IResource $resource, ?IUser $user): bool { private function getBoard(IResource $resource) { try { - return $this->boardMapper->find($resource->getId(), false, true); + return $this->boardMapper->find((int)$resource->getId(), false, true); } catch (DoesNotExistException $e) { } catch (MultipleObjectsReturnedException $e) { return null; diff --git a/lib/Db/BoardMapper.php b/lib/Db/BoardMapper.php index e4a04a9ac..6c2a79451 100644 --- a/lib/Db/BoardMapper.php +++ b/lib/Db/BoardMapper.php @@ -80,12 +80,14 @@ public function __construct( * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException * @throws DoesNotExistException */ - public function find($id, $withLabels = false, $withAcl = false): Board { + public function find(int $id, bool $withLabels = false, bool $withAcl = false, bool $allowDeleted = false): Board { if (!isset($this->boardCache[$id])) { $qb = $this->db->getQueryBuilder(); + $deletedWhere = $allowDeleted ? $qb->expr()->gte('deleted_at', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT)) : $qb->expr()->eq('deleted_at', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT)); $qb->select('*') ->from('deck_boards') ->where($qb->expr()->eq('id', $qb->createNamedParameter($id, IQueryBuilder::PARAM_INT))) + ->andWhere($deletedWhere) ->orderBy('id'); $this->boardCache[$id] = $this->findEntity($qb); } diff --git a/lib/Service/BoardService.php b/lib/Service/BoardService.php index fe00b9fc5..e0c8aa7fc 100644 --- a/lib/Service/BoardService.php +++ b/lib/Service/BoardService.php @@ -171,7 +171,7 @@ public function findAll(int $since = -1, bool $fullDetails = false, bool $includ * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException * @throws BadRequestException */ - public function find(int $boardId, bool $fullDetails = true): Board { + public function find(int $boardId, bool $fullDetails = true, bool $allowDeleted = false): Board { $this->boardServiceValidator->check(compact('boardId')); if (isset($this->boardsCacheFull[$boardId]) && $fullDetails) { @@ -184,7 +184,7 @@ public function find(int $boardId, bool $fullDetails = true): Board { $this->permissionService->checkPermission($this->boardMapper, $boardId, Acl::PERMISSION_READ); /** @var Board $board */ - $board = $this->boardMapper->find($boardId, true, true); + $board = $this->boardMapper->find($boardId, true, true, $allowDeleted); [$board] = $this->enrichBoards([$board], $fullDetails); return $board; } @@ -329,7 +329,7 @@ public function deleteUndo($id) { $this->boardServiceValidator->check(compact('id')); $this->permissionService->checkPermission($this->boardMapper, $id, Acl::PERMISSION_MANAGE); - $board = $this->find($id); + $board = $this->find($id, allowDeleted: true); $board->setDeletedAt(0); $board = $this->boardMapper->update($board); $this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_BOARD, $board, ActivityManager::SUBJECT_BOARD_RESTORE); @@ -350,7 +350,7 @@ public function deleteForce($id) { $this->boardServiceValidator->check(compact('id')); $this->permissionService->checkPermission($this->boardMapper, $id, Acl::PERMISSION_MANAGE); - $board = $this->find($id); + $board = $this->find($id, allowDeleted: true); $delete = $this->boardMapper->delete($board); return $delete; @@ -637,7 +637,7 @@ public function export($id) : Board { } $this->permissionService->checkPermission($this->boardMapper, $id, Acl::PERMISSION_READ); - $board = $this->boardMapper->find($id); + $board = $this->boardMapper->find((int)$id); $this->enrichWithCards($board); $this->enrichWithLabels($board); diff --git a/lib/Service/CardService.php b/lib/Service/CardService.php index 374111cb9..cc30b3f4a 100644 --- a/lib/Service/CardService.php +++ b/lib/Service/CardService.php @@ -298,6 +298,14 @@ public function update($id, $title, $stackId, $type, $owner, $description = '', if ($archived !== null && $card->getArchived() && $archived === true) { throw new StatusException('Operation not allowed. This card is archived.'); } + + if ($card->getDeletedAt() !== 0) { + if ($deletedAt === null) { + // Only allow operations when restoring the card + throw new StatusException('Operation not allowed. This card was deleted.'); + } + } + $changes = new ChangeSet($card); if ($card->getLastEditor() !== $this->currentUser && $card->getLastEditor() !== null) { $this->activityManager->triggerEvent( diff --git a/lib/Service/PermissionService.php b/lib/Service/PermissionService.php index c1c0e9eaa..64c4305a8 100644 --- a/lib/Service/PermissionService.php +++ b/lib/Service/PermissionService.php @@ -187,11 +187,11 @@ public function userIsBoardOwner($boardId, $userId = null) { * @throws MultipleObjectsReturnedException * @throws DoesNotExistException */ - private function getBoard($boardId): Board { - if (!isset($this->boardCache[$boardId])) { - $this->boardCache[$boardId] = $this->boardMapper->find($boardId, false, true); + private function getBoard(int $boardId): Board { + if (!isset($this->boardCache[(string)$boardId])) { + $this->boardCache[(string)$boardId] = $this->boardMapper->find($boardId, false, true); } - return $this->boardCache[$boardId]; + return $this->boardCache[(string)$boardId]; } /** diff --git a/tests/unit/Activity/ActivityManagerTest.php b/tests/unit/Activity/ActivityManagerTest.php index 700d72b19..0e277f35c 100644 --- a/tests/unit/Activity/ActivityManagerTest.php +++ b/tests/unit/Activity/ActivityManagerTest.php @@ -131,6 +131,7 @@ private function expectEventCreation($subject, $subjectParams) { public function testCreateEvent() { $board = new Board(); + $board->setId(123); $board->setTitle(''); $this->boardMapper->expects(self::once()) ->method('find') @@ -148,6 +149,7 @@ public function testCreateEvent() { public function testCreateEventDescription() { $board = new Board(); + $board->setId(123); $board->setTitle(''); $this->boardMapper->expects(self::once()) ->method('find') @@ -162,7 +164,9 @@ public function testCreateEventDescription() { ->method('find') ->willReturn($card); - $stack = Stack::fromRow([]); + $stack = Stack::fromRow([ + 'boardId' => 123, + ]); $this->stackMapper->expects(self::any()) ->method('find') ->willReturn($stack); @@ -192,6 +196,7 @@ public function testCreateEventDescription() { public function testCreateEventLongDescription() { $board = new Board(); + $board->setId(123); $board->setTitle(''); $this->boardMapper->expects(self::once()) ->method('find') @@ -205,7 +210,9 @@ public function testCreateEventLongDescription() { ->method('find') ->willReturn($card); - $stack = new Stack(); + $stack = Stack::fromRow([ + 'boardId' => 123, + ]); $this->stackMapper->expects(self::any()) ->method('find') ->willReturn($stack); @@ -235,6 +242,7 @@ public function testCreateEventLongDescription() { public function testCreateEventLabel() { $board = Board::fromRow([ + 'id' => 123, 'title' => 'My board' ]); $this->boardMapper->expects(self::once()) @@ -249,7 +257,9 @@ public function testCreateEventLabel() { ->method('find') ->willReturn($card); - $stack = Stack::fromParams([]); + $stack = Stack::fromRow([ + 'boardId' => 123, + ]); $this->stackMapper->expects(self::any()) ->method('find') ->willReturn($stack);