Fix group avatar display in space members list (#8248)

* feat: introduce shareType 8 --------- Co-authored-by: Benedikt Kulmann <[email protected]>
owncloud · Feb 1, 2023 · a0314e3 · a0314e3
1 parent 0947e75
commit a0314e3
Show file tree

Hide file tree

Showing 19 changed files with 145 additions and 109 deletions.
diff --git a/.drone.env b/.drone.env
@@ -1,3 +1,3 @@
 # The version of OCIS to use in pipelines that test against OCIS
-OCIS_COMMITID=770fc74a14d2150c51c088e6966434faef8b5557
+OCIS_COMMITID=e1a4ac6b330079e3488152f4c638b1e32743224a
 OCIS_BRANCH=master
diff --git a/changelog/unreleased/enhancement-share-group-recipient b/changelog/unreleased/enhancement-share-group-recipient
@@ -4,4 +4,5 @@ We've added the possibility to share a space with a group.
 
 https://github.com/owncloud/web/pull/8161
 https://github.com/owncloud/web/issues/8160
-https://github.com/owncloud/web/pull/8185
+https://github.com/owncloud/web/pull/8185
+https://github.com/owncloud/web/pull/8248
diff --git a/changelog/unreleased/enhancement-update-sdk b/changelog/unreleased/enhancement-update-sdk
@@ -1,9 +1,12 @@
-Enhancement: Update SDK to v3.1.0-alpha.2
+Enhancement: Update SDK to v3.1.0-alpha.3
 
-We updated the ownCloud SDK to version v3.1.0-alpha.2. Please refer to the full changelog in the SDK release (linked) for more details. Summary:
+We updated the ownCloud SDK to version v3.1.0-alpha.3. Please refer to the full changelog in the SDK release (linked) for more details. Summary:
+
+* Bugfix - Allow removing expiration dates from space shares: [owncloud/owncloud-sdk#1204](https://github.com/owncloud/owncloud-sdk/pull/1204)
+* Enhancement - Resource processing: [owncloud/owncloud-sdk#1109](https://github.com/owncloud/owncloud-sdk/pull/1109)
+* Enhancement - Share space with group: [owncloud/owncloud-sdk#1207](https://github.com/owncloud/owncloud-sdk/pull/1207)
 
-* Bugfix - Allow removing expiration dates from space shares: [#1204](https://github.com/owncloud/owncloud-sdk/pull/1204)
-* Enhancement - Resource processing: [#1109](https://github.com/owncloud/owncloud-sdk/pull/1109)
 
 https://github.com/owncloud/web/pull/8320
-https://github.com/owncloud/owncloud-sdk/releases/tag/v3.1.0-alpha.2
+https://github.com/owncloud/web/pull/8248
+https://github.com/owncloud/owncloud-sdk/releases/tag/v3.1.0-alpha.3
diff --git a/...files/src/components/SideBar/Shares/Collaborators/InviteCollaborator/AutocompleteItem.vue b/...files/src/components/SideBar/Shares/Collaborators/InviteCollaborator/AutocompleteItem.vue
@@ -5,7 +5,7 @@
     :class="collaboratorClass"
   >
     <avatar-image
-      v-if="isUser || isSpace"
+      v-if="isUser || isSpaceUser"
       class="oc-mr-s"
       :width="36"
       :userid="item.value.shareWith"
@@ -21,7 +21,7 @@
     >
     </oc-icon>
     <oc-icon
-      v-else-if="isGroup"
+      v-else-if="isGroup || isSpaceGroup"
       key="avatar-group"
       class="oc-mr-s files-recipient-suggestion-avatar"
       name="group"
@@ -76,8 +76,8 @@ export default {
       return this.shareType === ShareTypes.user
     },
 
-    isSpace() {
-      return this.shareType === ShareTypes.space
+    isSpaceUser() {
+      return this.shareType === ShareTypes.spaceUser
     },
 
     isGuest() {
@@ -88,6 +88,10 @@ export default {
       return this.shareType === ShareTypes.group
     },
 
+    isSpaceGroup() {
+      return this.shareType === ShareTypes.spaceGroup
+    },
+
     collaboratorClass() {
       return `files-collaborators-search-${this.shareType.key}`
     }

diff --git a/...src/components/SideBar/Shares/Collaborators/InviteCollaborator/InviteCollaboratorForm.vue b/...src/components/SideBar/Shares/Collaborators/InviteCollaborator/InviteCollaboratorForm.vue
@@ -195,31 +195,29 @@ export default defineComponent({
           this.configuration?.options?.sharingRecipientsPerPage
         )
 
-        // fixMe: head-breaking logic
         const users = recipients.exact.users
           .concat(recipients.users)
           .filter((user) => user.value.shareWith !== this.user.id)
           .map((result) => {
-            // Inject the correct share type here if space
-            const shareType = this.resourceIsSpace ? ShareTypes.space.value : result.value.shareType
-            return {
-              ...result,
-              value: {
-                ...result.value,
-                shareType
-              }
+            if (this.resourceIsSpace) {
+              result.value.shareType = ShareTypes.spaceUser.value
             }
+            return result
           })
-
-        const groups = recipients.exact.groups.concat(recipients.groups)
+        const groups = recipients.exact.groups.concat(recipients.groups).map((result) => {
+          if (this.resourceIsSpace) {
+            result.value.shareType = ShareTypes.spaceGroup.value
+          }
+          return result
+        })
         const remotes = recipients.exact.remotes.concat(recipients.remotes)
 
-        this.autocompleteResults = users.concat(groups, remotes).filter((collaborator) => {
+        this.autocompleteResults = [...users, ...groups, ...remotes].filter((collaborator) => {
           const selected = this.selectedCollaborators.find((selectedCollaborator) => {
             return (
               collaborator.value.shareWith === selectedCollaborator.value.shareWith &&
-              parseInt(collaborator.value.shareType, 10) ===
-                parseInt(selectedCollaborator.value.shareType, 10)
+              parseInt(collaborator.value.shareType) ===
+                parseInt(selectedCollaborator.value.shareType)
             )
           })
 
@@ -230,16 +228,7 @@ export default defineComponent({
             const shareCollaboratorIdentifier =
               share.collaborator.name || share.collaborator.displayName
             const isSameByIdentifier = collaborator.value.shareWith === shareCollaboratorIdentifier
-            const isSameByType = parseInt(collaborator.value.shareType, 10) === share.shareType
-            const isGroupShareCollaborator =
-              parseInt(collaborator.value.shareType, 10) == ShareTypes.group.value
-
-            // we cannot differentiate by isSameByType for spaces group collaborators, collaborators shareType and group shareType is not the same
-            // should be part of head-breaking fixMe above..
-            if (this.resourceIsSpace && isGroupShareCollaborator && isSameByIdentifier) {
-              return true
-            }
-
+            const isSameByType = parseInt(collaborator.value.shareType) === share.shareType
             return isSameByIdentifier && isSameByType
           })
 

diff --git a/...les/src/components/SideBar/Shares/Collaborators/InviteCollaborator/RecipientContainer.vue b/...les/src/components/SideBar/Shares/Collaborators/InviteCollaborator/RecipientContainer.vue
@@ -49,7 +49,7 @@ export default {
       formattedRecipient: {
         name: this.recipient.label,
         icon: this.getRecipientIcon(),
-        hasAvatar: [ShareTypes.user.value, ShareTypes.space.value].includes(
+        hasAvatar: [ShareTypes.user.value, ShareTypes.spaceUser.value].includes(
           this.recipient.value.shareType
         ),
         isLoadingAvatar: true

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
@@ -1,6 +1,6 @@
 <template>
   <div
-    :data-testid="`collaborator-${isUser || isSpace ? 'user' : 'group'}-item-${
+    :data-testid="`collaborator-${isAnyUserShareType ? 'user' : 'group'}-item-${
       share.collaborator.name
     }`"
     class="files-collaborators-collaborator oc-py-xs"
@@ -9,7 +9,7 @@
       <div class="oc-width-2-3 oc-flex oc-flex-middle">
         <div>
           <avatar-image
-            v-if="isUser || isSpace"
+            v-if="isAnyUserShareType"
             :userid="share.collaborator.name"
             :user-name="share.collaborator.displayName"
             :width="36"
@@ -40,7 +40,7 @@
                 :dom-selector="shareDomSelector"
                 :existing-permissions="share.customPermissions"
                 :existing-role="share.role"
-                :allow-share-permission="hasResharing || isSpace"
+                :allow-share-permission="hasResharing || isAnySpaceShareType"
                 class="files-collaborators-collaborator-role"
                 @option-change="shareRoleChanged"
               />
@@ -92,7 +92,7 @@
         <oc-info-drop
           ref="accessDetailsDrop"
           class="share-access-details-drop"
-          v-bind="isSpace ? accessDetailsPropsSpace : accessDetailsProps"
+          v-bind="isAnySpaceShareType ? accessDetailsPropsSpace : accessDetailsProps"
           mode="manual"
           :target="`#edit-drop-down-${editDropDownToggleId}`"
         />
@@ -165,12 +165,12 @@ export default defineComponent({
       return extractDomSelector(this.share.id)
     },
 
-    isUser() {
-      return this.shareType === ShareTypes.user
+    isAnyUserShareType() {
+      return [ShareTypes.user, ShareTypes.spaceUser].includes(this.shareType)
     },
 
-    isSpace() {
-      return this.shareType === ShareTypes.space
+    isAnySpaceShareType() {
+      return [ShareTypes.spaceUser, ShareTypes.spaceGroup].includes(this.shareType)
     },
 
     shareTypeText() {
@@ -358,8 +358,10 @@ export default defineComponent({
     saveShareChanges({ role, permissions, expirationDate }) {
       const bitmask = role.hasCustomPermissions
         ? SharePermissions.permissionsToBitmask(permissions)
-        : SharePermissions.permissionsToBitmask(role.permissions(this.hasResharing || this.isSpace))
-      const changeMethod = this.isSpace ? this.changeSpaceMember : this.changeShare
+        : SharePermissions.permissionsToBitmask(
+            role.permissions(this.hasResharing || this.isAnySpaceShareType)
+          )
+      const changeMethod = this.isAnySpaceShareType ? this.changeSpaceMember : this.changeShare
       changeMethod({
         ...this.$language,
         client: this.$client,

diff --git a/packages/web-app-files/src/helpers/resources.ts b/packages/web-app-files/src/helpers/resources.ts
@@ -9,9 +9,7 @@ import {
   Share,
   ShareStatus,
   ShareTypes,
-  spaceRoleEditor,
-  spaceRoleManager,
-  spaceRoleViewer
+  buildSpaceShare
 } from 'web-client/src/helpers/share'
 import {
   buildWebDavSpacesPath,
@@ -236,44 +234,13 @@ export function buildShare(s, file, allowSharePermission): Share {
   if (parseInt(s.share_type) === ShareTypes.link.value) {
     return _buildLink(s)
   }
-  if (parseInt(s.share_type) === ShareTypes.space.value) {
+  if ([ShareTypes.spaceUser.value, ShareTypes.spaceGroup.value].includes(parseInt(s.share_type))) {
     return buildSpaceShare(s, file)
   }
 
   return buildCollaboratorShare(s, file, allowSharePermission)
 }
 
-export function buildSpaceShare(s, storageId): Share {
-  let permissions, role
-
-  switch (s.role) {
-    case spaceRoleManager.name:
-      permissions = spaceRoleManager.bitmask(true)
-      role = spaceRoleManager
-      break
-    case spaceRoleEditor.name:
-      permissions = spaceRoleEditor.bitmask(true)
-      role = spaceRoleEditor
-      break
-    case spaceRoleViewer.name:
-      permissions = spaceRoleViewer.bitmask(true)
-      role = spaceRoleViewer
-      break
-  }
-
-  return {
-    shareType: ShareTypes.space.value,
-    id: storageId,
-    collaborator: {
-      name: s.onPremisesSamAccountName,
-      displayName: s.displayName,
-      additionalInfo: null
-    },
-    permissions,
-    role
-  }
-}
-
 function _buildLink(link): Share {
   let description = ''
   const permissions = parseInt(link.permissions)

diff --git a/packages/web-app-files/src/services/folder/loaderSharedWithOthers.ts b/packages/web-app-files/src/services/folder/loaderSharedWithOthers.ts
@@ -35,7 +35,9 @@ export class FolderLoaderSharedWithOthers implements FolderLoader {
       store.commit('Files/CLEAR_CURRENT_FILES_LIST')
 
       const shareTypes = ShareTypes.authenticated
-        .filter((type) => type.value !== ShareTypes.space.value)
+        .filter(
+          (type) => ![ShareTypes.spaceUser.value, ShareTypes.spaceGroup.value].includes(type.value)
+        )
         .map((share) => share.value)
         .join(',')
 

diff --git a/...Bar/Shares/Collaborators/InviteCollaborator/__snapshots__/RecipientContainer.spec.ts.snap b/...Bar/Shares/Collaborators/InviteCollaborator/__snapshots__/RecipientContainer.spec.ts.snap
@@ -67,3 +67,14 @@ exports[`InviteCollaborator RecipientContainer renders a recipient with a desele
   </button>
 </span>
 `;
+
+exports[`InviteCollaborator RecipientContainer renders a recipient with a deselect button different recipients for different shareTypes 6`] = `
+<span class="oc-recipient files-share-invite-recipient">
+  <oc-icon-stub accessiblelabel="User" class="oc-recipient-icon" color="" filltype="fill" name="user" size="small" type="span" variation="passive"></oc-icon-stub>
+  <p class="oc-recipient-name">Albert Einstein</p> <!-- @slot Append content (actions, additional info, etc.)  -->
+  <button aria-label="Deselect Albert Einstein" class="oc-button oc-rounded oc-button-m oc-button-justify-content-center oc-button-gap-m oc-button-passive oc-button-passive-raw files-share-invite-recipient-btn-remove" type="button">
+    <!-- @slot Content of the button -->
+    <oc-icon-stub accessiblelabel="" color="" filltype="fill" name="close" size="small" type="span" variation="passive"></oc-icon-stub>
+  </button>
+</span>
+`;
diff --git a/packages/web-app-files/tests/unit/components/SideBar/Shares/Collaborators/ListItem.spec.ts b/packages/web-app-files/tests/unit/components/SideBar/Shares/Collaborators/ListItem.spec.ts
@@ -34,8 +34,8 @@ const selectors = {
 
 describe('Collaborator ListItem component', () => {
   describe('displays the correct image/icon according to the shareType', () => {
-    describe('user and space share type', () => {
-      it.each([ShareTypes.user.value, ShareTypes.space.value])(
+    describe('user and spaceUser share type', () => {
+      it.each([ShareTypes.user.value, ShareTypes.spaceUser.value])(
         'should display a users avatar',
         (shareType) => {
           const { wrapper } = createWrapper({ shareType })
@@ -54,14 +54,22 @@ describe('Collaborator ListItem component', () => {
     describe('non-user share types', () => {
       it.each(
         ShareTypes.all.filter(
-          (shareType) => ![ShareTypes.user, ShareTypes.space].includes(shareType)
+          (shareType) => ![ShareTypes.user, ShareTypes.spaceUser].includes(shareType)
         )
       )('should display an oc-avatar-item for any non-user share types', (shareType) => {
         const { wrapper } = createWrapper({ shareType: shareType.value })
         expect(wrapper.find(selectors.userAvatarImage).exists()).toBeFalsy()
         expect(wrapper.find(selectors.notUserAvatar).exists()).toBeTruthy()
         expect(wrapper.find(selectors.notUserAvatar).attributes().name).toEqual(shareType.key)
       })
+      it('should display an oc-avatar-item for space group shares', () => {
+        const { wrapper } = createWrapper({
+          shareType: ShareTypes.spaceGroup.value,
+          collaborator: { name: undefined, displayName: 'someGroup', additionalInfo: undefined }
+        })
+        expect(wrapper.find(selectors.userAvatarImage).exists()).toBeFalsy()
+        expect(wrapper.find(selectors.notUserAvatar).exists()).toBeTruthy()
+      })
     })
   })
   describe('share info', () => {
@@ -115,7 +123,7 @@ describe('Collaborator ListItem component', () => {
       expect(changeShareStub).toHaveBeenCalled()
     })
     it('calls "changeSpaceMember" for space resources', () => {
-      const { wrapper } = createWrapper({ shareType: ShareTypes.space.value })
+      const { wrapper } = createWrapper({ shareType: ShareTypes.spaceUser.value })
       const changeShareStub = jest.spyOn(wrapper.vm, 'changeSpaceMember')
       ;(wrapper.findComponent<any>('role-dropdown-stub').vm as any).$emit('optionChange', {
         role: peopleRoleViewerFile,

diff --git a/packages/web-app-files/tests/unit/components/SideBar/Shares/SpaceMembers.spec.ts b/packages/web-app-files/tests/unit/components/SideBar/Shares/SpaceMembers.spec.ts
@@ -21,7 +21,7 @@ import {
 const memberMocks = {
   [spaceRoleManager.name]: {
     id: '1',
-    shareType: ShareTypes.space.value,
+    shareType: ShareTypes.spaceUser.value,
     collaborator: {
       name: 'alice',
       displayName: 'alice'
@@ -32,7 +32,7 @@ const memberMocks = {
   },
   [spaceRoleEditor.name]: {
     id: '2',
-    shareType: ShareTypes.space.value,
+    shareType: ShareTypes.spaceUser.value,
     collaborator: {
       onPremisesSamAccountName: 'Einstein',
       displayName: 'einstein'
@@ -43,7 +43,7 @@ const memberMocks = {
   },
   [spaceRoleViewer.name]: {
     id: '3',
-    shareType: ShareTypes.space.value,
+    shareType: ShareTypes.spaceUser.value,
     collaborator: {
       onPremisesSamAccountName: 'Marie',
       displayName: 'marie'

diff --git a/packages/web-app-files/tests/unit/store/mutations.spec.ts b/packages/web-app-files/tests/unit/store/mutations.spec.ts
@@ -105,7 +105,7 @@ describe('vuex store mutations', () => {
     it('removes an outgoing space share', () => {
       const shareToRemove = {
         id: 1,
-        shareType: ShareTypes.space.value,
+        shareType: ShareTypes.spaceUser.value,
         collaborator: { name: 'admin' }
       }
       const state = { currentFileOutgoingShares: [shareToRemove] }
@@ -127,7 +127,7 @@ describe('vuex store mutations', () => {
     it('updates an outgoing space share', () => {
       const share = {
         id: 1,
-        shareType: ShareTypes.space.value,
+        shareType: ShareTypes.spaceUser.value,
         permissions: 1,
         collaborator: { name: 'admin' }
       }