mirror of
https://github.com/zulip/zulip.git
synced 2026-06-24 21:08:25 +08:00
notifications: Calculate PMs/mentions settings like other settings.
Previously, we checked for the `enable_offline_email_notifications` and
`enable_offline_push_notifications` settings (which determine whether the
user will receive notifications for PMs and mentions) just before sending
notifications. This has a few problem:
1. We do not have access to all the user settings in the notification
handlers (`handle_missedmessage_emails` and `handle_push_notifications`),
and therefore, we cannot correctly determine whether the notification should
be sent. Checks like the following which existed previously, will, for
example, incorrectly not send notifications even when stream email
notifications are enabled-
```
if not receives_offline_email_notifications(user_profile):
return
```
With this commit, we simply do not enqueue notifications if the "offline"
settings are disabled, which fixes that bug.
Additionally, this also fixes a bug with the "online push notifications"
feature, which was, if someone were to:
* turn off notifications for PMs and mentions (`enable_offline_push_notifications`)
* turn on stream push notifications (`enable_stream_push_notifications`)
* turn on "online push" (`enable_online_push_notifications`)
then, they would still receive notifications for PMs when online.
This isn't how the "online push enabled" feature is supposed to work;
it should only act as a wrapper around the other notification settings.
The buggy code was this in `handle_push_notifications`:
```
if not (
receives_offline_push_notifications(user_profile)
or receives_online_push_notifications(user_profile)
):
return
// send notifications
```
This commit removes that code, and extends our `notification_data.py` logic
to cover this case, along with tests.
2. The name for these settings is slightly misleading. They essentially
talk about "what to send notifications for" (PMs and mentions), and not
"when to send notifications" (offline). This commit improves this condition
by restricting the use of this term only to the database field, and using
clearer names everywhere else. This distinction will be important to have
non-confusing code when we implement multiple options for notifications
in the future as dropdown (never/when offline/when offline or online, etc).
3. We should ideally re-check all notification settings just before the
notifications are sent. This is especially important for email notifications,
which may be sent after a long time after the message was sent. We will
in the future add code to thoroughly re-check settings before sending
notifications in a clean manner, but temporarily not re-checking isn't
a terrible scenario either.
This commit is contained in:
Abhijeet Prasad Bodas
Tim Abbott
parent
6eb88eb3ef
commit
de04f0ad67
@ -1456,6 +1456,8 @@ def render_incoming_message(
|
||||
class RecipientInfoResult(TypedDict):
|
||||
active_user_ids: Set[int]
|
||||
online_push_user_ids: Set[int]
|
||||
pm_mention_email_disabled_user_ids: Set[int]
|
||||
pm_mention_push_disabled_user_ids: Set[int]
|
||||
stream_email_user_ids: Set[int]
|
||||
stream_push_user_ids: Set[int]
|
||||
wildcard_mention_user_ids: Set[int]
|
||||
@ -1586,6 +1588,8 @@ def get_recipient_info(
|
||||
query = UserProfile.objects.filter(is_active=True).values(
|
||||
"id",
|
||||
"enable_online_push_notifications",
|
||||
"enable_offline_email_notifications",
|
||||
"enable_offline_push_notifications",
|
||||
"is_bot",
|
||||
"bot_type",
|
||||
"long_term_idle",
|
||||
@ -1628,6 +1632,16 @@ def get_recipient_info(
|
||||
lambda r: r["enable_online_push_notifications"],
|
||||
)
|
||||
|
||||
# We deal with only the users who have disabled this setting, since that
|
||||
# will ususally be much smaller a set than those who have enabled it (which
|
||||
# is the default)
|
||||
pm_mention_email_disabled_user_ids = get_ids_for(
|
||||
lambda r: not r["enable_offline_email_notifications"]
|
||||
)
|
||||
pm_mention_push_disabled_user_ids = get_ids_for(
|
||||
lambda r: not r["enable_offline_push_notifications"]
|
||||
)
|
||||
|
||||
# Service bots don't get UserMessage rows.
|
||||
um_eligible_user_ids = get_ids_for(
|
||||
lambda r: not is_service_bot(r),
|
||||
@ -1656,6 +1670,8 @@ def get_recipient_info(
|
||||
info: RecipientInfoResult = dict(
|
||||
active_user_ids=active_user_ids,
|
||||
online_push_user_ids=online_push_user_ids,
|
||||
pm_mention_email_disabled_user_ids=pm_mention_email_disabled_user_ids,
|
||||
pm_mention_push_disabled_user_ids=pm_mention_push_disabled_user_ids,
|
||||
stream_push_user_ids=stream_push_user_ids,
|
||||
stream_email_user_ids=stream_email_user_ids,
|
||||
wildcard_mention_user_ids=wildcard_mention_user_ids,
|
||||
@ -1859,6 +1875,8 @@ def build_message_send_dict(
|
||||
rendering_result=rendering_result,
|
||||
active_user_ids=info["active_user_ids"],
|
||||
online_push_user_ids=info["online_push_user_ids"],
|
||||
pm_mention_email_disabled_user_ids=info["pm_mention_email_disabled_user_ids"],
|
||||
pm_mention_push_disabled_user_ids=info["pm_mention_push_disabled_user_ids"],
|
||||
stream_push_user_ids=info["stream_push_user_ids"],
|
||||
stream_email_user_ids=info["stream_email_user_ids"],
|
||||
muted_sender_user_ids=info["muted_sender_user_ids"],
|
||||
@ -2012,14 +2030,20 @@ def do_send_messages(
|
||||
sender = send_request.message.sender
|
||||
message_type = wide_message_dict["type"]
|
||||
active_users_data = [
|
||||
UserMessageNotificationsData.from_user_id_sets(
|
||||
user_id=user_id,
|
||||
flags=user_flags.get(user_id, []),
|
||||
online_push_user_ids=send_request.online_push_user_ids,
|
||||
stream_push_user_ids=send_request.stream_push_user_ids,
|
||||
stream_email_user_ids=send_request.stream_email_user_ids,
|
||||
wildcard_mention_user_ids=send_request.wildcard_mention_user_ids,
|
||||
muted_sender_user_ids=send_request.muted_sender_user_ids,
|
||||
ActivePresenceIdleUserData(
|
||||
alerted="has_alert_word" in user_flags.get(user_id, []),
|
||||
notifications_data=UserMessageNotificationsData.from_user_id_sets(
|
||||
user_id=user_id,
|
||||
flags=user_flags.get(user_id, []),
|
||||
private_message=(message_type == "private"),
|
||||
online_push_user_ids=send_request.online_push_user_ids,
|
||||
pm_mention_push_disabled_user_ids=send_request.pm_mention_push_disabled_user_ids,
|
||||
pm_mention_email_disabled_user_ids=send_request.pm_mention_email_disabled_user_ids,
|
||||
stream_push_user_ids=send_request.stream_push_user_ids,
|
||||
stream_email_user_ids=send_request.stream_email_user_ids,
|
||||
wildcard_mention_user_ids=send_request.wildcard_mention_user_ids,
|
||||
muted_sender_user_ids=send_request.muted_sender_user_ids,
|
||||
),
|
||||
)
|
||||
for user_id in send_request.active_user_ids
|
||||
]
|
||||
@ -2027,7 +2051,6 @@ def do_send_messages(
|
||||
presence_idle_user_ids = get_active_presence_idle_user_ids(
|
||||
realm=sender.realm,
|
||||
sender_id=sender.id,
|
||||
message_type=message_type,
|
||||
active_users_data=active_users_data,
|
||||
)
|
||||
|
||||
@ -2037,6 +2060,10 @@ def do_send_messages(
|
||||
message_dict=wide_message_dict,
|
||||
presence_idle_user_ids=presence_idle_user_ids,
|
||||
online_push_user_ids=list(send_request.online_push_user_ids),
|
||||
pm_mention_push_disabled_user_ids=list(send_request.pm_mention_push_disabled_user_ids),
|
||||
pm_mention_email_disabled_user_ids=list(
|
||||
send_request.pm_mention_email_disabled_user_ids
|
||||
),
|
||||
stream_push_user_ids=list(send_request.stream_push_user_ids),
|
||||
stream_email_user_ids=list(send_request.stream_email_user_ids),
|
||||
wildcard_mention_user_ids=list(send_request.wildcard_mention_user_ids),
|
||||
@ -6057,6 +6084,10 @@ def do_update_message(
|
||||
)
|
||||
|
||||
event["online_push_user_ids"] = list(info["online_push_user_ids"])
|
||||
event["pm_mention_push_disabled_user_ids"] = list(info["pm_mention_push_disabled_user_ids"])
|
||||
event["pm_mention_email_disabled_user_ids"] = list(
|
||||
info["pm_mention_email_disabled_user_ids"]
|
||||
)
|
||||
event["stream_push_user_ids"] = list(info["stream_push_user_ids"])
|
||||
event["stream_email_user_ids"] = list(info["stream_email_user_ids"])
|
||||
event["muted_sender_user_ids"] = list(info["muted_sender_user_ids"])
|
||||
@ -6673,11 +6704,15 @@ def gather_subscriptions(
|
||||
return (subscribed, unsubscribed)
|
||||
|
||||
|
||||
class ActivePresenceIdleUserData(TypedDict):
|
||||
alerted: bool
|
||||
notifications_data: UserMessageNotificationsData
|
||||
|
||||
|
||||
def get_active_presence_idle_user_ids(
|
||||
realm: Realm,
|
||||
sender_id: int,
|
||||
message_type: str,
|
||||
active_users_data: List[UserMessageNotificationsData],
|
||||
active_users_data: List[ActivePresenceIdleUserData],
|
||||
) -> List[int]:
|
||||
"""
|
||||
Given a list of active_user_ids, we build up a subset
|
||||
@ -6691,17 +6726,16 @@ def get_active_presence_idle_user_ids(
|
||||
if realm.presence_disabled:
|
||||
return []
|
||||
|
||||
is_private_message = message_type == "private"
|
||||
|
||||
user_ids = set()
|
||||
for user_data in active_users_data:
|
||||
alerted = "has_alert_word" in user_data.flags
|
||||
user_notifications_data: UserMessageNotificationsData = user_data["notifications_data"]
|
||||
alerted = user_data["alerted"]
|
||||
|
||||
# We only need to know the presence idle state for a user if this message would be notifiable
|
||||
# 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.user_id)
|
||||
if user_notifications_data.is_notifiable(sender_id, idle=True) or alerted:
|
||||
user_ids.add(user_notifications_data.user_id)
|
||||
|
||||
return filter_presence_idle_user_ids(user_ids)
|
||||
|
||||
|
||||
@ -41,7 +41,6 @@ from zerver.models import (
|
||||
get_context_for_message,
|
||||
get_display_recipient,
|
||||
get_user_profile_by_id,
|
||||
receives_offline_email_notifications,
|
||||
)
|
||||
|
||||
|
||||
@ -585,8 +584,6 @@ def handle_missedmessage_emails(
|
||||
}
|
||||
|
||||
user_profile = get_user_profile_by_id(user_profile_id)
|
||||
if not receives_offline_email_notifications(user_profile):
|
||||
return
|
||||
|
||||
# Note: This query structure automatically filters out any
|
||||
# messages that were permanently deleted, since those would now be
|
||||
|
||||
@ -111,6 +111,8 @@ class SendMessageRequest:
|
||||
mentioned_user_groups_map: Dict[int, int]
|
||||
active_user_ids: Set[int]
|
||||
online_push_user_ids: Set[int]
|
||||
pm_mention_push_disabled_user_ids: Set[int]
|
||||
pm_mention_email_disabled_user_ids: Set[int]
|
||||
stream_push_user_ids: Set[int]
|
||||
stream_email_user_ids: Set[int]
|
||||
muted_sender_user_ids: Set[int]
|
||||
|
||||
@ -8,26 +8,34 @@ from zerver.models import NotificationTriggers
|
||||
@dataclass
|
||||
class UserMessageNotificationsData:
|
||||
user_id: int
|
||||
flags: Collection[str]
|
||||
mentioned: bool
|
||||
online_push_enabled: bool
|
||||
pm_email_notify: bool
|
||||
pm_push_notify: bool
|
||||
mention_email_notify: bool
|
||||
mention_push_notify: bool
|
||||
stream_push_notify: bool
|
||||
stream_email_notify: bool
|
||||
wildcard_mention_notify: bool
|
||||
sender_is_muted: bool
|
||||
|
||||
def __post_init__(self) -> None:
|
||||
if self.mentioned:
|
||||
assert "mentioned" in self.flags
|
||||
if self.wildcard_mention_notify:
|
||||
assert "wildcard_mentioned" in self.flags
|
||||
# Check that there's no dubious data.
|
||||
if self.pm_email_notify or self.pm_push_notify:
|
||||
assert not (self.stream_email_notify or self.stream_push_notify)
|
||||
|
||||
if self.stream_email_notify or self.stream_push_notify:
|
||||
assert not (self.pm_email_notify or self.pm_push_notify)
|
||||
|
||||
@classmethod
|
||||
def from_user_id_sets(
|
||||
cls,
|
||||
*,
|
||||
user_id: int,
|
||||
flags: Collection[str],
|
||||
private_message: bool,
|
||||
online_push_user_ids: Set[int],
|
||||
pm_mention_push_disabled_user_ids: Set[int],
|
||||
pm_mention_email_disabled_user_ids: Set[int],
|
||||
stream_push_user_ids: Set[int],
|
||||
stream_email_user_ids: Set[int],
|
||||
wildcard_mention_user_ids: Set[int],
|
||||
@ -36,10 +44,22 @@ class UserMessageNotificationsData:
|
||||
wildcard_mention_notify = (
|
||||
user_id in wildcard_mention_user_ids and "wildcard_mentioned" in flags
|
||||
)
|
||||
|
||||
pm_email_notify = user_id not in pm_mention_email_disabled_user_ids and private_message
|
||||
mention_email_notify = (
|
||||
user_id not in pm_mention_email_disabled_user_ids and "mentioned" in flags
|
||||
)
|
||||
|
||||
pm_push_notify = user_id not in pm_mention_push_disabled_user_ids and private_message
|
||||
mention_push_notify = (
|
||||
user_id not in pm_mention_push_disabled_user_ids and "mentioned" in flags
|
||||
)
|
||||
return cls(
|
||||
user_id=user_id,
|
||||
flags=flags,
|
||||
mentioned=("mentioned" in flags),
|
||||
pm_email_notify=pm_email_notify,
|
||||
mention_email_notify=mention_email_notify,
|
||||
pm_push_notify=pm_push_notify,
|
||||
mention_push_notify=mention_push_notify,
|
||||
online_push_enabled=(user_id in online_push_user_ids),
|
||||
stream_push_notify=(user_id in stream_push_user_ids),
|
||||
stream_email_notify=(user_id in stream_email_user_ids),
|
||||
@ -47,24 +67,18 @@ class UserMessageNotificationsData:
|
||||
sender_is_muted=(user_id in muted_sender_user_ids),
|
||||
)
|
||||
|
||||
# TODO: The following functions should also look at the `enable_offline_push_notifications` and
|
||||
# `enable_offline_email_notifications` settings (for PMs and mentions), but currently they
|
||||
# don't.
|
||||
|
||||
# For these functions, acting_user_id is the user sent a message
|
||||
# (or edited a message) triggering the event for which we need to
|
||||
# determine notifiability.
|
||||
def is_notifiable(self, private_message: bool, acting_user_id: int, idle: bool) -> bool:
|
||||
return self.is_email_notifiable(
|
||||
private_message, acting_user_id, idle
|
||||
) or self.is_push_notifiable(private_message, acting_user_id, idle)
|
||||
def is_notifiable(self, acting_user_id: int, idle: bool) -> bool:
|
||||
return self.is_email_notifiable(acting_user_id, idle) or self.is_push_notifiable(
|
||||
acting_user_id, idle
|
||||
)
|
||||
|
||||
def is_push_notifiable(self, private_message: bool, acting_user_id: int, idle: bool) -> bool:
|
||||
return self.get_push_notification_trigger(private_message, acting_user_id, idle) is not None
|
||||
def is_push_notifiable(self, acting_user_id: int, idle: bool) -> bool:
|
||||
return self.get_push_notification_trigger(acting_user_id, idle) is not None
|
||||
|
||||
def get_push_notification_trigger(
|
||||
self, private_message: bool, acting_user_id: int, idle: bool
|
||||
) -> Optional[str]:
|
||||
def get_push_notification_trigger(self, acting_user_id: int, idle: bool) -> Optional[str]:
|
||||
if not idle and not self.online_push_enabled:
|
||||
return None
|
||||
|
||||
@ -74,9 +88,12 @@ class UserMessageNotificationsData:
|
||||
if self.sender_is_muted:
|
||||
return None
|
||||
|
||||
if private_message:
|
||||
# The order here is important. If, for example, both
|
||||
# `mention_push_notify` and `stream_push_notify` are True, we
|
||||
# want to classify it as a mention, since that's more salient.
|
||||
if self.pm_push_notify:
|
||||
return NotificationTriggers.PRIVATE_MESSAGE
|
||||
elif self.mentioned:
|
||||
elif self.mention_push_notify:
|
||||
return NotificationTriggers.MENTION
|
||||
elif self.wildcard_mention_notify:
|
||||
return NotificationTriggers.WILDCARD_MENTION
|
||||
@ -85,14 +102,10 @@ class UserMessageNotificationsData:
|
||||
else:
|
||||
return None
|
||||
|
||||
def is_email_notifiable(self, private_message: bool, acting_user_id: int, idle: bool) -> bool:
|
||||
return (
|
||||
self.get_email_notification_trigger(private_message, acting_user_id, idle) is not None
|
||||
)
|
||||
def is_email_notifiable(self, acting_user_id: int, idle: bool) -> bool:
|
||||
return self.get_email_notification_trigger(acting_user_id, idle) is not None
|
||||
|
||||
def get_email_notification_trigger(
|
||||
self, private_message: bool, acting_user_id: int, idle: bool
|
||||
) -> Optional[str]:
|
||||
def get_email_notification_trigger(self, acting_user_id: int, idle: bool) -> Optional[str]:
|
||||
if not idle:
|
||||
return None
|
||||
|
||||
@ -102,9 +115,12 @@ class UserMessageNotificationsData:
|
||||
if self.sender_is_muted:
|
||||
return None
|
||||
|
||||
if private_message:
|
||||
# The order here is important. If, for example, both
|
||||
# `mention_email_notify` and `stream_email_notify` are True, we
|
||||
# want to classify it as a mention, since that's more salient.
|
||||
if self.pm_email_notify:
|
||||
return NotificationTriggers.PRIVATE_MESSAGE
|
||||
elif self.mentioned:
|
||||
elif self.mention_email_notify:
|
||||
return NotificationTriggers.MENTION
|
||||
elif self.wildcard_mention_notify:
|
||||
return NotificationTriggers.WILDCARD_MENTION
|
||||
|
||||
@ -35,8 +35,6 @@ from zerver.models import (
|
||||
UserProfile,
|
||||
get_display_recipient,
|
||||
get_user_profile_by_id,
|
||||
receives_offline_push_notifications,
|
||||
receives_online_push_notifications,
|
||||
)
|
||||
|
||||
if TYPE_CHECKING:
|
||||
@ -903,11 +901,6 @@ def handle_push_notification(user_profile_id: int, missed_message: Dict[str, Any
|
||||
if not push_notifications_enabled():
|
||||
return
|
||||
user_profile = get_user_profile_by_id(user_profile_id)
|
||||
if not (
|
||||
receives_offline_push_notifications(user_profile)
|
||||
or receives_online_push_notifications(user_profile)
|
||||
):
|
||||
return
|
||||
|
||||
try:
|
||||
(message, user_message) = access_message(user_profile, missed_message["message_id"])
|
||||
|
||||
@ -1339,9 +1339,11 @@ Output:
|
||||
) -> UserMessageNotificationsData:
|
||||
return UserMessageNotificationsData(
|
||||
user_id=user_id,
|
||||
flags=kwargs.get("flags", []),
|
||||
mentioned=kwargs.get("mentioned", False),
|
||||
online_push_enabled=kwargs.get("online_push_enabled", False),
|
||||
pm_email_notify=kwargs.get("pm_email_notify", False),
|
||||
pm_push_notify=kwargs.get("pm_push_notify", False),
|
||||
mention_email_notify=kwargs.get("mention_email_notify", False),
|
||||
mention_push_notify=kwargs.get("mention_push_notify", False),
|
||||
stream_email_notify=kwargs.get("stream_email_notify", False),
|
||||
stream_push_notify=kwargs.get("stream_push_notify", False),
|
||||
wildcard_mention_notify=kwargs.get("wildcard_mention_notify", False),
|
||||
@ -1363,7 +1365,6 @@ Output:
|
||||
user_notifications_data=user_notifications_data,
|
||||
message_id=message_id,
|
||||
acting_user_id=acting_user_id,
|
||||
private_message=kwargs.get("private_message", False),
|
||||
mentioned_user_group_id=kwargs.get("mentioned_user_group_id", None),
|
||||
idle=kwargs.get("idle", True),
|
||||
already_notified=kwargs.get(
|
||||
|
||||
@ -1854,18 +1854,6 @@ class UserGroupMembership(models.Model):
|
||||
unique_together = (("user_group", "user_profile"),)
|
||||
|
||||
|
||||
def receives_offline_push_notifications(user_profile: UserProfile) -> bool:
|
||||
return user_profile.enable_offline_push_notifications and not user_profile.is_bot
|
||||
|
||||
|
||||
def receives_offline_email_notifications(user_profile: UserProfile) -> bool:
|
||||
return user_profile.enable_offline_email_notifications and not user_profile.is_bot
|
||||
|
||||
|
||||
def receives_online_push_notifications(user_profile: UserProfile) -> bool:
|
||||
return user_profile.enable_online_push_notifications and not user_profile.is_bot
|
||||
|
||||
|
||||
def remote_user_to_email(remote_user: str) -> str:
|
||||
if settings.SSO_APPEND_DOMAIN is not None:
|
||||
remote_user += "@" + settings.SSO_APPEND_DOMAIN
|
||||
|
||||
@ -23,14 +23,7 @@ from zerver.lib.email_notifications import (
|
||||
from zerver.lib.send_email import FromAddress, send_custom_email
|
||||
from zerver.lib.test_classes import ZulipTestCase
|
||||
from zerver.lib.user_groups import create_user_group
|
||||
from zerver.models import (
|
||||
ScheduledEmail,
|
||||
UserMessage,
|
||||
UserProfile,
|
||||
get_realm,
|
||||
get_stream,
|
||||
receives_offline_email_notifications,
|
||||
)
|
||||
from zerver.models import ScheduledEmail, UserMessage, UserProfile, get_realm, get_stream
|
||||
|
||||
|
||||
class TestCustomEmails(ZulipTestCase):
|
||||
@ -1361,37 +1354,3 @@ class TestMissedMessages(ZulipTestCase):
|
||||
+ 'title="cloud with lightning and rain" style="height: 20px;">.</p>'
|
||||
)
|
||||
self.assertEqual(actual_output, expected_output)
|
||||
|
||||
|
||||
class TestReceivesNotificationsFunctions(ZulipTestCase):
|
||||
def test_receivers_offline_notifications_when_user_is_a_bot(self) -> None:
|
||||
hamlet = self.example_user("hamlet")
|
||||
hamlet.is_bot = True
|
||||
|
||||
hamlet.enable_offline_email_notifications = True
|
||||
self.assertFalse(receives_offline_email_notifications(hamlet))
|
||||
|
||||
hamlet.enable_offline_email_notifications = False
|
||||
self.assertFalse(receives_offline_email_notifications(hamlet))
|
||||
|
||||
hamlet.enable_offline_email_notifications = True
|
||||
self.assertFalse(receives_offline_email_notifications(hamlet))
|
||||
|
||||
hamlet.enable_offline_email_notifications = False
|
||||
self.assertFalse(receives_offline_email_notifications(hamlet))
|
||||
|
||||
def test_receivers_offline_notifications_when_user_is_not_a_bot(self) -> None:
|
||||
hamlet = self.example_user("hamlet")
|
||||
hamlet.is_bot = False
|
||||
|
||||
hamlet.enable_offline_email_notifications = True
|
||||
self.assertTrue(receives_offline_email_notifications(hamlet))
|
||||
|
||||
hamlet.enable_offline_email_notifications = False
|
||||
self.assertFalse(receives_offline_email_notifications(hamlet))
|
||||
|
||||
hamlet.enable_offline_email_notifications = True
|
||||
self.assertTrue(receives_offline_email_notifications(hamlet))
|
||||
|
||||
hamlet.enable_offline_email_notifications = False
|
||||
self.assertFalse(receives_offline_email_notifications(hamlet))
|
||||
|
||||
@ -44,7 +44,9 @@ class MissedMessageNotificationsTest(ZulipTestCase):
|
||||
with mock_queue_publish(
|
||||
"zerver.tornado.event_queue.queue_json_publish"
|
||||
) as mock_queue_json_publish:
|
||||
params["private_message"] = True
|
||||
params["user_notifications_data"] = self.create_user_notifications_data_object(
|
||||
user_id=1, pm_push_notify=True, pm_email_notify=True
|
||||
)
|
||||
notified = maybe_enqueue_notifications(**params)
|
||||
self.assertTrue(mock_queue_json_publish.call_count, 2)
|
||||
|
||||
@ -62,9 +64,8 @@ class MissedMessageNotificationsTest(ZulipTestCase):
|
||||
message_id=1,
|
||||
acting_user_id=2,
|
||||
user_id=3,
|
||||
private_message=False,
|
||||
flags=["mentioned"],
|
||||
mentioned=True,
|
||||
mention_push_notify=True,
|
||||
mention_email_notify=True,
|
||||
mentioned_user_group_id=33,
|
||||
)
|
||||
notified = maybe_enqueue_notifications(**params)
|
||||
@ -225,11 +226,35 @@ class MissedMessageNotificationsTest(ZulipTestCase):
|
||||
assert_maybe_enqueue_notifications_call_args(
|
||||
args_dict=args_dict,
|
||||
message_id=msg_id,
|
||||
private_message=True,
|
||||
pm_email_notify=True,
|
||||
pm_push_notify=True,
|
||||
already_notified={"email_notified": True, "push_notified": True},
|
||||
)
|
||||
destroy_event_queue(client_descriptor.event_queue.id)
|
||||
|
||||
# If `enable_offline_email_notifications` is disabled, email otifications shouldn't
|
||||
# be sent even for PMs
|
||||
user_profile.enable_offline_email_notifications = False
|
||||
user_profile.save()
|
||||
client_descriptor = allocate_event_queue()
|
||||
self.assertTrue(client_descriptor.event_queue.empty())
|
||||
msg_id = self.send_personal_message(iago, user_profile)
|
||||
with mock.patch("zerver.tornado.event_queue.maybe_enqueue_notifications") as mock_enqueue:
|
||||
missedmessage_hook(user_profile.id, client_descriptor, True)
|
||||
mock_enqueue.assert_called_once()
|
||||
args_dict = mock_enqueue.call_args_list[0][1]
|
||||
|
||||
assert_maybe_enqueue_notifications_call_args(
|
||||
args_dict=args_dict,
|
||||
message_id=msg_id,
|
||||
pm_email_notify=False,
|
||||
pm_push_notify=True,
|
||||
already_notified={"email_notified": False, "push_notified": True},
|
||||
)
|
||||
destroy_event_queue(client_descriptor.event_queue.id)
|
||||
user_profile.enable_offline_email_notifications = True
|
||||
user_profile.save()
|
||||
|
||||
# Test the hook with a mention; this should trigger notifications
|
||||
client_descriptor = allocate_event_queue()
|
||||
self.assertTrue(client_descriptor.event_queue.empty())
|
||||
@ -244,12 +269,35 @@ class MissedMessageNotificationsTest(ZulipTestCase):
|
||||
assert_maybe_enqueue_notifications_call_args(
|
||||
args_dict=args_dict,
|
||||
message_id=msg_id,
|
||||
flags=["mentioned"],
|
||||
mentioned=True,
|
||||
mention_push_notify=True,
|
||||
mention_email_notify=True,
|
||||
already_notified={"email_notified": True, "push_notified": True},
|
||||
)
|
||||
destroy_event_queue(client_descriptor.event_queue.id)
|
||||
|
||||
# If `enable_offline_push_notifications` is disabled, push otifications shouldn't
|
||||
# be sent even for mentions
|
||||
user_profile.enable_offline_push_notifications = False
|
||||
user_profile.save()
|
||||
client_descriptor = allocate_event_queue()
|
||||
self.assertTrue(client_descriptor.event_queue.empty())
|
||||
msg_id = self.send_personal_message(iago, user_profile)
|
||||
with mock.patch("zerver.tornado.event_queue.maybe_enqueue_notifications") as mock_enqueue:
|
||||
missedmessage_hook(user_profile.id, client_descriptor, True)
|
||||
mock_enqueue.assert_called_once()
|
||||
args_dict = mock_enqueue.call_args_list[0][1]
|
||||
|
||||
assert_maybe_enqueue_notifications_call_args(
|
||||
args_dict=args_dict,
|
||||
message_id=msg_id,
|
||||
pm_email_notify=True,
|
||||
pm_push_notify=False,
|
||||
already_notified={"email_notified": True, "push_notified": False},
|
||||
)
|
||||
destroy_event_queue(client_descriptor.event_queue.id)
|
||||
user_profile.enable_offline_push_notifications = True
|
||||
user_profile.save()
|
||||
|
||||
# Test the hook with a wildcard mention; this should trigger notifications
|
||||
client_descriptor = allocate_event_queue()
|
||||
self.assertTrue(client_descriptor.event_queue.empty())
|
||||
@ -262,7 +310,6 @@ class MissedMessageNotificationsTest(ZulipTestCase):
|
||||
assert_maybe_enqueue_notifications_call_args(
|
||||
args_dict=args_dict,
|
||||
message_id=msg_id,
|
||||
flags=["wildcard_mentioned"],
|
||||
wildcard_mention_notify=True,
|
||||
already_notified={"email_notified": True, "push_notified": True},
|
||||
)
|
||||
@ -280,7 +327,7 @@ class MissedMessageNotificationsTest(ZulipTestCase):
|
||||
|
||||
assert_maybe_enqueue_notifications_call_args(
|
||||
args_dict=args_dict,
|
||||
flags=["wildcard_mentioned"],
|
||||
wildcard_mention_notify=False,
|
||||
message_id=msg_id,
|
||||
already_notified={"email_notified": False, "push_notified": False},
|
||||
)
|
||||
@ -301,7 +348,7 @@ class MissedMessageNotificationsTest(ZulipTestCase):
|
||||
assert_maybe_enqueue_notifications_call_args(
|
||||
args_dict=args_dict,
|
||||
message_id=msg_id,
|
||||
flags=["wildcard_mentioned"],
|
||||
wildcard_mention_notify=False,
|
||||
already_notified={"email_notified": False, "push_notified": False},
|
||||
)
|
||||
destroy_event_queue(client_descriptor.event_queue.id)
|
||||
@ -325,7 +372,6 @@ class MissedMessageNotificationsTest(ZulipTestCase):
|
||||
assert_maybe_enqueue_notifications_call_args(
|
||||
args_dict=args_dict,
|
||||
message_id=msg_id,
|
||||
flags=["wildcard_mentioned"],
|
||||
wildcard_mention_notify=True,
|
||||
already_notified={"email_notified": True, "push_notified": True},
|
||||
)
|
||||
@ -352,8 +398,8 @@ class MissedMessageNotificationsTest(ZulipTestCase):
|
||||
assert_maybe_enqueue_notifications_call_args(
|
||||
args_dict=args_dict,
|
||||
message_id=msg_id,
|
||||
flags=["mentioned"],
|
||||
mentioned=True,
|
||||
mention_push_notify=True,
|
||||
mention_email_notify=True,
|
||||
mentioned_user_group_id=hamlet_and_cordelia.id,
|
||||
already_notified={"email_notified": True, "push_notified": True},
|
||||
)
|
||||
@ -469,9 +515,9 @@ class MissedMessageNotificationsTest(ZulipTestCase):
|
||||
assert_maybe_enqueue_notifications_call_args(
|
||||
args_dict=args_dict,
|
||||
message_id=msg_id,
|
||||
private_message=True,
|
||||
sender_is_muted=True,
|
||||
flags=["read"],
|
||||
pm_email_notify=True,
|
||||
pm_push_notify=True,
|
||||
already_notified={"email_notified": False, "push_notified": False},
|
||||
)
|
||||
destroy_event_queue(client_descriptor.event_queue.id)
|
||||
|
||||
@ -188,8 +188,8 @@ class EditMessageSideEffectsTest(ZulipTestCase):
|
||||
user_id=cordelia.id,
|
||||
acting_user_id=hamlet.id,
|
||||
message_id=message_id,
|
||||
mentioned=True,
|
||||
flags=["mentioned"],
|
||||
mention_email_notify=True,
|
||||
mention_push_notify=True,
|
||||
already_notified={},
|
||||
)
|
||||
|
||||
@ -317,8 +317,8 @@ class EditMessageSideEffectsTest(ZulipTestCase):
|
||||
user_id=cordelia.id,
|
||||
acting_user_id=hamlet.id,
|
||||
message_id=message_id,
|
||||
mentioned=True,
|
||||
flags=["mentioned"],
|
||||
mention_push_notify=True,
|
||||
mention_email_notify=True,
|
||||
online_push_enabled=True,
|
||||
idle=False,
|
||||
already_notified={},
|
||||
@ -362,9 +362,9 @@ class EditMessageSideEffectsTest(ZulipTestCase):
|
||||
|
||||
queue_messages = info["queue_messages"]
|
||||
|
||||
# Even though Cordelia has enable_online_push_notifications set
|
||||
# to True, we don't send her any offline notifications, since she
|
||||
# was not mentioned.
|
||||
# Cordelia being present and having `enable_online_push_notifications`
|
||||
# does not mean we'll send her notifications for messages which she
|
||||
# wouldn't otherwise have received notifications for.
|
||||
self.assert_length(queue_messages, 0)
|
||||
|
||||
def test_updates_with_stream_mention_of_sorta_present_user(self) -> None:
|
||||
@ -388,8 +388,8 @@ class EditMessageSideEffectsTest(ZulipTestCase):
|
||||
user_id=cordelia.id,
|
||||
message_id=message_id,
|
||||
acting_user_id=self.example_user("hamlet").id,
|
||||
mentioned=True,
|
||||
flags=["mentioned"],
|
||||
mention_email_notify=True,
|
||||
mention_push_notify=True,
|
||||
already_notified={},
|
||||
)
|
||||
self.assertEqual(info["enqueue_kwargs"], expected_enqueue_kwargs)
|
||||
@ -421,7 +421,6 @@ class EditMessageSideEffectsTest(ZulipTestCase):
|
||||
acting_user_id=hamlet.id,
|
||||
message_id=message_id,
|
||||
wildcard_mention_notify=True,
|
||||
flags=["wildcard_mentioned"],
|
||||
already_notified={},
|
||||
)
|
||||
self.assertEqual(info["enqueue_kwargs"], expected_enqueue_kwargs)
|
||||
@ -477,8 +476,8 @@ class EditMessageSideEffectsTest(ZulipTestCase):
|
||||
user_id=cordelia.id,
|
||||
acting_user_id=hamlet.id,
|
||||
message_id=message_id,
|
||||
mentioned=True,
|
||||
flags=["mentioned"],
|
||||
mention_email_notify=True,
|
||||
mention_push_notify=True,
|
||||
idle=False,
|
||||
already_notified={},
|
||||
)
|
||||
|
||||
@ -23,18 +23,27 @@ class MissedMessageTest(ZulipTestCase):
|
||||
realm = sender.realm
|
||||
hamlet = self.example_user("hamlet")
|
||||
othello = self.example_user("othello")
|
||||
user_data_objects = [
|
||||
self.create_user_notifications_data_object(user_id=hamlet.id),
|
||||
self.create_user_notifications_data_object(user_id=othello.id),
|
||||
]
|
||||
message_type = "private"
|
||||
|
||||
hamlet_alerted = False
|
||||
hamlet_notifications_data = self.create_user_notifications_data_object(user_id=hamlet.id)
|
||||
|
||||
othello_alerted = False
|
||||
othello_notifications_data = self.create_user_notifications_data_object(user_id=othello.id)
|
||||
|
||||
def assert_missing(user_ids: List[int]) -> None:
|
||||
presence_idle_user_ids = get_active_presence_idle_user_ids(
|
||||
realm=realm,
|
||||
sender_id=sender.id,
|
||||
message_type=message_type,
|
||||
active_users_data=user_data_objects,
|
||||
active_users_data=[
|
||||
dict(
|
||||
alerted=hamlet_alerted,
|
||||
notifications_data=hamlet_notifications_data,
|
||||
),
|
||||
dict(
|
||||
alerted=othello_alerted,
|
||||
notifications_data=othello_notifications_data,
|
||||
),
|
||||
],
|
||||
)
|
||||
self.assertEqual(sorted(user_ids), sorted(presence_idle_user_ids))
|
||||
|
||||
@ -47,7 +56,8 @@ class MissedMessageTest(ZulipTestCase):
|
||||
timestamp=when,
|
||||
)
|
||||
|
||||
message_type = "private"
|
||||
hamlet_notifications_data.pm_push_notify = True
|
||||
othello_notifications_data.pm_push_notify = True
|
||||
assert_missing([hamlet.id, othello.id])
|
||||
|
||||
# We have already thoroughly tested the `is_notifiable` function elsewhere,
|
||||
@ -56,16 +66,17 @@ class MissedMessageTest(ZulipTestCase):
|
||||
# at `private_message` and the `mentioned` flag, not stream level notifications.
|
||||
# Simulate Hamlet has turned on notifications for the stream, and test that he's
|
||||
# in the list.
|
||||
message_type = "stream"
|
||||
user_data_objects[0].stream_email_notify = True
|
||||
hamlet_notifications_data.pm_push_notify = False
|
||||
othello_notifications_data.pm_push_notify = False
|
||||
hamlet_notifications_data.stream_email_notify = True
|
||||
assert_missing([hamlet.id])
|
||||
|
||||
# We don't currently send push or email notifications for alert words -- only
|
||||
# desktop notifications, so `is_notifiable` will return False even if the flags contain
|
||||
# desktop notifications, so `is_notifiable` will return False even if the message contains
|
||||
# alert words. Test that `get_active_presence_idle_user_ids` correctly includes even
|
||||
# the alert word case in the list.
|
||||
user_data_objects[0].stream_email_notify = False
|
||||
user_data_objects[0].flags = ["has_alert_word"]
|
||||
hamlet_notifications_data.stream_email_notify = False
|
||||
hamlet_alerted = True
|
||||
assert_missing([hamlet.id])
|
||||
|
||||
# Hamlet is idle (and the message has an alert word), so he should be in the list.
|
||||
@ -78,7 +89,9 @@ class MissedMessageTest(ZulipTestCase):
|
||||
|
||||
# Hamlet is active now, so only Othello should be in the list for a huddle
|
||||
# message.
|
||||
message_type = "private"
|
||||
hamlet_alerted = False
|
||||
hamlet_notifications_data.pm_push_notify = False
|
||||
othello_notifications_data.pm_push_notify = True
|
||||
assert_missing([othello.id])
|
||||
|
||||
|
||||
|
||||
@ -12,155 +12,99 @@ class TestNotificationData(ZulipTestCase):
|
||||
# Boring case
|
||||
user_data = self.create_user_notifications_data_object(user_id=user_id)
|
||||
self.assertEqual(
|
||||
user_data.get_push_notification_trigger(
|
||||
private_message=False, acting_user_id=acting_user_id, idle=True
|
||||
),
|
||||
user_data.get_push_notification_trigger(acting_user_id=acting_user_id, idle=True),
|
||||
None,
|
||||
)
|
||||
self.assertFalse(
|
||||
user_data.is_push_notifiable(
|
||||
private_message=False, acting_user_id=acting_user_id, idle=True
|
||||
)
|
||||
)
|
||||
self.assertFalse(user_data.is_push_notifiable(acting_user_id=acting_user_id, idle=True))
|
||||
|
||||
# Private message
|
||||
user_data = self.create_user_notifications_data_object(user_id=user_id)
|
||||
user_data = self.create_user_notifications_data_object(user_id=user_id, pm_push_notify=True)
|
||||
self.assertEqual(
|
||||
user_data.get_push_notification_trigger(
|
||||
private_message=True, acting_user_id=acting_user_id, idle=True
|
||||
),
|
||||
user_data.get_push_notification_trigger(acting_user_id=acting_user_id, idle=True),
|
||||
"private_message",
|
||||
)
|
||||
self.assertTrue(
|
||||
user_data.is_push_notifiable(
|
||||
private_message=True, acting_user_id=acting_user_id, idle=True
|
||||
)
|
||||
)
|
||||
self.assertTrue(user_data.is_push_notifiable(acting_user_id=acting_user_id, idle=True))
|
||||
|
||||
# Mention
|
||||
user_data = self.create_user_notifications_data_object(
|
||||
user_id=user_id, flags=["mentioned"], mentioned=True
|
||||
user_id=user_id, mention_push_notify=True
|
||||
)
|
||||
self.assertEqual(
|
||||
user_data.get_push_notification_trigger(
|
||||
private_message=False, acting_user_id=acting_user_id, idle=True
|
||||
),
|
||||
user_data.get_push_notification_trigger(acting_user_id=acting_user_id, idle=True),
|
||||
"mentioned",
|
||||
)
|
||||
self.assertTrue(
|
||||
user_data.is_push_notifiable(
|
||||
private_message=False, acting_user_id=acting_user_id, idle=True
|
||||
)
|
||||
)
|
||||
self.assertTrue(user_data.is_push_notifiable(acting_user_id=acting_user_id, idle=True))
|
||||
|
||||
# Wildcard mention
|
||||
user_data = self.create_user_notifications_data_object(
|
||||
user_id=user_id, flags=["wildcard_mentioned"], wildcard_mention_notify=True
|
||||
user_id=user_id, wildcard_mention_notify=True
|
||||
)
|
||||
self.assertEqual(
|
||||
user_data.get_push_notification_trigger(
|
||||
private_message=False, acting_user_id=acting_user_id, idle=True
|
||||
),
|
||||
user_data.get_push_notification_trigger(acting_user_id=acting_user_id, idle=True),
|
||||
"wildcard_mentioned",
|
||||
)
|
||||
self.assertTrue(
|
||||
user_data.is_push_notifiable(
|
||||
private_message=False, acting_user_id=acting_user_id, idle=True
|
||||
)
|
||||
)
|
||||
self.assertTrue(user_data.is_push_notifiable(acting_user_id=acting_user_id, idle=True))
|
||||
|
||||
# Stream notification
|
||||
user_data = self.create_user_notifications_data_object(
|
||||
user_id=user_id, stream_push_notify=True
|
||||
)
|
||||
self.assertEqual(
|
||||
user_data.get_push_notification_trigger(
|
||||
private_message=False, acting_user_id=acting_user_id, idle=True
|
||||
),
|
||||
user_data.get_push_notification_trigger(acting_user_id=acting_user_id, idle=True),
|
||||
"stream_push_notify",
|
||||
)
|
||||
self.assertTrue(
|
||||
user_data.is_push_notifiable(
|
||||
private_message=False, acting_user_id=acting_user_id, idle=True
|
||||
)
|
||||
)
|
||||
self.assertTrue(user_data.is_push_notifiable(acting_user_id=acting_user_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_id=user_id)
|
||||
user_data = self.create_user_notifications_data_object(user_id=user_id, pm_push_notify=True)
|
||||
self.assertEqual(
|
||||
user_data.get_push_notification_trigger(
|
||||
private_message=True, acting_user_id=acting_user_id, idle=False
|
||||
),
|
||||
user_data.get_push_notification_trigger(acting_user_id=acting_user_id, idle=False),
|
||||
None,
|
||||
)
|
||||
self.assertFalse(
|
||||
user_data.is_push_notifiable(
|
||||
private_message=True, acting_user_id=acting_user_id, idle=False
|
||||
)
|
||||
)
|
||||
self.assertFalse(user_data.is_push_notifiable(acting_user_id=acting_user_id, idle=False))
|
||||
|
||||
# Test notifications are sent when not idle but `online_push_enabled = True`
|
||||
user_data = self.create_user_notifications_data_object(
|
||||
user_id=user_id, online_push_enabled=True
|
||||
user_id=user_id, online_push_enabled=True, pm_push_notify=True
|
||||
)
|
||||
self.assertEqual(
|
||||
user_data.get_push_notification_trigger(
|
||||
private_message=True, acting_user_id=acting_user_id, idle=False
|
||||
),
|
||||
user_data.get_push_notification_trigger(acting_user_id=acting_user_id, idle=False),
|
||||
"private_message",
|
||||
)
|
||||
self.assertTrue(
|
||||
user_data.is_push_notifiable(
|
||||
private_message=True, acting_user_id=acting_user_id, idle=False
|
||||
)
|
||||
)
|
||||
self.assertTrue(user_data.is_push_notifiable(acting_user_id=acting_user_id, idle=False))
|
||||
|
||||
# The following are hypothetical cases, since a private message can never have `stream_push_notify = True`.
|
||||
# We just want to test the early (False) return patterns in these special cases:
|
||||
# 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,
|
||||
mentioned=True,
|
||||
stream_email_notify=True,
|
||||
stream_push_notify=True,
|
||||
pm_push_notify=True,
|
||||
pm_email_notify=True,
|
||||
mention_push_notify=True,
|
||||
mention_email_notify=True,
|
||||
)
|
||||
self.assertEqual(
|
||||
user_data.get_push_notification_trigger(
|
||||
private_message=True, acting_user_id=acting_user_id, idle=True
|
||||
),
|
||||
user_data.get_push_notification_trigger(acting_user_id=acting_user_id, idle=True),
|
||||
None,
|
||||
)
|
||||
self.assertFalse(
|
||||
user_data.is_push_notifiable(
|
||||
private_message=True, acting_user_id=acting_user_id, idle=True
|
||||
)
|
||||
)
|
||||
self.assertFalse(user_data.is_push_notifiable(acting_user_id=acting_user_id, idle=True))
|
||||
|
||||
# Message sender is the user the object corresponds to.
|
||||
user_data = self.create_user_notifications_data_object(
|
||||
user_id=acting_user_id,
|
||||
sender_is_muted=False,
|
||||
flags=["mentioned", "wildcard_mentioned"],
|
||||
wildcard_mention_notify=True,
|
||||
mentioned=True,
|
||||
stream_email_notify=True,
|
||||
stream_push_notify=True,
|
||||
pm_push_notify=True,
|
||||
pm_email_notify=True,
|
||||
mention_push_notify=True,
|
||||
mention_email_notify=True,
|
||||
)
|
||||
self.assertEqual(
|
||||
user_data.get_push_notification_trigger(
|
||||
private_message=True, acting_user_id=acting_user_id, idle=True
|
||||
),
|
||||
user_data.get_push_notification_trigger(acting_user_id=acting_user_id, idle=True),
|
||||
None,
|
||||
)
|
||||
self.assertFalse(
|
||||
user_data.is_push_notifiable(
|
||||
private_message=True, acting_user_id=acting_user_id, idle=True
|
||||
)
|
||||
)
|
||||
self.assertFalse(user_data.is_push_notifiable(acting_user_id=acting_user_id, idle=True))
|
||||
|
||||
def test_is_email_notifiable(self) -> None:
|
||||
user_id = self.example_user("hamlet").id
|
||||
@ -169,148 +113,100 @@ class TestNotificationData(ZulipTestCase):
|
||||
# Boring case
|
||||
user_data = self.create_user_notifications_data_object(user_id=user_id)
|
||||
self.assertEqual(
|
||||
user_data.get_email_notification_trigger(
|
||||
private_message=False, acting_user_id=acting_user_id, idle=True
|
||||
),
|
||||
user_data.get_email_notification_trigger(acting_user_id=acting_user_id, idle=True),
|
||||
None,
|
||||
)
|
||||
self.assertFalse(
|
||||
user_data.is_email_notifiable(
|
||||
private_message=False, acting_user_id=acting_user_id, idle=True
|
||||
)
|
||||
)
|
||||
self.assertFalse(user_data.is_email_notifiable(acting_user_id=acting_user_id, idle=True))
|
||||
|
||||
# Private message
|
||||
user_data = self.create_user_notifications_data_object(user_id=user_id)
|
||||
user_data = self.create_user_notifications_data_object(
|
||||
user_id=user_id, pm_email_notify=True
|
||||
)
|
||||
self.assertEqual(
|
||||
user_data.get_email_notification_trigger(
|
||||
private_message=True, acting_user_id=acting_user_id, idle=True
|
||||
),
|
||||
user_data.get_email_notification_trigger(acting_user_id=acting_user_id, idle=True),
|
||||
"private_message",
|
||||
)
|
||||
self.assertTrue(
|
||||
user_data.is_email_notifiable(
|
||||
private_message=True, acting_user_id=acting_user_id, idle=True
|
||||
)
|
||||
)
|
||||
self.assertTrue(user_data.is_email_notifiable(acting_user_id=acting_user_id, idle=True))
|
||||
|
||||
# Mention
|
||||
user_data = self.create_user_notifications_data_object(
|
||||
user_id=user_id, flags=["mentioned"], mentioned=True
|
||||
user_id=user_id, mention_email_notify=True
|
||||
)
|
||||
self.assertEqual(
|
||||
user_data.get_email_notification_trigger(
|
||||
private_message=False, acting_user_id=acting_user_id, idle=True
|
||||
),
|
||||
user_data.get_email_notification_trigger(acting_user_id=acting_user_id, idle=True),
|
||||
"mentioned",
|
||||
)
|
||||
self.assertTrue(
|
||||
user_data.is_email_notifiable(
|
||||
private_message=False, acting_user_id=acting_user_id, idle=True
|
||||
)
|
||||
)
|
||||
self.assertTrue(user_data.is_email_notifiable(acting_user_id=acting_user_id, idle=True))
|
||||
|
||||
# Wildcard mention
|
||||
user_data = self.create_user_notifications_data_object(
|
||||
user_id=user_id, flags=["wildcard_mentioned"], wildcard_mention_notify=True
|
||||
user_id=user_id, wildcard_mention_notify=True
|
||||
)
|
||||
self.assertEqual(
|
||||
user_data.get_email_notification_trigger(
|
||||
private_message=False, acting_user_id=acting_user_id, idle=True
|
||||
),
|
||||
user_data.get_email_notification_trigger(acting_user_id=acting_user_id, idle=True),
|
||||
"wildcard_mentioned",
|
||||
)
|
||||
self.assertTrue(
|
||||
user_data.is_email_notifiable(
|
||||
private_message=False, acting_user_id=acting_user_id, idle=True
|
||||
)
|
||||
)
|
||||
self.assertTrue(user_data.is_email_notifiable(acting_user_id=acting_user_id, idle=True))
|
||||
|
||||
# Stream notification
|
||||
user_data = self.create_user_notifications_data_object(
|
||||
user_id=user_id, stream_email_notify=True
|
||||
)
|
||||
self.assertEqual(
|
||||
user_data.get_email_notification_trigger(
|
||||
private_message=False, acting_user_id=acting_user_id, idle=True
|
||||
),
|
||||
user_data.get_email_notification_trigger(acting_user_id=acting_user_id, idle=True),
|
||||
"stream_email_notify",
|
||||
)
|
||||
self.assertTrue(
|
||||
user_data.is_email_notifiable(
|
||||
private_message=False, acting_user_id=acting_user_id, idle=True
|
||||
)
|
||||
)
|
||||
self.assertTrue(user_data.is_email_notifiable(acting_user_id=acting_user_id, idle=True))
|
||||
|
||||
# Test no notifications when not idle
|
||||
user_data = self.create_user_notifications_data_object(user_id=user_id)
|
||||
user_data = self.create_user_notifications_data_object(
|
||||
user_id=user_id, pm_email_notify=True
|
||||
)
|
||||
self.assertEqual(
|
||||
user_data.get_email_notification_trigger(
|
||||
private_message=True, acting_user_id=acting_user_id, idle=False
|
||||
),
|
||||
user_data.get_email_notification_trigger(acting_user_id=acting_user_id, idle=False),
|
||||
None,
|
||||
)
|
||||
self.assertFalse(
|
||||
user_data.is_email_notifiable(
|
||||
private_message=True, acting_user_id=acting_user_id, idle=False
|
||||
)
|
||||
)
|
||||
self.assertFalse(user_data.is_email_notifiable(acting_user_id=acting_user_id, idle=False))
|
||||
|
||||
# The following are hypothetical cases, since a private message can never have `stream_email_notify = True`.
|
||||
# We just want to test the early (False) return patterns in these special cases:
|
||||
# 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,
|
||||
mentioned=True,
|
||||
stream_email_notify=True,
|
||||
stream_push_notify=True,
|
||||
pm_push_notify=True,
|
||||
pm_email_notify=True,
|
||||
mention_push_notify=True,
|
||||
mention_email_notify=True,
|
||||
)
|
||||
self.assertEqual(
|
||||
user_data.get_email_notification_trigger(
|
||||
private_message=True, acting_user_id=acting_user_id, idle=True
|
||||
),
|
||||
user_data.get_email_notification_trigger(acting_user_id=acting_user_id, idle=True),
|
||||
None,
|
||||
)
|
||||
self.assertFalse(
|
||||
user_data.is_email_notifiable(
|
||||
private_message=True, acting_user_id=acting_user_id, idle=True
|
||||
)
|
||||
)
|
||||
self.assertFalse(user_data.is_email_notifiable(acting_user_id=acting_user_id, idle=True))
|
||||
|
||||
# Message sender is the user the object corresponds to.
|
||||
user_data = self.create_user_notifications_data_object(
|
||||
user_id=acting_user_id,
|
||||
sender_is_muted=False,
|
||||
flags=["mentioned", "wildcard_mentioned"],
|
||||
wildcard_mention_notify=True,
|
||||
mentioned=True,
|
||||
stream_email_notify=True,
|
||||
stream_push_notify=True,
|
||||
pm_push_notify=True,
|
||||
pm_email_notify=True,
|
||||
mention_push_notify=True,
|
||||
mention_email_notify=True,
|
||||
)
|
||||
self.assertEqual(
|
||||
user_data.get_email_notification_trigger(
|
||||
private_message=True, acting_user_id=acting_user_id, idle=True
|
||||
),
|
||||
user_data.get_email_notification_trigger(acting_user_id=acting_user_id, idle=True),
|
||||
None,
|
||||
)
|
||||
self.assertFalse(
|
||||
user_data.is_email_notifiable(
|
||||
private_message=True, acting_user_id=acting_user_id, idle=True
|
||||
)
|
||||
)
|
||||
self.assertFalse(user_data.is_email_notifiable(acting_user_id=acting_user_id, idle=True))
|
||||
|
||||
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
|
||||
acting_user_id = self.example_user("cordelia").id
|
||||
user_data = self.create_user_notifications_data_object(user_id=user_id)
|
||||
self.assertTrue(
|
||||
user_data.is_notifiable(private_message=True, acting_user_id=acting_user_id, idle=True)
|
||||
)
|
||||
user_data = self.create_user_notifications_data_object(user_id=user_id, pm_push_notify=True)
|
||||
self.assertTrue(user_data.is_notifiable(acting_user_id=acting_user_id, idle=True))
|
||||
|
||||
def test_user_group_mentions_map(self) -> None:
|
||||
hamlet = self.example_user("hamlet")
|
||||
|
||||
@ -76,8 +76,6 @@ from zerver.models import (
|
||||
get_display_recipient,
|
||||
get_realm,
|
||||
get_stream,
|
||||
receives_offline_push_notifications,
|
||||
receives_online_push_notifications,
|
||||
)
|
||||
|
||||
if settings.ZILENCER_ENABLED:
|
||||
@ -928,18 +926,6 @@ class HandlePushNotificationTest(PushNotificationTest):
|
||||
with self.assertRaises(PushNotificationBouncerRetryLaterError):
|
||||
handle_push_notification(self.user_profile.id, missed_message)
|
||||
|
||||
@mock.patch("zerver.lib.push_notifications.push_notifications_enabled", return_value=True)
|
||||
def test_disabled_notifications(self, mock_push_notifications: mock.MagicMock) -> None:
|
||||
user_profile = self.example_user("hamlet")
|
||||
user_profile.enable_online_email_notifications = False
|
||||
user_profile.enable_online_push_notifications = False
|
||||
user_profile.enable_offline_email_notifications = False
|
||||
user_profile.enable_offline_push_notifications = False
|
||||
user_profile.enable_stream_push_notifications = False
|
||||
user_profile.save()
|
||||
handle_push_notification(user_profile.id, {})
|
||||
mock_push_notifications.assert_called()
|
||||
|
||||
@mock.patch("zerver.lib.push_notifications.push_notifications_enabled", return_value=True)
|
||||
def test_read_message(self, mock_push_notifications: mock.MagicMock) -> None:
|
||||
user_profile = self.example_user("hamlet")
|
||||
@ -2314,60 +2300,6 @@ class TestClearOnRead(ZulipTestCase):
|
||||
self.assertEqual({id for g in groups for id in g}, set(message_ids))
|
||||
|
||||
|
||||
class TestReceivesNotificationsFunctions(ZulipTestCase):
|
||||
def setUp(self) -> None:
|
||||
super().setUp()
|
||||
self.user = self.example_user("cordelia")
|
||||
|
||||
def test_receivers_online_notifications_when_user_is_a_bot(self) -> None:
|
||||
self.user.is_bot = True
|
||||
|
||||
self.user.enable_online_push_notifications = True
|
||||
self.assertFalse(receives_online_push_notifications(self.user))
|
||||
|
||||
self.user.enable_online_push_notifications = False
|
||||
self.assertFalse(receives_online_push_notifications(self.user))
|
||||
|
||||
def test_receivers_online_notifications_when_user_is_not_a_bot(self) -> None:
|
||||
self.user.is_bot = False
|
||||
|
||||
self.user.enable_online_push_notifications = True
|
||||
self.assertTrue(receives_online_push_notifications(self.user))
|
||||
|
||||
self.user.enable_online_push_notifications = False
|
||||
self.assertFalse(receives_online_push_notifications(self.user))
|
||||
|
||||
def test_receivers_offline_notifications_when_user_is_a_bot(self) -> None:
|
||||
self.user.is_bot = True
|
||||
|
||||
self.user.enable_offline_push_notifications = True
|
||||
self.assertFalse(receives_offline_push_notifications(self.user))
|
||||
|
||||
self.user.enable_offline_push_notifications = False
|
||||
self.assertFalse(receives_offline_push_notifications(self.user))
|
||||
|
||||
self.user.enable_offline_push_notifications = False
|
||||
self.assertFalse(receives_offline_push_notifications(self.user))
|
||||
|
||||
self.user.enable_offline_push_notifications = True
|
||||
self.assertFalse(receives_offline_push_notifications(self.user))
|
||||
|
||||
def test_receivers_offline_notifications_when_user_is_not_a_bot(self) -> None:
|
||||
self.user.is_bot = False
|
||||
|
||||
self.user.enable_offline_push_notifications = True
|
||||
self.assertTrue(receives_offline_push_notifications(self.user))
|
||||
|
||||
self.user.enable_offline_push_notifications = False
|
||||
self.assertFalse(receives_offline_push_notifications(self.user))
|
||||
|
||||
self.user.enable_offline_push_notifications = False
|
||||
self.assertFalse(receives_offline_push_notifications(self.user))
|
||||
|
||||
self.user.enable_offline_push_notifications = True
|
||||
self.assertTrue(receives_offline_push_notifications(self.user))
|
||||
|
||||
|
||||
class TestPushNotificationsContent(ZulipTestCase):
|
||||
def test_fixtures(self) -> None:
|
||||
fixtures = orjson.loads(self.fixture_data("markdown_test_cases.json"))
|
||||
|
||||
@ -1520,6 +1520,8 @@ class RecipientInfoTest(ZulipTestCase):
|
||||
expected_info = dict(
|
||||
active_user_ids=all_user_ids,
|
||||
online_push_user_ids=set(),
|
||||
pm_mention_email_disabled_user_ids=set(),
|
||||
pm_mention_push_disabled_user_ids=set(),
|
||||
stream_push_user_ids=set(),
|
||||
stream_email_user_ids=set(),
|
||||
wildcard_mention_user_ids=set(),
|
||||
@ -1532,6 +1534,22 @@ class RecipientInfoTest(ZulipTestCase):
|
||||
|
||||
self.assertEqual(info, expected_info)
|
||||
|
||||
hamlet.enable_offline_email_notifications = False
|
||||
hamlet.enable_offline_push_notifications = False
|
||||
hamlet.save()
|
||||
info = get_recipient_info(
|
||||
realm_id=realm.id,
|
||||
recipient=recipient,
|
||||
sender_id=hamlet.id,
|
||||
stream_topic=stream_topic,
|
||||
possible_wildcard_mention=False,
|
||||
)
|
||||
self.assertEqual(info["pm_mention_email_disabled_user_ids"], set([hamlet.id]))
|
||||
self.assertEqual(info["pm_mention_push_disabled_user_ids"], set([hamlet.id]))
|
||||
hamlet.enable_offline_email_notifications = True
|
||||
hamlet.enable_offline_push_notifications = True
|
||||
hamlet.save()
|
||||
|
||||
cordelia.wildcard_mentions_notify = False
|
||||
cordelia.save()
|
||||
hamlet.enable_stream_push_notifications = True
|
||||
|
||||
@ -744,9 +744,11 @@ def missedmessage_hook(
|
||||
|
||||
user_notifications_data = UserMessageNotificationsData(
|
||||
user_id=user_profile_id,
|
||||
flags=event.get("flags", []),
|
||||
sender_is_muted=internal_data.get("sender_is_muted", False),
|
||||
mentioned=internal_data.get("mentioned", False),
|
||||
pm_push_notify=internal_data.get("pm_push_notify", False),
|
||||
pm_email_notify=internal_data.get("pm_email_notify", False),
|
||||
mention_push_notify=internal_data.get("mention_push_notify", False),
|
||||
mention_email_notify=internal_data.get("mention_email_notify", 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),
|
||||
@ -754,7 +756,6 @@ def missedmessage_hook(
|
||||
online_push_enabled=False,
|
||||
)
|
||||
|
||||
private_message = event["message"]["type"] == "private"
|
||||
mentioned_user_group_id = internal_data.get("mentioned_user_group_id")
|
||||
|
||||
# Since we just GC'd the last event queue, the user is definitely idle.
|
||||
@ -770,7 +771,6 @@ def missedmessage_hook(
|
||||
user_notifications_data=user_notifications_data,
|
||||
acting_user_id=sender_id,
|
||||
message_id=message_id,
|
||||
private_message=private_message,
|
||||
mentioned_user_group_id=mentioned_user_group_id,
|
||||
idle=idle,
|
||||
already_notified=already_notified,
|
||||
@ -793,7 +793,6 @@ def maybe_enqueue_notifications(
|
||||
user_notifications_data: UserMessageNotificationsData,
|
||||
acting_user_id: int,
|
||||
message_id: int,
|
||||
private_message: bool,
|
||||
mentioned_user_group_id: Optional[int],
|
||||
idle: bool,
|
||||
already_notified: Dict[str, bool],
|
||||
@ -807,10 +806,10 @@ def maybe_enqueue_notifications(
|
||||
"""
|
||||
notified: Dict[str, bool] = {}
|
||||
|
||||
if user_notifications_data.is_push_notifiable(private_message, acting_user_id, idle):
|
||||
if user_notifications_data.is_push_notifiable(acting_user_id, idle):
|
||||
notice = build_offline_notification(user_notifications_data.user_id, message_id)
|
||||
notice["trigger"] = user_notifications_data.get_push_notification_trigger(
|
||||
private_message, acting_user_id, idle
|
||||
acting_user_id, idle
|
||||
)
|
||||
notice["type"] = "add"
|
||||
notice["mentioned_user_group_id"] = mentioned_user_group_id
|
||||
@ -822,10 +821,10 @@ def maybe_enqueue_notifications(
|
||||
# mention. Eventually, we'll add settings to allow email
|
||||
# notifications to match the model of push notifications
|
||||
# above.
|
||||
if user_notifications_data.is_email_notifiable(private_message, acting_user_id, idle):
|
||||
if user_notifications_data.is_email_notifiable(acting_user_id, idle):
|
||||
notice = build_offline_notification(user_notifications_data.user_id, message_id)
|
||||
notice["trigger"] = user_notifications_data.get_email_notification_trigger(
|
||||
private_message, acting_user_id, idle
|
||||
acting_user_id, idle
|
||||
)
|
||||
notice["mentioned_user_group_id"] = mentioned_user_group_id
|
||||
if not already_notified.get("email_notified"):
|
||||
@ -894,6 +893,12 @@ def process_message_event(
|
||||
|
||||
presence_idle_user_ids = set(event_template.get("presence_idle_user_ids", []))
|
||||
online_push_user_ids = set(event_template.get("online_push_user_ids", []))
|
||||
pm_mention_push_disabled_user_ids = set(
|
||||
event_template.get("pm_mention_push_disabled_user_ids", [])
|
||||
)
|
||||
pm_mention_email_disabled_user_ids = set(
|
||||
event_template.get("pm_mention_email_disabled_user_ids", [])
|
||||
)
|
||||
stream_push_user_ids = set(event_template.get("stream_push_user_ids", []))
|
||||
stream_email_user_ids = set(event_template.get("stream_email_user_ids", []))
|
||||
wildcard_mention_user_ids = set(event_template.get("wildcard_mention_user_ids", []))
|
||||
@ -937,7 +942,10 @@ def process_message_event(
|
||||
user_notifications_data = UserMessageNotificationsData.from_user_id_sets(
|
||||
user_id=user_profile_id,
|
||||
flags=flags,
|
||||
private_message=private_message,
|
||||
online_push_user_ids=online_push_user_ids,
|
||||
pm_mention_push_disabled_user_ids=pm_mention_push_disabled_user_ids,
|
||||
pm_mention_email_disabled_user_ids=pm_mention_email_disabled_user_ids,
|
||||
stream_push_user_ids=stream_push_user_ids,
|
||||
stream_email_user_ids=stream_email_user_ids,
|
||||
wildcard_mention_user_ids=wildcard_mention_user_ids,
|
||||
@ -946,7 +954,6 @@ def process_message_event(
|
||||
|
||||
internal_data = asdict(user_notifications_data)
|
||||
# Remove fields sent through other pipes to save some space.
|
||||
internal_data.pop("flags")
|
||||
internal_data.pop("user_id")
|
||||
internal_data["mentioned_user_group_id"] = mentioned_user_group_id
|
||||
extra_user_data[user_profile_id] = dict(internal_data=internal_data)
|
||||
@ -955,9 +962,7 @@ def process_message_event(
|
||||
# shouldn't receive notifications even if they were online. In that case we can
|
||||
# avoid the more expensive `receiver_is_off_zulip` call, and move on to process
|
||||
# the next user.
|
||||
if not user_notifications_data.is_notifiable(
|
||||
private_message, acting_user_id=sender_id, idle=True
|
||||
):
|
||||
if not user_notifications_data.is_notifiable(acting_user_id=sender_id, idle=True):
|
||||
continue
|
||||
|
||||
idle = receiver_is_off_zulip(user_profile_id) or (user_profile_id in presence_idle_user_ids)
|
||||
@ -967,7 +972,6 @@ def process_message_event(
|
||||
user_notifications_data=user_notifications_data,
|
||||
acting_user_id=sender_id,
|
||||
message_id=message_id,
|
||||
private_message=private_message,
|
||||
mentioned_user_group_id=mentioned_user_group_id,
|
||||
idle=idle,
|
||||
already_notified={},
|
||||
@ -1082,6 +1086,12 @@ def process_message_update_event(
|
||||
event_template = dict(orig_event)
|
||||
prior_mention_user_ids = set(event_template.pop("prior_mention_user_ids", []))
|
||||
presence_idle_user_ids = set(event_template.pop("presence_idle_user_ids", []))
|
||||
pm_mention_push_disabled_user_ids = set(
|
||||
event_template.pop("pm_mention_push_disabled_user_ids", [])
|
||||
)
|
||||
pm_mention_email_disabled_user_ids = set(
|
||||
event_template.pop("pm_mention_email_disabled_user_ids", [])
|
||||
)
|
||||
stream_push_user_ids = set(event_template.pop("stream_push_user_ids", []))
|
||||
stream_email_user_ids = set(event_template.pop("stream_email_user_ids", []))
|
||||
wildcard_mention_user_ids = set(event_template.pop("wildcard_mention_user_ids", []))
|
||||
@ -1124,7 +1134,10 @@ def process_message_update_event(
|
||||
user_notifications_data = UserMessageNotificationsData.from_user_id_sets(
|
||||
user_id=user_profile_id,
|
||||
flags=flags,
|
||||
private_message=(stream_name is None),
|
||||
online_push_user_ids=online_push_user_ids,
|
||||
pm_mention_push_disabled_user_ids=pm_mention_push_disabled_user_ids,
|
||||
pm_mention_email_disabled_user_ids=pm_mention_email_disabled_user_ids,
|
||||
stream_push_user_ids=stream_push_user_ids,
|
||||
stream_email_user_ids=stream_email_user_ids,
|
||||
wildcard_mention_user_ids=wildcard_mention_user_ids,
|
||||
@ -1199,7 +1212,6 @@ def maybe_enqueue_notifications_for_message_update(
|
||||
user_notifications_data=user_notifications_data,
|
||||
message_id=message_id,
|
||||
acting_user_id=acting_user_id,
|
||||
private_message=private_message,
|
||||
mentioned_user_group_id=mentioned_user_group_id,
|
||||
idle=idle,
|
||||
already_notified={},
|
||||
|
||||
Loading…
Reference in New Issue
Block a user