Merge pull request #5420 from nextcloud/backport/5296/stable27

[stable27] Fix small issues around delete/undo

lib/Collaboration/Resources/ResourceProvider.php

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

lib/Db/BoardMapper.php

@@ -80,12 +80,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);
 		}

lib/Service/BoardService.php

@@ -171,7 +171,7 @@ class BoardService {
 	 * @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 @@ 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($boardId, true, true, $allowDeleted);
 		[$board] = $this->enrichBoards([$board], $fullDetails);
 		return $board;
 	}
@@ -329,7 +329,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, 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 @@ class BoardService {
 		$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 @@ class BoardService {
 		}
 
 		$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);
 

lib/Service/CardService.php

@@ -304,6 +304,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(

lib/Service/PermissionService.php

@@ -187,11 +187,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];
 	}
 
 	/**

src/components/board/Board.vue

@@ -30,6 +30,11 @@
 				<h2>{{ t('deck', 'Loading board') }}</h2>
 				<p />
 			</div>
+			<div v-else-if="!board" key="notfound" class="emptycontent">
+				<div class="icon icon-deck" />
+				<h2>{{ t('deck', 'Board not found') }}</h2>
+				<p />
+			</div>
 			<NcEmptyContent v-else-if="isEmpty" key="empty">
 				<template #icon>
 					<DeckIcon />
@@ -70,11 +75,6 @@
 					</Draggable>
 				</Container>
 			</div>
-			<div v-else key="notfound" class="emptycontent">
-				<div class="icon icon-deck" />
-				<h2>{{ t('deck', 'Board not found') }}</h2>
-				<p />
-			</div>
 		</transition>
 		<GlobalSearchResults />
 	</div>
@@ -140,12 +140,6 @@ export default {
 	},
 	watch: {
 		id(newValue, oldValue) {
-			if (this.session) {
-				// close old session
-				this.session.close()
-			}
-			this.session = createSession(newValue)
-
 			this.fetchData()
 		},
 		showArchived() {
@@ -165,11 +159,16 @@ export default {
 			try {
 				await this.$store.dispatch('loadBoardById', this.id)
 				await this.$store.dispatch('loadStacks', this.id)
+
+				this.session?.close()
+				this.session = createSession(this.id)
 			} catch (e) {
+				this.loading = false
 				console.error(e)
 				showError(e)
+			} finally {
+				this.loading = false
 			}
-			this.loading = false
 		},
 
 		onDropStack({ removedIndex, addedIndex }) {

src/components/cards/CardMenu.vue

@@ -163,7 +163,8 @@ export default {
 		},
 		deleteCard() {
 			this.$store.dispatch('deleteCard', this.card)
-			showUndo(t('deck', 'Card deleted'), () => this.$store.dispatch('cardUndoDelete', this.card))
+			const undoCard = { ...this.card, deletedAt: 0 }
+			showUndo(t('deck', 'Card deleted'), () => this.$store.dispatch('cardUndoDelete', undoCard))
 		},
 		archiveUnarchiveCard() {
 			this.$store.dispatch('archiveUnarchiveCard', { ...this.card, archived: !this.card.archived })

tests/unit/Activity/ActivityManagerTest.php

@@ -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);