-
Notifications
You must be signed in to change notification settings - Fork 11k
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
chore: Remove hideRoom client meteor method #33766
Conversation
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #33766 +/- ##
========================================
Coverage 75.79% 75.79%
========================================
Files 510 510
Lines 22063 22063
Branches 5385 5385
========================================
Hits 16723 16723
Misses 4694 4694
Partials 646 646
Flags with carried forward coverage won't be shown. Click here to find out more. |
6f6c56c
to
8637ced
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.
Nice working creating the new action hook using the mutation! I have a feeling that we should keep using hide in the name, despite the name of the endpoint we don't use to say "we closed the room", so in terms of dev exp should be better to recognize.
I realized we're missing the optimistic update the old client method does, we should implement it in the new mutation. Here's a reference on how to do it using react-query
fa7206d
to
802172b
Compare
ff1da07
to
88969d9
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.
This is a really cool refactoring job!
@tiagoevanp as discussed useMutation() won't be affected by the v5 upgrade
4e2f0d9
to
8a37af4
Compare
8a37af4
to
971ffd6
Compare
971ffd6
to
fb7516b
Compare
Proposed changes (including videos or screenshots)
This PR removes
hideRoom
meteor method and unifies hide room calls across the application.Issue(s)
Steps to test or reproduce
Further comments
ARCH-1340