Skip to content

Commit

Permalink
Merge pull request #3298 from nextcloud/bugfix/attachment-delete
Browse files Browse the repository at this point in the history 
Delete file shares through attachments API
  • Loading branch information
@juliusknorr
juliusknorr authored Sep 7, 2021
2 parents b9758a7 + 13dcacc commit 6caa7bc
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 51 deletions.
1 change: 1 addition & 0 deletions docs/API.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down
47 changes: 18 additions & 29 deletions lib/Service/AttachmentService.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
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;
Expand Down Expand Up @@ -320,14 +321,10 @@ public function update($cardId, $attachmentId, $data, $type = 'deck_file') {
* 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) {
Expand All @@ -340,40 +337,32 @@ public function delete($cardId, $attachmentId, $type = 'deck_file') {
$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;
}

try {
$attachment = $this->attachmentMapper->find($attachmentId);
} catch (\Exception $e) {
throw new NoPermissionException('Permission denied');
} else {
try {
$attachment = $this->attachmentMapper->find($attachmentId);
} catch (IMapperException $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) {
Expand Down
45 changes: 33 additions & 12 deletions lib/Service/FilesAppService.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -38,6 +41,7 @@
use OCP\Share\Exceptions\ShareNotFound;
use OCP\Share\IManager;
use OCP\Share\IShare;
use Psr\Log\LoggerInterface;

class FilesAppService implements IAttachmentService, ICustomAttachmentService {
private $request;
Expand All @@ -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,
Expand All @@ -59,8 +65,10 @@ public function __construct(
ConfigService $configService,
DeckShareProvider $shareProvider,
IPreview $preview,
PermissionService $permissionService,
IMimeTypeDetector $mimeTypeDetector,
PermissionService $permissionService,
CardMapper $cardMapper,
LoggerInterface $logger,
string $userId = null
) {
$this->request = $request;
Expand All @@ -72,15 +80,20 @@ public function __construct(
$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());
Expand All @@ -89,9 +102,9 @@ public function listAttachments(int $cardId): array {
$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 {
Expand Down Expand Up @@ -144,6 +157,7 @@ public function extendData(Attachment $attachment) {
}

public function display(Attachment $attachment) {
// Problem: Folders
/** @psalm-suppress InvalidCatch */
try {
$share = $this->shareProvider->getShareById($attachment->getId());
Expand All @@ -165,6 +179,9 @@ public function create(Attachment $attachment) {
$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());
Expand Down Expand Up @@ -245,12 +262,16 @@ public function delete(Attachment $attachment) {
$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() {
Expand Down
29 changes: 19 additions & 10 deletions src/components/card/AttachmentList.vue
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@

<template>
<AttachmentDragAndDrop :card-id="cardId" class="drop-upload--sidebar">
<div class="button-group">
<div class="button-group" v-if="!isReadOnly">
<button class="icon-upload" @click="uploadNewFile()">
{{ t('deck', 'Upload new files') }}
</button>
Expand All @@ -49,31 +49,38 @@
</li>
<li v-for="attachment in attachments"
:key="attachment.id"
class="attachment">
class="attachment"
:class="{ 'attachment--deleted': attachment.deletedAt > 0 }">
<a class="fileicon"
:href="internalLink(attachment)"
:style="mimetypeForAttachment(attachment)"
@click.prevent="showViewer(attachment)" />
<div class="details">
<a @click.prevent="showViewer(attachment)">
<a :href="internalLink(attachment)" @click.prevent="showViewer(attachment)">
<div class="filename">
<span class="basename">{{ attachment.data }}</span>
</div>
<span class="filesize">{{ formattedFileSize(attachment.extendedData.filesize) }}</span>
<span class="filedate">{{ relativeDate(attachment.createdAt*1000) }}</span>
<span class="filedate">{{ attachment.createdBy }}</span>
<div v-if="attachment.deletedAt === 0">
<span class="filesize">{{ formattedFileSize(attachment.extendedData.filesize) }}</span>
<span class="filedate">{{ relativeDate(attachment.createdAt*1000) }}</span>
<span class="filedate">{{ attachment.createdBy }}</span>
</div>
<div v-else>
<span class="attachment--info">{{ t('deck', 'Pending share') }}</span>
</div>
</a>
</div>
<Actions v-if="selectable">
<ActionButton icon="icon-confirm" @click="$emit('select-attachment', attachment)">
{{ t('deck', 'Add this attachment') }}
</ActionButton>
</Actions>
<Actions v-if="removable" :force-menu="true">
<Actions v-if="removable && !isReadOnly" :force-menu="true">
<ActionLink v-if="attachment.extendedData.fileid" icon="icon-folder" :href="internalLink(attachment)">
{{ t('deck', 'Show in Files') }}
</ActionLink>
<ActionButton v-if="attachment.extendedData.fileid" icon="icon-delete" @click="unshareAttachment(attachment)">
{{ t('deck', 'Unshare file') }}
<ActionButton v-if="attachment.extendedData.fileid && !isReadOnly" icon="icon-delete" @click="unshareAttachment(attachment)">
{{ t('deck', 'Remove attachment') }}
</ActionButton>

<ActionButton v-if="!attachment.extendedData.fileid && attachment.deletedAt === 0" icon="icon-delete" @click="$emit('delete-attachment', attachment)">
Expand Down Expand Up @@ -143,6 +150,7 @@ export default {
},
computed: {
attachments() {
// FIXME sort propertly by last modified / deleted at
return [...this.$store.getters.attachmentsByCard(this.cardId)].filter(attachment => attachment.deletedAt >= 0).sort((a, b) => b.id - a.id)
},
mimetypeForAttachment() {
Expand Down Expand Up @@ -320,9 +328,10 @@ export default {
opacity: 0.7;
}
}
.attachment--info,
.filesize, .filedate {
font-size: 90%;
color: darkgray;
color: var(--color-text-maxcontrast);
}
.app-popover-menu-utils {
position: relative;
Expand Down

0 comments on commit 6caa7bc

Please sign in to comment.