Skip to content

Conversation

@JammingBen
Copy link
Contributor

We don't want to display this kind of information anymore due to performance reasons.

Also removes unnecessary isMember checks on project spaces since /me/drives only returns spaces where the current user is member of.

Preparation for #706

We don't want to display this kind of information anymore due to performance reasons.

Also removes unnecessary `isMember` checks on project spaces since `/me/drives` only returns spaces where the current user is member of.
@JammingBen JammingBen force-pushed the feat/remove-space-members-in-file-list branch from fdbe3f3 to e7212ba Compare May 20, 2025 09:20
@JammingBen JammingBen marked this pull request as ready for review May 20, 2025 09:31
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR removes the display of space membership information in file lists to improve performance and simplifies the related logic by eliminating unnecessary isMember checks. Key changes include modifying unit tests to remove isMember mocks, updating helper and composable functions to use simplified membership logic, and removing UI elements that displayed membership details.

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/web-pkg/tests/unit/helpers/statusIndicator.spec.ts Removed tests relying on isMember check for project spaces.
packages/web-pkg/tests/unit/components/sidebar/Spaces/Details/SpaceDetails.spec.ts Updated space mocks and expectations to reflect removal of membership info.
packages/web-pkg/src/helpers/statusIndicators.ts Simplified share indicator condition by removing isMember dependency.
packages/web-pkg/src/composables/spaces/useGetMatchingSpace.ts & resources.ts Refactored membership checking using Array.some instead of find with isMember.
packages/web-pkg/src/components/SideBar/Spaces/Details/SpaceDetails.vue Replaced getSpaceManagers with computed spaceMembers and adjusted member count logic.
packages/web-client/src/helpers/space/types.ts & functions.ts Removed the isMember method and introduced the isManager helper.
packages/web-app-files (tests and views) Removed templates and tests using space membership information and updated conditions in FileShares component.
Comments suppressed due to low confidence (2)

packages/web-pkg/src/helpers/statusIndicators.ts:140

  • The removal of the isMember check for project spaces aligns with the new API behavior, but please ensure that this simplified logic fully captures the intended sharing indicator behavior.
isProjectSpaceResource(space) || (isPersonalSpaceResource(space) && space.isOwner(user))

packages/web-app-files/src/components/SideBar/Shares/FileShares.vue:310

  • [nitpick] The updated condition omits the isMember check; please verify that this change comprehensively addresses the intended cases for displaying space members without unintended side effects.
return isProjectSpaceResource(this.space) && this.resource.type !== 'space'

)
}

export function isManager(share: CollaboratorShare) {
Copy link

Copilot AI May 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Consider adding a code comment to clarify that the isManager function interprets delete permissions as a sign of management rights, which aids clarity for future maintainers.

Copilot uses AI. Check for mistakes.
@JammingBen JammingBen merged commit 2b840d8 into main May 20, 2025
18 checks passed
@JammingBen JammingBen deleted the feat/remove-space-members-in-file-list branch May 20, 2025 12:52
@openclouders openclouders mentioned this pull request May 20, 2025
1 task
@openclouders openclouders mentioned this pull request Jun 2, 2025
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants