-
Notifications
You must be signed in to change notification settings - Fork 25
feat: remove space membership info in file list #721
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
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.
fdbe3f3 to
e7212ba
Compare
There was a problem hiding this 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) { |
Copilot
AI
May 20, 2025
There was a problem hiding this comment.
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.
We don't want to display this kind of information anymore due to performance reasons.
Also removes unnecessary
isMemberchecks on project spaces since/me/drivesonly returns spaces where the current user is member of.Preparation for #706