-
-
Notifications
You must be signed in to change notification settings - Fork 11
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
Feature: Add member count for groups in VaultDetail view #329
base: develop
Are you sure you want to change the base?
Conversation
WalkthroughThe changes add functionality to support returning and displaying the member size of group entities. In the backend, an overloaded method in the AuthorityDto allows the conversion of entities with an optional boolean flag to include member size data. The GroupDto and MemberDto classes have been updated with a new property, Suggested reviewers
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
frontend/src/common/backend.ts (1)
287-287
: Consider handling singular/plural forms for member count.The search method properly includes the new
withMemberSize=true
parameter. However, you might want to consider handling singular/plural forms when displaying member counts in the UI (e.g., "1 Member" vs. "2 Members").This could be implemented with a conditional in the template or by adding additional translation keys:
// In the i18n file "vaultDetails.sharedWith.member": "Member", "vaultDetails.sharedWith.members": "Members", // Then in the component template {{ member.memberSize }} {{ member.memberSize === 1 ? t('vaultDetails.sharedWith.member') : t('vaultDetails.sharedWith.members') }}frontend/src/components/VaultDetails.vue (1)
50-52
: LGTM! Good implementation of member count display.The implementation correctly displays the member size for group entities with appropriate styling. The italicized gray text matches the requirements specified in the PR objectives.
Consider adding a fallback value in case
memberSize
is undefined:- {{ member.memberSize }} {{ t('vaultDetails.sharedWith.members') }} + {{ member.memberSize || 0 }} {{ t('vaultDetails.sharedWith.members') }}Additionally, for better user experience, consider handling singular/plural forms as mentioned in the previous comment.
backend/src/test/java/org/cryptomator/hub/api/VaultResourceIT.java (1)
59-59
: Unused import detected.The import for
nullValue
from Hamcrest Matchers is not used in the changed code.-import static org.hamcrest.Matchers.nullValue;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
backend/src/main/java/org/cryptomator/hub/api/AuthorityDto.java
(1 hunks)backend/src/main/java/org/cryptomator/hub/api/AuthorityResource.java
(2 hunks)backend/src/main/java/org/cryptomator/hub/api/GroupDto.java
(1 hunks)backend/src/main/java/org/cryptomator/hub/api/MemberDto.java
(1 hunks)backend/src/main/java/org/cryptomator/hub/entities/Group.java
(2 hunks)backend/src/test/java/org/cryptomator/hub/api/AuthorityResourceIT.java
(3 hunks)backend/src/test/java/org/cryptomator/hub/api/VaultResourceIT.java
(2 hunks)frontend/src/common/backend.ts
(2 hunks)frontend/src/components/SearchInputGroup.vue
(2 hunks)frontend/src/components/VaultDetails.vue
(1 hunks)frontend/src/i18n/en-US.json
(1 hunks)
🔇 Additional comments (26)
backend/src/main/java/org/cryptomator/hub/entities/Group.java (2)
12-12
: Ensure Jakarta Persistence API import is used consistently.Good addition of the
@Transient
annotation import from the Jakarta Persistence API, maintaining consistency with the existing imports.
37-40
: LGTM! Clean implementation of the member size method.The
@Transient
annotation correctly indicates this is a calculated field rather than a persisted property. The implementation is straightforward and efficient, simply returning the size of the members collection.frontend/src/i18n/en-US.json (1)
266-266
: LGTM! Good addition of localization entry.The new localization entry "Members" will support displaying the member count in the UI, maintaining the application's internationalization capabilities.
frontend/src/common/backend.ts (1)
89-89
: LGTM! Good addition of the memberSize property.The optional
memberSize
property is correctly added to theGroupDto
type, making it available for display in the UI while maintaining backward compatibility with existing code.backend/src/main/java/org/cryptomator/hub/api/AuthorityResource.java (3)
5-5
: LGTM! Added import for Transactional annotation.The import for
jakarta.transaction.Transactional
is appropriate for the changes in this file.
32-33
: Good addition of @transactional annotation with updated method signature.Adding the
@Transactional
annotation ensures that the database operations occur within a transaction context, which is important when retrieving related entities like group members to calculate the member size.
34-34
: LGTM! Implementation properly uses the new withMemberSize parameter.The implementation correctly passes the
withMemberSize
parameter to theAuthorityDto.fromEntity
method, enabling the retrieval of member size information when requested.backend/src/test/java/org/cryptomator/hub/api/VaultResourceIT.java (2)
617-617
: LGTM! Updated test display name to reflect member size checking.The test name now accurately describes what is being tested - verifying that group2 has a member size of 2.
621-621
: Good implementation of member size validation in test.The assertion has been updated to use Groovy-style closure syntax to find the object with id 'group2' and assert that its memberSize equals 2, which properly tests the new functionality.
frontend/src/components/SearchInputGroup.vue (4)
12-17
: Great enhancement to show member count for selected groups.The implementation elegantly displays the member count as italicized gray text next to the group name when a group is selected, which aligns with the PR objectives.
25-27
: LGTM! Consistent display of member count in dropdown options.This implementation maintains UI consistency by showing the member count in the dropdown list in the same style as the selected item display.
49-49
: Good addition of new properties to support member size display.The Item type has been properly updated with optional
type
andmemberSize
properties, and the i18n import has been added to support localization of the "members" text.Also applies to: 55-56
59-59
: LGTM! Properly initialized i18n for localization.The internationalization setup is correctly implemented, allowing for localized display of the "members" text.
backend/src/main/java/org/cryptomator/hub/api/AuthorityDto.java (3)
33-35
: Good implementation of backward compatibility.The original
fromEntity
method has been updated to call the new overloaded method with a default value offalse
for thewithMemberSize
parameter, maintaining backward compatibility.
37-37
: LGTM! Well-designed overloaded method.Adding an overloaded method with the
withMemberSize
parameter is a clean approach to extend functionality while maintaining API compatibility.
40-40
: LGTM! Correctly passes the withMemberSize parameter.The switch statement properly passes the
withMemberSize
parameter to theGroupDto.fromEntity
method, enabling conditional retrieval of member size information.backend/src/test/java/org/cryptomator/hub/api/AuthorityResourceIT.java (2)
81-83
: Good update to existing test to verify memberSize is null by defaultThe updated test assertions correctly verify that both user and group entities don't include the memberSize property when the withMemberSize parameter is not provided. This ensures backward compatibility with existing API clients.
85-92
: Well-structured test for withMemberSize parameterThis test properly verifies the new functionality by checking that:
- When withMemberSize=true is passed, groups include their member count (1 in this case)
- Users still have null memberSize even with the parameter set
The test clearly documents the expected behavior and helps ensure the feature works as intended.
backend/src/main/java/org/cryptomator/hub/api/MemberDto.java (4)
16-17
: Good addition of memberSize property with proper annotationThe memberSize property is correctly annotated with @JsonProperty to ensure proper JSON serialization.
19-25
: Constructor properly updated to include memberSize parameterThe constructor has been correctly updated to include the new memberSize parameter with proper JSON annotation, maintaining the class's immutability pattern.
28-28
: Appropriate null value for user memberSizeCorrectly passing null for memberSize when creating a MemberDto from a User entity, as users don't have members.
32-32
: Group member size correctly passed to DTOProperly retrieving and passing the group's member size when creating a MemberDto from a Group entity.
backend/src/main/java/org/cryptomator/hub/api/GroupDto.java (4)
8-9
: Well-defined memberSize property with JSON annotationThe memberSize property is correctly defined as an Integer (allowing for null values) and properly annotated for JSON serialization.
11-14
: Constructor properly updated to handle memberSizeThe constructor has been correctly modified to accept and initialize the new memberSize field, maintaining the class's immutability pattern.
16-18
: Good backward compatibility approachThe original fromEntity method now calls the new overloaded version with false for withMemberSize, maintaining backward compatibility with existing code while supporting the new functionality.
20-23
: Well-implemented conditional member size inclusionThe new overloaded fromEntity method properly:
- Conditionally retrieves the member size based on the withMemberSize flag
- Sets it to null when not needed to avoid unnecessary calculation
- Creates the DTO with the appropriate values
This implementation aligns perfectly with the PR objective to display member counts for groups.
This PR addresses #173 by displaying the number of members for shared groups in the VaultDetail view.
The member count appears as italicized gray text (e.g., 3 Members) next to group names.
This information is visible both in the search results and the "Shared with" list, helping distinguish groups from individual users.