-
Notifications
You must be signed in to change notification settings - Fork 10.7k
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: Update Omnichannel's room closing mechanism to use transactions. #32896
Conversation
Looks like this PR is ready to merge! 🎉 |
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #32896 +/- ##
===========================================
- Coverage 59.39% 59.38% -0.01%
===========================================
Files 2547 2547
Lines 63241 63241
Branches 14223 14225 +2
===========================================
- Hits 37559 37558 -1
- Misses 22974 22975 +1
Partials 2708 2708
Flags with carried forward coverage won't be shown. Click here to find out more. |
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.
I wouldn't like to start to spread MongoDB (or any other DB) transaction mechanism into business rules code, this could open a wormhole to dark places, but I understand your point here. For this specific case I'm okay with it, but for sure we need a more concrete and robust solution for using DB transactions.
Yup, kinda the same shared on the chapter. This facilitates this specific problem but it can create new ones (hopefully not). We would need to check on how this behaves during testing to ensure it won't add more problems than it solves. And ideally, we would have a nicer api for dealing with this after mongo3 (where we can have some async storage that allows to pass data down to the models without editing the whole chain). |
This PR currently has a merge conflict. Please resolve this and then re-add the |
b2d91f7
Proposed changes (including videos or screenshots)
livechat-close
message after the room is closed is not part of the scope (mainly because of the involved code changes to make all callbacks accept a session.Issue(s)
https://rocketchat.atlassian.net/browse/CORE-238
Steps to test or reproduce
Further comments