-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
refactor: Reactions set/unset #32994
Conversation
Looks like this PR is ready to merge! 🎉 |
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #32994 +/- ##
===========================================
+ Coverage 59.85% 59.93% +0.08%
===========================================
Files 2554 2556 +2
Lines 63008 63400 +392
Branches 14129 14246 +117
===========================================
+ Hits 37711 37998 +287
- Misses 22890 22969 +79
- Partials 2407 2433 +26
Flags with carried forward coverage won't be shown. Click here to find out more. |
setReaction
and executeSetReaction
functions
This PR currently has a merge conflict. Please resolve this and then re-add the |
|
Proposed changes (including videos or screenshots)
chat.react
endpoint to remove check for message (executeSetReaction already does it)executeSetReaction
to perform emoji & reactions check before all DB callsexecuteSetReaction
to use newcountByNameOrAlias
query instead of afind().count()
executeSetReaction
to only request the required data when requesting room from DBsetReaction
to not await for callbacks (we don't need the return value anyways)setReaction
to notify of room changed at the end of the function (instead of next to every update). This doesn't gain any perf, but looks better (and also fixes a missing notify).Issue(s)
https://rocketchat.atlassian.net/browse/CORE-597
Steps to test or reproduce
Further comments