paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-11-03 21:43:21 +00:00
Code Issues Packages Projects Releases Wiki Activity

maybe_enqueue_notifications: Take in notification_data dataclass.

Browse Source 
* Modify `maybe_enqueue_notifications` to take in an instance of the
dataclass introduced in 951b49c048.

* The `check_notify` tests tested the "when to notify" logic in a way
which involved `maybe_enqueue_notifications`. To simplify things, we've
earlier extracted this logic in 8182632d7e.
So, we just kill off the `check_notify` test, and keep only those parts
which verify the queueing and return value behavior of that funtion.

* We retain the the missedmessage_hook and message
message_edit_notifications since they are more integration-style.

* There's a slightly subtle change with the missedmessage_hook tests.
Before this commit, we short-circuited the hook if the sender was muted
(5a642cea11).
With this commit, we delegate the check to our dataclass methods.
So, `maybe_enqueue_notifications` will be called even if the sender was
muted, and the test needs to be updated.

* In our test helper `get_maybe_enqueue_notifications_parameters` which
generates default values for testing `maybe_enqueue_notifications` calls,
we keep `message_id`, `sender_id`, and `user_id` as required arguments,
so that the tests are super-clear and avoid accidental false positives.

* Because `do_update_embedded_data` also sends `update_message` events,
we deal with that case with some hacky code for now. See the comment
there.

This mostly completes the extraction of the "when to notify" logic into
our new `notification_data` module.
This commit is contained in:
Abhijeet Prasad Bodas
2021-06-23 17:42:32 +05:30
committed by Tim Abbott
parent dedc39f139
commit 66192825c0
5 changed files with 135 additions and 211 deletions
Download Patch File Download Diff File Expand all files Collapse all files

30
zerver/lib/test_classes.py
View File

@@ -1337,26 +1337,28 @@ Output:
sender_is_muted=kwargs.get("sender_is_muted", False), sender_is_muted=kwargs.get("sender_is_muted", False),
) )
def get_maybe_enqueue_notifications_parameters(self, **kwargs: Any) -> Dict[str, Any]: def get_maybe_enqueue_notifications_parameters(
self, *, message_id: int, user_id: int, sender_id: int, **kwargs: Any
) -> Dict[str, Any]:
""" """
Returns a dictionary with the passed parameters, after filling up the Returns a dictionary with the passed parameters, after filling up the
missing data with default values, for testing what was passed to the missing data with default values, for testing what was passed to the
`maybe_enqueue_notifications` method. `maybe_enqueue_notifications` method.
""" """
parameters: Dict[str, Any] = dict( user_notifications_data = self.create_user_notifications_data_object(
private_message=False, user_id=user_id, **kwargs
mentioned=False, )
wildcard_mention_notify=False, return dict(
stream_push_notify=False, user_data=user_notifications_data,
stream_email_notify=False, message_id=message_id,
stream_name=None, sender_id=sender_id,
online_push_enabled=False, private_message=kwargs.get("private_message", False),
idle=True, stream_name=kwargs.get("stream_name", None),
already_notified={"email_notified": False, "push_notified": False}, idle=kwargs.get("idle", True),
already_notified=kwargs.get(
"already_notified", {"email_notified": False, "push_notified": False}
),
) )
# Values from `kwargs` will replace those from `parameters`
return {**parameters, **kwargs}
class WebhookTestCase(ZulipTestCase): class WebhookTestCase(ZulipTestCase):

183
zerver/tests/test_event_queue.py
View File

