diff --git a/docs/API.md b/docs/API.md index ba4328009..376720a50 100644 --- a/docs/API.md +++ b/docs/API.md @@ -959,6 +959,7 @@ For now only `deck_file` is supported as an attachment type. ### DELETE /boards/{boardId}/stacks/{stackId}/cards/{cardId}/attachments/{attachmentId} - Delete an attachment + #### Request parameters | Parameter | Type | Description | diff --git a/lib/Service/AttachmentService.php b/lib/Service/AttachmentService.php index 7ff427b5f..dc9cbd5f8 100644 --- a/lib/Service/AttachmentService.php +++ b/lib/Service/AttachmentService.php @@ -35,6 +35,7 @@ use OCA\Deck\InvalidAttachmentType; use OCA\Deck\NoPermissionException; use OCA\Deck\NotFoundException; use OCA\Deck\StatusException; +use OCP\AppFramework\Db\IMapperException; use OCP\AppFramework\Http\Response; use OCP\ICache; use OCP\ICacheFactory; @@ -320,14 +321,10 @@ class AttachmentService { * Either mark an attachment as deleted for later removal or just remove it depending * on the IAttachmentService implementation * - * @param $attachmentId - * @return \OCP\AppFramework\Db\Entity - * @throws \OCA\Deck\NoPermissionException - * @throws \OCP\AppFramework\Db\DoesNotExistException - * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException - * @throws BadRequestException + * @throws NoPermissionException + * @throws NotFoundException */ - public function delete($cardId, $attachmentId, $type = 'deck_file') { + public function delete(int $cardId, int $attachmentId, string $type = 'deck_file'): Attachment { try { $service = $this->getService($type); } catch (InvalidAttachmentType $e) { @@ -340,40 +337,32 @@ class AttachmentService { $attachment->setType($type); $attachment->setCardId($cardId); $service->extendData($attachment); - $service->delete($attachment); - $this->changeHelper->cardChanged($attachment->getCardId()); - $this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_CARD, $attachment, ActivityManager::SUBJECT_ATTACHMENT_DELETE); - return $attachment; + } else { + try { + $attachment = $this->attachmentMapper->find($attachmentId); + } catch (IMapperException $e) { + throw new NoPermissionException('Permission denied'); + } } - - try { - $attachment = $this->attachmentMapper->find($attachmentId); - } catch (\Exception $e) { - throw new NoPermissionException('Permission denied'); - } - $this->permissionService->checkPermission($this->cardMapper, $attachment->getCardId(), Acl::PERMISSION_EDIT); - $this->cache->clear('card-' . $attachment->getCardId()); if ($service->allowUndo()) { $service->markAsDeleted($attachment); - $this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_CARD, $attachment, ActivityManager::SUBJECT_ATTACHMENT_DELETE); - $this->changeHelper->cardChanged($attachment->getCardId()); - return $this->attachmentMapper->update($attachment); + $attachment = $this->attachmentMapper->update($attachment); + } else { + $service->delete($attachment); + if (!$service instanceof ICustomAttachmentService) { + $attachment = $this->attachmentMapper->delete($attachment); + } } - $service->delete($attachment); - $attachment = $this->attachmentMapper->delete($attachment); + $this->cache->clear('card-' . $attachment->getCardId()); $this->changeHelper->cardChanged($attachment->getCardId()); $this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_CARD, $attachment, ActivityManager::SUBJECT_ATTACHMENT_DELETE); return $attachment; } - public function restore($cardId, $attachmentId, $type = 'deck_file') { - if (is_numeric($attachmentId) === false) { - throw new BadRequestException('attachment id must be a number'); - } - + public function restore(int $cardId, int $attachmentId, string $type = 'deck_file'): Attachment { try { $attachment = $this->attachmentMapper->find($attachmentId); } catch (\Exception $e) { diff --git a/lib/Service/FilesAppService.php b/lib/Service/FilesAppService.php index 832565a27..2f978f1cf 100644 --- a/lib/Service/FilesAppService.php +++ b/lib/Service/FilesAppService.php @@ -23,7 +23,10 @@ namespace OCA\Deck\Service; +use OCA\Deck\Db\Acl; use OCA\Deck\Db\Attachment; +use OCA\Deck\Db\CardMapper; +use OCA\Deck\NoPermissionException; use OCA\Deck\Sharing\DeckShareProvider; use OCA\Deck\StatusException; use OCP\AppFramework\Http\StreamResponse; @@ -38,6 +41,7 @@ use OCP\IRequest; use OCP\Share\Exceptions\ShareNotFound; use OCP\Share\IManager; use OCP\Share\IShare; +use Psr\Log\LoggerInterface; class FilesAppService implements IAttachmentService, ICustomAttachmentService { private $request; @@ -48,8 +52,10 @@ class FilesAppService implements IAttachmentService, ICustomAttachmentService { private $configService; private $l10n; private $preview; - private $permissionService; private $mimeTypeDetector; + private $permissionService; + private $cardMapper; + private $logger; public function __construct( IRequest $request, @@ -59,8 +65,10 @@ class FilesAppService implements IAttachmentService, ICustomAttachmentService { ConfigService $configService, DeckShareProvider $shareProvider, IPreview $preview, - PermissionService $permissionService, IMimeTypeDetector $mimeTypeDetector, + PermissionService $permissionService, + CardMapper $cardMapper, + LoggerInterface $logger, string $userId = null ) { $this->request = $request; @@ -72,15 +80,20 @@ class FilesAppService implements IAttachmentService, ICustomAttachmentService { $this->userId = $userId; $this->preview = $preview; $this->mimeTypeDetector = $mimeTypeDetector; + $this->permissionService = $permissionService; + $this->cardMapper = $cardMapper; + $this->logger = $logger; } public function listAttachments(int $cardId): array { $shares = $this->shareProvider->getSharedWithByType($cardId, IShare::TYPE_DECK, -1, 0); - $shares = array_filter($shares, function ($share) { - return $share->getPermissions() > 0; - }); - return array_map(function (IShare $share) use ($cardId) { - $file = $share->getNode(); + return array_filter(array_map(function (IShare $share) use ($cardId) { + try { + $file = $share->getNode(); + } catch (NotFoundException $e) { + $this->logger->debug('Unable to find node for share with ID ' . $share->getId()); + return null; + } $attachment = new Attachment(); $attachment->setType('file'); $attachment->setId((int)$share->getId()); @@ -89,9 +102,9 @@ class FilesAppService implements IAttachmentService, ICustomAttachmentService { $attachment->setData($file->getName()); $attachment->setLastModified($file->getMTime()); $attachment->setCreatedAt($share->getShareTime()->getTimestamp()); - $attachment->setDeletedAt(0); + $attachment->setDeletedAt($share->getPermissions() === 0 ? $share->getShareTime()->getTimestamp() : 0); return $attachment; - }, $shares); + }, $shares)); } public function getAttachmentCount(int $cardId): int { @@ -144,6 +157,7 @@ class FilesAppService implements IAttachmentService, ICustomAttachmentService { } public function display(Attachment $attachment) { + // Problem: Folders /** @psalm-suppress InvalidCatch */ try { $share = $this->shareProvider->getShareById($attachment->getId()); @@ -165,6 +179,9 @@ class FilesAppService implements IAttachmentService, ICustomAttachmentService { $file = $this->getUploadedFile(); $fileName = $file['name']; + // get shares for current card + // check if similar filename already exists + $userFolder = $this->rootFolder->getUserFolder($this->userId); try { $folder = $userFolder->get($this->configService->getAttachmentFolder()); @@ -245,12 +262,16 @@ class FilesAppService implements IAttachmentService, ICustomAttachmentService { $file = $share->getNode(); $attachment->setData($file->getName()); - if ($file->getOwner() !== null && $file->getOwner()->getUID() === $this->userId) { - $file->delete(); + // Deleting a Nextcloud file attachment will remove the share to the card, keeping the source file untouched + // Opt-out of individual shares per user is no longer performed within deck but can still be done through the files app + $canEdit = $this->permissionService->checkPermission($this->cardMapper, $attachment->getCardId(), Acl::PERMISSION_EDIT); + $isFileOwner = $file->getOwner() !== null && $file->getOwner()->getUID() === $this->userId; + if ($isFileOwner || $canEdit) { + $this->shareManager->deleteShare($share); return; } - $this->shareManager->deleteFromSelf($share, $this->userId); + throw new NoPermissionException('No permission to remove the attachment from the card'); } public function allowUndo() { diff --git a/src/components/card/AttachmentList.vue b/src/components/card/AttachmentList.vue index cdcc738ac..b29d4af93 100644 --- a/src/components/card/AttachmentList.vue +++ b/src/components/card/AttachmentList.vue @@ -22,7 +22,7 @@