Fix several issues when setting image and readme for spaces (#6898)

Fix several issues when setting image and readme for spaces
owncloud · May 9, 2022 · 716a475 · 716a475
1 parent 8433aa3
commit 716a475
Show file tree

Hide file tree

Showing 10 changed files with 65 additions and 44 deletions.
diff --git a/changelog/unreleased/bugfix-set-space-image-and-readme b/changelog/unreleased/bugfix-set-space-image-and-readme
@@ -0,0 +1,8 @@
+Bugfix: Setting image and readme for spaces
+
+* An issue where setting a space-image or -readme would corrupt the file list has been fixed.
+* Setting images from within the `.space` folder has been fixed.
+* Setting images and readme files with specials characters in their names has been fixed.
+
+https://github.com/owncloud/web/pull/6898
+https://github.com/owncloud/web/issues/6875
diff --git a/packages/web-app-files/src/mixins/spaces/actions/setImage.js b/packages/web-app-files/src/mixins/spaces/actions/setImage.js
@@ -2,7 +2,6 @@ import { isLocationSpacesActive } from '../../../router'
 import { clientService } from 'web-pkg/src/services'
 import { mapMutations, mapActions, mapGetters, mapState } from 'vuex'
 import { buildResource } from '../../../helpers/resources'
-import { bus } from 'web-pkg/src/instance'
 import { thumbnailService } from '../../../services'
 
 export default {
@@ -54,19 +53,18 @@ export default {
     }
   },
   methods: {
-    ...mapMutations('Files', ['UPDATE_RESOURCE_FIELD']),
+    ...mapMutations('Files', ['UPDATE_SPACE_FIELD']),
     ...mapActions(['showMessage']),
     async $_setSpaceImage_trigger({ resources }) {
       const graphClient = clientService.graphAuthenticated(this.configuration.server, this.getToken)
       const storageId = this.$route.params.storageId
       const sourcePath = resources[0].webDavPath
       const destinationPath = `/spaces/${storageId}/.space/${resources[0].name}`
 
-      if (sourcePath === destinationPath) {
-        return
-      }
       try {
-        await this.$client.files.copy(sourcePath, destinationPath)
+        if (sourcePath !== destinationPath) {
+          await this.$client.files.copy(sourcePath, destinationPath)
+        }
         const fileInfo = await this.$client.files.fileInfo(destinationPath)
         const file = buildResource(fileInfo)
         const { data } = await graphClient.drives.updateDrive(
@@ -83,7 +81,8 @@ export default {
           },
           {}
         )
-        this.UPDATE_RESOURCE_FIELD({
+
+        this.UPDATE_SPACE_FIELD({
           id: storageId,
           field: 'spaceImageData',
           value: data.special.find((special) => special.specialFolder.name === 'image')
@@ -92,7 +91,6 @@ export default {
         this.showMessage({
           title: this.$gettext('Space image was set successfully')
         })
-        bus.publish('app.files.list.load')
       } catch (error) {
         console.error(error)
         this.showMessage({

diff --git a/packages/web-app-files/src/mixins/spaces/actions/setReadme.js b/packages/web-app-files/src/mixins/spaces/actions/setReadme.js
@@ -1,6 +1,5 @@
 import { isLocationSpacesActive } from '../../../router'
 import { mapMutations, mapState, mapActions } from 'vuex'
-import { bus } from 'web-pkg/src/instance'
 
 export default {
   inject: {
@@ -9,7 +8,7 @@ export default {
     }
   },
   computed: {
-    ...mapState('Files', ['currentFolder']),
+    ...mapState('Files', ['currentFolder', 'spaces']),
     ...mapState(['user']),
     $_setSpaceReadme_items() {
       return [
@@ -48,26 +47,23 @@ export default {
     }
   },
   methods: {
-    ...mapMutations('Files', ['UPDATE_RESOURCE_FIELD']),
+    ...mapMutations('Files', ['UPDATE_SPACE_FIELD']),
     ...mapActions(['showMessage']),
     async $_setSpaceReadme_trigger({ resources }) {
-      const space = this.currentFolder
-
       try {
         const fileContent = await this.$client.files.getFileContents(resources[0].webDavPath)
         const fileMetaData = await this.$client.files.putFileContents(
-          `/spaces/${space.id}/.space/readme.md`,
+          `/spaces/${this.space.id}/.space/readme.md`,
           fileContent
         )
-        this.UPDATE_RESOURCE_FIELD({
-          id: space.id,
+        this.UPDATE_SPACE_FIELD({
+          id: this.space.id,
           field: 'spaceReadmeData',
-          value: { ...space.spaceReadmeData, ...{ etag: fileMetaData?.ETag } }
+          value: { ...this.space.spaceReadmeData, ...{ etag: fileMetaData?.ETag } }
         })
         this.showMessage({
           title: this.$gettext('Space description was set successfully')
         })
-        bus.publish('app.files.list.load')
       } catch (error) {
         console.error(error)
         this.showMessage({

diff --git a/packages/web-app-files/src/services/folder/spaces/loaderProject.ts b/packages/web-app-files/src/services/folder/spaces/loaderProject.ts
@@ -72,6 +72,7 @@ export class FolderLoaderSpacesProject implements FolderLoader {
         currentFolder: currentFolder?.path,
         storageId: space.id
       })
+      ref.UPSERT_SPACE(space)
 
       if (!sameRoute) {
         ref.space = space

diff --git a/packages/web-app-files/src/store/getters.js b/packages/web-app-files/src/store/getters.js
@@ -7,6 +7,9 @@ export default {
   files: (state) => {
     return state.files
   },
+  spaces: (state) => {
+    return state.spaces
+  },
   filesAll: (state) => state.filesSearched || state.files,
   currentFolder: (state) => {
     return state.currentFolder

diff --git a/packages/web-app-files/src/store/mutations.js b/packages/web-app-files/src/store/mutations.js
@@ -36,7 +36,16 @@ export default {
     Vue.set(spaceSource, index, newResource)
   },
   UPSERT_SPACE(state, space) {
-    state.spaces.push(space)
+    const spaces = [...state.spaces]
+    const index = spaces.findIndex((r) => r.id === space.id)
+    const found = index > -1
+
+    if (found) {
+      spaces.splice(index, 1, space)
+    } else {
+      spaces.push(space)
+    }
+    state.spaces = spaces
   },
   REMOVE_SPACE(state, space) {
     state.spaces = state.spaces.filter((s) => s.id !== space.id)
@@ -51,6 +60,9 @@ export default {
   SET_CURRENT_FOLDER(state, currentFolder) {
     state.currentFolder = currentFolder
   },
+  SET_CURRENT_SPACE(state, currentSpace) {
+    state.currentSpace = currentSpace
+  },
   CLEAR_FILES(state) {
     state.files = []
   },

diff --git a/packages/web-app-files/src/views/spaces/Project.vue b/packages/web-app-files/src/views/spaces/Project.vue
@@ -265,19 +265,21 @@ export default defineComponent({
           .slice(webDavPathComponents.indexOf(this.space.id) + 1)
           .join('/')
 
-        this.$client.files.fileInfo(buildWebDavSpacesPath(this.space.id, path)).then((data) => {
-          const resource = buildResource(data)
-          loadPreview({
-            resource,
-            isPublic: false,
-            dimensions: ImageDimension.Preview,
-            server: this.configuration.server,
-            userId: this.user.id,
-            token: this.getToken
-          }).then((imageBlob) => {
-            this.imageContent = imageBlob
+        this.$client.files
+          .fileInfo(buildWebDavSpacesPath(this.space.id, decodeURIComponent(path)))
+          .then((data) => {
+            const resource = buildResource(data)
+            loadPreview({
+              resource,
+              isPublic: false,
+              dimensions: ImageDimension.Preview,
+              server: this.configuration.server,
+              userId: this.user.id,
+              token: this.getToken
+            }).then((imageBlob) => {
+              this.imageContent = imageBlob
+            })
           })
-        })
       },
       deep: true
     },
@@ -292,7 +294,7 @@ export default defineComponent({
           .join('/')
 
         this.$client.files
-          .getFileContents(buildWebDavSpacesPath(this.space.id, path))
+          .getFileContents(buildWebDavSpacesPath(this.space.id, decodeURIComponent(path)))
           .then((fileContents) => {
             if (this.markdownResizeObserver && this.$refs.markdownContainer) {
               this.markdownResizeObserver.unobserve(this.$refs.markdownContainer)
@@ -338,6 +340,7 @@ export default defineComponent({
     ...mapMutations('Files', [
       'SET_CURRENT_FOLDER',
       'LOAD_FILES',
+      'UPSERT_SPACE',
       'CLEAR_CURRENT_FILES_LIST',
       'REMOVE_FILE',
       'REMOVE_FILE_FROM_SEARCHED',

diff --git a/packages/web-app-files/tests/unit/mixins/spaces/setImage.spec.js b/packages/web-app-files/tests/unit/mixins/spaces/setImage.spec.js
@@ -7,7 +7,6 @@ import mockAxios from 'jest-mock-axios'
 // eslint-disable-next-line jest/no-mocks-import
 import sdkMock from '@/__mocks__/sdk'
 import fileFixtures from '__fixtures__/files'
-import { bus } from 'web-pkg/src/instance'
 import { thumbnailService } from '../../../../src/services'
 import { buildSpace } from '../../../../src/helpers/resources'
 
@@ -73,7 +72,7 @@ describe('setImage', () => {
           Files: {
             namespaced: true,
             mutations: {
-              UPDATE_RESOURCE_FIELD: jest.fn()
+              UPDATE_SPACE_FIELD: jest.fn()
             }
           }
         }
@@ -162,7 +161,6 @@ describe('setImage', () => {
 
       const wrapper = getWrapper()
       const showMessageStub = jest.spyOn(wrapper.vm, 'showMessage')
-      bus.publish = jest.fn((path) => path)
       await wrapper.vm.$_setSpaceImage_trigger({
         resources: [
           {
@@ -173,7 +171,6 @@ describe('setImage', () => {
       })
 
       expect(showMessageStub).toHaveBeenCalledTimes(1)
-      expect(bus.publish).toHaveBeenCalledTimes(1)
     })
 
     it('should show message on error', async () => {
@@ -196,7 +193,10 @@ describe('setImage', () => {
       expect(showMessageStub).toHaveBeenCalledTimes(1)
     })
 
-    it('should not set the image if source and destination path are the same', async () => {
+    it('should not copy the image if source and destination path are the same', async () => {
+      mockAxios.request.mockImplementationOnce(() => {
+        return Promise.resolve({ data: { special: [{ specialFolder: { name: 'image' } }] } })
+      })
       const wrapper = getWrapper()
       await wrapper.vm.$_setSpaceImage_trigger({
         resources: [

diff --git a/packages/web-app-files/tests/unit/mixins/spaces/setReadme.spec.js b/packages/web-app-files/tests/unit/mixins/spaces/setReadme.spec.js
@@ -5,9 +5,7 @@ import setReadme from '@files/src/mixins/spaces/actions/setReadme.js'
 import { createLocationSpaces } from '../../../../src/router'
 // eslint-disable-next-line jest/no-mocks-import
 import sdkMock from '@/__mocks__/sdk'
-import { bus } from 'web-pkg/src/instance'
 import { buildSpace } from '../../../../src/helpers/resources'
-
 const localVue = createLocalVue()
 localVue.use(Vuex)
 
@@ -36,6 +34,9 @@ describe('setReadme', () => {
               .mockImplementation(() => Promise.resolve({ ETag: '60c7243c2e7f1' }))
           }
         },
+        $route: {
+          params: { storageId: 1 }
+        },
         $router: {
           currentRoute: createLocationSpaces('files-spaces-projects'),
           resolve: (r) => {
@@ -72,7 +73,7 @@ describe('setReadme', () => {
           Files: {
             namespaced: true,
             mutations: {
-              UPDATE_RESOURCE_FIELD: jest.fn()
+              UPDATE_SPACE_FIELD: jest.fn()
             },
             state: {
               currentFolder: {
@@ -134,9 +135,8 @@ describe('setReadme', () => {
   })
   describe('method "$_setSpaceReadme_trigger"', () => {
     it('should show message on success', async () => {
-      const wrapper = getWrapper()
+      const wrapper = getWrapper(true, { id: 1 })
       const showMessageStub = jest.spyOn(wrapper.vm, 'showMessage')
-      bus.publish = jest.fn((path) => path)
       await wrapper.vm.$_setSpaceReadme_trigger({
         resources: [
           {
@@ -147,12 +147,11 @@ describe('setReadme', () => {
       })
 
       expect(showMessageStub).toHaveBeenCalledTimes(1)
-      expect(bus.publish).toHaveBeenCalledTimes(1)
     })
 
     it('should show message on error', async () => {
       jest.spyOn(console, 'error').mockImplementation(() => {})
-      const wrapper = getWrapper(false)
+      const wrapper = getWrapper(false, { id: 1 })
       const showMessageStub = jest.spyOn(wrapper.vm, 'showMessage')
       await wrapper.vm.$_setSpaceReadme_trigger({
         resources: [

diff --git a/packages/web-app-files/tests/unit/views/spaces/Project.spec.js b/packages/web-app-files/tests/unit/views/spaces/Project.spec.js
@@ -280,6 +280,7 @@ function getMountedWrapper(spaceResources = [], spaceItem = null, imageContent =
           mutations: {
             CLEAR_CURRENT_FILES_LIST: jest.fn(),
             CLEAR_FILES_SEARCHED: jest.fn(),
+            UPSERT_SPACE: jest.fn(),
             LOAD_FILES: jest.fn()
           },
           actions: {