zulip/zerver/tests/test_notification_data.py

376 lines
16 KiB
Python
Raw Normal View History

from zerver.actions.user_groups import check_add_user_group
from zerver.lib.mention import MentionBackend, MentionData
from zerver.lib.notification_data import UserMessageNotificationsData, get_user_group_mentions_data
from zerver.lib.test_classes import ZulipTestCase
class TestNotificationData(ZulipTestCase):
"""
Because the `UserMessageNotificationsData` does not do any database queries, all user IDs
used in the following tests are arbitrary, and do not represent real users.
"""
def test_is_push_notifiable(self) -> None:
user_id = self.example_user("hamlet").id
acting_user_id = self.example_user("cordelia").id
# Boring case
user_data = self.create_user_notifications_data_object(user_id=user_id)
self.assertEqual(
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
user_data.get_push_notification_trigger(acting_user_id=acting_user_id, idle=True),
None,
)
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.assertFalse(user_data.is_push_notifiable(acting_user_id=acting_user_id, idle=True))
# Private message
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
user_data = self.create_user_notifications_data_object(user_id=user_id, pm_push_notify=True)
self.assertEqual(
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
user_data.get_push_notification_trigger(acting_user_id=acting_user_id, idle=True),
"private_message",
)
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.assertTrue(user_data.is_push_notifiable(acting_user_id=acting_user_id, idle=True))
# Mention
user_data = self.create_user_notifications_data_object(
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
user_id=user_id, mention_push_notify=True
)
self.assertEqual(
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
user_data.get_push_notification_trigger(acting_user_id=acting_user_id, idle=True),
"mentioned",
)
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.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, wildcard_mention_push_notify=True
)
self.assertEqual(
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
user_data.get_push_notification_trigger(acting_user_id=acting_user_id, idle=True),
"wildcard_mentioned",
)
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.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(
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
user_data.get_push_notification_trigger(acting_user_id=acting_user_id, idle=True),
"stream_push_notify",
)
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.assertTrue(user_data.is_push_notifiable(acting_user_id=acting_user_id, idle=True))
# Followed Topic notification
user_data = self.create_user_notifications_data_object(
user_id=user_id, followed_topic_push_notify=True
)
self.assertEqual(
user_data.get_push_notification_trigger(acting_user_id=acting_user_id, idle=True),
"followed_topic_push_notify",
)
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
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
user_data = self.create_user_notifications_data_object(user_id=user_id, pm_push_notify=True)
self.assertEqual(
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
user_data.get_push_notification_trigger(acting_user_id=acting_user_id, idle=False),
None,
)
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.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(
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
user_id=user_id, online_push_enabled=True, pm_push_notify=True
)
self.assertEqual(
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
user_data.get_push_notification_trigger(acting_user_id=acting_user_id, idle=False),
"private_message",
)
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.assertTrue(user_data.is_push_notifiable(acting_user_id=acting_user_id, idle=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
# 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,
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
pm_push_notify=True,
pm_email_notify=True,
mention_push_notify=True,
mention_email_notify=True,
wildcard_mention_push_notify=True,
wildcard_mention_email_notify=True,
)
self.assertEqual(
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
user_data.get_push_notification_trigger(acting_user_id=acting_user_id, idle=True),
None,
)
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.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,
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
pm_push_notify=True,
pm_email_notify=True,
mention_push_notify=True,
mention_email_notify=True,
wildcard_mention_push_notify=True,
wildcard_mention_email_notify=True,
)
self.assertEqual(
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
user_data.get_push_notification_trigger(acting_user_id=acting_user_id, idle=True),
None,
)
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.assertFalse(user_data.is_push_notifiable(acting_user_id=acting_user_id, idle=True))
# 'disable_external_notifications' takes precedence over other flags.
user_data = self.create_user_notifications_data_object(
user_id=user_id,
pm_push_notify=True,
pm_email_notify=True,
mention_push_notify=True,
mention_email_notify=True,
wildcard_mention_push_notify=True,
wildcard_mention_email_notify=True,
disable_external_notifications=True,
)
self.assertEqual(
user_data.get_push_notification_trigger(acting_user_id=acting_user_id, idle=True),
None,
)
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
acting_user_id = self.example_user("cordelia").id
# Boring case
user_data = self.create_user_notifications_data_object(user_id=user_id)
self.assertEqual(
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
user_data.get_email_notification_trigger(acting_user_id=acting_user_id, idle=True),
None,
)
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.assertFalse(user_data.is_email_notifiable(acting_user_id=acting_user_id, idle=True))
# Private message
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
user_data = self.create_user_notifications_data_object(
user_id=user_id, pm_email_notify=True
)
self.assertEqual(
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
user_data.get_email_notification_trigger(acting_user_id=acting_user_id, idle=True),
"private_message",
)
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.assertTrue(user_data.is_email_notifiable(acting_user_id=acting_user_id, idle=True))
# Mention
user_data = self.create_user_notifications_data_object(
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
user_id=user_id, mention_email_notify=True
)
self.assertEqual(
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
user_data.get_email_notification_trigger(acting_user_id=acting_user_id, idle=True),
"mentioned",
)
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.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, wildcard_mention_email_notify=True
)
self.assertEqual(
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
user_data.get_email_notification_trigger(acting_user_id=acting_user_id, idle=True),
"wildcard_mentioned",
)
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.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(
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
user_data.get_email_notification_trigger(acting_user_id=acting_user_id, idle=True),
"stream_email_notify",
)
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.assertTrue(user_data.is_email_notifiable(acting_user_id=acting_user_id, idle=True))
# Followed Topic notification
user_data = self.create_user_notifications_data_object(
user_id=user_id, followed_topic_email_notify=True
)
self.assertEqual(
user_data.get_email_notification_trigger(acting_user_id=acting_user_id, idle=True),
"followed_topic_email_notify",
)
self.assertTrue(user_data.is_email_notifiable(acting_user_id=acting_user_id, idle=True))
# Test no notifications when not idle
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
user_data = self.create_user_notifications_data_object(
user_id=user_id, pm_email_notify=True
)
self.assertEqual(
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
user_data.get_email_notification_trigger(acting_user_id=acting_user_id, idle=False),
None,
)
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.assertFalse(user_data.is_email_notifiable(acting_user_id=acting_user_id, idle=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
# 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,
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
pm_push_notify=True,
pm_email_notify=True,
mention_push_notify=True,
mention_email_notify=True,
wildcard_mention_push_notify=True,
wildcard_mention_email_notify=True,
)
self.assertEqual(
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
user_data.get_email_notification_trigger(acting_user_id=acting_user_id, idle=True),
None,
)
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.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,
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
pm_push_notify=True,
pm_email_notify=True,
mention_push_notify=True,
mention_email_notify=True,
wildcard_mention_push_notify=True,
wildcard_mention_email_notify=True,
)
self.assertEqual(
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
user_data.get_email_notification_trigger(acting_user_id=acting_user_id, idle=True),
None,
)
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.assertFalse(user_data.is_email_notifiable(acting_user_id=acting_user_id, idle=True))
# Message sender is the welcome bot.
user_data = self.create_user_notifications_data_object(
user_id=user_id,
pm_push_notify=True,
pm_email_notify=True,
mention_push_notify=True,
mention_email_notify=True,
wildcard_mention_push_notify=True,
wildcard_mention_email_notify=True,
disable_external_notifications=True,
)
self.assertEqual(
user_data.get_email_notification_trigger(acting_user_id=acting_user_id, idle=True),
None,
)
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
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
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_bot_user_notifiability(self) -> None:
# Non-bot user (`user_id=9`) should get notified, while bot user (`user_id=10`) shouldn't
for user_id, notifiable in [(9, True), (10, False)]:
user_data = UserMessageNotificationsData.from_user_id_sets(
user_id=user_id,
flags=["mentioned"],
private_message=True,
disable_external_notifications=False,
online_push_user_ids=set(),
pm_mention_email_disabled_user_ids=set(),
pm_mention_push_disabled_user_ids=set(),
all_bot_user_ids={10, 11, 12},
muted_sender_user_ids=set(),
stream_email_user_ids=set(),
stream_push_user_ids=set(),
wildcard_mention_user_ids=set(),
followed_topic_email_user_ids=set(),
followed_topic_push_user_ids=set(),
)
self.assertEqual(user_data.is_notifiable(acting_user_id=1000, idle=True), notifiable)
def test_user_group_mentions_map(self) -> None:
hamlet = self.example_user("hamlet")
cordelia = self.example_user("cordelia")
realm = hamlet.realm
hamlet_only = check_add_user_group(realm, "hamlet_only", [hamlet], acting_user=None)
hamlet_and_cordelia = check_add_user_group(
realm, "hamlet_and_cordelia", [hamlet, cordelia], acting_user=None
)
mention_backend = MentionBackend(realm.id)
# Base case. No user/user group mentions
result = get_user_group_mentions_data(
mentioned_user_ids=set(),
mentioned_user_group_ids=[],
mention_data=MentionData(mention_backend, "no group mentioned"),
)
self.assertDictEqual(result, {})
# Only user group mentions, no personal mentions
result = get_user_group_mentions_data(
mentioned_user_ids=set(),
mentioned_user_group_ids=[hamlet_and_cordelia.id],
mention_data=MentionData(mention_backend, "hey @*hamlet_and_cordelia*!"),
)
self.assertDictEqual(
result,
{
hamlet.id: hamlet_and_cordelia.id,
cordelia.id: hamlet_and_cordelia.id,
},
)
# Hamlet is mentioned in two user groups
# Test that we consider the smaller user group
result = get_user_group_mentions_data(
mentioned_user_ids=set(),
mentioned_user_group_ids=[hamlet_and_cordelia.id, hamlet_only.id],
mention_data=MentionData(
mention_backend, "hey @*hamlet_and_cordelia* and @*hamlet_only*"
),
)
self.assertDictEqual(
result,
{
hamlet.id: hamlet_only.id,
cordelia.id: hamlet_and_cordelia.id,
},
)
# To make sure we aren't getting the expected data from over-writing in a loop,
# test the same setup as above, but with reversed group ids.
result = get_user_group_mentions_data(
mentioned_user_ids=set(),
mentioned_user_group_ids=[hamlet_only.id, hamlet_and_cordelia.id],
mention_data=MentionData(
mention_backend, "hey @*hamlet_only* and @*hamlet_and_cordelia*"
),
)
self.assertDictEqual(
result,
{
hamlet.id: hamlet_only.id,
cordelia.id: hamlet_and_cordelia.id,
},
)
# Personal and user group mentioned. Test that we don't consider the user
# group mention for Hamlet in this case.
result = get_user_group_mentions_data(
mentioned_user_ids={hamlet.id},
mentioned_user_group_ids=[hamlet_and_cordelia.id],
mention_data=MentionData(mention_backend, "hey @*hamlet_and_cordelia*!"),
)
self.assertDictEqual(
result,
{
cordelia.id: hamlet_and_cordelia.id,
},
)