From 339c6cf63b39573e1d9811df453cf6cc05207518 Mon Sep 17 00:00:00 2001 From: Florian Schade Date: Mon, 23 May 2022 10:10:56 +0200 Subject: [PATCH] use ocis storageId whenever possible use roles storageId whenever possible refactor share loading out of link and share panel component into parent component and load the data only once --- .drone.env | 4 +- .../SideBar/Details/FileDetails.vue | 2 +- .../InviteCollaboratorForm.vue | 10 +- .../SideBar/Shares/Collaborators/ListItem.vue | 1 + .../components/SideBar/Shares/FileLinks.vue | 37 +---- .../components/SideBar/Shares/FileShares.vue | 38 +---- .../components/SideBar/Shares/SharesPanel.vue | 55 ++++++++ packages/web-app-files/src/store/actions.js | 130 ++++++++---------- .../SideBar/Shares/FileShares.spec.js | 12 -- .../SideBar/Shares/SharesPanel.spec.js | 49 +++++++ .../tests/unit/store/actions.spec.js | 6 +- packages/web-runtime/package.json | 2 +- tests/e2e/support/api/graph/user.ts | 1 + yarn.lock | 16 +-- 14 files changed, 181 insertions(+), 182 deletions(-) create mode 100644 packages/web-app-files/tests/unit/components/SideBar/Shares/SharesPanel.spec.js diff --git a/.drone.env b/.drone.env index d35bb02f812..2fbaa38f281 100644 --- a/.drone.env +++ b/.drone.env @@ -1,3 +1,3 @@ # The version of OCIS to use in pipelines that test against OCIS -OCIS_COMMITID=09ff21ace56566127e3e328dd1a179936f918734 -OCIS_BRANCH=master +OCIS_COMMITID=4206509dbedc1b1c8d5a1ef459e424706c5155e9 +OCIS_BRANCH=Resharing diff --git a/packages/web-app-files/src/components/SideBar/Details/FileDetails.vue b/packages/web-app-files/src/components/SideBar/Details/FileDetails.vue index f0864dda9d1..29ebc3ece0b 100644 --- a/packages/web-app-files/src/components/SideBar/Details/FileDetails.vue +++ b/packages/web-app-files/src/components/SideBar/Details/FileDetails.vue @@ -392,7 +392,7 @@ export default defineComponent({ client: this.$client, path: this.file.path, $gettext: this.$gettext, - storageId: this.$route.params.storageId + storageId: this.file.fileId }) this.shareIndicators = getIndicators(this.file, this.sharesTree) }, diff --git a/packages/web-app-files/src/components/SideBar/Shares/Collaborators/InviteCollaborator/InviteCollaboratorForm.vue b/packages/web-app-files/src/components/SideBar/Shares/Collaborators/InviteCollaborator/InviteCollaboratorForm.vue index 52251275292..7ba0d2fe65a 100644 --- a/packages/web-app-files/src/components/SideBar/Shares/Collaborators/InviteCollaborator/InviteCollaboratorForm.vue +++ b/packages/web-app-files/src/components/SideBar/Shares/Collaborators/InviteCollaborator/InviteCollaboratorForm.vue @@ -323,13 +323,6 @@ export default { this.selectedRole.permissions(this.hasResharing || this.resourceIsSpace) ) - let storageId - if (this.resourceIsSpace) { - storageId = this.highlightedFile.id - } else if (this.$route.params.storageId) { - storageId = this.$route.params.storageId - } - this.addShare({ client: this.$client, graphClient: this.graphClient, @@ -339,8 +332,9 @@ export default { displayName: collaborator.label, shareType: collaborator.value.shareType, permissions: bitmask, + role: this.selectedRole, expirationDate: this.expirationDate, - storageId + storageId: this.highlightedFile.fileId }) }) ) diff --git a/packages/web-app-files/src/components/SideBar/Shares/Collaborators/ListItem.vue b/packages/web-app-files/src/components/SideBar/Shares/Collaborators/ListItem.vue index 1e6c4636827..bc1ae803f87 100644 --- a/packages/web-app-files/src/components/SideBar/Shares/Collaborators/ListItem.vue +++ b/packages/web-app-files/src/components/SideBar/Shares/Collaborators/ListItem.vue @@ -308,6 +308,7 @@ export default { client: this.$client, graphClient: this.graphClient, share: this.share, + role, permissions: bitmask, expirationDate: expirationDate || '' }) diff --git a/packages/web-app-files/src/components/SideBar/Shares/FileLinks.vue b/packages/web-app-files/src/components/SideBar/Shares/FileLinks.vue index 7741675dc20..010bd31dd53 100644 --- a/packages/web-app-files/src/components/SideBar/Shares/FileLinks.vue +++ b/packages/web-app-files/src/components/SideBar/Shares/FileLinks.vue @@ -107,7 +107,6 @@ import CreateQuickLink from './Links/CreateQuickLink.vue' import { ShareTypes, LinkShareRoles } from '../../../helpers/share' import { useStore, useCapabilitySpacesEnabled } from 'web-pkg/src/composables' import { clientService } from 'web-pkg/src/services' -import { dirname } from 'path' import { defineComponent } from '@vue/composition-api' import { shareViaLinkHelp } from '../../../helpers/contextualHelpers' @@ -315,48 +314,14 @@ export default defineComponent({ return this.$route.params.storageId || null } }, - watch: { - highlightedFile(newItem, oldItem) { - if (oldItem !== newItem) { - this.reloadLinks() - } - } - }, - mounted() { - this.reloadLinks() - }, - methods: { - ...mapActions('Files', [ - 'addLink', - 'updateLink', - 'removeLink', - 'loadSharesTree', - 'loadCurrentFileOutgoingShares' - ]), + ...mapActions('Files', ['addLink', 'updateLink', 'removeLink']), ...mapActions(['showMessage', 'createModal', 'hideModal']), toggleLinkListCollapsed() { this.linkListCollapsed = !this.linkListCollapsed }, - reloadLinks() { - this.loadCurrentFileOutgoingShares({ - client: this.$client, - graphClient: this.graphClient, - path: this.highlightedFile.path, - $gettext: this.$gettext, - storageId: this.currentStorageId, - resource: this.highlightedFile - }) - this.loadSharesTree({ - client: this.$client, - path: this.highlightedFile.path === '' ? '/' : dirname(this.highlightedFile.path), - $gettext: this.$gettext, - storageId: this.currentStorageId - }) - }, - linksComparator(l1, l2) { // sorting priority 1: display name (lower case, ascending), 2: creation time (descending) const name1 = l1.name.toLowerCase().trim() diff --git a/packages/web-app-files/src/components/SideBar/Shares/FileShares.vue b/packages/web-app-files/src/components/SideBar/Shares/FileShares.vue index 9315f9d39be..6cf2647c637 100644 --- a/packages/web-app-files/src/components/SideBar/Shares/FileShares.vue +++ b/packages/web-app-files/src/components/SideBar/Shares/FileShares.vue @@ -61,7 +61,6 @@ diff --git a/packages/web-app-files/src/store/actions.js b/packages/web-app-files/src/store/actions.js index 29daf55f1ca..b60085c84ff 100644 --- a/packages/web-app-files/src/store/actions.js +++ b/packages/web-app-files/src/store/actions.js @@ -13,7 +13,7 @@ import { $gettext, $gettextInterpolate } from '../gettext' import { loadPreview } from '../helpers/resource' import { avatarUrl } from '../helpers/user' import { has } from 'lodash-es' -import { ShareTypes, SpacePeopleShareRoles } from '../helpers/share' +import { ShareTypes } from '../helpers/share' import { sortSpaceMembers } from '../helpers/space' import get from 'lodash-es/get' @@ -150,10 +150,7 @@ export default { context.commit('CURRENT_FILE_OUTGOING_SHARES_ERROR', null) context.commit('CURRENT_FILE_OUTGOING_SHARES_LOADING', true) - let spaceRef - if (storageId) { - spaceRef = `${storageId}${path}` - } + const spaceRef = storageId if (resource?.type === 'space') { const promises = [] @@ -249,70 +246,56 @@ export default { context.commit('INCOMING_SHARES_LOADING', false) }) }, - changeShare( + async changeShare( { commit, getters, rootGetters }, - { client, graphClient, share, permissions, expirationDate } + { client, graphClient, share, permissions, expirationDate, role } ) { - const params = { - permissions: permissions, - expireDate: expirationDate + if (!permissions && !role) { + throw new Error('Nothing changed') } - if (!params.permissions) { - return new Promise((resolve, reject) => { - reject(new Error('Nothing changed')) + if (share.shareType === ShareTypes.space.value) { + await client.shares.shareSpaceWithUser('', share.collaborator.name, share.id, { + permissions, + role: role.name }) - } - if (share.shareType === ShareTypes.space.value) { - return new Promise((resolve, reject) => { - client.shares - .shareSpaceWithUser('', share.collaborator.name, share.id, { - permissions - }) - .then(() => { - const role = SpacePeopleShareRoles.getByBitmask(permissions) - const shareObj = { - role: role.name, - onPremisesSamAccountName: share.collaborator.name, - displayName: share.collaborator.displayName - } - const updatedShare = buildSpaceShare(shareObj, share.id) - commit('CURRENT_FILE_OUTGOING_SHARES_UPSERT', updatedShare) - - graphClient.drives.getDrive(share.id).then((response) => { - const space = buildSpace(response.data) - commit('UPDATE_RESOURCE_FIELD', { - id: share.id, - field: 'spaceRoles', - value: space.spaceRoles - }) - }) + const updatedShare = buildSpaceShare( + { + role: role.name, + onPremisesSamAccountName: share.collaborator.name, + displayName: share.collaborator.displayName + }, + share.id + ) - resolve(updatedShare) - }) - .catch((e) => { - reject(e) - }) + commit('CURRENT_FILE_OUTGOING_SHARES_UPSERT', updatedShare) + + const { data: drive } = await graphClient.drives.getDrive(share.id) + const space = buildSpace(drive) + commit('UPDATE_RESOURCE_FIELD', { + id: share.id, + field: 'spaceRoles', + value: space.spaceRoles }) + + return } - return new Promise((resolve, reject) => { - client.shares - .updateShare(share.id, params) - .then((updatedShare) => { - const share = buildCollaboratorShare( - updatedShare.shareInfo, - getters.highlightedFile, - allowSharePermissions(rootGetters) - ) - commit('CURRENT_FILE_OUTGOING_SHARES_UPSERT', share) - resolve(share) - }) - .catch((e) => { - reject(e) - }) + const updatedShare = await client.shares.updateShare(share.id, { + role: role.name, + permissions, + expirationDate }) + + commit( + 'CURRENT_FILE_OUTGOING_SHARES_UPSERT', + buildCollaboratorShare( + updatedShare.shareInfo, + getters.highlightedFile, + allowSharePermissions(rootGetters) + ) + ) }, addShare( context, @@ -323,22 +306,19 @@ export default { shareWith, shareType, permissions, + role, expirationDate, storageId, displayName } ) { - let spaceRef - if (storageId) { - spaceRef = `${storageId}${path}` - } - if (shareType === ShareTypes.group.value) { client.shares .shareFileWithGroup(path, shareWith, { - permissions: permissions, - expirationDate: expirationDate, - spaceRef + permissions, + role: role.name, + expirationDate, + spaceRef: storageId }) .then((share) => { context.commit( @@ -369,10 +349,10 @@ export default { if (shareType === ShareTypes.space.value) { client.shares .shareSpaceWithUser(path, shareWith, storageId, { - permissions + permissions, + role: role.name }) .then(() => { - const role = SpacePeopleShareRoles.getByBitmask(permissions) const shareObj = { role: role.name, onPremisesSamAccountName: shareWith, @@ -412,10 +392,11 @@ export default { const remoteShare = shareType === ShareTypes.remote.value client.shares .shareFileWithUser(path, shareWith, { - permissions: permissions, remoteUser: remoteShare, - expirationDate: expirationDate, - spaceRef + permissions, + role: role.name, + expirationDate, + spaceRef: storageId }) .then((share) => { context.commit( @@ -447,6 +428,8 @@ export default { additionalParams.shareWith = share.collaborator.name } + console.log(share) + client.shares .deleteShare(share.id, additionalParams) .then(() => { @@ -498,10 +481,7 @@ export default { return Promise.resolve() } - let spaceRef - if (storageId) { - spaceRef = `${storageId}${path}` - } + const spaceRef = storageId // remove last entry which is the root folder parentPaths.pop() diff --git a/packages/web-app-files/tests/unit/components/SideBar/Shares/FileShares.spec.js b/packages/web-app-files/tests/unit/components/SideBar/Shares/FileShares.spec.js index 62688aa4c63..db74760affe 100644 --- a/packages/web-app-files/tests/unit/components/SideBar/Shares/FileShares.spec.js +++ b/packages/web-app-files/tests/unit/components/SideBar/Shares/FileShares.spec.js @@ -89,18 +89,6 @@ describe('FileShares', () => { await wrapper.vm.$nextTick() expect(spyOnCollaboratorDeleteTrigger).toHaveBeenCalledTimes(1) }) - it('reloads shares if highlighted file is changed', async () => { - const spyOnReloadShares = jest - .spyOn(FileShares.methods, '$_reloadShares') - .mockImplementation() - const wrapper = getMountedWrapper({ - user, - outgoingCollaborators: collaborators - }) - wrapper.vm.$store.commit('Files/SET_HIGHLIGHTED_FILE', { name: 'testfile2' }) - await wrapper.vm.$nextTick() - expect(spyOnReloadShares).toHaveBeenCalledTimes(1) - }) it('correctly passes the shared parent route to the collaborator list item', () => { const wrapper = getShallowMountedWrapper({ user, diff --git a/packages/web-app-files/tests/unit/components/SideBar/Shares/SharesPanel.spec.js b/packages/web-app-files/tests/unit/components/SideBar/Shares/SharesPanel.spec.js new file mode 100644 index 00000000000..c31f11c18d9 --- /dev/null +++ b/packages/web-app-files/tests/unit/components/SideBar/Shares/SharesPanel.spec.js @@ -0,0 +1,49 @@ +import { createLocalVue, mount } from '@vue/test-utils' +import GetTextPlugin from 'vue-gettext' +import Vuex from 'vuex' +import DesignSystem from 'owncloud-design-system' +import SharesPanel from '@files/src/components/SideBar/Shares/SharesPanel.vue' + +const localVue = createLocalVue() +localVue.use(DesignSystem) +localVue.use(Vuex) +localVue.use(GetTextPlugin, { + translations: 'does-not-matter.json', + silent: true +}) + +jest.mock('web-pkg/src/composables/reactivity') +describe('SharesPanel', () => { + it('reloads shares if highlighted file is changed', async () => { + const spyOnReloadShares = jest.spyOn(SharesPanel.methods, '$_reloadShares').mockImplementation() + const wrapper = mount(SharesPanel, { + localVue, + store: new Vuex.Store({ + modules: { + Files: { + namespaced: true, + state: { + highlightedFile: { name: '1' } + }, + getters: { + highlightedFile: (state) => { + return state.highlightedFile + } + }, + mutations: { + SET_HIGHLIGHTED_FILE(state, file) { + state.highlightedFile = file + } + } + } + } + }), + stubs: { + 'file-shares': true + } + }) + wrapper.vm.$store.commit('Files/SET_HIGHLIGHTED_FILE', { name: '2' }) + await wrapper.vm.$nextTick() + expect(spyOnReloadShares).toHaveBeenCalledTimes(2) + }) +}) diff --git a/packages/web-app-files/tests/unit/store/actions.spec.js b/packages/web-app-files/tests/unit/store/actions.spec.js index 1dcffb8e5ce..63685553ea8 100644 --- a/packages/web-app-files/tests/unit/store/actions.spec.js +++ b/packages/web-app-files/tests/unit/store/actions.spec.js @@ -108,7 +108,8 @@ describe('vuex store actions', () => { client: clientMock, graphClient: graphClientMock, share: dataSet.share, - permissions: 1, + permissions: spaceRoleManager.bitmask(false), + role: spaceRoleManager, expirationDate: null }) @@ -128,7 +129,8 @@ describe('vuex store actions', () => { graphClient: graphClientMock, shareType: dataSet.shareType, storageId: dataSet.storageId, - permissions: 1, + permissions: spaceRoleManager.bitmask(false), + role: spaceRoleManager, expirationDate: null }) diff --git a/packages/web-runtime/package.json b/packages/web-runtime/package.json index abe0eb63276..729063f2dc5 100644 --- a/packages/web-runtime/package.json +++ b/packages/web-runtime/package.json @@ -26,7 +26,7 @@ "marked": "^4.0.12", "oidc-client": "1.11.5", "owncloud-design-system": "^13.1.0-rc.7", - "owncloud-sdk": "~3.0.0-alpha.9", + "owncloud-sdk": "owncloud/owncloud-sdk#experimental-next", "p-queue": "^6.6.2", "popper-max-size-modifier": "^0.2.0", "portal-vue": "^2.1.7", diff --git a/tests/e2e/support/api/graph/user.ts b/tests/e2e/support/api/graph/user.ts index be3bbb5d90e..500023588cf 100644 --- a/tests/e2e/support/api/graph/user.ts +++ b/tests/e2e/support/api/graph/user.ts @@ -26,6 +26,7 @@ export const createUser = async ({ user, admin }: { user: User; admin: User }): body, user: admin }) + checkResponseStatus(response, 'Failed while creating user') return user } diff --git a/yarn.lock b/yarn.lock index 4c90a3f98ec..9126bdf682e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -9722,8 +9722,8 @@ __metadata: linkType: hard "owncloud-design-system@npm:^13.1.0-rc.7": - version: 13.1.0-rc.7 - resolution: "owncloud-design-system@npm:13.1.0-rc.7" + version: 13.1.0-rc.9 + resolution: "owncloud-design-system@npm:13.1.0-rc.9" peerDependencies: "@popperjs/core": ^2.4.0 "@vue/composition-api": ^1.4.3 @@ -9740,13 +9740,13 @@ __metadata: vue-inline-svg: ^2.0.0 vue-select: ^3.12.0 webfontloader: ^1.6.28 - checksum: a3fa2f1d7f93acf7cda9f71eff88ee2820742c34e60eb5441d0077f4246a040d02a1dc3bcb49942401ce5ba107e1f83c256c8593b676bd20dcd68da05f112674 + checksum: af88c0a64b005a260d6748199c0dc5aa8337c91385a0e749699276087fa36b9043d4054982addf4dc4b17a2c344f0f31c99298fe91fe33f084a7f05c8645ade0 languageName: node linkType: hard -"owncloud-sdk@npm:~3.0.0-alpha.9": - version: 3.0.0-alpha.9 - resolution: "owncloud-sdk@npm:3.0.0-alpha.9" +"owncloud-sdk@owncloud/owncloud-sdk#experimental-next": + version: 2.0.1 + resolution: "owncloud-sdk@https://github.com/owncloud/owncloud-sdk.git#commit=bdf09a74a9940ad4547dc7ece00530ad5b153b1e" peerDependencies: axios: ^0.26.0 cross-fetch: ^3.0.6 @@ -9759,7 +9759,7 @@ __metadata: dependenciesMeta: "@pact-foundation/pact": built: true - checksum: 4ef6bcfc68c59739d40f09c18d34a9116e3e24930e3f659c0738900c6e00cf68c09083012da1deb619b31dbd7561d83006ca9c0fa120068bcd26409029b982f9 + checksum: 9007b36092dc98c26099cd48aa868af95f355cf1c57b85af1aa540f57c468e1278b37143664cd87036456f5e761552c60bc27795abf56334f559c723162413d0 languageName: node linkType: hard @@ -13797,7 +13797,7 @@ __metadata: marked: ^4.0.12 oidc-client: 1.11.5 owncloud-design-system: ^13.1.0-rc.7 - owncloud-sdk: ~3.0.0-alpha.9 + owncloud-sdk: "owncloud/owncloud-sdk#experimental-next" p-queue: ^6.6.2 popper-max-size-modifier: ^0.2.0 portal-vue: ^2.1.7