-
-
Notifications
You must be signed in to change notification settings - Fork 748
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
feat: manage tags UI (GSoC) #2185
feat: manage tags UI (GSoC) #2185
Conversation
WalkthroughThe update enhances the application's tag management functionality, introducing a new Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
Our Pull Request Approval ProcessWe have these basic policies to make the approval process smoother for our volunteer team. Testing Your CodePlease make sure your code passes all tests. Our test code coverage system will fail if these conditions occur:
The process helps maintain the overall reliability of the code base and is a prerequisite for getting your PR approved. Assigned reviewers regularly review the PR queue and tend to focus on PRs that are passing. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
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
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (15)
- public/locales/en/translation.json (1 hunks)
- public/locales/fr/translation.json (1 hunks)
- public/locales/hi/translation.json (1 hunks)
- public/locales/sp/translation.json (1 hunks)
- public/locales/zh/translation.json (1 hunks)
- src/App.tsx (2 hunks)
- src/GraphQl/Mutations/TagMutations.ts (1 hunks)
- src/GraphQl/Queries/userTagQueries.ts (1 hunks)
- src/screens/ManageTag/ManageTag.module.css (1 hunks)
- src/screens/ManageTag/ManageTag.test.tsx (1 hunks)
- src/screens/ManageTag/ManageTag.tsx (1 hunks)
- src/screens/ManageTag/ManageTagMocks.ts (1 hunks)
- src/screens/OrganizationTags/OrganizationTags.test.tsx (2 hunks)
- src/screens/OrganizationTags/OrganizationTags.tsx (1 hunks)
- src/utils/interfaces.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- src/screens/ManageTag/ManageTag.module.css
Additional comments not posted (26)
src/GraphQl/Mutations/TagMutations.ts (4)
32-37
: LGTM!The comments for the
UNASSIGN_USER_TAG
mutation are clear and provide a good explanation of the parameters.
39-45
: LGTM!The
UNASSIGN_USER_TAG
mutation is correctly defined and ready for use.
47-52
: LGTM!The comments for the
UPDATE_USER_TAG
mutation are clear and provide a good explanation of the parameters.
54-60
: LGTM!The
UPDATE_USER_TAG
mutation is correctly defined and ready for use.src/GraphQl/Queries/userTagQueries.ts (3)
1-36
: LGTM!The
USER_TAGS_ASSIGNED_MEMBERS
query is well-structured and includes pagination, which is essential for handling large datasets.
38-71
: LGTM!The
USER_TAG_CHILD_TAGS
query is well-structured and includes pagination. It effectively retrieves child tags and their associated data.
73-80
: LGTM!The
USER_TAG_ANCESTORS
query is straightforward and efficiently retrieves ancestor tags.src/screens/ManageTag/ManageTagMocks.ts (3)
1-5
: LGTM!The imports for GraphQL queries and mutations are correctly set up for use in mocks.
7-219
: LGTM!The mock data for
USER_TAGS_ASSIGNED_MEMBERS
andUSER_TAG_ANCESTORS
queries, as well as theUNASSIGN_USER_TAG
mutation, is comprehensive and well-structured.
222-245
: LGTM!The error mocks provide a good way to test error handling in the application.
src/screens/OrganizationTags/OrganizationTags.test.tsx (2)
62-63
: Route path updated successfully.The route path has been updated to
/orgtags/:orgId/managetag/:tagId
, reflecting the shift in functionality. Ensure that all test cases are aligned with this change.
165-165
: Test case for navigation to manage tag page is correct.The test case correctly verifies that the navigation to the manage tag page is functioning as expected.
src/screens/ManageTag/ManageTag.test.tsx (4)
1-23
: Test setup and imports are appropriate.The imports and setup for translations and mock links are appropriate for the tests being conducted.
51-64
: Caching strategy is well-defined.The
InMemoryCache
with a type policy forgetUserTag
is appropriately set up to handle data merging.
111-119
: Test case for component loading is correct.The test case correctly verifies that the
ManageTag
component loads as expected.
175-188
: Test case for navigation to member's profile is correct.The test case correctly verifies that navigation to the member's profile screen is functioning as expected.
src/App.tsx (2)
24-24
: Import ofManageTag
is correct.The
ManageTag
component is correctly imported, aligning with the new routing logic.
152-154
: New route forManageTag
is correctly defined.The route
orgtags/:orgId/managetag/:tagId
is correctly integrated into the application's routing logic.src/utils/interfaces.ts (1)
232-250
: Interface addition looks good.The
InterfaceQueryUserTagsAssignedMembers
interface is well-structured and follows TypeScript best practices. It includes all necessary properties for managing user tags and their assigned members.src/screens/OrganizationTags/OrganizationTags.tsx (1)
188-188
: Navigation path update is appropriate.The change in the
handleClick
function to navigate to/orgtags/${currentUrl}/managetag/${tagId}
aligns with the new functionality for managing tags. This enhances the user experience by directing users to the appropriate section for tag management.src/screens/ManageTag/ManageTag.tsx (1)
1-524
: Comprehensive component implementation.The
ManageTag
component is well-structured and implements the required functionality for managing tag assignments effectively. Here are some observations:
- State Management: The use of
useState
for managing local state is appropriate. Consider using a state management library if the state becomes more complex in the future.- Error Handling: Error handling is implemented using
try-catch
blocks andtoast
notifications, which is good for user feedback.- UI/UX: The component provides a clear interface for managing tags, with modals for adding and unassigning users. Ensure that all user interactions are intuitive.
- Performance: The use of
useQuery
anduseMutation
hooks for data fetching and mutations is efficient. Ensure that the network requests are optimized for performance.- Accessibility: Consider adding ARIA attributes to improve accessibility, especially for modals and interactive elements.
Overall, the component adheres to React best practices and provides a solid foundation for managing tags.
public/locales/zh/translation.json (1)
221-232
: Translations formanageTag
added successfully.The new translations for the
manageTag
section are well-structured and consistent with the existing format. Ensure that these entries are used correctly in the application.public/locales/en/translation.json (1)
221-232
: Translations formanageTag
added successfully.The new translations for the
manageTag
section are well-structured and consistent with the existing format. Ensure that these entries are used correctly in the application.public/locales/hi/translation.json (1)
221-232
: Translations formanageTag
are well-implemented.The translations for the
manageTag
section are accurate and consistent with the rest of the file. They effectively cover the necessary functionalities for managing tags in Hindi.public/locales/fr/translation.json (1)
221-231
: Translations look accurate and consistent.The added French translations for the "manageTag" section are well-structured and align with the application's functionality. Ensure these keys are used correctly in the UI components.
public/locales/sp/translation.json (1)
315-325
: Translations look accurate and consistent.The added Spanish translations for the "manageTag" section are well-structured and align with the application's functionality. Ensure these keys are used correctly in the UI components.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2185 +/- ##
===========================================
+ Coverage 97.57% 97.60% +0.02%
===========================================
Files 241 244 +3
Lines 6857 6942 +85
Branches 1993 2000 +7
===========================================
+ Hits 6691 6776 +85
Misses 151 151
Partials 15 15 ☔ View full report in Codecov by Sentry. |
efc5821
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
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (3)
- src/screens/ManageTag/ManageTag.test.tsx (1 hunks)
- src/screens/ManageTag/ManageTag.tsx (1 hunks)
- src/screens/ManageTag/ManageTagMocks.ts (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- src/screens/ManageTag/ManageTag.test.tsx
- src/screens/ManageTag/ManageTag.tsx
- src/screens/ManageTag/ManageTagMocks.ts
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.
That member profile screen is not associated with this issue, the view profile button just takes us to that existing screen. Do you want me to make changes there as part of this issue? |
Yes, you can create a separate issue and link it up with this PR to address these changes as well |
@meetulr |
What kind of change does this PR introduce?
Implements Manage Tag Screen.
Issue Number:
Fixes #2184
Did you add tests for your changes?
Yes
Snapshots/Videos:
manage.Tags.mp4
Does this PR introduce a breaking change?
No
Additional Context
Previous PR: #2175
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Style