fix: sort space members and managers by name within a role

packages/web-app-admin-settings/src/components/Spaces/SideBar/MembersPanel.vue

@@ -84,15 +84,28 @@ const availableRoles = computed<ShareRole[]>(() => {
     })
 })
 
-// group members by role before paginating, so a role section is not scattered across pages
+// group members by role before paginating, so a role section is not scattered across pages.
+// sort by name within a role, the server returns members in a different order on every request.
 const sortedPermissions = computed(() => {
   const roleIds = unref(availableRoles).map(({ id }) => id)
   function roleIndex(permission: Permission) {
     const index = roleIds.findIndex((id) => permission.roles.includes(id))
     return index === -1 ? roleIds.length : index
   }
+  function displayName({ grantedToV2 }: Permission) {
+    return grantedToV2?.user?.displayName || grantedToV2?.group?.displayName || ''
+  }
 
-  return [...unref(filteredPermissions)].sort((a, b) => roleIndex(a) - roleIndex(b))
+  return [...unref(filteredPermissions)].sort((a, b) => {
+    return (
+      roleIndex(a) - roleIndex(b) ||
+      displayName(a).localeCompare(displayName(b), undefined, {
+        sensitivity: 'base',
+        numeric: true
+      }) ||
+      (a.id || '').localeCompare(b.id || '')
+    )
+  })
 })
 
 const {

packages/web-app-admin-settings/tests/unit/components/Spaces/SideBar/MembersPanel.spec.ts

@@ -13,10 +13,16 @@ import MembersRoleSection from '../../../../../src/components/Spaces/SideBar/Mem
 import { Permission } from '@opencloud-eu/web-client/graph/generated'
 import { OcPaginationInline } from '@opencloud-eu/design-system/components'
 
+function roleWithActions(id: string, displayName: string, amountOfActions: number) {
+  const allowedResourceActions = Array.from({ length: amountOfActions }, (_, i) => `action-${i}`)
+  return mock<ShareRole>({ id, displayName, rolePermissions: [{ allowedResourceActions }] })
+}
+
+// roles are sorted by their amount of allowed actions
 const graphRoles = {
-  '1': mock<ShareRole>({ id: '1', displayName: 'Managers', rolePermissions: [] }),
-  '2': mock<ShareRole>({ id: '2', displayName: 'Editors', rolePermissions: [] }),
-  '3': mock<ShareRole>({ id: '3', displayName: 'Viewers', rolePermissions: [] })
+  '1': roleWithActions('1', 'Managers', 3),
+  '2': roleWithActions('2', 'Editors', 2),
+  '3': roleWithActions('3', 'Viewers', 1)
 }
 
 const spaceMock = {
@@ -65,6 +71,66 @@ describe('MembersPanel', () => {
         .map((section) => section.props().permissions[0].grantedToV2.user.displayName)
     ).toEqual(['admin', 'marie', 'einstein'])
   })
+  it('should sort members by name within a role, independent of the order returned by the server', () => {
+    const permissions = [
+      mock<Permission>({
+        id: 'p-zoe',
+        grantedToV2: { user: { displayName: 'Zoe' } },
+        roles: ['1']
+      }),
+      mock<Permission>({
+        id: 'p-marie',
+        grantedToV2: { user: { displayName: 'marie' } },
+        roles: ['3']
+      }),
+      mock<Permission>({
+        id: 'p-group',
+        grantedToV2: { group: { displayName: 'Physics' } },
+        roles: ['3']
+      }),
+      mock<Permission>({
+        id: 'p-admin-2',
+        grantedToV2: { user: { displayName: 'Admin' } },
+        roles: ['3']
+      }),
+      mock<Permission>({
+        id: 'p-einstein',
+        grantedToV2: { user: { displayName: 'Einstein' } },
+        roles: ['1']
+      }),
+      mock<Permission>({
+        id: 'p-admin-1',
+        grantedToV2: { user: { displayName: 'admin' } },
+        roles: ['3']
+      }),
+      mock<Permission>({
+        id: 'p-user-10',
+        grantedToV2: { user: { displayName: 'User 10' } },
+        roles: ['3']
+      }),
+      mock<Permission>({
+        id: 'p-user-2',
+        grantedToV2: { user: { displayName: 'User 2' } },
+        roles: ['3']
+      })
+    ]
+
+    for (const order of [permissions, [...permissions].reverse()]) {
+      const { wrapper } = getWrapper({
+        spaceResource: { root: { permissions: order } } as undefined as SpaceResource
+      })
+
+      expect(rolesOnPage(wrapper)).toEqual(['Managers', 'Viewers'])
+      expect(
+        wrapper
+          .findAllComponents<typeof MembersRoleSection>(selectors.membersRolePanelStub)
+          .map((section) => section.props().permissions.map(({ id }: Permission) => id))
+      ).toEqual([
+        ['p-einstein', 'p-zoe'],
+        ['p-admin-1', 'p-admin-2', 'p-marie', 'p-group', 'p-user-2', 'p-user-10']
+      ])
+    }
+  })
   it('should filter members accordingly to the entered search term', async () => {
     const userToFilterFor = spaceMock.root.permissions[2]
     const { wrapper } = getWrapper()

packages/web-client/src/helpers/space/functions.ts

@@ -16,7 +16,7 @@ import {
 import { DavProperty } from '../../webdav/constants'
 import { buildWebDavPublicPath, buildWebDavOcmPath } from '../publicLink'
 import { urlJoin } from '../../utils'
-import { Drive, DriveItem } from '@opencloud-eu/web-client/graph/generated'
+import { Drive, DriveItem, Permission } from '@opencloud-eu/web-client/graph/generated'
 import { CollaboratorShare, GraphSharePermission, ShareRole } from '../share'
 
 export function buildWebDavSpacesPath(storageId: string, path?: string) {
@@ -51,7 +51,7 @@ export function isManager(share: CollaboratorShare) {
 }
 
 export const getSpaceManagers = (space: SpaceResource, graphRoles: Record<string, ShareRole>) => {
-  return space.root.permissions?.filter((p) => {
+  const managers = space.root.permissions?.filter((p) => {
     let permissionActions: string[] = []
     if (p['@libre.graph.permissions.actions']) {
       permissionActions = p['@libre.graph.permissions.actions']
@@ -66,6 +66,20 @@ export const getSpaceManagers = (space: SpaceResource, graphRoles: Record<string
 
     return permissionActions.includes(GraphSharePermission.deletePermissions)
   })
+
+  // sort by name, the server returns the permissions in a different order on every request
+  function displayName({ grantedToV2 }: Permission) {
+    return grantedToV2?.user?.displayName || grantedToV2?.group?.displayName || ''
+  }
+
+  return managers?.sort((a, b) => {
+    return (
+      displayName(a).localeCompare(displayName(b), undefined, {
+        sensitivity: 'base',
+        numeric: true
+      }) || (a.id || '').localeCompare(b.id || '')
+    )
+  })
 }
 
 export type PublicLinkType = 'ocm' | 'public-link'

packages/web-client/tests/unit/helpers/space/functions.spec.ts

@@ -1,7 +1,7 @@
-import { buildSpace, SpaceResource } from '../../../../src/helpers/space'
+import { buildSpace, getSpaceManagers, SpaceResource } from '../../../../src/helpers/space'
 import { mock } from 'vitest-mock-extended'
 import { Ability, GraphSharePermission } from '@opencloud-eu/web-client'
-import { Drive, User } from '@opencloud-eu/web-client/graph/generated'
+import { Drive, Permission, User } from '@opencloud-eu/web-client/graph/generated'
 
 describe('buildSpace', () => {
   const id = '1'
@@ -285,3 +285,37 @@ describe('buildSpace', () => {
     )
   })
 })
+
+describe('getSpaceManagers', () => {
+  function permission(id: string, grantedToV2: Permission['grantedToV2'], isManager = true) {
+    return mock<Permission>({
+      id,
+      grantedToV2,
+      roles: [],
+      '@libre.graph.permissions.actions': isManager
+        ? [GraphSharePermission.deletePermissions]
+        : [GraphSharePermission.readBasic]
+    })
+  }
+
+  it('returns the managers sorted by name, independent of the order returned by the server', () => {
+    const permissions = [
+      permission('p-user-10', { user: { id: 'u10', displayName: 'User 10' } }),
+      permission('p-viewer', { user: { id: 'viewer', displayName: 'Anna' } }, false),
+      permission('p-admin-2', { user: { id: 'admin2', displayName: 'Admin' } }),
+      permission('p-group', { group: { id: 'group', displayName: 'physics' } }),
+      permission('p-user-2', { user: { id: 'u2', displayName: 'User 2' } }),
+      permission('p-admin-1', { user: { id: 'admin1', displayName: 'admin' } })
+    ]
+    const expectedOrder = ['p-admin-1', 'p-admin-2', 'p-group', 'p-user-2', 'p-user-10']
+
+    for (const order of [permissions, [...permissions].reverse()]) {
+      const space = mock<SpaceResource>({ root: { permissions: order } })
+      const serverOrder = order.map(({ id }) => id)
+
+      expect(getSpaceManagers(space, {}).map(({ id }) => id)).toEqual(expectedOrder)
+      // the permissions of the space itself are not reordered
+      expect(space.root.permissions.map(({ id }) => id)).toEqual(serverOrder)
+    }
+  })
+})

packages/web-pkg/src/composables/piniaStores/spaces.ts

@@ -19,9 +19,19 @@ import { useExtensionRegistry } from './extensionRegistry'
 import { markSpaceVaultStatus } from '../../helpers/vault'
 import { ListPermissionsSpaceRootSelectEnum } from '@opencloud-eu/web-client/graph/generated'
 
-// sort space members with higher permissions (managers) at the top
+// sort space members with higher permissions (managers) at the top, then by name.
+// the server returns members in a different order on every request, so the order must not depend on it.
 export const sortSpaceMembers = (shares: CollaboratorShare[]) => {
-  return shares.sort((a, b) => b.permissions.length - a.permissions.length)
+  return shares.sort((a, b) => {
+    return (
+      b.permissions.length - a.permissions.length ||
+      (a.sharedWith.displayName || '').localeCompare(b.sharedWith.displayName || '', undefined, {
+        sensitivity: 'base',
+        numeric: true
+      }) ||
+      (a.id || '').localeCompare(b.id || '')
+    )
+  })
 }
 
 export const getSpacesByType = async ({

packages/web-pkg/tests/unit/composables/piniaStores/spaces.spec.ts

@@ -32,6 +32,58 @@ describe('spaces', () => {
         sortedMembers[0].permissions.includes(GraphSharePermission.updatePermissions)
       ).toBeTruthy()
     })
+    it('sorts space members with the same permissions by name (numeric aware), independent of the input order', () => {
+      const manager = mock<CollaboratorShare>({
+        id: 'share-zoe',
+        permissions: [GraphSharePermission.updatePermissions],
+        sharedWith: { displayName: 'Zoe' }
+      })
+      const members = [
+        manager,
+        mock<CollaboratorShare>({
+          id: 'share-marie',
+          permissions: [],
+          sharedWith: { displayName: 'marie' }
+        }),
+        mock<CollaboratorShare>({
+          id: 'share-admin-2',
+          permissions: [],
+          sharedWith: { displayName: 'Admin' }
+        }),
+        mock<CollaboratorShare>({
+          id: 'share-einstein',
+          permissions: [],
+          sharedWith: { displayName: 'Einstein' }
+        }),
+        mock<CollaboratorShare>({
+          id: 'share-admin-1',
+          permissions: [],
+          sharedWith: { displayName: 'admin' }
+        }),
+        mock<CollaboratorShare>({
+          id: 'share-user-10',
+          permissions: [],
+          sharedWith: { displayName: 'User 10' }
+        }),
+        mock<CollaboratorShare>({
+          id: 'share-user-2',
+          permissions: [],
+          sharedWith: { displayName: 'User 2' }
+        })
+      ]
+      const expectedOrder = [
+        'share-zoe',
+        'share-admin-1',
+        'share-admin-2',
+        'share-einstein',
+        'share-marie',
+        'share-user-2',
+        'share-user-10'
+      ]
+
+      expect(sortSpaceMembers([...members]).map(({ id }) => id)).toEqual(expectedOrder)
+      expect(sortSpaceMembers([...members].reverse()).map(({ id }) => id)).toEqual(expectedOrder)
+    })
   })
 
   describe('computed "personalSpace"', () => {