Skip to content

Commit

Permalink
Merge pull request #5447 from nextcloud/backport/5446/stable23
Browse files Browse the repository at this point in the history
[stable23] Fix small issues around delete/undo
  • Loading branch information
juliusknorr authored Jan 9, 2024
2 parents b017f44 + eac673c commit 0b6df4b
Show file tree
Hide file tree
Showing 9 changed files with 26 additions and 53 deletions.
39 changes: 0 additions & 39 deletions .github/workflows/appbuild.yml

This file was deleted.

8 changes: 4 additions & 4 deletions .github/workflows/phpunit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ env:

jobs:
integration:
runs-on: ubuntu-18.04
runs-on: ubuntu-latest

strategy:
fail-fast: false
Expand Down Expand Up @@ -44,7 +44,7 @@ jobs:

steps:
- name: Checkout server
uses: actions/checkout@v2.4.0
uses: actions/checkout@v3
with:
repository: nextcloud/server
ref: ${{ matrix.server-versions }}
Expand All @@ -57,12 +57,12 @@ jobs:
git -c "http.extraheader=$auth_header" -c protocol.version=2 submodule update --init --force --recursive --depth=1
- name: Checkout app
uses: actions/checkout@v2.4.0
uses: actions/checkout@v3
with:
path: apps/${{ env.APP_NAME }}

- name: Set up php ${{ matrix.php-versions }}
uses: shivammathur/setup-php@2.15.0
uses: shivammathur/setup-php@2.18.0
with:
php-version: ${{ matrix.php-versions }}
tools: phpunit
Expand Down
2 changes: 1 addition & 1 deletion lib/Collaboration/Resources/ResourceProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,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;
Expand Down
4 changes: 3 additions & 1 deletion lib/Db/BoardMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -79,12 +79,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);
}
Expand Down
8 changes: 4 additions & 4 deletions lib/Service/BoardService.php
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ public function findAll($since = -1, $details = null, $includeArchived = true) {
* @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
* @throws BadRequestException
*/
public function find($boardId) {
public function find($boardId, bool $allowDeleted = false) {
$this->boardServiceValidator->check(compact('boardId'));
if ($this->boardsCache && isset($this->boardsCache[$boardId])) {
return $this->boardsCache[$boardId];
Expand All @@ -192,7 +192,7 @@ public function find($boardId) {

$this->permissionService->checkPermission($this->boardMapper, $boardId, Acl::PERMISSION_READ);
/** @var Board $board */
$board = $this->boardMapper->find($boardId, true, true);
$board = $this->boardMapper->find((int)$boardId, true, true, $allowDeleted);
$this->boardMapper->mapOwner($board);
if ($board->getAcl() !== null) {
foreach ($board->getAcl() as $acl) {
Expand Down Expand Up @@ -367,7 +367,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, true);
$board->setDeletedAt(0);
$board = $this->boardMapper->update($board);
$this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_BOARD, $board, ActivityManager::SUBJECT_BOARD_RESTORE);
Expand All @@ -388,7 +388,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, true);
$delete = $this->boardMapper->delete($board);

return $delete;
Expand Down
8 changes: 8 additions & 0 deletions lib/Service/CardService.php
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,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(
Expand Down
8 changes: 4 additions & 4 deletions lib/Service/PermissionService.php
Original file line number Diff line number Diff line change
Expand Up @@ -194,11 +194,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];
}

/**
Expand Down
1 change: 1 addition & 0 deletions tests/integration/base-query-count.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
68024
1 change: 1 addition & 0 deletions tests/unit/Activity/ActivityManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ public function testGetActivityFormatOwn() {

public function testCreateEvent() {
$board = new Board();
$board->setId(123);
$board->setTitle('');
$this->boardMapper->expects($this->once())
->method('find')
Expand Down

0 comments on commit 0b6df4b

Please sign in to comment.