From 9c4d87f92b533be0e3a22d2b54b39122b5ade11e Mon Sep 17 00:00:00 2001
From: Jannik Stehle <jannik.stehle@kiteworks.com>
Date: Fri, 23 Aug 2024 16:55:10 +0200
Subject: [PATCH] feat: mark external user shares in sidebar

Marks external user shares in the sidebar so they can be distinguished from internal shares.
---
 .../enhancement-mark-external-shares          |  6 +++
 .../InviteCollaboratorForm.vue                | 12 +++++-
 .../InviteCollaborator/RecipientContainer.vue |  2 +-
 .../SideBar/Shares/Collaborators/ListItem.vue | 12 ++++++
 .../RecipientContainer.spec.ts.snap           |  2 +-
 .../Shares/Collaborators/ListItem.spec.ts     | 13 +++++-
 .../__snapshots__/ListItem.spec.ts.snap       |  4 +-
 .../web-client/src/helpers/share/functions.ts | 43 +++++++++----------
 packages/web-client/src/helpers/share/type.ts |  2 +-
 .../unit/helpers/share/functions.spec.ts      | 21 ++++++++-
 10 files changed, 86 insertions(+), 31 deletions(-)
 create mode 100644 changelog/unreleased/enhancement-mark-external-shares

diff --git a/changelog/unreleased/enhancement-mark-external-shares b/changelog/unreleased/enhancement-mark-external-shares
new file mode 100644
index 00000000000..32ef717a5ad
--- /dev/null
+++ b/changelog/unreleased/enhancement-mark-external-shares
@@ -0,0 +1,6 @@
+Enhancement: Mark external shares
+
+External shares are now being marked in the right sidebar so they can be distinguished from regular shares.
+
+https://github.com/owncloud/web/pull/11394
+https://github.com/owncloud/web/issues/11361
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 f9e6fd7acc7..758a7d8b4d0 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
@@ -40,7 +40,7 @@
           <autocomplete-item class="mark-element" :item="option" />
         </template>
         <template #no-options>
-          <span v-text="$gettext('No users or groups found.')" />
+          <span v-text="noOptionsLabel" />
         </template>
         <template #selected-option-container="{ option, deselect }">
           <recipient-container :key="option.id" :recipient="option" :deselect="deselect" />
