notifications: Ignore mobile presence when sending notifications.

Previously, if the user had interacted with the Zulip mobile app in
the last ~140 seconds, it's likely the mobile app had sent presence
data to the Zulip server, which in turns means that the Zulip server
might not send that user mobile push notifications (or email
notifications) about new messages for the next few minutes.

The email notifications behavior is potentially desirable, but the
push notifications behavior is definitely not -- a private message
reply to something you sent 2 minutes ago is definitely something you
want a push notification for.

This commit partially addresses that issue, by ignoring presence data
from the ZulipMobile client when determining whether the user is
currently engaging with a Zulip client (essentially, we're only
considering desktop activity as something that predicts the user is
likely to see a desktop notification or is otherwise "online").
This commit is contained in:
Tim Abbott
2019-12-10 16:58:44 -08:00
parent 17bde5944d
commit 299896b6ce
2 changed files with 23 additions and 3 deletions

View File

@@ -4930,16 +4930,30 @@ def get_active_presence_idle_user_ids(realm: Realm,
return filter_presence_idle_user_ids(user_ids)
def filter_presence_idle_user_ids(user_ids: Set[int]) -> List[int]:
# Given a set of user IDs (the recipients of a message), accesses
# the UserPresence table to determine which of these users are
# currently idle and should potentially get email notifications
# (and push notifications with with
# user_profile.enable_online_push_notifications=False).
#
# We exclude any presence data from ZulipMobile for the purpose of
# triggering these notifications; the mobile app can more
# effectively do its own client-side filtering of notification
# sounds/etc. for the case that the user is actively doing a PM
# conversation in the app.
if not user_ids:
return []
# 140 seconds is consistent with presence.js:OFFLINE_THRESHOLD_SECS
recent = timezone_now() - datetime.timedelta(seconds=140)
# Matches presence.js constant
OFFLINE_THRESHOLD_SECS = 140
recent = timezone_now() - datetime.timedelta(seconds=OFFLINE_THRESHOLD_SECS)
rows = UserPresence.objects.filter(
user_profile_id__in=user_ids,
status=UserPresence.ACTIVE,
timestamp__gte=recent
).distinct('user_profile_id').values('user_profile_id')
).exclude(client__name="ZulipMobile").distinct('user_profile_id').values('user_profile_id')
active_user_ids = {row['user_profile_id'] for row in rows}
idle_user_ids = user_ids - active_user_ids
return sorted(list(idle_user_ids))

View File

@@ -278,6 +278,12 @@ class UserPresenceTests(ZulipTestCase):
self.assertEqual(filter_presence_idle_user_ids({user_profile.id}), [user_profile.id])
self.client_post("/json/users/me/presence", {'status': 'idle'})
self.assertEqual(filter_presence_idle_user_ids({user_profile.id}), [user_profile.id])
# Active presence from the mobile app doesn't count
self.client_post("/json/users/me/presence", {'status': 'active'},
HTTP_USER_AGENT="ZulipMobile/1.0")
self.assertEqual(filter_presence_idle_user_ids({user_profile.id}), [user_profile.id])
self.client_post("/json/users/me/presence", {'status': 'active'})
self.assertEqual(filter_presence_idle_user_ids({user_profile.id}), [])