Bugfix: Copy / move file name exists count duplicate

changelog/unreleased/enhancement-copy-move-conflict-dialog

@@ -2,5 +2,6 @@ Enhancement: Copy/Move conflict dialog
 
 We've added a conflict dialog for moving resources via drag&drop in the files list
 
+https://github.com/owncloud/web/pull/7119
 https://github.com/owncloud/web/pull/6994
-https://github.com/owncloud/web/issues/6996
+https://github.com/owncloud/web/issues/6996

packages/web-app-files/src/helpers/extensions/fileExtensions.ts

@@ -1,3 +1,3 @@
 export default {
-  complex: ['tar.bz2', 'tar.gz']
+  complex: ['tar.bz2', 'tar.gz', '.tar.xz']
 }

packages/web-app-files/src/helpers/resource/copyMove.ts

@@ -1,5 +1,6 @@
-import { Resource } from './index'
+import { Resource, extractNameWithoutExtension } from './index'
 import { join } from 'path'
+import { buildResource } from '../resources'
 
 export enum ResolveStrategy {
   SKIP,
@@ -64,19 +65,18 @@ export const resolveFileExists = (
     createModal(modal)
   })
 }
+
 export const resolveAllConflicts = async (
   resourcesToMove,
   targetFolder,
-  client,
+  targetFolderItems,
   createModal,
   hideModal,
   $gettext,
   $gettextInterpolate,
   resolveFileExistsMethod,
   copy = false
 ) => {
-  // if we implement MERGE, we need to use 'infinity' instead of 1
-  const targetFolderItems = await client.files.list(targetFolder.webDavPath, 1)
   const targetPath = targetFolder.path
   const index = targetFolder.webDavPath.lastIndexOf(targetPath)
   const webDavPrefix =
@@ -215,6 +215,20 @@ export const copy = async (
     true
   )
 }
