Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(attachments): add support for creating new attachments #6676

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions appinfo/routes.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
['name' => 'Attachment#insertAttachmentFile', 'url' => '/attachment/filepath', 'verb' => 'POST'],
/** @see Controller\AttachmentController::uploadAttachment() */
['name' => 'Attachment#uploadAttachment', 'url' => '/attachment/upload', 'verb' => 'POST'],
/** @see Controller\AttachmentController::createAttachment() */
['name' => 'Attachment#createAttachment', 'url' => '/attachment/create', 'verb' => 'POST'],
/** @see Controller\AttachmentController::getImageFile() */
['name' => 'Attachment#getImageFile', 'url' => '/image', 'verb' => 'GET'],
/** @see Controller\AttachmentController::getMediaFile() */
Expand Down
14 changes: 14 additions & 0 deletions cypress/e2e/attachments.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const attachmentFileNameToId = {}

const ACTION_UPLOAD_LOCAL_FILE = 'insert-attachment-upload'
const ACTION_INSERT_FROM_FILES = 'insert-attachment-insert'
const ACTION_CREATE_NEW_TEXT_FILE = 'insert-attachment-add-text-0'

/**
* @param {string} name name of file
Expand Down Expand Up @@ -279,6 +280,19 @@ describe('Test all attachment insertion methods', () => {
cy.closeFile()
})

it('Create a new text file as an attachment', () => {
cy.visit('/apps/files')
cy.openFile('test.md')

cy.log('Create a new text file as an attachment')
const requestAlias = 'create-attachment-request'
cy.intercept({ method: 'POST', url: '**/text/attachment/create' }).as(requestAlias)
clickOnAttachmentAction(ACTION_CREATE_NEW_TEXT_FILE)
.then(() => {
return waitForRequestAndCheckAttachment(requestAlias, undefined, false)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is failing on CI right now. I suspect the dom structure of the text attachment is different from the image dom structure.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@max-nextcloud: Would you mind helping me with this issue? Most of the Cypress tests on my system are failing, and I currently don't have enough time to investigate this further.

})
})

