Merge pull request #7357 from owncloud/batch-delete-performance

Fix batch deleting multiple files
owncloud · Jul 28, 2022 · fb16e4f · fb16e4f
2 parents 28736b6 + 7175dd9
commit fb16e4f
Show file tree

Hide file tree

Showing 10 changed files with 49 additions and 34 deletions.
diff --git a/changelog/unreleased/bugfix-batch-delete-multiple-files b/changelog/unreleased/bugfix-batch-delete-multiple-files
@@ -0,0 +1,6 @@
+Bugfix: Batch deleting multiple files
+
+We've fixed a bug where deleting many files in a batch action would fail.
+
+https://github.com/owncloud/web/pull/7357
+https://github.com/owncloud/web/issues/7329
diff --git a/packages/web-app-files/src/mixins/spaces/actions/delete.js b/packages/web-app-files/src/mixins/spaces/actions/delete.js
@@ -37,7 +37,7 @@ export default {
       'showMessage',
       'toggleModalConfirmButton'
     ]),
-    ...mapMutations('Files', ['REMOVE_FILE', 'REMOVE_SPACE']),
+    ...mapMutations('Files', ['REMOVE_FILES', 'REMOVE_SPACE']),
 
     $_delete_trigger({ resources }) {
       if (resources.length !== 1) {
@@ -72,7 +72,7 @@ export default {
         })
         .then(() => {
           this.hideModal()
-          this.REMOVE_FILE({ id })
+          this.REMOVE_FILES([{ id }])
           this.REMOVE_SPACE({ id })
           this.showMessage({
             title: this.$gettext('Space was deleted successfully')

diff --git a/packages/web-app-files/src/store/actions.js b/packages/web-app-files/src/store/actions.js
@@ -153,6 +153,7 @@ export default {
   },
   deleteFiles(context, { files, client, isPublicLinkContext, firstRun = true }) {
     const promises = []
+    const removedFiles = []
     for (const file of files) {
       let p = null
       if (isPublicLinkContext) {
@@ -166,10 +167,7 @@ export default {
       }
       const promise = p
         .then(() => {
-          context.dispatch('sidebar/close')
-          context.commit('REMOVE_FILE', file)
-          context.commit('REMOVE_FILE_SELECTION', file)
-          context.commit('REMOVE_FILE_FROM_SEARCHED', file)
+          removedFiles.push(file)
         })
         .catch((error) => {
           let translated = $gettext('Failed to delete "%{file}"')
@@ -197,7 +195,12 @@ export default {
         })
       promises.push(promise)
     }
-    return Promise.all(promises)
+    return Promise.all(promises).then(() => {
+      context.dispatch('sidebar/close')
+      context.commit('REMOVE_FILES', removedFiles)
+      context.commit('REMOVE_FILES_FROM_SEARCHED', removedFiles)
+      context.commit('RESET_SELECTION')
+    })
   },
   async clearTrashBin(context) {
     await context.dispatch('sidebar/close')
@@ -207,11 +210,9 @@ export default {
   },
   async removeFilesFromTrashbin(context, files) {
     await context.dispatch('sidebar/close')
-    for (const file of files) {
-      context.commit('REMOVE_FILE', file)
-      context.commit('REMOVE_FILE_SELECTION', file)
-      context.commit('REMOVE_FILE_FROM_SEARCHED', file)
-    }
+    context.commit('REMOVE_FILES', files)
+    context.commit('REMOVE_FILES_FROM_SEARCHED', files)
+    context.commit('RESET_SELECTION')
   },
   renameFile(context, { file, newValue, client, isPublicLinkContext, isSameResource }) {
     if (file !== undefined && newValue !== undefined && newValue !== file.name) {

diff --git a/packages/web-app-files/src/store/mutations.js b/packages/web-app-files/src/store/mutations.js
@@ -69,12 +69,12 @@ export default {
   LOAD_FILES_SEARCHED(state, files) {
     state.filesSearched = files
   },
-  REMOVE_FILE_FROM_SEARCHED(state, file) {
+  REMOVE_FILES_FROM_SEARCHED(state, files) {
     if (!state.filesSearched) {
       return
     }
 
-    state.filesSearched = state.filesSearched.filter((i) => file.id !== i.id)
+    state.filesSearched = state.filesSearched.filter((i) => !files.find((f) => f.id === i.id))
   },
   CLEAR_FILES_SEARCHED(state) {
     state.filesSearched = null
@@ -123,8 +123,8 @@ export default {
   RESET_SELECTION(state) {
     state.selectedIds = []
   },
-  REMOVE_FILE(state, removedFile) {
-    state.files = [...state.files].filter((file) => file.id !== removedFile.id)
+  REMOVE_FILES(state, removedFiles) {
+    state.files = [...state.files].filter((file) => !removedFiles.find((r) => r.id === file.id))
   },
   RENAME_FILE(state, { file, newValue, newPath }) {
     const resources = [...state.files]

diff --git a/packages/web-app-files/src/views/Personal.vue b/packages/web-app-files/src/views/Personal.vue
@@ -248,7 +248,11 @@ export default defineComponent({
   methods: {
     ...mapActions('Files', ['loadPreview']),
     ...mapActions(['showMessage', 'createModal', 'hideModal']),
-    ...mapMutations('Files', ['REMOVE_FILE', 'REMOVE_FILE_FROM_SEARCHED', 'REMOVE_FILE_SELECTION']),
+    ...mapMutations('Files', [
+      'REMOVE_FILES',
+      'REMOVE_FILES_FROM_SEARCHED',
+      'REMOVE_FILE_SELECTION'
+    ]),
 
     async fileDropped(fileIdTarget) {
       const selected = [...this.selectedResources]
@@ -269,8 +273,8 @@ export default defineComponent({
         this.$route.name
       )
       for (const resource of movedResources) {
-        this.REMOVE_FILE(resource)
-        this.REMOVE_FILE_FROM_SEARCHED(resource)
+        this.REMOVE_FILES([resource])
+        this.REMOVE_FILES_FROM_SEARCHED([resource])
         this.REMOVE_FILE_SELECTION(resource)
       }
     },

diff --git a/packages/web-app-files/src/views/PublicFiles.vue b/packages/web-app-files/src/views/PublicFiles.vue
@@ -186,8 +186,8 @@ export default defineComponent({
       'SET_CURRENT_FOLDER',
       'LOAD_FILES',
       'CLEAR_CURRENT_FILES_LIST',
-      'REMOVE_FILE',
-      'REMOVE_FILE_FROM_SEARCHED',
+      'REMOVE_FILES',
+      'REMOVE_FILES_FROM_SEARCHED',
       'REMOVE_FILE_SELECTION'
     ]),
 
@@ -211,8 +211,8 @@ export default defineComponent({
         this.publicLinkPassword
       )
       for (const resource of movedResources) {
-        this.REMOVE_FILE(resource)
-        this.REMOVE_FILE_FROM_SEARCHED(resource)
+        this.REMOVE_FILES([resource])
+        this.REMOVE_FILES_FROM_SEARCHED([resource])
         this.REMOVE_FILE_SELECTION(resource)
       }
     },

diff --git a/packages/web-app-files/src/views/shares/SharedResource.vue b/packages/web-app-files/src/views/shares/SharedResource.vue
@@ -216,7 +216,11 @@ export default defineComponent({
   methods: {
     ...mapActions('Files', ['loadPreview']),
     ...mapActions(['showMessage']),
-    ...mapMutations('Files', ['REMOVE_FILE', 'REMOVE_FILE_FROM_SEARCHED', 'REMOVE_FILE_SELECTION']),
+    ...mapMutations('Files', [
+      'REMOVE_FILES',
+      'REMOVE_FILES_FROM_SEARCHED',
+      'REMOVE_FILE_SELECTION'
+    ]),
 
     fetchResources,
 
@@ -239,8 +243,8 @@ export default defineComponent({
         this.$route.name
       )
       for (const resource of movedResources) {
-        this.REMOVE_FILE(resource)
-        this.REMOVE_FILE_FROM_SEARCHED(resource)
+        this.REMOVE_FILES([resource])
+        this.REMOVE_FILES_FROM_SEARCHED([resource])
         this.REMOVE_FILE_SELECTION(resource)
       }
     },

diff --git a/packages/web-app-files/src/views/spaces/Project.vue b/packages/web-app-files/src/views/spaces/Project.vue
@@ -376,8 +376,8 @@ export default defineComponent({
       'LOAD_FILES',
       'UPSERT_SPACE',
       'CLEAR_CURRENT_FILES_LIST',
-      'REMOVE_FILE',
-      'REMOVE_FILE_FROM_SEARCHED',
+      'REMOVE_FILES',
+      'REMOVE_FILES_FROM_SEARCHED',
       'REMOVE_FILE_SELECTION'
     ]),
     async fileDropped(fileIdTarget) {
@@ -399,8 +399,8 @@ export default defineComponent({
         this.$route.name
       )
       for (const resource of movedResources) {
-        this.REMOVE_FILE(resource)
-        this.REMOVE_FILE_FROM_SEARCHED(resource)
+        this.REMOVE_FILES([resource])
+        this.REMOVE_FILES_FROM_SEARCHED([resource])
         this.REMOVE_FILE_SELECTION(resource)
       }
     },

diff --git a/packages/web-app-files/tests/unit/components/components.setup.js b/packages/web-app-files/tests/unit/components/components.setup.js
@@ -142,8 +142,8 @@ export const getStore = function ({
           LOAD_FILES: () => {},
           SET_FILES_PAGE_LIMIT: () => {},
           SET_CURRENT_FOLDER: () => {},
-          REMOVE_FILE: () => {},
-          REMOVE_FILE_FROM_SEARCHED: () => {},
+          REMOVE_FILES: () => {},
+          REMOVE_FILES_FROM_SEARCHED: () => {},
           REMOVE_FILE_SELECTION: () => {},
           SET_FILE_SELECTION: () => {}
         },

diff --git a/packages/web-app-files/tests/unit/views/views.setup.js b/packages/web-app-files/tests/unit/views/views.setup.js
@@ -190,8 +190,8 @@ export const getStore = function ({
           LOAD_FILES: () => {},
           SET_FILES_PAGE_LIMIT: () => {},
           SET_CURRENT_FOLDER: () => {},
-          REMOVE_FILE: () => {},
-          REMOVE_FILE_FROM_SEARCHED: () => {},
+          REMOVE_FILES: () => {},
+          REMOVE_FILES_FROM_SEARCHED: () => {},
           REMOVE_FILE_SELECTION: () => {},
           SET_FILE_SELECTION: () => {}
         },