ahtlon/deck
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

Merge pull request #5440 from nextcloud/backport/5296/stable25

Browse Source 
[stable25] Fix small issues around delete/undo
This commit is contained in:
Julius Härtl
2024-01-09 19:36:55 +01:00
committed by GitHub
parent 9b785502b5 23901ff0b0
commit 041f3e6eb8
8 changed files with 36 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
.github/workflows/phpunit.yml vendored
View File

@@ -70,11 +70,12 @@ jobs:
path: apps/${{ env.APP_NAME }}
- name: Set up php ${{ matrix.php-versions }}
uses: shivammathur/setup-php@2.21.2
uses: shivammathur/setup-php@2.24.0
with:
php-version: ${{ matrix.php-versions }}
tools: phpunit
extensions: zip, gd, mbstring, iconv, fileinfo, intl, sqlite, pdo_sqlite, mysql, pdo_mysql, pgsql, pdo_pgsql
ini-file: development
coverage: none
- name: Set up PHPUnit

2
lib/Collaboration/Resources/ResourceProvider.php
View File

@@ -111,7 +111,7 @@ class ResourceProvider implements IProvider {
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;

4
lib/Db/BoardMapper.php
View File

@@ -79,12 +79,14 @@ class BoardMapper extends QBMapper implements IPermissionMapper {
* @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);
}

8
lib/Service/BoardService.php
View File

@@ -183,7 +183,7 @@ class BoardService {
* @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];
@@ -194,7 +194,7 @@ class BoardService {
$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) {
@@ -369,7 +369,7 @@ class BoardService {
$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);
@@ -390,7 +390,7 @@ class BoardService {
$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;

8
lib/Service/CardService.php
View File

@@ -277,6 +277,14 @@ class CardService {
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(

8
lib/Service/PermissionService.php
View File

@@ -194,11 +194,11 @@ class PermissionService {
* @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];
}
/**

1
tests/integration/base-query-count.txt Normal file
View File

@@ -0,0 +1 @@
68024

16
tests/unit/Activity/ActivityManagerTest.php
View File

@@ -131,6 +131,7 @@ class ActivityManagerTest extends TestCase {
public function testCreateEvent() {
$board = new Board();
$board->setId(123);
$board->setTitle('');
$this->boardMapper->expects(self::once())
->method('find')
@@ -148,6 +149,7 @@ class ActivityManagerTest extends TestCase {
public function testCreateEventDescription() {
$board = new Board();
$board->setId(123);
$board->setTitle('');
$this->boardMapper->expects(self::once())
->method('find')
@@ -162,7 +164,9 @@ class ActivityManagerTest extends TestCase {
->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 @@ class ActivityManagerTest extends TestCase {
public function testCreateEventLongDescription() {
$board = new Board();
$board->setId(123);
$board->setTitle('');
$this->boardMapper->expects(self::once())
->method('find')
@@ -205,7 +210,9 @@ class ActivityManagerTest extends TestCase {
->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 @@ class ActivityManagerTest extends TestCase {
public function testCreateEventLabel() {
$board = Board::fromRow([
'id' => 123,
'title' => 'My board'
]);
$this->boardMapper->expects(self::once())
@@ -249,7 +257,9 @@ class ActivityManagerTest extends TestCase {
->method('find')
->willReturn($card);
$stack = Stack::fromParams([]);
$stack = Stack::fromRow([
'boardId' => 123,
]);
$this->stackMapper->expects(self::any())
->method('find')
->willReturn($stack);