-
Notifications
You must be signed in to change notification settings - Fork 421
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
Use prepared queries in mod_event_pusher_push #3060
Conversation
chrzaszcz
commented
Mar 22, 2021
•
edited
edited
- Introduce prepared queries
- Update the backend API to remove an unused case that would require a separate prepared query
Codecov Report
@@ Coverage Diff @@
## master #3060 +/- ##
==========================================
+ Coverage 78.73% 78.77% +0.04%
==========================================
Files 378 378
Lines 31101 31096 -5
==========================================
+ Hits 24486 24497 +11
+ Misses 6615 6599 -16
Continue to review full report at Codecov.
|
Also: split the 'disable' callback There are two distinct use cases now: - disable/1 for cleanup on user removal - disable/3 for disabling pubsub jid with an optional node Before it was possible to specify the node without the pubsub JID. However, this was unused and there was no use case or test for it. XEP-0357 also does not allow this use case. There was no point in introducing an unused prepared query. Other changes: - Minor refactoring - Fix type specs
7f35e2e
to
ae33b21
Compare
big_tests/tests/push_SUITE.erl
Outdated
%% Remove account - this should disable the notifications | ||
escalus_connection:send(Bob, escalus_stanza:remove_account()), | ||
escalus:assert(is_iq_result, escalus:wait_for_stanza(Bob)), | ||
Pid = mongoose_helper:get_session_pid(Bob, distributed_helper:mim()), |
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.
Shouldn't we first get the session PID and only then remove the account?
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.
Generally looks good, thanks for extending push_SUITE.
I just have one small comment to the new testcase
Also: remove an unused function.
bd4bd87
to
a031068
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.
👍