From edce26b0810c213a22bf1950a928a5201fc8007a Mon Sep 17 00:00:00 2001 From: Jannik Stehle Date: Wed, 31 Jan 2024 08:49:03 +0100 Subject: [PATCH] refactor: remove share property on ShareResource Removes the `share` property on `ShareResource` because a) there is no need to expose the full share on a resource and b) a `ShareResource` can "hold" multiple shares, rendering that property useless. --- .../src/components/SideBar/Versions/FileVersions.vue | 5 ----- .../web-app-files/src/views/shares/SharedWithOthers.vue | 6 ++++-- .../unit/components/SideBar/Versions/FileVersions.spec.ts | 7 ++----- .../tests/unit/views/shares/SharedWithOthers.spec.ts | 6 +++--- packages/web-client/src/helpers/share/functions.ts | 1 - packages/web-client/src/helpers/share/functionsNG.ts | 2 -- packages/web-client/src/helpers/share/types.ts | 2 -- 7 files changed, 9 insertions(+), 20 deletions(-) diff --git a/packages/web-app-files/src/components/SideBar/Versions/FileVersions.vue b/packages/web-app-files/src/components/SideBar/Versions/FileVersions.vue index ca8650460ff..84820d78c29 100644 --- a/packages/web-app-files/src/components/SideBar/Versions/FileVersions.vue +++ b/packages/web-app-files/src/components/SideBar/Versions/FileVersions.vue @@ -65,7 +65,6 @@ import { } from '@ownclouders/web-pkg' import { computed, defineComponent, inject, Ref, ref, unref, watch } from 'vue' import { isShareSpaceResource, Resource, SpaceResource } from '@ownclouders/web-client/src/helpers' -import { SharePermissions, isShareResource } from '@ownclouders/web-client/src/helpers/share' import { useTask } from 'vue-concurrency' import { useGettext } from 'vue3-gettext' @@ -120,10 +119,6 @@ export default defineComponent({ if (unref(resource).permissions !== undefined) { return unref(resource).permissions.includes(DavPermission.Updateable) } - const res = unref(resource) - if (isShareResource(res) && res.share?.role) { - return res.share.role.hasPermission(SharePermissions.update) - } } return true diff --git a/packages/web-app-files/src/views/shares/SharedWithOthers.vue b/packages/web-app-files/src/views/shares/SharedWithOthers.vue index 94a2deba7ae..462fe663e87 100644 --- a/packages/web-app-files/src/views/shares/SharedWithOthers.vue +++ b/packages/web-app-files/src/views/shares/SharedWithOthers.vue @@ -145,7 +145,7 @@ export default defineComponent({ resourcesViewDefaults const shareTypes = computed(() => { - const uniqueShareTypes = uniq(unref(paginatedResources).map((i) => i.share?.shareType)) + const uniqueShareTypes = uniq(unref(paginatedResources).flatMap((i) => i.shareTypes)) return ShareTypes.getByValues(uniqueShareTypes) }) const selectedShareTypesQuery = useRouteQuery('q_shareType') @@ -155,7 +155,9 @@ export default defineComponent({ return unref(paginatedResources) } return unref(paginatedResources).filter((item) => { - return selectedShareTypes.map((t) => ShareTypes[t].value).includes(item.share.shareType) + return selectedShareTypes + .map((t) => ShareTypes[t].value) + .some((t) => item.shareTypes.includes(t)) }) }) diff --git a/packages/web-app-files/tests/unit/components/SideBar/Versions/FileVersions.spec.ts b/packages/web-app-files/tests/unit/components/SideBar/Versions/FileVersions.spec.ts index b1c1d5c5880..725548739c6 100644 --- a/packages/web-app-files/tests/unit/components/SideBar/Versions/FileVersions.spec.ts +++ b/packages/web-app-files/tests/unit/components/SideBar/Versions/FileVersions.spec.ts @@ -111,10 +111,7 @@ describe('FileVersions', () => { expect(revertVersionButton.length).toBe(defaultVersions.length) }) it('should be possible for a share with write permissions', async () => { - const resource = mockDeep({ - permissions: DavPermission.Updateable, - share: undefined - }) + const resource = mockDeep({ permissions: DavPermission.Updateable }) const space = mockDeep({ driveType: 'share' }) const { wrapper } = getMountedWrapper({ resource, space }) await wrapper.vm.fetchVersionsTask.last @@ -122,7 +119,7 @@ describe('FileVersions', () => { expect(revertVersionButton.length).toBe(defaultVersions.length) }) it('should not be possible for a share with read-only permissions', async () => { - const resource = mockDeep({ permissions: '', share: undefined }) + const resource = mockDeep({ permissions: '' }) const space = mockDeep({ driveType: 'share' }) const { wrapper } = getMountedWrapper({ resource, space }) await wrapper.vm.fetchVersionsTask.last diff --git a/packages/web-app-files/tests/unit/views/shares/SharedWithOthers.spec.ts b/packages/web-app-files/tests/unit/views/shares/SharedWithOthers.spec.ts index 5531504caff..b79644ee907 100644 --- a/packages/web-app-files/tests/unit/views/shares/SharedWithOthers.spec.ts +++ b/packages/web-app-files/tests/unit/views/shares/SharedWithOthers.spec.ts @@ -52,15 +52,15 @@ describe('SharedWithOthers view', () => { it('shows filter if multiple share types are present', () => { const { wrapper } = getMountedWrapper({ files: [ - mock({ share: { shareType: ShareTypes.user.value } }), - mock({ share: { shareType: ShareTypes.group.value } }) + mock({ shareTypes: [ShareTypes.user.value] }), + mock({ shareTypes: [ShareTypes.group.value] }) ] }) expect(wrapper.find('.share-type-filter').exists()).toBeTruthy() }) it('does not show filter if only one share type is present', () => { const { wrapper } = getMountedWrapper({ - files: [mock({ share: { shareType: ShareTypes.user.value } })] + files: [mock({ shareTypes: [ShareTypes.user.value] })] }) expect(wrapper.find('.share-type-filter').exists()).toBeFalsy() }) diff --git a/packages/web-client/src/helpers/share/functions.ts b/packages/web-client/src/helpers/share/functions.ts index 1e8d9a8cfdf..44d12573c22 100644 --- a/packages/web-client/src/helpers/share/functions.ts +++ b/packages/web-client/src/helpers/share/functions.ts @@ -211,7 +211,6 @@ export function buildSharedResource( resource.canUpload = () => SharePermissions.create.enabled(share.permissions) resource.canCreate = () => SharePermissions.create.enabled(share.permissions) resource.isMounted = () => false - resource.share = buildShare(share, resource, allowSharePermission) resource.canDeny = () => SharePermissions.denied.enabled(share.permissions) resource.getDomSelector = () => extractDomSelector(share.id) diff --git a/packages/web-client/src/helpers/share/functionsNG.ts b/packages/web-client/src/helpers/share/functionsNG.ts index 09821eae501..23a2cef1788 100644 --- a/packages/web-client/src/helpers/share/functionsNG.ts +++ b/packages/web-client/src/helpers/share/functionsNG.ts @@ -93,7 +93,6 @@ export function buildIncomingShareResource({ : { ...permission?.grantedToV2.user, shareType } ], shareTypes: [shareType], - share: driveItem, isFolder: !!driveItem.remoteItem.folder, type: !!driveItem.remoteItem.folder ? 'folder' : 'file', mimeType: driveItem.remoteItem.file?.mimeType || 'httpd/unix-directory', @@ -166,7 +165,6 @@ export function buildOutgoingShareResource({ } return ShareTypes.user.value }), - share: driveItem, isFolder: !!driveItem.folder, type: !!driveItem.folder ? 'folder' : 'file', mimeType: driveItem.file?.mimeType || 'httpd/unix-directory', diff --git a/packages/web-client/src/helpers/share/types.ts b/packages/web-client/src/helpers/share/types.ts index 6330404ee77..4280e3e03b0 100644 --- a/packages/web-client/src/helpers/share/types.ts +++ b/packages/web-client/src/helpers/share/types.ts @@ -23,8 +23,6 @@ export interface ShareResource extends Resource { sharedWith: Array<{ shareType: number } & Identity> sharedBy: Identity outgoing: boolean - - share?: any // FIXME: type DriveItem OR remove? do we want to expose the share on each resource? } export interface OutgoingShareResource extends ShareResource {}