@@ -1,5 +1,5 @@
import time import time
from typing import Any, Callable, Collection, Dict, List, Optional, Tuple, Union from typing import Any, Callable, Collection, Dict, List
from unittest import mock from unittest import mock
import orjson import orjson
@@ -25,125 +25,34 @@ class MissedMessageNotificationsTest(ZulipTestCase):
"""Tests the logic for when missed-message notifications """Tests the logic for when missed-message notifications
should be triggered, based on user settings""" should be triggered, based on user settings"""
def check_will_notify(self, **kwargs: Any) -> Tuple[str, str]: def test_maybe_enqueue_notifications(self) -> None:
hamlet = self.example_user("hamlet") # We've already tested the "when to send notifications" logic as part of the
kwargs["user_profile_id"] = hamlet.id # notification_data module.
kwargs["message_id"] = 32 # This test is for verifying whether `maybe_enqueue_notifications` returns the
# `already_notified` data correctly.
params = self.get_maybe_enqueue_notifications_parameters(
message_id=1, user_id=1, sender_id=2
)
# Fill up the parameters which weren't sent with defaults.
kwargs = self.get_maybe_enqueue_notifications_parameters(**kwargs)
email_notice = None
mobile_notice = None
with mock_queue_publish( with mock_queue_publish(
"zerver.tornado.event_queue.queue_json_publish" "zerver.tornado.event_queue.queue_json_publish"
) as mock_queue_json_publish: ) as mock_queue_json_publish:
notified = maybe_enqueue_notifications(**kwargs) notified = maybe_enqueue_notifications(**params)
for entry in mock_queue_json_publish.call_args_list: mock_queue_json_publish.assert_not_called()
args = entry[0]
if args[0] == "missedmessage_mobile_notifications":
mobile_notice = args[1]
if args[0] == "missedmessage_emails":
email_notice = args[1]
# Now verify the return value matches the queue actions with mock_queue_publish(
if email_notice: "zerver.tornado.event_queue.queue_json_publish"
self.assertTrue(notified["email_notified"]) ) as mock_queue_json_publish:
else: params["private_message"] = True
self.assertFalse(notified.get("email_notified", False)) notified = maybe_enqueue_notifications(**params)
if mobile_notice: self.assertTrue(mock_queue_json_publish.call_count, 2)
self.assertTrue(notified["push_notified"])
else:
self.assertFalse(notified.get("push_notified", False))
return email_notice, mobile_notice
def test_enqueue_notifications(self) -> None: queues_pushed = [entry[0][0] for entry in mock_queue_json_publish.call_args_list]
# Boring message doesn't send a notice self.assertIn("missedmessage_mobile_notifications", queues_pushed)
email_notice, mobile_notice = self.check_will_notify() self.assertIn("missedmessage_emails", queues_pushed)
self.assertTrue(email_notice is None)
self.assertTrue(mobile_notice is None)
# Private message sends a notice self.assertTrue(notified["email_notified"])
email_notice, mobile_notice = self.check_will_notify(private_message=True) self.assertTrue(notified["push_notified"])
self.assertTrue(email_notice is not None)
self.assertTrue(mobile_notice is not None)
# Private message won't double-send either notice if we've
# already sent notices before.
email_notice, mobile_notice = self.check_will_notify(
private_message=True, already_notified={"push_notified": True, "email_notified": False}
)
self.assertTrue(email_notice is not None)
self.assertTrue(mobile_notice is None)
email_notice, mobile_notice = self.check_will_notify(
private_message=True, already_notified={"push_notified": False, "email_notified": True}
)
self.assertTrue(email_notice is None)
self.assertTrue(mobile_notice is not None)
# Mention sends a notice
email_notice, mobile_notice = self.check_will_notify(mentioned=True)
self.assertTrue(email_notice is not None)
self.assertTrue(mobile_notice is not None)
# Wildcard mention triggers both email and push notices (Like a
# direct mention, whether the notice is actually delivered is
# determined later, in the email/push notification code)
email_notice, mobile_notice = self.check_will_notify(wildcard_mention_notify=True)
self.assertTrue(email_notice is not None)
self.assertTrue(mobile_notice is not None)
# stream_push_notify pushes but doesn't email
email_notice, mobile_notice = self.check_will_notify(
stream_push_notify=True, stream_name="Denmark"
)
self.assertTrue(email_notice is None)
self.assertTrue(mobile_notice is not None)
# stream_email_notify emails but doesn't push
email_notice, mobile_notice = self.check_will_notify(
stream_email_notify=True, stream_name="Denmark"
)
self.assertTrue(email_notice is not None)
self.assertTrue(mobile_notice is None)
# Private message doesn't send a notice if not idle
email_notice, mobile_notice = self.check_will_notify(private_message=True, idle=False)
self.assertTrue(email_notice is None)
self.assertTrue(mobile_notice is None)
# Mention doesn't send a notice if not idle
email_notice, mobile_notice = self.check_will_notify(mentioned=True, idle=False)
self.assertTrue(email_notice is None)
self.assertTrue(mobile_notice is None)
# Wildcard mention doesn't send a notice if not idle
email_notice, mobile_notice = self.check_will_notify(
wildcard_mention_notify=True, idle=False
)
self.assertTrue(email_notice is None)
self.assertTrue(mobile_notice is None)
# Private message sends push but not email if not idle but online_push_enabled
email_notice, mobile_notice = self.check_will_notify(
private_message=True, online_push_enabled=True, idle=False
)
self.assertTrue(email_notice is None)
self.assertTrue(mobile_notice is not None)
# Stream message sends push (if enabled for that stream) but not email if not
# idle but online_push_enabled
email_notice, mobile_notice = self.check_will_notify(
stream_push_notify=True,
stream_email_notify=True,
stream_name="Denmark",
online_push_enabled=True,
idle=False,
already_notified={},
)
self.assertTrue(email_notice is None)
self.assertTrue(mobile_notice is not None)
def tornado_call( def tornado_call(
self, self,
@@ -246,23 +155,14 @@ class MissedMessageNotificationsTest(ZulipTestCase):
def assert_maybe_enqueue_notifications_call_args( def assert_maybe_enqueue_notifications_call_args(
args_dict: Collection[Any], args_dict: Collection[Any],
user_profile_id: int,
message_id: int, message_id: int,
**kwargs: Union[bool, Dict[str, bool], Optional[str]], **kwargs: Any,
) -> None: ) -> None:
kwargs = self.get_maybe_enqueue_notifications_parameters(**kwargs) expected_args_dict = self.get_maybe_enqueue_notifications_parameters(
expected_args_dict = dict( user_id=user_profile.id,
user_profile_id=user_profile_id, sender_id=iago.id,
message_id=message_id, message_id=message_id,
private_message=kwargs["private_message"], **kwargs,
mentioned=kwargs["mentioned"],
wildcard_mention_notify=kwargs["wildcard_mention_notify"],
stream_push_notify=kwargs["stream_push_notify"],
stream_email_notify=kwargs["stream_email_notify"],
stream_name=kwargs["stream_name"],
online_push_enabled=kwargs["online_push_enabled"],
idle=kwargs["idle"],
already_notified=kwargs["already_notified"],
) )
self.assertEqual(args_dict, expected_args_dict) self.assertEqual(args_dict, expected_args_dict)
@@ -285,7 +185,6 @@ class MissedMessageNotificationsTest(ZulipTestCase):
assert_maybe_enqueue_notifications_call_args( assert_maybe_enqueue_notifications_call_args(
args_dict=args_dict, args_dict=args_dict,
user_profile_id=user_profile.id,
message_id=msg_id, message_id=msg_id,
stream_name="Denmark", stream_name="Denmark",
already_notified={"email_notified": False, "push_notified": False}, already_notified={"email_notified": False, "push_notified": False},
@@ -303,7 +202,6 @@ class MissedMessageNotificationsTest(ZulipTestCase):
assert_maybe_enqueue_notifications_call_args( assert_maybe_enqueue_notifications_call_args(
args_dict=args_dict, args_dict=args_dict,
user_profile_id=user_profile.id,
message_id=msg_id, message_id=msg_id,
private_message=True, private_message=True,
already_notified={"email_notified": True, "push_notified": True}, already_notified={"email_notified": True, "push_notified": True},
@@ -323,8 +221,8 @@ class MissedMessageNotificationsTest(ZulipTestCase):
assert_maybe_enqueue_notifications_call_args( assert_maybe_enqueue_notifications_call_args(
args_dict=args_dict, args_dict=args_dict,
user_profile_id=user_profile.id,
message_id=msg_id, message_id=msg_id,
flags=["mentioned"],
mentioned=True, mentioned=True,
stream_name="Denmark", stream_name="Denmark",
already_notified={"email_notified": True, "push_notified": True}, already_notified={"email_notified": True, "push_notified": True},
@@ -342,8 +240,8 @@ class MissedMessageNotificationsTest(ZulipTestCase):
assert_maybe_enqueue_notifications_call_args( assert_maybe_enqueue_notifications_call_args(
args_dict=args_dict, args_dict=args_dict,
user_profile_id=user_profile.id,
message_id=msg_id, message_id=msg_id,
flags=["wildcard_mentioned"],
wildcard_mention_notify=True, wildcard_mention_notify=True,
stream_name="Denmark", stream_name="Denmark",
already_notified={"email_notified": True, "push_notified": True}, already_notified={"email_notified": True, "push_notified": True},
@@ -362,7 +260,7 @@ class MissedMessageNotificationsTest(ZulipTestCase):
assert_maybe_enqueue_notifications_call_args( assert_maybe_enqueue_notifications_call_args(
args_dict=args_dict, args_dict=args_dict,
user_profile_id=user_profile.id, flags=["wildcard_mentioned"],
message_id=msg_id, message_id=msg_id,
stream_name="Denmark", stream_name="Denmark",
already_notified={"email_notified": False, "push_notified": False}, already_notified={"email_notified": False, "push_notified": False},
@@ -383,8 +281,8 @@ class MissedMessageNotificationsTest(ZulipTestCase):
assert_maybe_enqueue_notifications_call_args( assert_maybe_enqueue_notifications_call_args(
args_dict=args_dict, args_dict=args_dict,
user_profile_id=user_profile.id,
message_id=msg_id, message_id=msg_id,
flags=["wildcard_mentioned"],
stream_name="Denmark", stream_name="Denmark",
already_notified={"email_notified": False, "push_notified": False}, already_notified={"email_notified": False, "push_notified": False},
) )
@@ -408,8 +306,8 @@ class MissedMessageNotificationsTest(ZulipTestCase):
assert_maybe_enqueue_notifications_call_args( assert_maybe_enqueue_notifications_call_args(
args_dict=args_dict, args_dict=args_dict,
user_profile_id=user_profile.id,
message_id=msg_id, message_id=msg_id,
flags=["wildcard_mentioned"],
wildcard_mention_notify=True, wildcard_mention_notify=True,
stream_name="Denmark", stream_name="Denmark",
already_notified={"email_notified": True, "push_notified": True}, already_notified={"email_notified": True, "push_notified": True},
@@ -432,7 +330,6 @@ class MissedMessageNotificationsTest(ZulipTestCase):
assert_maybe_enqueue_notifications_call_args( assert_maybe_enqueue_notifications_call_args(
args_dict=args_dict, args_dict=args_dict,
user_profile_id=user_profile.id,
message_id=msg_id, message_id=msg_id,
stream_push_notify=True, stream_push_notify=True,
stream_email_notify=False, stream_email_notify=False,
@@ -455,7 +352,6 @@ class MissedMessageNotificationsTest(ZulipTestCase):
assert_maybe_enqueue_notifications_call_args( assert_maybe_enqueue_notifications_call_args(
args_dict=args_dict, args_dict=args_dict,
user_profile_id=user_profile.id,
message_id=msg_id, message_id=msg_id,
stream_push_notify=False, stream_push_notify=False,
stream_email_notify=True, stream_email_notify=True,
@@ -486,7 +382,6 @@ class MissedMessageNotificationsTest(ZulipTestCase):
assert_maybe_enqueue_notifications_call_args( assert_maybe_enqueue_notifications_call_args(
args_dict=args_dict, args_dict=args_dict,
user_profile_id=user_profile.id,
message_id=msg_id, message_id=msg_id,
stream_name="Denmark", stream_name="Denmark",
already_notified={"email_notified": False, "push_notified": False}, already_notified={"email_notified": False, "push_notified": False},
@@ -510,7 +405,6 @@ class MissedMessageNotificationsTest(ZulipTestCase):
assert_maybe_enqueue_notifications_call_args( assert_maybe_enqueue_notifications_call_args(
args_dict=args_dict, args_dict=args_dict,
user_profile_id=user_profile.id,
message_id=msg_id, message_id=msg_id,
stream_name="Denmark", stream_name="Denmark",
already_notified={"email_notified": False, "push_notified": False}, already_notified={"email_notified": False, "push_notified": False},
@@ -530,8 +424,17 @@ class MissedMessageNotificationsTest(ZulipTestCase):
msg_id = self.send_personal_message(iago, user_profile) msg_id = self.send_personal_message(iago, user_profile)
with mock.patch("zerver.tornado.event_queue.maybe_enqueue_notifications") as mock_enqueue: with mock.patch("zerver.tornado.event_queue.maybe_enqueue_notifications") as mock_enqueue:
missedmessage_hook(user_profile.id, client_descriptor, True) missedmessage_hook(user_profile.id, client_descriptor, True)
# If the sender is muted, we don't call `maybe_enqueue_notifications` at all. mock_enqueue.assert_called_once()
mock_enqueue.assert_not_called() args_dict = mock_enqueue.call_args_list[0][1]
assert_maybe_enqueue_notifications_call_args(
args_dict=args_dict,
message_id=msg_id,
private_message=True,
sender_is_muted=True,
flags=["read"],
already_notified={"email_notified": False, "push_notified": False},
)
destroy_event_queue(client_descriptor.event_queue.id) destroy_event_queue(client_descriptor.event_queue.id)
result = self.api_delete(user_profile, f"/api/v1/users/me/muted_users/{iago.id}") result = self.api_delete(user_profile, f"/api/v1/users/me/muted_users/{iago.id}")
self.assert_json_success(result) self.assert_json_success(result)

30
zerver/tests/test_message_edit_notifications.py
View File

@@ -105,7 +105,7 @@ class EditMessageSideEffectsTest(ZulipTestCase):
cordelia_calls = [ cordelia_calls = [
call_args call_args
for call_args in m.call_args_list for call_args in m.call_args_list
if call_args[1]["user_profile_id"] == cordelia.id if call_args[1]["user_data"].user_id == cordelia.id
] ]
if expect_short_circuit: if expect_short_circuit:
@@ -184,10 +184,13 @@ class EditMessageSideEffectsTest(ZulipTestCase):
info = notification_message_data["info"] info = notification_message_data["info"]
cordelia = self.example_user("cordelia") cordelia = self.example_user("cordelia")
hamlet = self.example_user("hamlet")
expected_enqueue_kwargs = self.get_maybe_enqueue_notifications_parameters( expected_enqueue_kwargs = self.get_maybe_enqueue_notifications_parameters(
user_profile_id=cordelia.id, user_id=cordelia.id,
sender_id=hamlet.id,
message_id=message_id, message_id=message_id,
mentioned=True, mentioned=True,
flags=["mentioned"],
stream_name="Scotland", stream_name="Scotland",
already_notified={}, already_notified={},
) )
@@ -295,6 +298,7 @@ class EditMessageSideEffectsTest(ZulipTestCase):
def test_online_push_enabled_for_fully_present_mentioned_user(self) -> None: def test_online_push_enabled_for_fully_present_mentioned_user(self) -> None:
cordelia = self.example_user("cordelia") cordelia = self.example_user("cordelia")
hamlet = self.example_user("hamlet")
# Simulate Cordelia is FULLY present, not just in term of # Simulate Cordelia is FULLY present, not just in term of
# browser activity, but also in terms of her client descriptors. # browser activity, but also in terms of her client descriptors.
@@ -312,10 +316,12 @@ class EditMessageSideEffectsTest(ZulipTestCase):
info = notification_message_data["info"] info = notification_message_data["info"]
expected_enqueue_kwargs = self.get_maybe_enqueue_notifications_parameters( expected_enqueue_kwargs = self.get_maybe_enqueue_notifications_parameters(
user_profile_id=cordelia.id, user_id=cordelia.id,
sender_id=hamlet.id,
message_id=message_id, message_id=message_id,
mentioned=True, mentioned=True,
stream_name="Scotland", stream_name="Scotland",
flags=["mentioned"],
online_push_enabled=True, online_push_enabled=True,
idle=False, idle=False,
already_notified={}, already_notified={},
@@ -329,6 +335,7 @@ class EditMessageSideEffectsTest(ZulipTestCase):
def test_online_push_enabled_for_fully_present_boring_user(self) -> None: def test_online_push_enabled_for_fully_present_boring_user(self) -> None:
cordelia = self.example_user("cordelia") cordelia = self.example_user("cordelia")
hamlet = self.example_user("hamlet")
# Simulate Cordelia is FULLY present, not just in term of # Simulate Cordelia is FULLY present, not just in term of
# browser activity, but also in terms of her client descriptors. # browser activity, but also in terms of her client descriptors.
@@ -346,7 +353,8 @@ class EditMessageSideEffectsTest(ZulipTestCase):
info = notification_message_data["info"] info = notification_message_data["info"]
expected_enqueue_kwargs = self.get_maybe_enqueue_notifications_parameters( expected_enqueue_kwargs = self.get_maybe_enqueue_notifications_parameters(
user_profile_id=cordelia.id, user_id=cordelia.id,
sender_id=hamlet.id,
message_id=message_id, message_id=message_id,
stream_name="Scotland", stream_name="Scotland",
online_push_enabled=True, online_push_enabled=True,
@@ -381,9 +389,11 @@ class EditMessageSideEffectsTest(ZulipTestCase):
info = notification_message_data["info"] info = notification_message_data["info"]
expected_enqueue_kwargs = self.get_maybe_enqueue_notifications_parameters( expected_enqueue_kwargs = self.get_maybe_enqueue_notifications_parameters(
user_profile_id=cordelia.id, user_id=cordelia.id,
message_id=message_id, message_id=message_id,
sender_id=self.example_user("hamlet").id,
mentioned=True, mentioned=True,
flags=["mentioned"],
stream_name="Scotland", stream_name="Scotland",
already_notified={}, already_notified={},
) )
@@ -395,6 +405,7 @@ class EditMessageSideEffectsTest(ZulipTestCase):
def test_updates_with_wildcard_mention(self) -> None: def test_updates_with_wildcard_mention(self) -> None:
cordelia = self.example_user("cordelia") cordelia = self.example_user("cordelia")
hamlet = self.example_user("hamlet")
# We will simulate that the user still has a an active client, # We will simulate that the user still has a an active client,
# but they don't have UserPresence rows, so we will still # but they don't have UserPresence rows, so we will still
@@ -411,9 +422,11 @@ class EditMessageSideEffectsTest(ZulipTestCase):
info = notification_message_data["info"] info = notification_message_data["info"]
expected_enqueue_kwargs = self.get_maybe_enqueue_notifications_parameters( expected_enqueue_kwargs = self.get_maybe_enqueue_notifications_parameters(
user_profile_id=cordelia.id, user_id=cordelia.id,
sender_id=hamlet.id,
message_id=message_id, message_id=message_id,
wildcard_mention_notify=True, wildcard_mention_notify=True,
flags=["wildcard_mentioned"],
stream_name="Scotland", stream_name="Scotland",
already_notified={}, already_notified={},
) )
@@ -450,6 +463,7 @@ class EditMessageSideEffectsTest(ZulipTestCase):
def test_updates_with_stream_mention_of_fully_present_user(self) -> None: def test_updates_with_stream_mention_of_fully_present_user(self) -> None:
cordelia = self.example_user("cordelia") cordelia = self.example_user("cordelia")
hamlet = self.example_user("hamlet")
# Simulate Cordelia is FULLY present, not just in term of # Simulate Cordelia is FULLY present, not just in term of
# browser activity, but also in terms of her client descriptors. # browser activity, but also in terms of her client descriptors.
@@ -466,9 +480,11 @@ class EditMessageSideEffectsTest(ZulipTestCase):
info = notification_message_data["info"] info = notification_message_data["info"]
expected_enqueue_kwargs = self.get_maybe_enqueue_notifications_parameters( expected_enqueue_kwargs = self.get_maybe_enqueue_notifications_parameters(
user_profile_id=cordelia.id, user_id=cordelia.id,
sender_id=hamlet.id,
message_id=message_id, message_id=message_id,
mentioned=True, mentioned=True,
flags=["mentioned"],
stream_name="Scotland", stream_name="Scotland",
idle=False, idle=False,
already_notified={}, already_notified={},

2
zerver/tests/test_muting_users.py
View File

@@ -312,7 +312,7 @@ class MutedUsersTests(ZulipTestCase):
self.assert_json_success(result) self.assert_json_success(result)
m.assert_called_once() m.assert_called_once()
# `maybe_enqueue_notificaions` was called for Hamlet after message edit mentioned him. # `maybe_enqueue_notificaions` was called for Hamlet after message edit mentioned him.
self.assertEqual(m.call_args_list[0][1]["user_profile_id"], hamlet.id) self.assertEqual(m.call_args_list[0][1]["user_data"].user_id, hamlet.id)
# Hamlet mutes Cordelia. # Hamlet mutes Cordelia.
self.login("hamlet") self.login("hamlet")

101
zerver/tornado/event_queue.py
View File

@@ -737,26 +737,26 @@ def missedmessage_hook(
if event["type"] != "message": if event["type"] != "message":
continue continue
internal_data = event.get("internal_data", {}) internal_data = event.get("internal_data", {})
sender_id = event["message"]["sender_id"]
sender_is_muted = internal_data.get("sender_is_muted", False) user_notifications_data = UserMessageNotificationsData(
if sender_is_muted: user_id=user_profile_id,
continue flags=event.get("flags", []),
sender_is_muted=internal_data.get("sender_is_muted", False),
mentioned=internal_data.get("mentioned", False),
stream_push_notify=internal_data.get("stream_push_notify", False),
stream_email_notify=internal_data.get("stream_email_notify", False),
wildcard_mention_notify=internal_data.get("wildcard_mention_notify", False),
# Since one is by definition idle, we don't need to check online_push_enabled
online_push_enabled=False,
)
assert "flags" in event
mentioned = internal_data.get("mentioned", False)
private_message = event["message"]["type"] == "private" private_message = event["message"]["type"] == "private"
# stream_push_notify is set in process_message_event.
stream_push_notify = internal_data.get("stream_push_notify", False)
stream_email_notify = internal_data.get("stream_email_notify", False)
wildcard_mention_notify = internal_data.get("wildcard_mention_notify", False)
stream_name = None stream_name = None
if not private_message: if not private_message:
stream_name = event["message"]["display_recipient"] stream_name = event["message"]["display_recipient"]
# Since one is by definition idle, we don't need to check online_push_enabled
online_push_enabled = False
# Since we just GC'd the last event queue, the user is definitely idle. # Since we just GC'd the last event queue, the user is definitely idle.
idle = True idle = True
@@ -767,15 +767,11 @@ def missedmessage_hook(
email_notified=internal_data.get("email_notified", False), email_notified=internal_data.get("email_notified", False),
) )
maybe_enqueue_notifications( maybe_enqueue_notifications(
user_profile_id=user_profile_id, user_data=user_notifications_data,
sender_id=sender_id,
message_id=message_id, message_id=message_id,
private_message=private_message, private_message=private_message,
mentioned=mentioned,
wildcard_mention_notify=wildcard_mention_notify,
stream_push_notify=stream_push_notify,
stream_email_notify=stream_email_notify,
stream_name=stream_name, stream_name=stream_name,
online_push_enabled=online_push_enabled,
idle=idle, idle=idle,
already_notified=already_notified, already_notified=already_notified,
) )
@@ -794,15 +790,11 @@ def receiver_is_off_zulip(user_profile_id: int) -> bool:
def maybe_enqueue_notifications( def maybe_enqueue_notifications(
*, *,
user_profile_id: int, user_data: UserMessageNotificationsData,
sender_id: int,
message_id: int, message_id: int,
private_message: bool, private_message: bool,
mentioned: bool,
wildcard_mention_notify: bool,
stream_push_notify: bool,
stream_email_notify: bool,
stream_name: Optional[str], stream_name: Optional[str],
online_push_enabled: bool,
idle: bool, idle: bool,
already_notified: Dict[str, bool], already_notified: Dict[str, bool],
) -> Dict[str, bool]: ) -> Dict[str, bool]:
@@ -815,17 +807,15 @@ def maybe_enqueue_notifications(
""" """
notified: Dict[str, bool] = {} notified: Dict[str, bool] = {}
if (idle or online_push_enabled) and ( if user_data.is_push_notifiable(private_message, sender_id, idle):
private_message or mentioned or wildcard_mention_notify or stream_push_notify notice = build_offline_notification(user_data.user_id, message_id)
):
notice = build_offline_notification(user_profile_id, message_id)
if private_message: if private_message:
notice["trigger"] = "private_message" notice["trigger"] = "private_message"
elif mentioned: elif user_data.mentioned:
notice["trigger"] = "mentioned" notice["trigger"] = "mentioned"
elif wildcard_mention_notify: elif user_data.wildcard_mention_notify:
notice["trigger"] = "wildcard_mentioned" notice["trigger"] = "wildcard_mentioned"
elif stream_push_notify: elif user_data.stream_push_notify:
notice["trigger"] = "stream_push_notify" notice["trigger"] = "stream_push_notify"
else: else:
raise AssertionError("Unknown notification trigger!") raise AssertionError("Unknown notification trigger!")
@@ -838,15 +828,15 @@ def maybe_enqueue_notifications(
# mention. Eventually, we'll add settings to allow email # mention. Eventually, we'll add settings to allow email
# notifications to match the model of push notifications # notifications to match the model of push notifications
# above. # above.
if idle and (private_message or mentioned or wildcard_mention_notify or stream_email_notify): if user_data.is_email_notifiable(private_message, sender_id, idle):
notice = build_offline_notification(user_profile_id, message_id) notice = build_offline_notification(user_data.user_id, message_id)
if private_message: if private_message:
notice["trigger"] = "private_message" notice["trigger"] = "private_message"
elif mentioned: elif user_data.mentioned:
notice["trigger"] = "mentioned" notice["trigger"] = "mentioned"
elif wildcard_mention_notify: elif user_data.wildcard_mention_notify:
notice["trigger"] = "wildcard_mentioned" notice["trigger"] = "wildcard_mentioned"
elif stream_email_notify: elif user_data.stream_email_notify:
notice["trigger"] = "stream_email_notify" notice["trigger"] = "stream_email_notify"
else: else:
raise AssertionError("Unknown notification trigger!") raise AssertionError("Unknown notification trigger!")
@@ -966,8 +956,8 @@ def process_message_event(
muted_sender_user_ids=muted_sender_user_ids, muted_sender_user_ids=muted_sender_user_ids,
) )
# Remove fields sent through other pipes to save some space.
internal_data = asdict(user_notifications_data) internal_data = asdict(user_notifications_data)
# Remove fields sent through other pipes to save some space.
internal_data.pop("flags") internal_data.pop("flags")
internal_data.pop("user_id") internal_data.pop("user_id")
extra_user_data[user_profile_id] = dict(internal_data=internal_data) extra_user_data[user_profile_id] = dict(internal_data=internal_data)
@@ -985,15 +975,11 @@ def process_message_event(
extra_user_data[user_profile_id]["internal_data"].update( extra_user_data[user_profile_id]["internal_data"].update(
maybe_enqueue_notifications( maybe_enqueue_notifications(
user_profile_id=user_profile_id, user_data=user_notifications_data,
sender_id=sender_id,
message_id=message_id, message_id=message_id,
private_message=private_message, private_message=private_message,
mentioned=user_notifications_data.mentioned,
wildcard_mention_notify=user_notifications_data.wildcard_mention_notify,
stream_push_notify=user_notifications_data.stream_push_notify,
stream_email_notify=user_notifications_data.stream_email_notify,
stream_name=stream_name, stream_name=stream_name,
online_push_enabled=user_notifications_data.online_push_enabled,
idle=idle, idle=idle,
already_notified={}, already_notified={},
) )
@@ -1126,6 +1112,25 @@ def process_message_update_event(
for user_data in users: for user_data in users:
user_profile_id = user_data["id"] user_profile_id = user_data["id"]
if "user_id" in event_template:
# This is inaccurate: the user we'll get here will be the
# sender if the message's content was edited, which is
# typically where we might send new notifications.
# However, for topic/stream edits, it could be another
# user. We may need to adjust the format for
# update_message events to address this issue.
message_sender_id = event_template["user_id"]
else:
# This is also inaccurate, but usefully so. Events
# without a `user_id` field come from the
# do_update_embedded_data code path, and represent not an
# update to the raw content, but instead just rendering
# previews. Setting the current user at the sender is a
# hack to simplify notifications logic for this code
# path. TODO: Change this to short-circuit more directly.
message_sender_id = user_profile_id
user_event = dict(event_template) # shallow copy, but deep enough for our needs user_event = dict(event_template) # shallow copy, but deep enough for our needs
for key in user_data.keys(): for key in user_data.keys():
if key != "id": if key != "id":
@@ -1145,6 +1150,7 @@ def process_message_update_event(
maybe_enqueue_notifications_for_message_update( maybe_enqueue_notifications_for_message_update(
user_data=user_notifications_data, user_data=user_notifications_data,
message_id=message_id, message_id=message_id,
sender_id=message_sender_id,
private_message=(stream_name is None), private_message=(stream_name is None),
stream_name=stream_name, stream_name=stream_name,
presence_idle=(user_profile_id in presence_idle_user_ids), presence_idle=(user_profile_id in presence_idle_user_ids),
@@ -1161,6 +1167,7 @@ def process_message_update_event(
def maybe_enqueue_notifications_for_message_update( def maybe_enqueue_notifications_for_message_update(
user_data: UserMessageNotificationsData, user_data: UserMessageNotificationsData,
message_id: int, message_id: int,
sender_id: int,
private_message: bool, private_message: bool,
stream_name: Optional[str], stream_name: Optional[str],
presence_idle: bool, presence_idle: bool,
@@ -1202,15 +1209,11 @@ def maybe_enqueue_notifications_for_message_update(
idle = presence_idle or receiver_is_off_zulip(user_data.user_id) idle = presence_idle or receiver_is_off_zulip(user_data.user_id)
maybe_enqueue_notifications( maybe_enqueue_notifications(
user_profile_id=user_data.user_id, user_data=user_data,
message_id=message_id, message_id=message_id,
sender_id=sender_id,
private_message=private_message, private_message=private_message,
mentioned=user_data.mentioned,
wildcard_mention_notify=user_data.wildcard_mention_notify,
stream_push_notify=user_data.stream_push_notify,
stream_email_notify=user_data.stream_email_notify,
stream_name=stream_name, stream_name=stream_name,
online_push_enabled=user_data.online_push_enabled,
idle=idle, idle=idle,
already_notified={}, already_notified={},
) )