Introduce UPSERT_RESOURCE to files mutations

changelog/unreleased/bugfix-upsert-resource-in-filestable

@@ -0,0 +1,7 @@
+Bugfix: Upsert resource in filestable
+
+When uploading an already existing resource in the filestable, we sometimes displayed both 
+files in the filestable until the page got refreshed. We now check when uploading a file if 
+it exists in the filestable and replace it there if that is the case.
+
+https://github.com/owncloud/web/pull/5130

packages/web-app-files/src/components/AppBar.vue

@@ -277,7 +277,7 @@ export default {
       'loadIndicators'
     ]),
     ...mapActions(['openFile', 'showMessage', 'createModal', 'setModalInputErrorMessage']),
-    ...mapMutations('Files', ['PUSH_NEW_RESOURCE']),
+    ...mapMutations('Files', ['UPSERT_RESOURCE']),
     ...mapMutations(['SET_QUOTA']),
 
     showCreateResourceModal(isFolder = true, ext = 'txt', openAction = null) {
@@ -338,7 +338,7 @@ export default {
         }
         resource = buildResource(resource)
 
-        this.PUSH_NEW_RESOURCE(resource)
+        this.UPSERT_RESOURCE(resource)
         this.hideModal()
 
         if (this.isPersonalRoute) {
@@ -427,7 +427,7 @@ export default {
 
         resource = buildResource(resource)
 
-        this.PUSH_NEW_RESOURCE(resource)
+        this.UPSERT_RESOURCE(resource)
         this.hideModal()
 
         if (this.isPersonalRoute) {
@@ -500,7 +500,7 @@ export default {
             )
 
         resource = buildResource(resource)
-        this.PUSH_NEW_RESOURCE(resource)
+        this.UPSERT_RESOURCE(resource)
 
         if (this.isPersonalRoute) {
           await this.loadIndicators({

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

@@ -302,23 +302,48 @@ export default {
     state.files = files
   },
 
-  PUSH_NEW_RESOURCE(state, resource) {
-    const files = [...state.files]
-    files.push(resource)
-    state.files = files
-  },
-
   SELECT_RESOURCES(state, resources) {
     state.selected = resources
   },
 
+  /**
+   * Inserts or updates the given resource, depending on whether or not the resource is already loaded.
+   * Updating the resource always takes precedence, so that we don't duplicate resources in the store
+   * accidentally.
+   *
+   * @param state Current state of this store module
+   * @param resource A new or updated resource
+   */
+  UPSERT_RESOURCE(state, resource) {
+    $_upsertResource(state, resource, true)
+  },
+
+  /**
+   * Updates the given resource in the store. If the resource doesn't exist in the store, the update
+   * will be ignored. If you also want to allow inserts, use UPSERT_RESOURCE instead.
+   *
+   * @param state Current state of this store module
+   * @param resource An updated resource
+   */
   UPDATE_RESOURCE(state, resource) {
-    const files = [...state.files]
-    const index = files.findIndex(r => r.id === resource.id)
+    $_upsertResource(state, resource, false)
+  }
+}
 
-    if (index > -1) {
-      files.splice(index, 1, resource)
-      state.files = files
-    }
+// eslint-disable-next-line camelcase
+function $_upsertResource(state, resource, allowInsert) {
+  const files = [...state.files]
+  const index = files.findIndex(r => r.id === resource.id)
+  const found = index > -1
+
+  if (!found && !allowInsert) {
+    return
+  }
+
+  if (found) {
+    files.splice(index, 1, resource)
+  } else {
+    files.push(resource)
   }
+  state.files = files
 }

packages/web-app-files/tests/store/mutations.spec.js

@@ -0,0 +1,61 @@
+import mutations from '../../src/store/mutations'
+import { cloneDeep } from 'lodash-es'
+
+const stateFixture = {
+  files: [
+    {
+      id: 1,
+      name: 'test1'
+    },
+    {
+      id: 2,
+      name: 'test2'
+    },
+    {
+      id: 5,
+      name: 'test5'
+    }
+  ]
+}
+let stateMock = {}
+const resetState = () => {
+  stateMock = cloneDeep(stateFixture)
+}
+
+describe('vuex store mutations', () => {
+  describe('update resources by id', () => {
+    beforeEach(resetState)
+    it.each([
+      [mutations.UPSERT_RESOURCE.name, mutations.UPSERT_RESOURCE],
+      [mutations.UPDATE_RESOURCE.name, mutations.UPDATE_RESOURCE]
+    ])('succeeds using mutation %s', (name, m) => {
+      expect(stateMock.files).toEqual(stateFixture.files)
+      expect(stateMock.files[1].name).toBe('test2')
+      m(stateMock, {
+        id: 2,
+        name: 'test2-updated'
+      })
+      expect(stateMock.files[1].name).toBe('test2-updated')
+      expect(stateMock.files.length).toBe(stateFixture.files.length)
+    })
+  })
+  describe('insert resources', () => {
+    beforeEach(resetState)
+    it('succeeds using mutation UPSERT_RESOURCE', () => {
+      expect(stateMock.files).toEqual(stateFixture.files)
+      mutations.UPSERT_RESOURCE(stateMock, {
+        id: 3,
+        name: 'test3-inserted'
+      })
+      expect(stateMock.files).toEqual([...stateFixture.files, { id: 3, name: 'test3-inserted' }])
+    })
+    it('is ignored using mutation UPDATE_RESOURCE', () => {
+      expect(stateMock.files).toEqual(stateFixture.files)
+      mutations.UPDATE_RESOURCE(stateMock, {
+        id: 3,
+        name: 'test3-inserted'
+      })
+      expect(stateMock.files).toEqual(stateFixture.files)
+    })
+  })
+})