zulip/zerver/tests/test_messages.py

106 lines
4.5 KiB
Python
Raw Normal View History

import datetime
from typing import List
from django.utils.timezone import now as timezone_now
from zerver.actions.message_send import get_active_presence_idle_user_ids
from zerver.lib.test_classes import ZulipTestCase
from zerver.models import (
Message,
UserPresence,
UserProfile,
bulk_get_huddle_user_ids,
get_client,
get_huddle_user_ids,
)
class MissedMessageTest(ZulipTestCase):
def test_presence_idle_user_ids(self) -> None:
UserPresence.objects.all().delete()
sender = self.example_user("cordelia")
realm = sender.realm
hamlet = self.example_user("hamlet")
othello = self.example_user("othello")
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.
2021-07-14 15:34:01 +02:00
hamlet_notifications_data = self.create_user_notifications_data_object(user_id=hamlet.id)
othello_notifications_data = self.create_user_notifications_data_object(user_id=othello.id)
def assert_active_presence_idle_user_ids(user_ids: List[int]) -> None:
presence_idle_user_ids = get_active_presence_idle_user_ids(
realm=realm,
sender_id=sender.id,
user_notifications_data_list=[
hamlet_notifications_data,
othello_notifications_data,
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.
2021-07-14 15:34:01 +02:00
],
)
self.assertEqual(sorted(user_ids), sorted(presence_idle_user_ids))
def set_presence(user: UserProfile, client_name: str, ago: int) -> None:
when = timezone_now() - datetime.timedelta(seconds=ago)
UserPresence.objects.create(
user_profile_id=user.id,
realm_id=user.realm_id,
client=get_client(client_name),
timestamp=when,
)
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.
2021-07-14 15:34:01 +02:00
hamlet_notifications_data.pm_push_notify = True
othello_notifications_data.pm_push_notify = True
assert_active_presence_idle_user_ids([hamlet.id, othello.id])
# We have already thoroughly tested the `is_notifiable` function elsewhere,
# so we needn't test all cases here. This test exists mainly to avoid a bug
# which existed earlier with `get_active_presence_idle_user_ids` only looking
# 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.
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.
2021-07-14 15:34:01 +02:00
hamlet_notifications_data.pm_push_notify = False
othello_notifications_data.pm_push_notify = False
hamlet_notifications_data.stream_email_notify = True
assert_active_presence_idle_user_ids([hamlet.id])
# Hamlet is idle (and is supposed to receive stream notifications), so he should be in the list.
set_presence(hamlet, "iPhone", ago=5000)
assert_active_presence_idle_user_ids([hamlet.id])
# If Hamlet is active, don't include him in the `presence_idle` list.
set_presence(hamlet, "website", ago=15)
assert_active_presence_idle_user_ids([])
# Hamlet is active now, so only Othello should be in the list for a huddle
# message.
hamlet_notifications_data.stream_email_notify = False
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.
2021-07-14 15:34:01 +02:00
hamlet_notifications_data.pm_push_notify = False
othello_notifications_data.pm_push_notify = True
assert_active_presence_idle_user_ids([othello.id])
class TestBulkGetHuddleUserIds(ZulipTestCase):
def test_bulk_get_huddle_user_ids(self) -> None:
hamlet = self.example_user("hamlet")
cordelia = self.example_user("cordelia")
othello = self.example_user("othello")
iago = self.example_user("iago")
message_ids = [
self.send_huddle_message(hamlet, [cordelia, othello], "test"),
self.send_huddle_message(cordelia, [hamlet, othello, iago], "test"),
]
messages = Message.objects.filter(id__in=message_ids).order_by("id")
first_huddle_recipient = messages[0].recipient
first_huddle_user_ids = list(get_huddle_user_ids(first_huddle_recipient))
second_huddle_recipient = messages[1].recipient
second_huddle_user_ids = list(get_huddle_user_ids(second_huddle_recipient))
huddle_user_ids = bulk_get_huddle_user_ids(
[first_huddle_recipient, second_huddle_recipient]
)
self.assertEqual(huddle_user_ids[first_huddle_recipient.id], first_huddle_user_ids)
self.assertEqual(huddle_user_ids[second_huddle_recipient.id], second_huddle_user_ids)
def test_bulk_get_huddle_user_ids_empty_list(self) -> None:
self.assertEqual(bulk_get_huddle_user_ids([]), {})