notifications_data: Rename `id` -> `user_id`.

We also make this a mandatory named argument for our test helper
for clarity.
This commit is contained in:
Abhijeet Prasad Bodas 2021-06-23 14:14:34 +05:30
parent f6e705d477
commit dedc39f139
6 changed files with 45 additions and 28 deletions

View File

@ -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)

View File

@ -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:

View File

@ -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),

View File

@ -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"

View File

@ -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)
)

View File

@ -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,