+
+export const resolveFileNameDuplicate = (name, extension, existingFiles, iteration = 1) => {
+  let potentialName
+  if (extension.length === 0) {
+    potentialName = `${name} (${iteration})`
+  } else {
+    const nameWithoutExtension = extractNameWithoutExtension({ name, extension } as Resource)
+    potentialName = `${nameWithoutExtension} (${iteration}).${extension}`
+  }
+  const hasConflict = existingFiles.some((f) => f.name === potentialName)
+  if (!hasConflict) return potentialName
+  return resolveFileNameDuplicate(name, extension, existingFiles, iteration + 1)
+}
+
 export const copyMoveResource = async (
   resourcesToMove,
   targetFolder,
@@ -228,10 +242,13 @@ export const copyMoveResource = async (
   copy = false
 ) => {
   const errors = []
+  // if we implement MERGE, we need to use 'infinity' instead of 1
+  const targetFolderItems = await client.files.list(targetFolder.webDavPath, 1)
+  const targetFolderResources = targetFolderItems.map((i) => buildResource(i))
   const resolvedConflicts = await resolveAllConflicts(
     resourcesToMove,
     targetFolder,
-    client,
+    targetFolderItems,
     createModal,
     hideModal,
     $gettext,
@@ -241,7 +258,9 @@ export const copyMoveResource = async (
   )
   const movedResources = []
 
-  for (const resource of resourcesToMove) {
+  for (let resource of resourcesToMove) {
+    // shallow copy of resources to prevent modifing existing rows
+    resource = { ...resource }
     const hasConflict = resolvedConflicts.some((e) => e.resource.id === resource.id)
     let targetName = resource.name
     let overwriteTarget = false
@@ -254,17 +273,18 @@ export const copyMoveResource = async (
         overwriteTarget = true
       }
       if (resolveStrategy === ResolveStrategy.KEEP_BOTH) {
-        targetName = $gettextInterpolate($gettext('%{name} copy'), { name: resource.name }, true)
+        targetName = resolveFileNameDuplicate(resource.name, resource.extension, [
+          ...movedResources,
+          ...targetFolderResources
+        ])
         resource.name = targetName
       }
     }
-    resource.path = join(targetFolder.path, resource.name)
     try {
-      if (copy) {
+      if (copy && !overwriteTarget) {
         await client.files.copy(
           resource.webDavPath,
-          join(targetFolder.webDavPath, targetName),
-          overwriteTarget
+          join(targetFolder.webDavPath, targetName)
         )
       } else {
         await client.files.move(
@@ -273,6 +293,7 @@ export const copyMoveResource = async (
           overwriteTarget
         )
       }
+      resource.path = join(targetFolder.path, resource.name)
       resource.webDavPath = join(targetFolder.webDavPath, resource.name)
       movedResources.push(resource)
     } catch (error) {

packages/web-app-files/src/store/mutations.js

@@ -324,7 +324,10 @@ export default {
 // eslint-disable-next-line camelcase
 function $_upsertResource(state, resource, allowInsert) {
   const files = [...state.files]
-  const index = files.findIndex((r) => r.id === resource.id)
+  let index = files.findIndex((r) => r.id === resource.id)
+  if(resource.webDavPath && resource.webDavPath.length && index === -1) {
+    index = files.findIndex((r) => r.webDavPath === resource.webDavPath)
+  }
   const found = index > -1
 
   state.filesSearched = null

packages/web-app-files/tests/unit/helpers/resource/copyMove.spec.ts

@@ -23,6 +23,17 @@ describe('copyMove', () => {
       webDavPath: '/target'
     }
   })
+  it.each([
+    { name: 'a', extension: '', expectName: 'a (1)' },
+    { name: 'a', extension: '', expectName: 'a (2)', existing: [{ name: 'a (1)' }] },
+    { name: 'a (1)', extension: '', expectName: 'a (1) (1)' },
+    { name: 'b.png', extension: 'png', expectName: 'b (1).png' },
+    { name: 'b.png', extension: 'png', expectName: 'b (2).png', existing: [{ name: 'b (1).png' }] }
+  ])('should name duplicate file correctly', (dataSet) => {
+    const existing = dataSet.existing ? [...resourcesToMove, ...dataSet.existing] : resourcesToMove
+    const result = Resource.resolveFileNameDuplicate(dataSet.name, dataSet.extension, existing)
+    expect(result).toEqual(dataSet.expectName)
+  })
   it('should copy files if no conflicts exist', async () => {
     const client = {
       files: {
@@ -43,8 +54,8 @@ describe('copyMove', () => {
       jest.fn(),
       jest.fn()
     )
-    expect(client.files.copy).toHaveBeenCalledWith('/a', '/target/a', false)
-    expect(client.files.copy).toHaveBeenCalledWith('/b', '/target/b', false)
+    expect(client.files.copy).toHaveBeenCalledWith('/a', '/target/a')
+    expect(client.files.copy).toHaveBeenCalledWith('/b', '/target/b')
   })
   it('should move files if no conflicts exist', async () => {
     const client = {
@@ -70,27 +81,21 @@ describe('copyMove', () => {
     expect(client.files.move).toHaveBeenCalledWith('/b', '/target/b', false)
   })
   it('should not show message if no conflict exists', async () => {
-    const client = {
-      files: {
-        list: async () => {
-          return [
-            {
-              id: 'c',
-              path: 'target/c',
-              webDavPath: '/target/c',
-              name: '/target/c'
-            }
-          ]
-        }
+    const targetFolderItems = [
+      {
+        id: 'c',
+        path: 'target/c',
+        webDavPath: '/target/c',
+        name: '/target/c'
       }
-    }
+    ]
     const resolveFileExistsMethod = jest
       .fn()
       .mockImplementation(() => Promise.resolve({ strategy: 0 } as Resource.ResolveConflict))
     await Resource.resolveAllConflicts(
       resourcesToMove,
       targetFolder,
-      client,
+      targetFolderItems,
       jest.fn(),
       jest.fn(),
       jest.fn(),
@@ -100,27 +105,21 @@ describe('copyMove', () => {
     expect(resolveFileExistsMethod).not.toHaveBeenCalled()
   })
   it('should show message if conflict exists', async () => {
-    const client = {
-      files: {
-        list: async () => {
-          return [
-            {
-              id: 'a',
-              path: 'target/a',
-              webDavPath: '/target/a',
-              name: '/target/a'
-            }
-          ]
-        }
+    const targetFolderItems = [
+      {
+        id: 'a',
+        path: 'target/a',
+        webDavPath: '/target/a',
+        name: '/target/a'
       }
-    }
+    ]
     const resolveFileExistsMethod = jest
       .fn()
       .mockImplementation(() => Promise.resolve({ strategy: 0 } as Resource.ResolveConflict))
     await Resource.resolveAllConflicts(
       resourcesToMove,
       targetFolder,
-      client,
+      targetFolderItems,
       jest.fn(),
       jest.fn(),
       jest.fn(),