Skip to content

Commit

Permalink
Merge pull request #5296 from nextcloud/bugfix/noid/deleted-board-fro…
Browse files Browse the repository at this point in the history
…ntend
  • Loading branch information
juliushaertl committed Nov 20, 2023
2 parents 1e5b4fe + 3c047d3 commit ebdbe65
Show file tree
Hide file tree
Showing 8 changed files with 47 additions and 27 deletions.
2 changes: 1 addition & 1 deletion lib/Collaboration/Resources/ResourceProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 3 additions & 1 deletion lib/Db/BoardMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
10 changes: 5 additions & 5 deletions lib/Service/BoardService.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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;
}
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -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);

Expand Down
8 changes: 8 additions & 0 deletions lib/Service/CardService.php
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,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 @@ -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];
}

/**
Expand Down
23 changes: 11 additions & 12 deletions src/components/board/Board.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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 />
Expand Down Expand Up @@ -75,11 +80,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>
Expand Down Expand Up @@ -147,12 +147,6 @@ export default {
},
watch: {
id(newValue, oldValue) {
if (this.session) {
// close old session
this.session.close()
}
this.session = createSession(newValue)
this.fetchData()
},
showArchived() {
Expand All @@ -172,11 +166,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 }) {
Expand Down
3 changes: 2 additions & 1 deletion src/components/cards/CardMenuEntries.vue
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,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))
},
changeCardDoneStatus() {
this.$store.dispatch('changeCardDoneStatus', { ...this.card, done: !this.card.done })
Expand Down
16 changes: 13 additions & 3 deletions tests/unit/Activity/ActivityManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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')
Expand All @@ -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);
Expand Down Expand Up @@ -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')
Expand All @@ -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);
Expand Down Expand Up @@ -235,6 +242,7 @@ public function testCreateEventLongDescription() {

public function testCreateEventLabel() {
$board = Board::fromRow([
'id' => 123,
'title' => 'My board'
]);
$this->boardMapper->expects(self::once())
Expand All @@ -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);
Expand Down

0 comments on commit ebdbe65

Please sign in to comment.