prevented unnecessary page reload with complementary test #3202
Conversation
|
Warning Rate limit exceeded@Dhiren-Mhatre has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 11 minutes and 39 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThe pull request addresses an issue with page reloading after removing a member from an organization. The changes modify the Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
Finishing Touches
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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/components/OrgPeopleListCard/OrgPeopleListCard.tsx (1)
Line range hint
62-65: Consider enhancing error handling for better UX.The current error handling could be improved to provide more specific feedback to users.
Consider this enhancement:
} catch (error: unknown) { - errorHandler(t, error); + errorHandler(t, error); + // Close modal on error to prevent it from staying open + props.toggleRemoveModal(); }src/components/OrgPeopleListCard/OrgPeopleListCard.spec.tsx (1)
165-165: LGTM! Updated test expectations match new behavior.The test now correctly verifies that the modal is toggled instead of checking for page reload.
Consider adding a test to verify the UI state after successful member removal:
// Add after successful removal verification expect(screen.queryByText(removedMemberName)).not.toBeInTheDocument();
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/OrgPeopleListCard/OrgPeopleListCard.spec.tsx(2 hunks)src/components/OrgPeopleListCard/OrgPeopleListCard.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Test Application
- GitHub Check: Analyse Code With CodeQL (javascript)
🔇 Additional comments (3)
src/components/OrgPeopleListCard/OrgPeopleListCard.tsx (1)
61-61: LGTM! Removing unnecessary page reload improves UX.The change aligns with the PR objectives by eliminating the disruptive page reload after member removal.
However, let's verify that Apollo Client's cache is properly updated after the mutation:
src/components/OrgPeopleListCard/OrgPeopleListCard.spec.tsx (2)
86-99: LGTM! Well-structured mock for null data scenario.The mock is properly configured to test the null data response from the mutation.
101-123: LGTM! Comprehensive test coverage for null data handling.The test thoroughly verifies that neither success toast nor modal toggle are called when receiving null data.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #3202 +/- ##
=====================================================
+ Coverage 12.22% 90.06% +77.84%
=====================================================
Files 308 329 +21
Lines 7839 8525 +686
Branches 1709 1913 +204
=====================================================
+ Hits 958 7678 +6720
+ Misses 6815 613 -6202
- Partials 66 234 +168 ☔ View full report in Codecov by Sentry. |
|
LGTM, this PR can be merged after the stale comment is updated |
…dation#3165) * UI fixes on organisation pages * Added TSDoc for Truncated Text * Added Debouncer * Update src/components/OrgListCard/OrgListCard.tsx Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Added code rabbit suggestions * Fixed test error --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…adoesFoundation#3203) * Improved Code Coverage in src/components/Venues/VenueModal.tsx * removed the ignore statements from VenueModal.tsx
…ation#3149) * code coverage * jest global coverage decreased * global jest coverage * rename file problem solved * changes requested resolved
This reverts commit e0fa894.
…/user-delete-page-reload
rishav-jha-mech
merged commit 011f9ab
into
PalisadoesFoundation:develop-postgres
Fix Unnecessary Page Reload After User Deletion
What kind of change does this PR introduce?
Bugfix - Removes unnecessary page reload after deleting a user from organization people list
Issue Number:
Fixes #2700
Did you add tests for your changes?
Yes - Added unit tests to verify the user deletion flow and UI update behavior
Snapshots/Videos:
Screencast.from.2025-01-08.01-16-28.webm
Screencast.from.2025-01-08.01-08-03.webm
Summary
This PR addresses the unnecessary page reload that occurs when deleting a user from the organization people list. Currently, the page reloads 2 seconds after successful user deletion, which disrupts the user experience. This PR implements the following changes:
Does this PR introduce a breaking change?
No - This is a behavioral improvement that maintains all existing functionality
Testing Performed
Have you read the contributing guide?
Yes
Please review and let me know if any additional changes are needed.
Summary by CodeRabbit
Tests
Bug Fixes