From bc7d9205c0a7fe96e2cb9d612ad42e2c79a20aa6 Mon Sep 17 00:00:00 2001 From: Jannik Stehle Date: Fri, 6 May 2022 14:07:56 +0200 Subject: [PATCH 1/3] Fix several issues when setting image and readme for spaces --- .../bugfix-set-space-image-and-readme | 8 +++++ .../src/mixins/spaces/actions/setImage.js | 14 ++++----- .../src/mixins/spaces/actions/setReadme.js | 13 ++++----- .../services/folder/spaces/loaderProject.ts | 1 + packages/web-app-files/src/store/getters.js | 3 ++ packages/web-app-files/src/store/mutations.js | 14 ++++++++- .../src/views/spaces/Project.vue | 29 ++++++++++--------- .../tests/unit/mixins/spaces/setImage.spec.js | 10 +++---- .../unit/mixins/spaces/setReadme.spec.js | 11 +++---- .../tests/unit/views/spaces/Project.spec.js | 1 + 10 files changed, 65 insertions(+), 39 deletions(-) create mode 100644 changelog/unreleased/bugfix-set-space-image-and-readme diff --git a/changelog/unreleased/bugfix-set-space-image-and-readme b/changelog/unreleased/bugfix-set-space-image-and-readme new file mode 100644 index 00000000000..5861510ccfd --- /dev/null +++ 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 index 947ac382a6f..2c6d2640513 100644 --- 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,7 +53,7 @@ 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) @@ -62,11 +61,10 @@ export default { 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 index d132e6ac5f1..53af7804ab0 100644 --- 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,18 +47,19 @@ 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 storageId = this.$route.params.storageId + const space = this.spaces.find((s) => s.id === storageId) + const fileContent = await this.$client.files.getFileContents(resources[0].webDavPath) const fileMetaData = await this.$client.files.putFileContents( `/spaces/${space.id}/.space/readme.md`, fileContent ) - this.UPDATE_RESOURCE_FIELD({ + this.UPDATE_SPACE_FIELD({ id: space.id, field: 'spaceReadmeData', value: { ...space.spaceReadmeData, ...{ etag: fileMetaData?.ETag } } @@ -67,7 +67,6 @@ export default { 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 index 1792c2e7f81..c3750b24ab6 100644 --- a/packages/web-app-files/src/services/folder/spaces/loaderProject.ts +++ b/packages/web-app-files/src/services/folder/spaces/loaderProject.ts @@ -74,6 +74,7 @@ export class FolderLoaderSpacesProject implements FolderLoader { }) if (!sameRoute) { + ref.UPSERT_SPACE(space) ref.space = space } }) diff --git a/packages/web-app-files/src/store/getters.js b/packages/web-app-files/src/store/getters.js index 4bb5e89edb7..083dcc9ebc5 100644 --- 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 index 7406a204c79..d1a6d4a0dde 100644 --- 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 index 46dc0326011..3b1e3988ba5 100644 --- 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 index e908a9842f0..bed16a47f2c 100644 --- 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 index ba8e7bb7079..d9bf1fecb00 100644 --- 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,7 +5,6 @@ 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() @@ -36,6 +35,9 @@ describe('setReadme', () => { .mockImplementation(() => Promise.resolve({ ETag: '60c7243c2e7f1' })) } }, + $route: { + params: { storageId: 1 } + }, $router: { currentRoute: createLocationSpaces('files-spaces-projects'), resolve: (r) => { @@ -72,12 +74,13 @@ describe('setReadme', () => { Files: { namespaced: true, mutations: { - UPDATE_RESOURCE_FIELD: jest.fn() + UPDATE_SPACE_FIELD: jest.fn() }, state: { currentFolder: { id: '1fe58d8b-aa69-4c22-baf7-97dd57479f22' - } + }, + spaces: [{ id: 1 }] } } } @@ -136,7 +139,6 @@ describe('setReadme', () => { it('should show message on success', async () => { const wrapper = getWrapper() const showMessageStub = jest.spyOn(wrapper.vm, 'showMessage') - bus.publish = jest.fn((path) => path) await wrapper.vm.$_setSpaceReadme_trigger({ resources: [ { @@ -147,7 +149,6 @@ describe('setReadme', () => { }) expect(showMessageStub).toHaveBeenCalledTimes(1) - expect(bus.publish).toHaveBeenCalledTimes(1) }) it('should show message on error', async () => { 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 index b91666aee11..d8191fd9db9 100644 --- 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: { From fb0e0110fc60818b5673a64d3569b907d4e06c2f Mon Sep 17 00:00:00 2001 From: Jannik Stehle Date: Mon, 9 May 2022 10:04:29 +0200 Subject: [PATCH 2/3] Simplify space fetching in setReadme mutation --- .../web-app-files/src/mixins/spaces/actions/setReadme.js | 9 +++------ .../tests/unit/mixins/spaces/setReadme.spec.js | 8 +++----- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/packages/web-app-files/src/mixins/spaces/actions/setReadme.js b/packages/web-app-files/src/mixins/spaces/actions/setReadme.js index 53af7804ab0..4714e4bf422 100644 --- a/packages/web-app-files/src/mixins/spaces/actions/setReadme.js +++ b/packages/web-app-files/src/mixins/spaces/actions/setReadme.js @@ -51,18 +51,15 @@ export default { ...mapActions(['showMessage']), async $_setSpaceReadme_trigger({ resources }) { try { - const storageId = this.$route.params.storageId - const space = this.spaces.find((s) => s.id === storageId) - 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_SPACE_FIELD({ - id: space.id, + 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') 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 index d9bf1fecb00..1295876b56b 100644 --- a/packages/web-app-files/tests/unit/mixins/spaces/setReadme.spec.js +++ b/packages/web-app-files/tests/unit/mixins/spaces/setReadme.spec.js @@ -6,7 +6,6 @@ import { createLocationSpaces } from '../../../../src/router' // eslint-disable-next-line jest/no-mocks-import import sdkMock from '@/__mocks__/sdk' import { buildSpace } from '../../../../src/helpers/resources' - const localVue = createLocalVue() localVue.use(Vuex) @@ -79,8 +78,7 @@ describe('setReadme', () => { state: { currentFolder: { id: '1fe58d8b-aa69-4c22-baf7-97dd57479f22' - }, - spaces: [{ id: 1 }] + } } } } @@ -137,7 +135,7 @@ 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') await wrapper.vm.$_setSpaceReadme_trigger({ resources: [ @@ -153,7 +151,7 @@ describe('setReadme', () => { 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: [ From 7eec83fe95d50233d75628f68e1b9d5bb0b0fdf0 Mon Sep 17 00:00:00 2001 From: Jannik Stehle Date: Mon, 9 May 2022 10:10:43 +0200 Subject: [PATCH 3/3] Move upsert space statement out of any condition --- .../web-app-files/src/services/folder/spaces/loaderProject.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/web-app-files/src/services/folder/spaces/loaderProject.ts b/packages/web-app-files/src/services/folder/spaces/loaderProject.ts index c3750b24ab6..0abf4c17c5b 100644 --- a/packages/web-app-files/src/services/folder/spaces/loaderProject.ts +++ b/packages/web-app-files/src/services/folder/spaces/loaderProject.ts @@ -72,9 +72,9 @@ export class FolderLoaderSpacesProject implements FolderLoader { currentFolder: currentFolder?.path, storageId: space.id }) + ref.UPSERT_SPACE(space) if (!sameRoute) { - ref.UPSERT_SPACE(space) ref.space = space } })