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.
owncloud · Jan 31, 2024 · edce26b · edce26b
1 parent 3443f1c
commit edce26b
Show file tree

Hide file tree

Showing 7 changed files with 9 additions and 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
@@ -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
@@ -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
@@ -111,18 +111,15 @@ describe('FileVersions', () => {
               expect(revertVersionButton.length).toBe(defaultVersions.length)
             })
             it('should be possible for a share with write permissions', async () => {
-              const resource = mockDeep<ShareResource>({
-                permissions: DavPermission.Updateable,
-                share: undefined
-              })
+              const resource = mockDeep<ShareResource>({ permissions: DavPermission.Updateable })
               const space = mockDeep<ShareSpaceResource>({ driveType: 'share' })
               const { wrapper } = getMountedWrapper({ resource, space })
               await wrapper.vm.fetchVersionsTask.last
               const revertVersionButton = wrapper.findAll(selectors.revertVersionButton)
               expect(revertVersionButton.length).toBe(defaultVersions.length)
             })
             it('should not be possible for a share with read-only permissions', async () => {
-              const resource = mockDeep<ShareResource>({ permissions: '', share: undefined })
+              const resource = mockDeep<ShareResource>({ permissions: '' })
               const space = mockDeep<ShareSpaceResource>({ 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
@@ -52,15 +52,15 @@ describe('SharedWithOthers view', () => {
       it('shows filter if multiple share types are present', () => {
         const { wrapper } = getMountedWrapper({
           files: [
-            mock<ShareResource>({ share: { shareType: ShareTypes.user.value } }),
-            mock<ShareResource>({ share: { shareType: ShareTypes.group.value } })
+            mock<ShareResource>({ shareTypes: [ShareTypes.user.value] }),
+            mock<ShareResource>({ 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<ShareResource>({ share: { shareType: ShareTypes.user.value } })]
+          files: [mock<ShareResource>({ 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
@@ -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
@@ -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
@@ -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 {}