Add unavailable presence after blocking contact #900
Conversation
| %%%---------------------------------------------------------------------- | ||
|
|
||
| maybe_update_presence(StateData = #state{jid = JID}, NewList) -> | ||
| {Froms, _Tos, _Pending} = ejabberd_hooks:run_fold( |
There was a problem hiding this comment.
This hook is not necessary here, I think. If I understand it correctly, all you need (subscription lists) are already in the StateData var. Take a look at fields pres_*, they are documented quite well in the ejabberd_c2s.hrl file.
There was a problem hiding this comment.
Moreover running the roster_get_subscription_lists hook will touch DB every time which is not the most efficient way :)
There was a problem hiding this comment.
Ah, thanks. I hadn't noticed that they were already in the state. I've pushed a fix, as well as tweaking some of the test code a little to avoid a couple of intermittent failures I was getting.
There was a problem hiding this comment.
This was quick :) thanks @bernardd
| @@ -711,6 +711,7 @@ block_jid_message_but_not_presence(Config) -> | |||
| end). | |||
|
|
|||
| newly_blocked_presense_jid_by_new_list(Config) -> | |||
| reset_users(Config), | |||
There was a problem hiding this comment.
Instead of reseting users you can use escalus_fresh:story/3 instead of escalus:story/3. This will create new users for you based on the provided specs. Please remember to call escalus_fresh:clean/0 in end_per_suite to remove this fresh users.
There was a problem hiding this comment.
The only catch with that is that it creates new usernames as well, so the ones hardcoded into the privacy_helper lists don't work. Still, it feels like a neater solution, so I'll sort it out.
There was a problem hiding this comment.
Okay, fixed that and did a couple more little cleanups too.
The previous pres_f fix introduced a bug that would send 'unavailable' back to the user making the privacy change. I've added a test to cover this case and fixed the bug.
|
Thanks a lot @bernardd for your patch! I like your code and the tests you've written for the functionality! |
Add unavailable presence after blocking contact
XEP-0016 section 2.11 contains the following requirement:
However for this case no unavailable message was being sent. This patch adds this behaviour, sending an "unavailable" from A to B when all the following conditions are met:
This covers the unambiguous case described in the XEP. It's not clear to me whether an 'unavailable' is also required to be sent when some second-order condition changes the blocking state. For example, if A has an existing privacy list that blocks all presence information to group X, and A adds B to group X, should that trigger an unavailable message? XE-0016 isn't really explcit on the matter. For now I've left those other cases out.