From 6bcb0cde4758759bd845a1062dcc7805c2db4594 Mon Sep 17 00:00:00 2001 From: Max Date: Thu, 24 Jul 2025 14:57:56 +0200 Subject: [PATCH 1/4] fix(share): allow opening attachments Provide relative path to attachments in public shares. The path needs to be relative to the shared folder. Open attachments in public collective shares as well as in folder descriptions of public shares. Signed-off-by: Max --- lib/Service/AttachmentService.php | 37 +++++++++++++++++++++++++++++-- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/lib/Service/AttachmentService.php b/lib/Service/AttachmentService.php index 18f3e8b32a8..fc5e0214fa7 100755 --- a/lib/Service/AttachmentService.php +++ b/lib/Service/AttachmentService.php @@ -220,7 +220,11 @@ public function getAttachmentList(int $documentId, ?string $userId = null, ?Sess : '?documentId=' . $documentId . $shareTokenUrlString; $attachments = []; - $userFolder = $userId !== null ? $this->rootFolder->getUserFolder($userId) : null; + + // Folder davPath need to be relative to. + $davFolder = $userId !== null + ? $this->rootFolder->getUserFolder($userId) + : $this->getShareFolder($shareToken); $fileNodes = []; $fileIds = []; @@ -247,7 +251,7 @@ public function getAttachmentList(int $documentId, ?string $userId = null, ?Sess 'mimetype' => $node->getMimeType(), 'mtime' => $node->getMTime(), 'isImage' => $isImage, - 'davPath' => $userFolder?->getRelativePath($node->getPath()), + 'davPath' => $davFolder?->getRelativePath($node->getPath()), 'metadata' => $metadata, 'fullUrl' => $isImage ? $this->urlGenerator->linkToRouteAbsolute('text.Attachment.getImageFile') . $urlParamsBase . '&imageFileName=' . rawurlencode($name) . '&preferRawImage=1' @@ -574,6 +578,35 @@ private function getTextFilePublic(?int $documentId, string $shareToken): File { throw new NotFoundException('Text file with id=' . $documentId . ' and shareToken ' . $shareToken . ' was not found.'); } + /** + * Get share folder + * + * @param string $shareToken + * + * @throws NotFoundException + */ + private function getShareFolder(string $shareToken): ?Folder { + // is the file shared with this token? + try { + $share = $this->shareManager->getShareByToken($shareToken); + if (in_array($share->getShareType(), [IShare::TYPE_LINK, IShare::TYPE_EMAIL])) { + // shared file or folder? + if ($share->getNodeType() === 'file') { + return null; + } elseif ($share->getNodeType() === 'folder') { + $folder = $share->getNode(); + if ($folder instanceof Folder) { + return $folder; + } + throw new NotFoundException('Share folder for ' . $shareToken . ' was not a folder.'); + } + } + } catch (ShareNotFound $e) { + // same as below + } + throw new NotFoundException('Share folder for ' . $shareToken . ' was not found.'); + } + /** * Actually delete attachment files which are not pointed in the markdown content * From 3e0958c18d2643aa72bb5d145383c0ddfc530c2b Mon Sep 17 00:00:00 2001 From: Max Date: Sat, 26 Jul 2025 20:11:04 +0200 Subject: [PATCH 2/4] chore(split): shareWithAttachments from attachments spec Signed-off-by: Max --- cypress/e2e/attachments.spec.js | 63 ------------------ cypress/e2e/shareWithAttachments.spec.js | 83 ++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 63 deletions(-) mode change 100755 => 100644 cypress/e2e/attachments.spec.js create mode 100644 cypress/e2e/shareWithAttachments.spec.js diff --git a/cypress/e2e/attachments.spec.js b/cypress/e2e/attachments.spec.js old mode 100755 new mode 100644 index 4f80fb139b2..d080ffb87cd --- a/cypress/e2e/attachments.spec.js +++ b/cypress/e2e/attachments.spec.js @@ -447,67 +447,4 @@ describe('Test all attachment insertion methods', () => { cy.getFile('.attachments.' + documentId).should('not.exist') }) }) - - it('[share] check everything behaves correctly on the share target user side', () => { - const fileName = 'testShared.md' - cy.createMarkdown( - fileName, - '![git](.attachments.123/github.png)', - false, - ).then((fileId) => { - const attachmentsFolder = `.attachments.${fileId}` - cy.createFolder(attachmentsFolder) - cy.uploadFile( - 'github.png', - 'image/png', - `${attachmentsFolder}/github.png`, - ) - cy.shareFileToUser(fileName, recipient) - }) - - cy.login(recipient) - cy.showHiddenFiles() - - cy.visit('/apps/files') - // check the file list - cy.getFile('testShared.md').should('exist') - cy.getFile('github.png').should('not.exist') - - // check the attachment folder is not there - cy.getFile('testShared.md') - .should('exist') - .should('have.attr', 'data-cy-files-list-row-fileid') - .then((documentId) => { - cy.getFile('.attachments.' + documentId).should('not.exist') - }) - - // move the file and check the attachment folder is still not there - cy.moveFile('testShared.md', 'testMoved.md') - cy.reloadFileList() - cy.getFile('testMoved.md') - .should('exist') - .should('have.attr', 'data-cy-files-list-row-fileid') - .then((documentId) => { - cy.getFile('.attachments.' + documentId).should('not.exist') - }) - - // copy the file and check the attachment folder was copied - cy.copyFile('testMoved.md', 'testCopied.md') - cy.reloadFileList() - cy.getFile('testCopied.md') - .should('exist') - .should('have.attr', 'data-cy-files-list-row-fileid') - .then((documentId) => { - const files = attachmentFileNameToId[documentId] - cy.openFolder('.attachments.' + documentId) - for (const name in files) { - cy.getFile(name) - .should('exist') - .should('have.attr', 'data-cy-files-list-row-fileid') - // these are new copied attachment files - // so they should not have the same IDs than the ones created when uploading the files - .should('not.eq', String(files[name])) - } - }) - }) }) diff --git a/cypress/e2e/shareWithAttachments.spec.js b/cypress/e2e/shareWithAttachments.spec.js new file mode 100644 index 00000000000..834d8e7f7c3 --- /dev/null +++ b/cypress/e2e/shareWithAttachments.spec.js @@ -0,0 +1,83 @@ +/** + * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import { randUser } from '../utils/index.js' + +const user = randUser() +const recipient = randUser() + +describe('Share with attachments', () => { + + before(() => { + cy.createUser(user) + cy.createUser(recipient) + }) + + it('handle file operations by recipient user', () => { + cy.login(user) + const fileName = 'testShared.md' + cy.createMarkdown( + fileName, + '![git](.attachments.123/github.png)', + false, + ) + .as('textFileId') + .then((fileId) => { + const attachmentsFolder = `.attachments.${fileId}` + cy.createFolder(attachmentsFolder) + cy.uploadFile( + 'github.png', + 'image/png', + `${attachmentsFolder}/github.png`, + ).as('attachmentId') + cy.shareFileToUser(fileName, recipient) + }) + + cy.login(recipient) + cy.showHiddenFiles() + + cy.visit('/apps/files') + // check the file list + cy.getFile('testShared.md').should('exist') + cy.getFile('github.png').should('not.exist') + + // check the attachment folder is not there + cy.getFile('testShared.md') + .should('exist') + .should('have.attr', 'data-cy-files-list-row-fileid') + .then((documentId) => { + cy.getFile('.attachments.' + documentId).should('not.exist') + }) + + // move the file and check the attachment folder is still not there + cy.moveFile('testShared.md', 'testMoved.md') + cy.reloadFileList() + cy.getFile('testMoved.md') + .should('exist') + .should('have.attr', 'data-cy-files-list-row-fileid') + .then((documentId) => { + cy.getFile('.attachments.' + documentId).should('not.exist') + }) + + // copy the file and check the attachment folder was copied + cy.copyFile('testMoved.md', 'testCopied.md') + cy.reloadFileList() + cy.getFile('testCopied.md') + .should('exist') + .should('have.attr', 'data-cy-files-list-row-fileid') + .then((documentId) => { + cy.openFolder('.attachments.' + documentId) + }) + cy.get('@attachmentId') + .then((attachmentId) => { + cy.getFile('github.png') + .should('exist') + .should('have.attr', 'data-cy-files-list-row-fileid') + // these are new copied attachment files + // so they should not have the same IDs than the ones created when uploading the files + .should('not.eq', String(attachmentId)) + }) + }) +}) From bdf48f42448a72be3428168454549c98a32a4e76 Mon Sep 17 00:00:00 2001 From: Max Date: Sat, 26 Jul 2025 20:20:08 +0200 Subject: [PATCH 3/4] chore(refactor): use cy.getFileId Signed-off-by: Max --- cypress/e2e/shareWithAttachments.spec.js | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/cypress/e2e/shareWithAttachments.spec.js b/cypress/e2e/shareWithAttachments.spec.js index 834d8e7f7c3..2895ae35167 100644 --- a/cypress/e2e/shareWithAttachments.spec.js +++ b/cypress/e2e/shareWithAttachments.spec.js @@ -44,9 +44,7 @@ describe('Share with attachments', () => { cy.getFile('github.png').should('not.exist') // check the attachment folder is not there - cy.getFile('testShared.md') - .should('exist') - .should('have.attr', 'data-cy-files-list-row-fileid') + cy.getFileId('testShared.md') .then((documentId) => { cy.getFile('.attachments.' + documentId).should('not.exist') }) @@ -54,9 +52,7 @@ describe('Share with attachments', () => { // move the file and check the attachment folder is still not there cy.moveFile('testShared.md', 'testMoved.md') cy.reloadFileList() - cy.getFile('testMoved.md') - .should('exist') - .should('have.attr', 'data-cy-files-list-row-fileid') + cy.getFileId('testMoved.md') .then((documentId) => { cy.getFile('.attachments.' + documentId).should('not.exist') }) @@ -64,20 +60,17 @@ describe('Share with attachments', () => { // copy the file and check the attachment folder was copied cy.copyFile('testMoved.md', 'testCopied.md') cy.reloadFileList() - cy.getFile('testCopied.md') - .should('exist') - .should('have.attr', 'data-cy-files-list-row-fileid') + cy.getFileId('testCopied.md') .then((documentId) => { cy.openFolder('.attachments.' + documentId) }) cy.get('@attachmentId') .then((attachmentId) => { - cy.getFile('github.png') - .should('exist') - .should('have.attr', 'data-cy-files-list-row-fileid') - // these are new copied attachment files - // so they should not have the same IDs than the ones created when uploading the files + // these are new copied attachment files + // so they should not have the same IDs than the ones created when uploading the files + cy.getFileId('github.png') .should('not.eq', String(attachmentId)) }) }) + }) From 8b0456472144c17746f60ca27007f1bdbf1de71c Mon Sep 17 00:00:00 2001 From: Max Date: Sat, 26 Jul 2025 21:18:13 +0200 Subject: [PATCH 4/4] test(cy): open attachment in public share Only in folder description though as we do not allow it from the viewer. Signed-off-by: Max --- cypress/e2e/shareWithAttachments.spec.js | 96 +++++++++++++++--------- 1 file changed, 60 insertions(+), 36 deletions(-) diff --git a/cypress/e2e/shareWithAttachments.spec.js b/cypress/e2e/shareWithAttachments.spec.js index 2895ae35167..5d5b13127cf 100644 --- a/cypress/e2e/shareWithAttachments.spec.js +++ b/cypress/e2e/shareWithAttachments.spec.js @@ -9,7 +9,6 @@ const user = randUser() const recipient = randUser() describe('Share with attachments', () => { - before(() => { cy.createUser(user) cy.createUser(recipient) @@ -18,22 +17,18 @@ describe('Share with attachments', () => { it('handle file operations by recipient user', () => { cy.login(user) const fileName = 'testShared.md' - cy.createMarkdown( - fileName, - '![git](.attachments.123/github.png)', - false, - ) - .as('textFileId') - .then((fileId) => { - const attachmentsFolder = `.attachments.${fileId}` - cy.createFolder(attachmentsFolder) - cy.uploadFile( - 'github.png', - 'image/png', - `${attachmentsFolder}/github.png`, - ).as('attachmentId') - cy.shareFileToUser(fileName, recipient) - }) + cy.createFile(fileName, '![git](.attachments.123/github.png)') + .as('textFileId') + .then((fileId) => { + const attachmentsFolder = `.attachments.${fileId}` + cy.createFolder(attachmentsFolder) + cy.uploadFile( + 'github.png', + 'image/png', + `${attachmentsFolder}/github.png`, + ).as('attachmentId') + cy.shareFileToUser(fileName, recipient) + }) cy.login(recipient) cy.showHiddenFiles() @@ -44,33 +39,62 @@ describe('Share with attachments', () => { cy.getFile('github.png').should('not.exist') // check the attachment folder is not there - cy.getFileId('testShared.md') - .then((documentId) => { - cy.getFile('.attachments.' + documentId).should('not.exist') - }) + cy.getFileId('testShared.md').then((documentId) => { + cy.getFile('.attachments.' + documentId).should('not.exist') + }) // move the file and check the attachment folder is still not there cy.moveFile('testShared.md', 'testMoved.md') cy.reloadFileList() - cy.getFileId('testMoved.md') - .then((documentId) => { - cy.getFile('.attachments.' + documentId).should('not.exist') - }) + cy.getFileId('testMoved.md').then((documentId) => { + cy.getFile('.attachments.' + documentId).should('not.exist') + }) // copy the file and check the attachment folder was copied cy.copyFile('testMoved.md', 'testCopied.md') cy.reloadFileList() - cy.getFileId('testCopied.md') - .then((documentId) => { - cy.openFolder('.attachments.' + documentId) - }) - cy.get('@attachmentId') - .then((attachmentId) => { - // these are new copied attachment files - // so they should not have the same IDs than the ones created when uploading the files - cy.getFileId('github.png') - .should('not.eq', String(attachmentId)) - }) + cy.getFileId('testCopied.md').then((documentId) => { + cy.openFolder('.attachments.' + documentId) + }) + cy.get('@attachmentId').then((attachmentId) => { + // these are new copied attachment files + // so they should not have the same IDs than the ones created when uploading the files + cy.getFileId('github.png').should('not.eq', String(attachmentId)) + }) + }) +}) + +describe('Public Share with attachments', () => { + before(function () { + cy.createUser(user) + }) + + beforeEach(function () { + cy.login(user) + cy.createTestFolder().as('folder').then(cy.shareFile).as('token') + cy.then(function () { + cy.createFile( + `${this.folder}/Readme.md`, + '![Attached text](.attachments.123/lines.txt)', + ).as('fileId') + }) + cy.then(function () { + const attachmentsFolder = `${this.folder}/.attachments.${this.fileId}` + cy.createFolder(attachmentsFolder) + cy.uploadFile( + 'lines.txt', + 'text/plain', + `${attachmentsFolder}/lines.txt`, + ) + }) + cy.clearCookies() }) + it('open attached files in folder description', function () { + cy.visit(`/s/${this.token}`) + cy.get('.content-wrapper').should('exist') + cy.get('.content-wrapper .name', { timeout: 10_000 }).click() + cy.get('.viewer').should('exist') + cy.get('.language-plaintext').should('contain', 'multiple lines') + }) })