-
-
Notifications
You must be signed in to change notification settings - Fork 848
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
improve coverage for Organizations component #3434
improve coverage for Organizations component #3434
Conversation
WalkthroughThis pull request focuses on improving the code coverage and test scenarios for the Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. 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.
Congratulations on making your first PR! 🎊 If you haven't already, check out our Contributing Guidelines and PR Reporting Guidelines to ensure that you are following our guidelines for contributing and creating PR.
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 (1)
src/screens/UserPortal/Organizations/Organizations.tsx (1)
231-260
: Consider extracting complex logic into separate functions.The useEffect hook contains complex nested logic that could be more maintainable if broken down into separate functions.
Consider this refactor to improve readability and maintainability:
+const mapOrganizationData = (organization: InterfaceOrganization, userId: string) => { + let membershipRequestStatus = ''; + if (organization.members.find((member: { _id: string }) => member._id === userId)) { + membershipRequestStatus = 'accepted'; + } else if (organization.membershipRequests.find( + (request: { user: { _id: string } }) => request.user._id === userId + )) { + membershipRequestStatus = 'pending'; + } + return { ...organization, membershipRequestStatus }; +}; + +const getOrganizationsForMode = ( + mode: number, + data: any, + joinedOrganizationsData: any, + createdOrganizationsData: any, + userId: string +) => { + if (mode === 0 && data) { + return data.organizationsConnection.map((org: InterfaceOrganization) => + mapOrganizationData(org, userId) + ); + } + if (mode === 1 && joinedOrganizationsData?.users?.length > 0) { + return joinedOrganizationsData.users[0]?.user?.joinedOrganizations || []; + } + if (mode === 2 && createdOrganizationsData?.users?.length > 0) { + return createdOrganizationsData.users[0]?.appUserProfile?.createdOrganizations || []; + } + return []; +}; + useEffect(() => { - if (mode === 0 && data) { - const organizations = data.organizationsConnection.map( - (organization: InterfaceOrganization) => { - let membershipRequestStatus = ''; - if ( - organization.members.find( - (member: { _id: string }) => member._id === userId, - ) - ) - membershipRequestStatus = 'accepted'; - else if ( - organization.membershipRequests.find( - (request: { user: { _id: string } }) => - request.user._id === userId, - ) - ) - membershipRequestStatus = 'pending'; - return { ...organization, membershipRequestStatus }; - }, - ); - setOrganizations(organizations); - } else if (mode === 1 && joinedOrganizationsData?.users?.length > 0) { - const organizations = - joinedOrganizationsData.users[0]?.user?.joinedOrganizations || []; - setOrganizations(organizations); - } else if (mode === 2 && createdOrganizationsData?.users?.length > 0) { - const organizations = - createdOrganizationsData.users[0]?.appUserProfile - ?.createdOrganizations || []; - setOrganizations(organizations); - } + const newOrganizations = getOrganizationsForMode( + mode, + data, + joinedOrganizationsData, + createdOrganizationsData, + userId + ); + setOrganizations(newOrganizations); }, [mode, data, joinedOrganizationsData, createdOrganizationsData, userId]);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/screens/UserPortal/Organizations/Organizations.spec.tsx
(2 hunks)src/screens/UserPortal/Organizations/Organizations.tsx
(1 hunks)
🔇 Additional comments (3)
src/screens/UserPortal/Organizations/Organizations.tsx (1)
225-226
: LGTM! Good catch adding userId to dependency array.Adding userId to the dependency array ensures that organizations are properly updated when the user changes.
src/screens/UserPortal/Organizations/Organizations.spec.tsx (2)
323-381
: LGTM! Excellent test coverage with comprehensive mock scenarios.The addition of ERROR_MOCKS, EMPTY_MOCKS, and LOADING_MOCKS provides thorough coverage of edge cases and error scenarios. The 2000ms delay in LOADING_MOCKS effectively simulates real-world loading states.
652-748
: LGTM! Excellent test coverage improvements.The new test cases thoroughly cover important scenarios:
- Pagination functionality
- Loading states with simulated delays
- Empty state handling
- Error state handling
- Responsive behavior with window resize events
This aligns perfectly with the PR objective of improving test coverage.
@Harshvardhan-91 Please make sure you're committing to the correct branch. |
@Cioppolo14 just to confirm i have to commit in develop-postgres |
Yes commit to develop-postgres branch |
Please submit your PRs against our Also if applicable, make sure your branch name is different from Closing. |
What kind of change does this PR introduce?
Issue Number:
Fixes #3040
Snapshots/Videos:
If relevant, did you update the documentation?
Summary
Does this PR introduce a breaking change?
Checklist
CodeRabbit AI Review
Test Coverage
Have you read the contributing guide?
Yes
Summary by CodeRabbit
Tests
Improvements
useEffect
hooks to be more responsive to user-specific data