@@ -342,7 +342,7 @@ export default defineComponent({
 
       const users = (userData || []).map((u) => ({
         ...u,
-        shareType: ShareTypes.user.value
+        shareType: unref(isExternalShareRoleType) ? ShareTypes.remote.value : ShareTypes.user.value
       })) as CollaboratorAutoCompleteItem[]
 
       const groups = (groupData || []).map((u) => ({
@@ -492,6 +492,13 @@ export default defineComponent({
       }
     }
 
+    const noOptionsLabel = computed(() => {
+      if (unref(isExternalShareRoleType)) {
+        return $gettext('No external users found.')
+      }
+      return $gettext('No users or groups found.')
+    })
+
     return {
       minSearchLength: capabilityRefs.sharingSearchMinLength,
       isRunningOnEos: computed(() => configStore.options.runningOnEos),
@@ -514,6 +521,7 @@ export default defineComponent({
       isExternalShareRoleType,
       selectShareRoleType,
       focusShareInput,
+      noOptionsLabel,
       DateTime,
 
       // CERN
diff --git a/packages/web-app-files/src/components/SideBar/Shares/Collaborators/InviteCollaborator/RecipientContainer.vue b/packages/web-app-files/src/components/SideBar/Shares/Collaborators/InviteCollaborator/RecipientContainer.vue
index 8c39fdcc79f..4af3e37bf8e 100644
--- a/packages/web-app-files/src/components/SideBar/Shares/Collaborators/InviteCollaborator/RecipientContainer.vue
+++ b/packages/web-app-files/src/components/SideBar/Shares/Collaborators/InviteCollaborator/RecipientContainer.vue
@@ -100,7 +100,7 @@ export default defineComponent({
         case ShareTypes.remote.value:
           return {
             name: ShareTypes.remote.icon,
-            label: this.$gettext('Remote user')
+            label: this.$gettext('External user')
           }
 
         default:
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 3205ef4d53a..586ea78fb14 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
@@ -43,6 +43,14 @@
               v-text="shareDisplayName"
             />
             <span class="oc-invisible-sr" v-text="screenreaderShareDisplayName" />
+            <oc-contextual-helper
+              v-if="isExternalShare"
+              :text="
+                $gettext(
+                  'External user, registered with another organization’s account but granted access to your resources. External users can only have “view” or “edit” permission.'
+                )
+              "
+            />
           </div>
           <div>
             <div
@@ -58,6 +66,7 @@
                   :existing-share-role="share.role"
                   :existing-share-permissions="share.permissions"
                   :is-locked="isLocked"
+                  :is-external="isExternalShare"
                   class="files-collaborators-collaborator-role"
                   mode="edit"
                   @option-change="shareRoleChanged"
@@ -218,6 +227,8 @@ export default defineComponent({
       return formatDateFromDateTime(DateTime.fromISO(props.share.createdDateTime), language.current)
     })
 
+    const isExternalShare = computed(() => props.share.shareType === ShareTypes.remote.value)
+
     const setDenyShare = (value: boolean) => {
       emit('onSetDeny', { share: props.share, value })
     }
@@ -252,6 +263,7 @@ export default defineComponent({
       showErrorMessage,
       upsertSpace,
       upsertSpaceMember,
+      isExternalShare,
       DateTime
     }
   },
diff --git a/packages/web-app-files/tests/unit/components/SideBar/Shares/Collaborators/InviteCollaborator/__snapshots__/RecipientContainer.spec.ts.snap b/packages/web-app-files/tests/unit/components/SideBar/Shares/Collaborators/InviteCollaborator/__snapshots__/RecipientContainer.spec.ts.snap
index 241708720ea..da676d87456 100644
--- a/packages/web-app-files/tests/unit/components/SideBar/Shares/Collaborators/InviteCollaborator/__snapshots__/RecipientContainer.spec.ts.snap
+++ b/packages/web-app-files/tests/unit/components/SideBar/Shares/Collaborators/InviteCollaborator/__snapshots__/RecipientContainer.spec.ts.snap
@@ -22,4 +22,4 @@ exports[`InviteCollaborator RecipientContainer > renders a recipient with a dese
 
 exports[`InviteCollaborator RecipientContainer > renders a recipient with a deselect button > different recipients for different shareTypes 3`] = `"<span class="oc-recipient files-share-invite-recipient" data-testid="recipient-container-Albert Einstein"><oc-icon-stub name="global" filltype="fill" accessiblelabel="Guest user" type="span" size="small" variation="passive" color="" class="oc-recipient-icon" data-testid="recipient-icon"></oc-icon-stub> <p class="oc-recipient-name" data-testid="recipient-name">Albert Einstein</p> <!-- @slot Append content (actions, additional info, etc.)  --> <button type="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"><!--v-if--> <!-- @slot Content of the button --> <oc-icon-stub name="close" filltype="fill" accessiblelabel="" type="span" size="small" variation="passive" color=""></oc-icon-stub></button></span>"`;
 
-exports[`InviteCollaborator RecipientContainer > renders a recipient with a deselect button > different recipients for different shareTypes 4`] = `"<span class="oc-recipient files-share-invite-recipient" data-testid="recipient-container-Albert Einstein"><oc-icon-stub name="earth" filltype="fill" accessiblelabel="Remote user" type="span" size="small" variation="passive" color="" class="oc-recipient-icon" data-testid="recipient-icon"></oc-icon-stub> <p class="oc-recipient-name" data-testid="recipient-name">Albert Einstein</p> <!-- @slot Append content (actions, additional info, etc.)  --> <button type="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"><!--v-if--> <!-- @slot Content of the button --> <oc-icon-stub name="close" filltype="fill" accessiblelabel="" type="span" size="small" variation="passive" color=""></oc-icon-stub></button></span>"`;
+exports[`InviteCollaborator RecipientContainer > renders a recipient with a deselect button > different recipients for different shareTypes 4`] = `"<span class="oc-recipient files-share-invite-recipient" data-testid="recipient-container-Albert Einstein"><oc-icon-stub name="earth" filltype="fill" accessiblelabel="External user" type="span" size="small" variation="passive" color="" class="oc-recipient-icon" data-testid="recipient-icon"></oc-icon-stub> <p class="oc-recipient-name" data-testid="recipient-name">Albert Einstein</p> <!-- @slot Append content (actions, additional info, etc.)  --> <button type="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"><!--v-if--> <!-- @slot Content of the button --> <oc-icon-stub name="close" filltype="fill" accessiblelabel="" type="span" size="small" variation="passive" color=""></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
index e60eb6ba49a..dcb828fcbf9 100644
--- 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
@@ -39,7 +39,8 @@ const selectors = {
   collaboratorRole: '.files-collaborators-collaborator-role',
   collaboratorEdit: '.files-collaborators-collaborator-edit',
   shareInheritanceIndicators: '.oc-resource-indicators',
-  expirationDateIcon: '[data-testid="recipient-info-expiration-date"]'
+  expirationDateIcon: '[data-testid="recipient-info-expiration-date"]',
+  externalContextHelper: '.files-collaborators-collaborator-name-wrapper .oc-contextual-helper'
 }
 
 const getShareMock = ({
@@ -217,6 +218,16 @@ describe('Collaborator ListItem component', () => {
       expect(messagesStore.showErrorMessage).toHaveBeenCalled()
     })
   })
+  describe('external user shares', () => {
+    it('correctly identifies external shares', () => {
+      const share = getShareMock({ shareType: ShareTypes.remote.value })
+      const { wrapper } = createWrapper({ share })
+      const roleDropDown = wrapper.findComponent<typeof RoleDropdown>('role-dropdown-stub')
+
+      expect(roleDropDown.props('isExternal')).toBeTruthy()
+      expect(wrapper.find(selectors.externalContextHelper).exists()).toBeTruthy()
+    })
+  })
 })
 
 function createWrapper({
diff --git a/packages/web-app-files/tests/unit/components/SideBar/Shares/Collaborators/__snapshots__/ListItem.spec.ts.snap b/packages/web-app-files/tests/unit/components/SideBar/Shares/Collaborators/__snapshots__/ListItem.spec.ts.snap
index cdd7f0ef4ac..ec1d1787676 100644
--- a/packages/web-app-files/tests/unit/components/SideBar/Shares/Collaborators/__snapshots__/ListItem.spec.ts.snap
+++ b/packages/web-app-files/tests/unit/components/SideBar/Shares/Collaborators/__snapshots__/ListItem.spec.ts.snap
@@ -8,7 +8,9 @@ exports[`Collaborator ListItem component > share inheritance indicators > show w
         <avatar-image-stub data-v-ccf14be2="" userid="3" user-name="einstein" width="36" class="files-collaborators-collaborator-indicator"></avatar-image-stub>
       </div>
       <div data-v-ccf14be2="" class="files-collaborators-collaborator-name-wrapper oc-pl-s">
-        <div data-v-ccf14be2="" class="oc-text-truncate"><span data-v-ccf14be2="" aria-hidden="true" class="files-collaborators-collaborator-name">einstein</span> <span data-v-ccf14be2="" class="oc-invisible-sr">Share receiver name: einstein</span></div>
+        <div data-v-ccf14be2="" class="oc-text-truncate"><span data-v-ccf14be2="" aria-hidden="true" class="files-collaborators-collaborator-name">einstein</span> <span data-v-ccf14be2="" class="oc-invisible-sr">Share receiver name: einstein</span>
+          <!--v-if-->
+        </div>
         <div data-v-ccf14be2="">
           <div data-v-ccf14be2="" class="oc-flex oc-flex-nowrap oc-flex-middle">
             <role-dropdown-stub data-v-ccf14be2="" existingsharerole="undefined" existingsharepermissions="" domselector="1" mode="edit" showicon="false" islocked="false" isexternal="false" class="files-collaborators-collaborator-role"></role-dropdown-stub>
diff --git a/packages/web-client/src/helpers/share/functions.ts b/packages/web-client/src/helpers/share/functions.ts
index a969327abb2..217780cbe01 100644
--- a/packages/web-client/src/helpers/share/functions.ts
+++ b/packages/web-client/src/helpers/share/functions.ts
@@ -99,17 +99,13 @@ export function buildIncomingShareResource({
   const resourceName = driveItem.name || driveItem.remoteItem.name
   const storageId = extractStorageId(driveItem.remoteItem.id)
 
-  const shareTypes = uniq(
-    driveItem.remoteItem.permissions.map((p) =>
-      p.grantedToV2.group ? ShareTypes.group.value : ShareTypes.user.value
-    )
-  )
-
-  const sharedWith = driveItem.remoteItem.permissions.map(({ grantedToV2 }) => {
+  const shareTypes = uniq(driveItem.remoteItem.permissions.map(getShareTypeFromPermission))
+  const sharedWith = driveItem.remoteItem.permissions.map((permission) => {
+    const { grantedToV2 } = permission
     const identity = grantedToV2.group || grantedToV2.user
     return {
       ...identity,
-      shareType: grantedToV2.group ? ShareTypes.group.value : ShareTypes.user.value
+      shareType: getShareTypeFromPermission(permission)
     }
   })
 
@@ -199,20 +195,10 @@ export function buildOutgoingShareResource({
           shareType: ShareTypes.link.value
         }
       }
-      if (p.grantedToV2.group) {
-        return { ...p.grantedToV2.group, shareType: ShareTypes.group.value }
-      }
-      return { ...p.grantedToV2.user, shareType: ShareTypes.user.value }
-    }),
-    shareTypes: driveItem.permissions.map((p) => {
-      if (p.link) {
-        return ShareTypes.link.value
-      }
-      if (p.grantedToV2.group) {
-        return ShareTypes.group.value
-      }
-      return ShareTypes.user.value
+      const shareType = getShareTypeFromPermission(p)
+      return { ...(p.grantedToV2.user || p.grantedToV2.group), shareType }
     }),
+    shareTypes: driveItem.permissions.map(getShareTypeFromPermission),
     isFolder: !!driveItem.folder,
     type: !!driveItem.folder ? 'folder' : 'file',
     mimeType: driveItem.file?.mimeType || 'httpd/unix-directory',
@@ -253,7 +239,7 @@ export function buildCollaboratorShare({
     id: graphPermission.id,
     resourceId,
     indirect,
-    shareType: graphPermission.grantedToV2.group ? ShareTypes.group.value : ShareTypes.user.value,
+    shareType: getShareTypeFromPermission(graphPermission),
     role,
     sharedBy: { id: invitedBy?.id, displayName: invitedBy?.displayName },
     sharedWith: graphPermission.grantedToV2.user || graphPermission.grantedToV2.group,
@@ -292,3 +278,16 @@ export function buildLinkShare({
     preventsDownload: graphPermission.link.preventsDownload
   }
 }
+
+function getShareTypeFromPermission({ link, grantedToV2 }: Permission) {
+  if (link) {
+    return ShareTypes.link.value
+  }
+  if (grantedToV2?.group) {
+    return ShareTypes.group.value
+  }
+  if (grantedToV2?.user?.['@libre.graph.userType'] === 'Federated') {
+    return ShareTypes.remote.value
+  }
+  return ShareTypes.user.value
+}
diff --git a/packages/web-client/src/helpers/share/type.ts b/packages/web-client/src/helpers/share/type.ts
index 57369bb1d80..fd0cb817938 100644
--- a/packages/web-client/src/helpers/share/type.ts
+++ b/packages/web-client/src/helpers/share/type.ts
@@ -40,7 +40,7 @@ export abstract class ShareTypes {
   static readonly group = new ShareType('group', 1, $gettext('Group'), 'group')
   static readonly link = new ShareType('link', 3, $gettext('Link'), 'link')
   static readonly guest = new ShareType('guest', 4, $gettext('Guest'), 'global')
-  static readonly remote = new ShareType('remote', 6, $gettext('Federated'), 'earth')
+  static readonly remote = new ShareType('remote', 6, $gettext('External'), 'earth')
 
   static readonly individuals = [this.user, this.guest, this.remote]
   static readonly collectives = [this.group]
diff --git a/packages/web-client/tests/unit/helpers/share/functions.spec.ts b/packages/web-client/tests/unit/helpers/share/functions.spec.ts
index 27fec901112..b490e8c267c 100644
--- a/packages/web-client/tests/unit/helpers/share/functions.spec.ts
+++ b/packages/web-client/tests/unit/helpers/share/functions.spec.ts
@@ -223,7 +223,8 @@ describe('share helper functions', () => {
       it('is user type if grantedToV2 includes a user', () => {
         const graphPermission = mock<Permission>({
           '@libre.graph.permissions.actions': [],
-          grantedToV2: { user: {}, group: undefined }
+          grantedToV2: { user: {}, group: undefined },
+          link: undefined
         })
 
         const result = buildCollaboratorShare({
@@ -237,7 +238,8 @@ describe('share helper functions', () => {
       it('is group type if grantedToV2 includes a group', () => {
         const graphPermission = mock<Permission>({
           '@libre.graph.permissions.actions': [],
-          grantedToV2: { user: undefined, group: {} }
+          grantedToV2: { user: undefined, group: {} },
+          link: undefined
         })
 
         const result = buildCollaboratorShare({
@@ -248,6 +250,21 @@ describe('share helper functions', () => {
 
         expect(result.shareType).toEqual(ShareTypes.group.value)
       })
+      it('is external type if grantedToV2 includes a user that is external', () => {
+        const graphPermission = mock<Permission>({
+          '@libre.graph.permissions.actions': [],
+          grantedToV2: { user: { '@libre.graph.userType': 'Federated' }, group: undefined },
+          link: undefined
+        })
+
+        const result = buildCollaboratorShare({
+          graphPermission,
+          graphRoles,
+          resourceId
+        })
+
+        expect(result.shareType).toEqual(ShareTypes.remote.value)
+      })
     })
     describe('permissions', () => {
       it('sets permissions if given directly via property', () => {