From dedc39f139f7909f74cce8fc21019a3dff7870e7 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Bodas Date: Wed, 23 Jun 2021 14:14:34 +0530 Subject: [PATCH] notifications_data: Rename `id` -> `user_id`. We also make this a mandatory named argument for our test helper for clarity. --- zerver/lib/actions.py | 2 +- zerver/lib/notification_data.py | 8 ++--- zerver/lib/test_classes.py | 6 ++-- zerver/tests/test_messages.py | 4 +-- zerver/tests/test_notification_data.py | 47 +++++++++++++++++--------- zerver/tornado/event_queue.py | 6 ++-- 6 files changed, 45 insertions(+), 28 deletions(-) diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index 904f3afd4e..8b36fa5e33 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -6612,7 +6612,7 @@ def get_active_presence_idle_user_ids( # for them if they were indeed idle. Only including those users in the calculation below is a # very important optimization for open communities with many inactive users. if user_data.is_notifiable(is_private_message, sender_id, idle=True) or alerted: - user_ids.add(user_data.id) + user_ids.add(user_data.user_id) return filter_presence_idle_user_ids(user_ids) diff --git a/zerver/lib/notification_data.py b/zerver/lib/notification_data.py index 6a9ee78b67..9fbc1a715d 100644 --- a/zerver/lib/notification_data.py +++ b/zerver/lib/notification_data.py @@ -4,7 +4,7 @@ from typing import Collection, Set @dataclass class UserMessageNotificationsData: - id: int + user_id: int flags: Collection[str] mentioned: bool online_push_enabled: bool @@ -34,7 +34,7 @@ class UserMessageNotificationsData: user_id in wildcard_mention_user_ids and "wildcard_mentioned" in flags ) return cls( - id=user_id, + user_id=user_id, flags=flags, mentioned=("mentioned" in flags), online_push_enabled=(user_id in online_push_user_ids), @@ -57,7 +57,7 @@ class UserMessageNotificationsData: if not idle and not self.online_push_enabled: return False - if self.id == sender_id: + if self.user_id == sender_id: return False if self.sender_is_muted: @@ -74,7 +74,7 @@ class UserMessageNotificationsData: if not idle: return False - if self.id == sender_id: + if self.user_id == sender_id: return False if self.sender_is_muted: diff --git a/zerver/lib/test_classes.py b/zerver/lib/test_classes.py index f455478ef4..80a5dbd383 100644 --- a/zerver/lib/test_classes.py +++ b/zerver/lib/test_classes.py @@ -1323,9 +1323,11 @@ Output: self.assert_length(lst, expected_num_events) - def create_user_notifications_data_object(self, **kwargs: Any) -> UserMessageNotificationsData: + def create_user_notifications_data_object( + self, *, user_id: int, **kwargs: Any + ) -> UserMessageNotificationsData: return UserMessageNotificationsData( - id=kwargs.get("id", self.example_user("hamlet").id), + user_id=user_id, flags=kwargs.get("flags", []), mentioned=kwargs.get("mentioned", False), online_push_enabled=kwargs.get("online_push_enabled", False), diff --git a/zerver/tests/test_messages.py b/zerver/tests/test_messages.py index 7e11f4b65e..b63186556a 100644 --- a/zerver/tests/test_messages.py +++ b/zerver/tests/test_messages.py @@ -23,8 +23,8 @@ class MissedMessageTest(ZulipTestCase): hamlet = self.example_user("hamlet") othello = self.example_user("othello") user_data_objects = [ - self.create_user_notifications_data_object(id=hamlet.id), - self.create_user_notifications_data_object(id=othello.id), + self.create_user_notifications_data_object(user_id=hamlet.id), + self.create_user_notifications_data_object(user_id=othello.id), ] message_type = "private" diff --git a/zerver/tests/test_notification_data.py b/zerver/tests/test_notification_data.py index 8cbb4ec41d..3956175e2c 100644 --- a/zerver/tests/test_notification_data.py +++ b/zerver/tests/test_notification_data.py @@ -3,46 +3,53 @@ from zerver.lib.test_classes import ZulipTestCase class TestNotificationData(ZulipTestCase): def test_is_push_notifiable(self) -> None: + user_id = self.example_user("hamlet").id sender_id = self.example_user("cordelia").id # Boring case - user_data = self.create_user_notifications_data_object() + user_data = self.create_user_notifications_data_object(user_id=user_id) self.assertFalse( user_data.is_push_notifiable(private_message=False, sender_id=sender_id, idle=True) ) # Notifiable cases for PMs, mentions, stream notifications - user_data = self.create_user_notifications_data_object() + user_data = self.create_user_notifications_data_object(user_id=user_id) self.assertTrue( user_data.is_push_notifiable(private_message=True, sender_id=sender_id, idle=True) ) - user_data = self.create_user_notifications_data_object(flags=["mentioned"], mentioned=True) + user_data = self.create_user_notifications_data_object( + user_id=user_id, flags=["mentioned"], mentioned=True + ) self.assertTrue( user_data.is_push_notifiable(private_message=False, sender_id=sender_id, idle=True) ) user_data = self.create_user_notifications_data_object( - flags=["wildcard_mentioned"], wildcard_mention_notify=True + user_id=user_id, flags=["wildcard_mentioned"], wildcard_mention_notify=True ) self.assertTrue( user_data.is_push_notifiable(private_message=False, sender_id=sender_id, idle=True) ) - user_data = self.create_user_notifications_data_object(stream_push_notify=True) + user_data = self.create_user_notifications_data_object( + user_id=user_id, stream_push_notify=True + ) self.assertTrue( user_data.is_push_notifiable(private_message=False, sender_id=sender_id, idle=True) ) # Now, test the `online_push_enabled` property # Test no notifications when not idle - user_data = self.create_user_notifications_data_object() + user_data = self.create_user_notifications_data_object(user_id=user_id) self.assertFalse( user_data.is_push_notifiable(private_message=True, sender_id=sender_id, idle=False) ) # Test notifications are sent when not idle but `online_push_enabled = True` - user_data = self.create_user_notifications_data_object(online_push_enabled=True) + user_data = self.create_user_notifications_data_object( + user_id=user_id, online_push_enabled=True + ) self.assertTrue( user_data.is_push_notifiable(private_message=True, sender_id=sender_id, idle=False) ) @@ -51,6 +58,7 @@ class TestNotificationData(ZulipTestCase): # We just want to test the early (False) return patterns in these special cases: # Message sender is muted. user_data = self.create_user_notifications_data_object( + user_id=user_id, sender_is_muted=True, flags=["mentioned", "wildcard_mentioned"], wildcard_mention_notify=True, @@ -64,7 +72,7 @@ class TestNotificationData(ZulipTestCase): # Message sender is the user the object corresponds to. user_data = self.create_user_notifications_data_object( - id=sender_id, + user_id=sender_id, sender_is_muted=False, flags=["mentioned", "wildcard_mentioned"], wildcard_mention_notify=True, @@ -77,39 +85,44 @@ class TestNotificationData(ZulipTestCase): ) def test_is_email_notifiable(self) -> None: + user_id = self.example_user("hamlet").id sender_id = self.example_user("cordelia").id # Boring case - user_data = self.create_user_notifications_data_object() + user_data = self.create_user_notifications_data_object(user_id=user_id) self.assertFalse( user_data.is_email_notifiable(private_message=False, sender_id=sender_id, idle=True) ) # Notifiable cases for PMs, mentions, stream notifications - user_data = self.create_user_notifications_data_object() + user_data = self.create_user_notifications_data_object(user_id=user_id) self.assertTrue( user_data.is_email_notifiable(private_message=True, sender_id=sender_id, idle=True) ) - user_data = self.create_user_notifications_data_object(flags=["mentioned"], mentioned=True) + user_data = self.create_user_notifications_data_object( + user_id=user_id, flags=["mentioned"], mentioned=True + ) self.assertTrue( user_data.is_email_notifiable(private_message=False, sender_id=sender_id, idle=True) ) user_data = self.create_user_notifications_data_object( - flags=["wildcard_mentioned"], wildcard_mention_notify=True + user_id=user_id, flags=["wildcard_mentioned"], wildcard_mention_notify=True ) self.assertTrue( user_data.is_email_notifiable(private_message=False, sender_id=sender_id, idle=True) ) - user_data = self.create_user_notifications_data_object(stream_email_notify=True) + user_data = self.create_user_notifications_data_object( + user_id=user_id, stream_email_notify=True + ) self.assertTrue( user_data.is_email_notifiable(private_message=False, sender_id=sender_id, idle=True) ) # Test no notifications when not idle - user_data = self.create_user_notifications_data_object() + user_data = self.create_user_notifications_data_object(user_id=user_id) self.assertFalse( user_data.is_email_notifiable(private_message=True, sender_id=sender_id, idle=False) ) @@ -118,6 +131,7 @@ class TestNotificationData(ZulipTestCase): # We just want to test the early (False) return patterns in these special cases: # Message sender is muted. user_data = self.create_user_notifications_data_object( + user_id=user_id, sender_is_muted=True, flags=["mentioned", "wildcard_mentioned"], wildcard_mention_notify=True, @@ -131,7 +145,7 @@ class TestNotificationData(ZulipTestCase): # Message sender is the user the object corresponds to. user_data = self.create_user_notifications_data_object( - id=sender_id, + user_id=sender_id, sender_is_muted=False, flags=["mentioned", "wildcard_mentioned"], wildcard_mention_notify=True, @@ -146,8 +160,9 @@ class TestNotificationData(ZulipTestCase): def test_is_notifiable(self) -> None: # This is just for coverage purposes. We've already tested all scenarios above, # and `is_notifiable` is a simple OR of the email and push functions. + user_id = self.example_user("hamlet").id sender_id = self.example_user("cordelia").id - user_data = self.create_user_notifications_data_object() + user_data = self.create_user_notifications_data_object(user_id=user_id) self.assertTrue( user_data.is_notifiable(private_message=True, sender_id=sender_id, idle=True) ) diff --git a/zerver/tornado/event_queue.py b/zerver/tornado/event_queue.py index 4de7682ab8..2c86296d44 100644 --- a/zerver/tornado/event_queue.py +++ b/zerver/tornado/event_queue.py @@ -969,7 +969,7 @@ def process_message_event( # Remove fields sent through other pipes to save some space. internal_data = asdict(user_notifications_data) internal_data.pop("flags") - internal_data.pop("id") + internal_data.pop("user_id") extra_user_data[user_profile_id] = dict(internal_data=internal_data) # If the message isn't notifiable had the user been idle, then the user @@ -1199,10 +1199,10 @@ def maybe_enqueue_notifications_for_message_update( # model. return - idle = presence_idle or receiver_is_off_zulip(user_data.id) + idle = presence_idle or receiver_is_off_zulip(user_data.user_id) maybe_enqueue_notifications( - user_profile_id=user_data.id, + user_profile_id=user_data.user_id, message_id=message_id, private_message=private_message, mentioned=user_data.mentioned,