Skip to content

Commit

Permalink
refactor: remove GraphShareRoleIdMap
Browse files Browse the repository at this point in the history
  • Loading branch information
Jannik Stehle committed Aug 16, 2024
1 parent 4c3dc5e commit c4cc29f
Show file tree
Hide file tree
Showing 11 changed files with 40 additions and 66 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,6 @@ import ExpirationDatepicker from './ExpirationDatepicker.vue'
import {
CollaboratorAutoCompleteItem,
CollaboratorShare,
GraphShareRoleIdMap,
ShareRole,
ShareTypes,
call
Expand Down Expand Up @@ -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)
Expand All @@ -258,10 +257,9 @@ export default defineComponent({
const resource = inject<Resource>('resource')
const space = inject<SpaceResource>('space')
const availableInternalRoles = inject<Ref<ShareRole[]>>('availableInternalShareRoles')
const availableExternalRoles = inject<Ref<ShareRole[]>>('availableExternalShareRoles')
const resourceIsSpace = computed(() => unref(resource).type === 'space')
const markInstance = ref(null)
const isOpen = ref(false)
Expand Down Expand Up @@ -302,9 +300,9 @@ export default defineComponent({
})
onMounted(async () => {
selectedRole.value = unref(resourceIsSpace)
? unref(graphRoles).find(({ id }) => id === GraphShareRoleIdMap.SpaceViewer)
: unref(graphRoles).find(({ id }) => id === GraphShareRoleIdMap.Viewer)
selectedRole.value = unref(isExternalShareRoleType)
? unref(availableExternalRoles)[0]
: unref(availableInternalRoles)[0]
await nextTick()
markInstance.value = new Mark('.mark-element')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -209,16 +204,17 @@ function getWrapper({
capabilityState: { capabilities },
configState: { options: { concurrentRequests: { shares: { create: 1 } } } },
sharesState: {
graphRoles: [
mock<ShareRole>({ id: GraphShareRoleIdMap.Viewer }),
mock<ShareRole>({ id: GraphShareRoleIdMap.SpaceViewer })
],
collaboratorShares: existingCollaborators
}
}
})
],
provide: { ...mocks, resource, availableExternalShareRoles: externalShareRoles },
provide: {
...mocks,
resource,
availableExternalShareRoles: externalShareRoles,
availableInternalShareRoles: [mock<ShareRole>()]
},
mocks,
stubs: { OcSelect: false, VueSelect: false }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -31,8 +31,8 @@ const memberMocks = [
id: 'alice',
displayName: 'alice'
},
role: mock<ShareRole>({ id: GraphShareRoleIdMap.SpaceManager }),
permissions: [],
role: mock<ShareRole>(),
permissions: [GraphSharePermission.updatePermissions],
resourceId: '1',
indirect: false,
sharedBy: { id: 'admin', displayName: 'admin' }
Expand All @@ -44,7 +44,7 @@ const memberMocks = [
onPremisesSamAccountName: 'Einstein',
displayName: 'einstein'
},
role: mock<ShareRole>({ id: GraphShareRoleIdMap.SpaceEditor }),
role: mock<ShareRole>(),
permissions: [],
resourceId: '1',
indirect: false,
Expand All @@ -57,7 +57,7 @@ const memberMocks = [
onPremisesSamAccountName: 'Marie',
displayName: 'marie'
},
role: mock<ShareRole>({ id: GraphShareRoleIdMap.SpaceViewer }),
role: mock<ShareRole>(),
permissions: [],
resourceId: '1',
indirect: false,
Expand Down
11 changes: 0 additions & 11 deletions packages/web-client/src/helpers/share/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 1 addition & 4 deletions packages/web-client/src/helpers/share/functions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,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')
Expand Down Expand Up @@ -150,9 +149,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),
Expand Down
1 change: 1 addition & 0 deletions packages/web-client/src/helpers/share/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
}

Expand Down
1 change: 1 addition & 0 deletions packages/web-client/src/helpers/space/functions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
12 changes: 2 additions & 10 deletions packages/web-pkg/src/composables/piniaStores/spaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 ({
Expand Down
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -33,7 +27,7 @@ const spaceShare = {
id: 'Alice',
displayName: 'alice'
},
role: mock<ShareRole>({ id: GraphShareRoleIdMap.SpaceManager })
role: mock<ShareRole>()
} as CollaboratorShare

const selectors = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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<CollaboratorShare>({
role: { id: GraphShareRoleIdMap.SpaceEditor },
permissions: [],
sharedWith: { displayName: 'user1' }
}),
mock<CollaboratorShare>({
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()
})
})

Expand Down Expand Up @@ -278,6 +280,7 @@ describe('spaces', () => {
setup: async (instance) => {
const share = mock<CollaboratorShare>({
id: '1',
permissions: [],
role: mock<ShareRole>({ id: 'roleId' })
})
const clientService = mockDeep<ClientService>()
Expand Down

0 comments on commit c4cc29f

Please sign in to comment.