it('test if attachment files are in the attachment folder', () => {
cy.visit('/apps/files')

Expand Down
24 changes: 24 additions & 0 deletions lib/Controller/AttachmentController.php
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,30 @@ public function uploadAttachment(string $token = ''): DataResponse {
}
}

#[NoAdminRequired]
#[PublicPage]
#[RequireDocumentSession]
public function createAttachment(string $token = ''): DataResponse {
$documentId = $this->getSession()->getDocumentId();
try {
$userId = $this->getSession()->getUserId();
$newFileName = $this->request->getParam('fileName', 'text.md');
$createResult = $this->attachmentService->createAttachmentFile($documentId, $newFileName, $userId);
if (isset($createResult['error'])) {
return new DataResponse($createResult, Http::STATUS_BAD_REQUEST);
} else {
return new DataResponse($createResult);
}
} catch (InvalidPathException $e) {
$this->logger->error('File creation error', ['exception' => $e]);
$error = $e->getMessage() ?: 'Upload error';
return new DataResponse(['error' => $error], Http::STATUS_BAD_REQUEST);
} catch (Exception $e) {
$this->logger->error('File creation error', ['exception' => $e]);
return new DataResponse(['error' => 'File creation error'], Http::STATUS_BAD_REQUEST);
}
}

private function getUploadedFile(string $key): array {
$file = $this->request->getUploadedFile($key);
$error = null;
Expand Down
66 changes: 58 additions & 8 deletions lib/Service/AttachmentService.php
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,35 @@ public function insertAttachmentFile(int $documentId, string $path, string $user
return $this->copyFile($originalFile, $saveDir, $textFile);
}

/**
* create a new file in the attachment folder
*
* @param int $documentId
* @param string $userId
*
* @return array
* @throws NotFoundException
* @throws NotPermittedException
* @throws InvalidPathException
* @throws NoUserException
*/
public function createAttachmentFile(int $documentId, string $newFileName, string $userId): array {
$textFile = $this->getTextFile($documentId, $userId);
if (!$textFile->isUpdateable()) {
throw new NotPermittedException('No write permissions');
}
$saveDir = $this->getAttachmentDirectoryForFile($textFile, true);
$fileName = self::getUniqueFileName($saveDir, $newFileName);
$newFile = $saveDir->newFile($fileName);
return [
'name' => $newFile->getName(),
'dirname' => $saveDir->getName(),
'id' => $newFile->getId(),
'documentId' => $textFile->getId(),
'mimetype' => $newFile->getMimetype(),
];
}

/**
* @param File $originalFile
* @param Folder $saveDir
Expand Down Expand Up @@ -552,23 +581,44 @@ public function cleanupAttachments(int $fileId): int {
// this only happens if the attachment dir was deleted by the user while editing the document
return 0;
}
$attachmentsByName = [];
foreach ($attachmentDir->getDirectoryListing() as $attNode) {
$attachmentsByName[$attNode->getName()] = $attNode;
}

$contentAttachmentFileIds = self::getAttachmentIdsFromContent($textFile->getContent());
$contentAttachmentNames = self::getAttachmentNamesFromContent($textFile->getContent(), $fileId);

$toDelete = array_diff(array_keys($attachmentsByName), $contentAttachmentNames);
foreach ($toDelete as $name) {
$attachmentsByName[$name]->delete();
$toDelete = array_filter($attachmentDir->getDirectoryListing(),
function ($node) use ($contentAttachmentFileIds, $contentAttachmentNames) {
return !in_array($node->getName(), $contentAttachmentNames) &&
!in_array($node->getId(), $contentAttachmentFileIds);
}
);
foreach ($toDelete as $node) {
$node->delete();
}
return count($toDelete);
}
}
return 0;
}

/**
* Get attachment file ids listed in the markdown file content
*
* @param string $content
*
* @return array
*/
public static function getAttachmentIdsFromContent(string $content): array {
$matches = [];
// matches [ANY_CONSIDERED_CORRECT_BY_PHP-MARKDOWN](ANY_URL/f/FILE_ID and captures FILE_ID
preg_match_all(
'/\[(?>[^\[\]]+|\[(?>[^\[\]]+|\[(?>[^\[\]]+|\[(?>[^\[\]]+|\[(?>[^\[\]]+|\[(?>[^\[\]]+|\[\])*\])*\])*\])*\])*\])*\]\(\S+\/f\/(\d+)/',
$content,
$matches,
PREG_SET_ORDER
);
return array_map(static function (array $match) {
return intval($match[1]);
}, $matches);
}

/**
* Get attachment file names listed in the markdown file content
Expand Down
7 changes: 7 additions & 0 deletions src/components/Editor/MediaHandler.provider.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
export const STATE_UPLOADING = Symbol('state:uploading-state')
export const ACTION_ATTACHMENT_PROMPT = Symbol('editor:action:attachment-prompt')
export const ACTION_CHOOSE_LOCAL_ATTACHMENT = Symbol('editor:action:upload-attachment')
export const ACTION_CREATE_ATTACHMENT = Symbol('editor:action:create-attachment')

export const useUploadingStateMixin = {
inject: {
Expand All @@ -29,3 +30,9 @@ export const useActionChooseLocalAttachmentMixin = {
$callChooseLocalAttachment: { from: ACTION_CHOOSE_LOCAL_ATTACHMENT, default: () => {} },
},
}

export const useActionCreateAttachmentMixin = {
inject: {
$callCreateAttachment: { from: ACTION_CREATE_ATTACHMENT, default: () => (template) => {} },
},
}
25 changes: 25 additions & 0 deletions src/components/Editor/MediaHandler.vue
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import { getCurrentUser } from '@nextcloud/auth'
import { showError } from '@nextcloud/dialogs'
import { emit } from '@nextcloud/event-bus'
import { generateUrl } from '@nextcloud/router'

Check warning on line 30 in src/components/Editor/MediaHandler.vue

View check run for this annotation

Codecov / codecov/patch

src/components/Editor/MediaHandler.vue#L30

Added line #L30 was not covered by tests
import { logger } from '../../helpers/logger.js'

import {
Expand All @@ -38,6 +39,7 @@
import {
ACTION_ATTACHMENT_PROMPT,
ACTION_CHOOSE_LOCAL_ATTACHMENT,
ACTION_CREATE_ATTACHMENT,
STATE_UPLOADING,
} from './MediaHandler.provider.js'

Expand All @@ -56,6 +58,9 @@
[ACTION_CHOOSE_LOCAL_ATTACHMENT]: {
get: () => this.chooseLocalFile,
},
[ACTION_CREATE_ATTACHMENT]: {
get: () => this.createAttachment,
},

Check warning on line 63 in src/components/Editor/MediaHandler.vue

View check run for this annotation

Codecov / codecov/patch

src/components/Editor/MediaHandler.vue#L61-L63

Added lines #L61 - L63 were not covered by tests
[STATE_UPLOADING]: {
get: () => this.state,
},
Expand Down Expand Up @@ -166,6 +171,26 @@
this.state.isUploadingAttachments = false
})
},
createAttachment(template) {
this.state.isUploadingAttachments = true
return this.$syncService.createAttachment(template).then((response) => {
this.insertAttachmentPreview(response.data?.id)
}).catch((error) => {
logger.error('Failed to create attachment', { error })
showError(t('text', 'Failed to create attachment'))
}).then(() => {
this.state.isUploadingAttachments = false
})
},
insertAttachmentPreview(fileId) {
const url = new URL(generateUrl(`/f/${fileId}`), window.origin)
const href = url.href.replaceAll(' ', '%20')
this.$editor
.chain()
.focus()
.insertPreview(href)
.run()
},

Check warning on line 193 in src/components/Editor/MediaHandler.vue

View check run for this annotation

Codecov / codecov/patch

src/components/Editor/MediaHandler.vue#L174-L193

Added lines #L174 - L193 were not covered by tests
insertAttachment(name, fileId, mimeType, position = null, dirname = '') {
// inspired by the fixedEncodeURIComponent function suggested in
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/encodeURIComponent
Expand Down
33 changes: 31 additions & 2 deletions src/components/Menu/ActionAttachmentUpload.vue
Original file line number Diff line number Diff line change
Expand Up @@ -34,29 +34,49 @@
</template>
{{ t('text', 'Insert from Files') }}
</NcActionButton>
<template v-if="templates.length">
<NcActionSeparator />
<NcActionButton v-for="(template, index) in templates"
:key="`${template.app}-${index}`"
close-after-click
:disabled="isUploadingAttachments"
:data-text-action-entry="`${actionEntry.key}-add-${template.app}-${index}`"
@click="createAttachment(template)">
<template #icon>
<NcIconSvgWrapper v-if="template.iconSvgInline" :svg="template.iconSvgInline" />
<Plus v-else />
</template>
{{ template.actionLabel }}
</NcActionButton>
</template>
</NcActions>
</template>

<script>
import { NcActions, NcActionButton } from '@nextcloud/vue'
import { Loading, Folder, Upload } from '../icons.js'
import { NcActions, NcActionSeparator, NcActionButton, NcIconSvgWrapper } from '@nextcloud/vue'
import { loadState } from '@nextcloud/initial-state'
import { Loading, Folder, Upload, Plus } from '../icons.js'
import { useIsPublicMixin, useEditorUpload } from '../Editor.provider.js'
import { BaseActionEntry } from './BaseActionEntry.js'
import { useMenuIDMixin } from './MenuBar.provider.js'
import {
useActionAttachmentPromptMixin,
useUploadingStateMixin,
useActionChooseLocalAttachmentMixin,
useActionCreateAttachmentMixin,
} from '../Editor/MediaHandler.provider.js'

export default {
name: 'ActionAttachmentUpload',
components: {
NcActions,
NcActionSeparator,
NcActionButton,
NcIconSvgWrapper,
Loading,
Folder,
Upload,
Plus,
},
extends: BaseActionEntry,
mixins: [
Expand All @@ -65,6 +85,7 @@
useActionAttachmentPromptMixin,
useUploadingStateMixin,
useActionChooseLocalAttachmentMixin,
useActionCreateAttachmentMixin,
useMenuIDMixin,
],
computed: {
Expand All @@ -76,6 +97,14 @@
isUploadingAttachments() {
return this.$uploadingState.isUploadingAttachments
},
templates() {
return loadState('files', 'templates', [])
},

Check warning on line 102 in src/components/Menu/ActionAttachmentUpload.vue

View check run for this annotation

Codecov / codecov/patch

src/components/Menu/ActionAttachmentUpload.vue#L100-L102

Added lines #L100 - L102 were not covered by tests
},
methods: {
createAttachment(template) {
this.$callCreateAttachment(template)
},

Check warning on line 107 in src/components/Menu/ActionAttachmentUpload.vue

View check run for this annotation

Codecov / codecov/patch

src/components/Menu/ActionAttachmentUpload.vue#L105-L107

Added lines #L105 - L107 were not covered by tests
},
}
</script>
2 changes: 2 additions & 0 deletions src/components/icons.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ import MDI_Upload from 'vue-material-design-icons/Upload.vue'
import MDI_Warn from 'vue-material-design-icons/Alert.vue'
import MDI_Web from 'vue-material-design-icons/Web.vue'
import MDI_TranslateVariant from 'vue-material-design-icons/TranslateVariant.vue'
import MDI_Plus from 'vue-material-design-icons/Plus.vue'

const DEFAULT_ICON_SIZE = 20

Expand Down Expand Up @@ -148,3 +149,4 @@ export const UnfoldMoreHorizontal = makeIcon(MDI_UnfoldMoreHorizontal)
export const Upload = makeIcon(MDI_Upload)
export const Warn = makeIcon(MDI_Warn)
export const Web = makeIcon(MDI_Web)
export const Plus = makeIcon(MDI_Plus)
9 changes: 9 additions & 0 deletions src/services/SessionApi.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,15 @@
})
}

createAttachment(template) {
return this.#post(_endpointUrl('attachment/create'), {
documentId: this.#document.id,
sessionId: this.#session.id,
sessionToken: this.#session.token,
fileName: `${template.app}${template.extension}`,
})
}

Check warning on line 165 in src/services/SessionApi.js

View check run for this annotation

Codecov / codecov/patch

src/services/SessionApi.js#L158-L165

Added lines #L158 - L165 were not covered by tests

insertAttachmentFile(filePath) {
return this.#post(_endpointUrl('attachment/filepath'), {
documentId: this.#document.id,
Expand Down
4 changes: 4 additions & 0 deletions src/services/SyncService.js
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,10 @@
return this.#connection.insertAttachmentFile(filePath)
}

createAttachment(template) {
return this.#connection.createAttachment(template)
}

Check warning on line 344 in src/services/SyncService.js

View check run for this annotation

Codecov / codecov/patch

src/services/SyncService.js#L342-L344

Added lines #L342 - L344 were not covered by tests

on(event, callback) {
this._bus.on(event, callback)
return this
Expand Down
27 changes: 27 additions & 0 deletions tests/unit/Service/AttachmentServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,33 @@ public function testGetAttachmentNamesFromContent() {
}
}

public function testGetAttachmentIdsFromContent() {
$urls = [
'www.example.com',
'http://example.com',
'https://www.example.com/path/to/page',
'http://sub.domain.co.uk/index.php',
'https://1.2.3.4:8080/path',
'http://localhost:3000/',
'https://[2001:db8::1]/ipv6-check',
];

$id = 1;
$content = "some content\n";
foreach (self::$attachmentNames as $name) {
$linkText = preg_replace('/[[\]]/', '', $name);
foreach ($urls as $url) {
$addon = $id % 2 ? ' (preview)' : '';
$content .= "[{$linkText}]({$url}/f/{$id}{$addon})\n";
$id++;
}
}
$content .= 'some content';

$computedIds = AttachmentService::getAttachmentIdsFromContent($content);
$this->assertEquals(range(1, $id - 1), $computedIds);
}

public function testGetUniqueFileName() {
$fileNameList = [
'foo.png',
Expand Down
Loading