From f8d451f95f42ef410c4144cd60c1f97c3a70ad69 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Fri, 22 Mar 2024 18:30:52 +0100 Subject: [PATCH] fix(files): When copying nodes only add the copy suffix for file before file extension Co-authored-by: Pytal <24800714+Pytal@users.noreply.github.com> Signed-off-by: Ferdinand Thiessen --- apps/files/src/actions/moveOrCopyAction.ts | 11 +++++++-- apps/files/src/utils/fileUtils.ts | 26 ++++++++++++++++---- cypress/e2e/files/files_copy-move.cy.ts | 28 +++++++++++++++++++++- 3 files changed, 57 insertions(+), 8 deletions(-) diff --git a/apps/files/src/actions/moveOrCopyAction.ts b/apps/files/src/actions/moveOrCopyAction.ts index eb54c4c3f82..2456340a02c 100644 --- a/apps/files/src/actions/moveOrCopyAction.ts +++ b/apps/files/src/actions/moveOrCopyAction.ts @@ -120,7 +120,14 @@ export const handleCopyMoveNodeTo = async (node: Node, destination: Folder, meth // If we do not allow overwriting then find an unique name if (!overwrite) { const otherNodes = await client.getDirectoryContents(destinationPath) as FileStat[] - target = getUniqueName(node.basename, otherNodes.map((n) => n.basename), copySuffix) + target = getUniqueName( + node.basename, + otherNodes.map((n) => n.basename), + { + suffix: copySuffix, + ignoreFileExtension: node.type === FileType.Folder, + }, + ) } await client.copyFile(currentPath, join(destinationPath, target)) // If the node is copied into current directory the view needs to be updated @@ -150,7 +157,7 @@ export const handleCopyMoveNodeTo = async (node: Node, destination: Folder, meth } } catch (error) { // User cancelled - showError(t('files','Move cancelled')) + showError(t('files', 'Move cancelled')) return } } diff --git a/apps/files/src/utils/fileUtils.ts b/apps/files/src/utils/fileUtils.ts index 126739242a0..180bec31004 100644 --- a/apps/files/src/utils/fileUtils.ts +++ b/apps/files/src/utils/fileUtils.ts @@ -12,7 +12,7 @@ * * This program is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU Affero General Public License for more details. * * You should have received a copy of the GNU Affero General Public License @@ -28,15 +28,31 @@ import { basename, extname } from 'path' * Create an unique file name * @param name The initial name to use * @param otherNames Other names that are already used - * @param suffix A function that takes an index an returns a suffix to add, defaults to '(index)' + * @param options Optional parameters for tuning the behavior + * @param options.suffix A function that takes an index and returns a suffix to add to the file name, defaults to '(index)' + * @param options.ignoreFileExtension Set to true to ignore the file extension when adding the suffix (when getting a unique directory name) * @return Either the initial name, if unique, or the name with the suffix so that the name is unique */ -export const getUniqueName = (name: string, otherNames: string[], suffix = (n: number) => `(${n})`): string => { +export const getUniqueName = ( + name: string, + otherNames: string[], + options: { + suffix?: (i: number) => string, + ignoreFileExtension?: boolean, + } = {}, +): string => { + const opts = { + suffix: (n: number) => `(${n})`, + ignoreFileExtension: false, + ...options, + } + let newName = name let i = 1 while (otherNames.includes(newName)) { - const ext = extname(name) - newName = `${basename(name, ext)} ${suffix(i++)}${ext}` + const ext = opts.ignoreFileExtension ? '' : extname(name) + const base = basename(name, ext) + newName = `${base} ${opts.suffix(i++)}${ext}` } return newName } diff --git a/cypress/e2e/files/files_copy-move.cy.ts b/cypress/e2e/files/files_copy-move.cy.ts index 22f8f765662..d7892dbe3ba 100644 --- a/cypress/e2e/files/files_copy-move.cy.ts +++ b/cypress/e2e/files/files_copy-move.cy.ts @@ -93,7 +93,9 @@ describe('Files: Move or copy files', { testIsolation: true }, () => { getRowForFile('new-folder').should('not.exist') }) - // This was a bug previously + /** + * Test for https://github.com/nextcloud/server/issues/41768 + */ it('Can move a file to folder with similar name', () => { cy.uploadContent(currentUser, new Blob(), 'text/plain', '/original') .mkdir(currentUser, '/original folder') @@ -180,6 +182,30 @@ describe('Files: Move or copy files', { testIsolation: true }, () => { getRowForFile('original (copy 2).txt').should('be.visible') }) + /** + * Test that a copied folder with a dot will be renamed correctly ('foo.bar' -> 'foo.bar (copy)') + * Test for: https://github.com/nextcloud/server/issues/43843 + */ + it('Can copy a folder to same folder', () => { + cy.mkdir(currentUser, '/foo.bar') + cy.login(currentUser) + cy.visit('/apps/files') + + // intercept the copy so we can wait for it + cy.intercept('COPY', /\/remote.php\/dav\/files\//).as('copyFile') + + getRowForFile('foo.bar').should('be.visible') + triggerActionForFile('foo.bar', 'move-copy') + + // click copy + cy.get('.file-picker').contains('button', 'Copy').should('be.visible').click() + + cy.wait('@copyFile') + + getRowForFile('foo.bar').should('be.visible') + getRowForFile('foo.bar (copy)').should('be.visible') + }) + /** Test for https://github.com/nextcloud/server/issues/43329 */ context('escaping file and folder names', () => { it('Can handle files with special characters', () => {