From 4ed87d2df55a8815c3b99d9d30bf2ea8a3007366 Mon Sep 17 00:00:00 2001 From: Jannik Stehle Date: Wed, 14 Aug 2024 07:09:31 +0200 Subject: [PATCH 1/3] refactor: remove GraphShareRoleIdMap --- .../InviteCollaboratorForm.vue | 12 +++++------- .../components/SideBar/Shares/SpaceMembers.vue | 13 ++++++++----- .../InviteCollaboratorForm.spec.ts | 18 +++++++----------- .../SideBar/Shares/SpaceMembers.spec.ts | 10 +++++----- .../web-client/src/helpers/share/constants.ts | 11 ----------- .../web-client/src/helpers/share/functions.ts | 5 +---- packages/web-client/src/helpers/share/types.ts | 1 + .../web-client/src/helpers/space/functions.ts | 1 + .../src/composables/piniaStores/spaces.ts | 12 ++---------- .../Spaces/Details/SpaceDetails.spec.ts | 10 ++-------- .../composables/piniaStores/spaces.spec.ts | 13 ++++++++----- 11 files changed, 40 insertions(+), 66 deletions(-) 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 52c7eef5605..350687fd129 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 @@ -169,7 +169,6 @@ import ExpirationDatepicker from './ExpirationDatepicker.vue' import { CollaboratorAutoCompleteItem, CollaboratorShare, - GraphShareRoleIdMap, ShareRole, ShareTypes, call @@ -242,7 +241,7 @@ export default defineComponent({ const sharesStore = useSharesStore() const { addShare } = sharesStore - const { collaboratorShares, graphRoles } = storeToRefs(sharesStore) + const { collaboratorShares } = storeToRefs(sharesStore) const searchQuery = ref('') const searchInProgress = ref(false) @@ -258,10 +257,9 @@ export default defineComponent({ const resource = inject('resource') const space = inject('space') + const availableInternalRoles = inject>('availableInternalShareRoles') const availableExternalRoles = inject>('availableExternalShareRoles') - const resourceIsSpace = computed(() => unref(resource).type === 'space') - const markInstance = ref(null) const isOpen = ref(false) @@ -302,9 +300,9 @@ export default defineComponent({ }) onMounted(async () => { - selectedRole.value = unref(resourceIsSpace) - ? unref(graphRoles)[GraphShareRoleIdMap.SpaceViewer] - : unref(graphRoles)[GraphShareRoleIdMap.Viewer] + selectedRole.value = unref(isExternalShareRoleType) + ? unref(availableExternalRoles)[0] + : unref(availableInternalRoles)[0] await nextTick() markInstance.value = new Mark('.mark-element') diff --git a/packages/web-app-files/src/components/SideBar/Shares/SpaceMembers.vue b/packages/web-app-files/src/components/SideBar/Shares/SpaceMembers.vue index a40c884fd27..0baef492782 100644 --- a/packages/web-app-files/src/components/SideBar/Shares/SpaceMembers.vue +++ b/packages/web-app-files/src/components/SideBar/Shares/SpaceMembers.vue @@ -71,7 +71,7 @@ import { storeToRefs } from 'pinia' import CollaboratorListItem from './Collaborators/ListItem.vue' import InviteCollaboratorForm from './Collaborators/InviteCollaborator/InviteCollaboratorForm.vue' -import { GraphShareRoleIdMap } from '@ownclouders/web-client' +import { GraphSharePermission } from '@ownclouders/web-client' import { createLocationSpaces, isLocationSpacesActive, @@ -193,13 +193,16 @@ export default defineComponent({ return false } - if (share.role.id !== GraphShareRoleIdMap.SpaceManager) { + const memberCanUpdateMembers = share.permissions.includes( + GraphSharePermission.updatePermissions + ) + if (!memberCanUpdateMembers) { return true } - // forbid to remove last manager of a space - const managers = this.spaceMembers.filter( - ({ role }) => role.id === GraphShareRoleIdMap.SpaceManager + // make sure at least one member can edit other members + const managers = this.spaceMembers.filter(({ permissions }) => + permissions.includes(GraphSharePermission.updatePermissions) ) return managers.length > 1 }, diff --git a/packages/web-app-files/tests/unit/components/SideBar/Shares/Collaborators/InviteCollaborator/InviteCollaboratorForm.spec.ts b/packages/web-app-files/tests/unit/components/SideBar/Shares/Collaborators/InviteCollaborator/InviteCollaboratorForm.spec.ts index 986c30ea383..c996a939db1 100644 --- a/packages/web-app-files/tests/unit/components/SideBar/Shares/Collaborators/InviteCollaborator/InviteCollaboratorForm.spec.ts +++ b/packages/web-app-files/tests/unit/components/SideBar/Shares/Collaborators/InviteCollaborator/InviteCollaboratorForm.spec.ts @@ -8,12 +8,7 @@ import { } from 'web-test-helpers' import { Resource, SpaceResource } from '@ownclouders/web-client' import { useSharesStore } from '@ownclouders/web-pkg' -import { - CollaboratorAutoCompleteItem, - CollaboratorShare, - GraphShareRoleIdMap, - ShareRole -} from '@ownclouders/web-client' +import { CollaboratorAutoCompleteItem, CollaboratorShare, ShareRole } from '@ownclouders/web-client' import { Group, User } from '@ownclouders/web-client/graph/generated' import OcButton from 'design-system/src/components/OcButton/OcButton.vue' import RoleDropdown from '../../../../../../../src/components/SideBar/Shares/Collaborators/RoleDropdown.vue' @@ -209,16 +204,17 @@ function getWrapper({ capabilityState: { capabilities }, configState: { options: { concurrentRequests: { shares: { create: 1 } } } }, sharesState: { - graphRoles: { - [GraphShareRoleIdMap.Viewer]: mock(), - [GraphShareRoleIdMap.SpaceViewer]: mock() - }, collaboratorShares: existingCollaborators } } }) ], - provide: { ...mocks, resource, availableExternalShareRoles: externalShareRoles }, + provide: { + ...mocks, + resource, + availableExternalShareRoles: externalShareRoles, + availableInternalShareRoles: [mock()] + }, mocks, stubs: { OcSelect: false, VueSelect: false } } 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 index 6621940985a..09052a3b814 100644 --- 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 @@ -3,7 +3,7 @@ import { ShareTypes, ShareRole, CollaboratorShare, - GraphShareRoleIdMap + GraphSharePermission } from '@ownclouders/web-client' import { mock } from 'vitest-mock-extended' import { ProjectSpaceResource, SpaceResource } from '@ownclouders/web-client' @@ -31,8 +31,8 @@ const memberMocks = [ id: 'alice', displayName: 'alice' }, - role: mock({ id: GraphShareRoleIdMap.SpaceManager }), - permissions: [], + role: mock(), + permissions: [GraphSharePermission.updatePermissions], resourceId: '1', indirect: false, sharedBy: { id: 'admin', displayName: 'admin' } @@ -44,7 +44,7 @@ const memberMocks = [ onPremisesSamAccountName: 'Einstein', displayName: 'einstein' }, - role: mock({ id: GraphShareRoleIdMap.SpaceEditor }), + role: mock(), permissions: [], resourceId: '1', indirect: false, @@ -57,7 +57,7 @@ const memberMocks = [ onPremisesSamAccountName: 'Marie', displayName: 'marie' }, - role: mock({ id: GraphShareRoleIdMap.SpaceViewer }), + role: mock(), permissions: [], resourceId: '1', indirect: false, diff --git a/packages/web-client/src/helpers/share/constants.ts b/packages/web-client/src/helpers/share/constants.ts index c2de08ac33d..b6567f2baee 100644 --- a/packages/web-client/src/helpers/share/constants.ts +++ b/packages/web-client/src/helpers/share/constants.ts @@ -7,14 +7,3 @@ export abstract class SharePermissionBit { static readonly Delete: number = 8 static readonly Share: number = 16 } - -// map human readable role names to graph share role ids -export const GraphShareRoleIdMap = { - Viewer: 'b1e2218d-eef8-4d4c-b82d-0f1a1b48f3b5', - SpaceViewer: 'a8d5fe5e-96e3-418d-825b-534dbdf22b99', - FileEditor: '2d00ce52-1fc2-4dbc-8b95-a73b73395f5a', - FolderEditor: 'fb6c3e19-e378-47e5-b277-9732f9de6e21', - SpaceEditor: '58c63c02-1d89-4572-916a-870abc5a1b7d', - SpaceManager: '312c0871-5ef7-4b3a-85b6-0e4074c64049', - SecureViewer: 'aa97fe03-7980-45ac-9e50-b325749fd7e6' -} as const diff --git a/packages/web-client/src/helpers/share/functions.ts b/packages/web-client/src/helpers/share/functions.ts index afce36d6675..a969327abb2 100644 --- a/packages/web-client/src/helpers/share/functions.ts +++ b/packages/web-client/src/helpers/share/functions.ts @@ -14,7 +14,6 @@ import { buildWebDavSpacesPath } from '../space' import { DriveItem, Identity, Permission, UnifiedRoleDefinition, User } from '../../graph/generated' import { urlJoin } from '../../utils' import { uniq } from 'lodash-es' -import { GraphShareRoleIdMap } from './constants' export const isShareResource = (resource: Resource): resource is ShareResource => { return Object.hasOwn(resource, 'sharedWith') @@ -151,9 +150,7 @@ export function buildIncomingShareResource({ sharePermissions, outgoing: false, canRename: () => driveItem['@client.synchronize'], - canDownload: () => - sharePermissions.includes(GraphSharePermission.readBasic) && - !shareRoles.some((shareRole) => shareRole.id === GraphShareRoleIdMap.SecureViewer), + canDownload: () => sharePermissions.includes(GraphSharePermission.readContent), canUpload: () => sharePermissions.includes(GraphSharePermission.createUpload), canCreate: () => sharePermissions.includes(GraphSharePermission.createChildren), canBeDeleted: () => sharePermissions.includes(GraphSharePermission.deleteStandard), diff --git a/packages/web-client/src/helpers/share/types.ts b/packages/web-client/src/helpers/share/types.ts index 7d1fa801181..9b7cf20b960 100644 --- a/packages/web-client/src/helpers/share/types.ts +++ b/packages/web-client/src/helpers/share/types.ts @@ -13,6 +13,7 @@ export enum GraphSharePermission { readDeleted = 'libre.graph/driveItem/deleted/read', updatePath = 'libre.graph/driveItem/path/update', updateDeleted = 'libre.graph/driveItem/deleted/update', + updatePermissions = 'libre.graph/driveItem/permissions/update', deleteStandard = 'libre.graph/driveItem/standard/delete' } diff --git a/packages/web-client/src/helpers/space/functions.ts b/packages/web-client/src/helpers/space/functions.ts index 83470ebbb96..eb6d8ad3c5c 100644 --- a/packages/web-client/src/helpers/space/functions.ts +++ b/packages/web-client/src/helpers/space/functions.ts @@ -131,6 +131,7 @@ export function buildSpace( } } + console.log('data', data) if (data.root?.permissions) { for (const permission of data.root.permissions) { spaceRoles[permission.roles[0] as keyof SpaceRoles].push( diff --git a/packages/web-pkg/src/composables/piniaStores/spaces.ts b/packages/web-pkg/src/composables/piniaStores/spaces.ts index 93e5a424def..570519ebb98 100644 --- a/packages/web-pkg/src/composables/piniaStores/spaces.ts +++ b/packages/web-pkg/src/composables/piniaStores/spaces.ts @@ -3,7 +3,6 @@ import { computed, ref, unref } from 'vue' import { isCollaboratorShare, SpaceResource } from '@ownclouders/web-client' import { Graph } from '@ownclouders/web-client/graph' import { - GraphShareRoleIdMap, buildSpace, extractStorageId, isPersonalSpaceResource, @@ -14,16 +13,9 @@ import { useUserStore } from './user' import { ConfigStore, useConfigStore } from './config' import { useSharesStore } from './shares' +// sort space members with higher permissions (managers) at the top export const sortSpaceMembers = (shares: CollaboratorShare[]) => { - const sortedManagers = shares - .filter((share) => share.role.id === GraphShareRoleIdMap.SpaceManager) - .sort((a, b) => a.sharedWith.displayName.localeCompare(b.sharedWith.displayName)) - - const sortedRest = shares - .filter((share) => share.role.id !== GraphShareRoleIdMap.SpaceManager) - .sort((a, b) => a.sharedWith.displayName.localeCompare(b.sharedWith.displayName)) - - return [...sortedManagers, ...sortedRest] + return shares.sort((a, b) => b.permissions.length - a.permissions.length) } export const getSpacesByType = async ({ diff --git a/packages/web-pkg/tests/unit/components/sidebar/Spaces/Details/SpaceDetails.spec.ts b/packages/web-pkg/tests/unit/components/sidebar/Spaces/Details/SpaceDetails.spec.ts index fb6c0bce721..f284f98257c 100644 --- a/packages/web-pkg/tests/unit/components/sidebar/Spaces/Details/SpaceDetails.spec.ts +++ b/packages/web-pkg/tests/unit/components/sidebar/Spaces/Details/SpaceDetails.spec.ts @@ -1,11 +1,5 @@ import SpaceDetails from '../../../../../../src/components/SideBar/Spaces/Details/SpaceDetails.vue' -import { - CollaboratorShare, - ShareRole, - GraphShareRoleIdMap, - SpaceResource, - Resource -} from '@ownclouders/web-client' +import { CollaboratorShare, ShareRole, SpaceResource, Resource } from '@ownclouders/web-client' import { mock } from 'vitest-mock-extended' import { defaultComponentMocks, defaultPlugins, shallowMount } from 'web-test-helpers' import { RouteLocation } from 'vue-router' @@ -33,7 +27,7 @@ const spaceShare = { id: 'Alice', displayName: 'alice' }, - role: mock({ id: GraphShareRoleIdMap.SpaceManager }) + role: mock() } as CollaboratorShare const selectors = { diff --git a/packages/web-pkg/tests/unit/composables/piniaStores/spaces.spec.ts b/packages/web-pkg/tests/unit/composables/piniaStores/spaces.spec.ts index d14f231e651..615033af6ec 100644 --- a/packages/web-pkg/tests/unit/composables/piniaStores/spaces.spec.ts +++ b/packages/web-pkg/tests/unit/composables/piniaStores/spaces.spec.ts @@ -9,7 +9,7 @@ import { createPinia, setActivePinia } from 'pinia' import { mock, mockDeep } from 'vitest-mock-extended' import { CollaboratorShare, - GraphShareRoleIdMap, + GraphSharePermission, ShareRole, SpaceResource } from '@ownclouders/web-client' @@ -23,20 +23,22 @@ describe('spaces', () => { }) describe('method "sortSpaceMembers"', () => { - it('always puts space managers first', () => { + it('sorts space members by amount of permissions', () => { const members = [ mock({ - role: { id: GraphShareRoleIdMap.SpaceEditor }, + permissions: [], sharedWith: { displayName: 'user1' } }), mock({ - role: { id: GraphShareRoleIdMap.SpaceManager }, + permissions: [GraphSharePermission.updatePermissions], sharedWith: { displayName: 'user2' } }) ] const sortedMembers = sortSpaceMembers(members) - expect(sortedMembers[0].role.id).toEqual(GraphShareRoleIdMap.SpaceManager) + expect( + sortedMembers[0].permissions.includes(GraphSharePermission.updatePermissions) + ).toBeTruthy() }) }) @@ -278,6 +280,7 @@ describe('spaces', () => { setup: async (instance) => { const share = mock({ id: '1', + permissions: [], role: mock({ id: 'roleId' }) }) const clientService = mockDeep() From 58f056351481e6f2079d2d2539c5241727d0cb56 Mon Sep 17 00:00:00 2001 From: Jannik Stehle Date: Wed, 14 Aug 2024 16:49:29 +0200 Subject: [PATCH 2/3] refactor: remove hard-coded space roles, use permissions instead --- .../Spaces/SideBar/MembersPanel.vue | 110 ++-- .../Spaces/SideBar/MembersRoleSection.vue | 13 +- .../src/components/Spaces/SpacesList.vue | 14 +- .../components/Users/SideBar/EditPanel.vue | 16 +- .../src/views/Spaces.vue | 6 +- .../Spaces/SideBar/MembersPanel.spec.ts | 48 +- .../Spaces/SideBar/MembersRoleSection.spec.ts | 8 +- .../__snapshots__/MembersPanel.spec.ts.snap | 41 +- .../unit/components/Spaces/SpacesList.spec.ts | 62 +- .../__snapshots__/SpacesList.spec.ts.snap | 4 +- .../src/components/AppBar/CreateAndUpload.vue | 4 +- .../InviteCollaboratorForm.vue | 3 +- .../SideBar/Shares/Collaborators/ListItem.vue | 7 +- .../components/SideBar/Shares/FileShares.vue | 4 +- .../SideBar/Shares/SpaceMembers.vue | 7 +- .../src/components/Spaces/SpaceHeader.vue | 2 +- .../spaces/useSpaceActionsUploadImage.ts | 16 +- .../composables/extensions/useFileSideBars.ts | 4 +- .../src/views/spaces/GenericTrash.vue | 4 +- .../src/views/spaces/Projects.vue | 9 +- .../SideBar/Shares/FileShares.spec.ts | 14 +- .../SideBar/Shares/SpaceMembers.spec.ts | 2 +- .../Spaces/SpaceContextActions.spec.ts | 6 +- .../components/Spaces/SpaceHeader.spec.ts | 30 +- .../__snapshots__/SpaceHeader.spec.ts.snap | 24 +- .../tests/unit/views/trash/Overview.spec.ts | 2 +- .../web-client/src/graph/drives/drives.ts | 52 +- packages/web-client/src/graph/drives/types.ts | 17 +- .../web-client/src/helpers/resource/types.ts | 1 - .../web-client/src/helpers/share/types.ts | 4 +- .../web-client/src/helpers/space/functions.ts | 267 +++++--- .../web-client/src/helpers/space/types.ts | 29 +- .../unit/helpers/space/functions.spec.ts | 619 +++++++++++------- .../components/FilesList/ResourceTable.vue | 4 +- .../SideBar/Spaces/Details/SpaceDetails.vue | 18 +- .../src/components/Spaces/QuotaModal.vue | 6 +- .../actions/files/useFileActionsDelete.ts | 2 +- .../files/useFileActionsEmptyTrashBin.ts | 2 +- .../actions/files/useFileActionsRename.ts | 8 +- .../actions/files/useFileActionsRestore.ts | 14 +- .../actions/files/useFileActionsSetImage.ts | 15 +- .../helpers/useFileActionsDeleteResources.ts | 9 +- .../spaces/useSpaceActionsDuplicate.ts | 14 +- .../spaces/useSpaceActionsEditDescription.ts | 11 +- .../useSpaceActionsEditReadmeContent.ts | 19 +- .../actions/spaces/useSpaceActionsRename.ts | 11 +- .../actions/spaces/useSpaceActionsRestore.ts | 13 +- .../actions/spaces/useSpaceActionsSetIcon.ts | 21 +- .../src/composables/piniaStores/spaces.ts | 51 +- .../resources/useGetResourceContext.ts | 5 +- .../src/composables/shares/useCanShare.ts | 2 +- .../src/composables/spaces/useCreateSpace.ts | 7 +- .../Spaces/Details/SpaceDetails.spec.ts | 20 +- .../actions/files/useFileActionsCopy.spec.ts | 3 +- .../files/useFileActionsDelete.spec.ts | 3 +- .../useFileActionsDownloadArchive.spec.ts | 3 +- .../files/useFileActionsEmptyTrashBin.spec.ts | 2 +- .../files/useFileActionsRestore.spec.ts | 6 +- .../files/useFileActionsSetImage.spec.ts | 51 +- .../spaces/useSpaceActionsDelete.spec.ts | 47 +- .../spaces/useSpaceActionsDisable.spec.ts | 45 +- .../spaces/useSpaceActionsEditQuota.spec.ts | 27 +- .../useSpaceActionsEditReadmeContent.spec.ts | 37 +- .../spaces/useSpaceActionsRestore.spec.ts | 46 +- .../composables/piniaStores/spaces.spec.ts | 46 +- .../resourcesTransfer.spec.ts | 4 +- .../web-runtime/src/container/sse/shares.ts | 18 +- packages/web-runtime/src/index.ts | 10 +- tests/e2e/cucumber/steps/ui/adminSettings.ts | 6 +- .../app-admin-settings/spaces/actions.ts | 6 +- 70 files changed, 1158 insertions(+), 903 deletions(-) diff --git a/packages/web-app-admin-settings/src/components/Spaces/SideBar/MembersPanel.vue b/packages/web-app-admin-settings/src/components/Spaces/SideBar/MembersPanel.vue index 4c11d536e81..2c067065ecd 100644 --- a/packages/web-app-admin-settings/src/components/Spaces/SideBar/MembersPanel.vue +++ b/packages/web-app-admin-settings/src/components/Spaces/SideBar/MembersPanel.vue @@ -9,103 +9,76 @@

-
-

- +
+
+

+ +

-
-

- -

-
-

- -

-
-

- +
+

+