2020-06-11 00:54:34 +02:00
|
|
|
import datetime
|
2021-06-18 14:16:16 +02:00
|
|
|
from typing import List
|
2020-06-11 00:54:34 +02:00
|
|
|
|
2017-04-15 04:03:56 +02:00
|
|
|
from django.utils.timezone import now as timezone_now
|
2018-04-17 17:16:02 +02:00
|
|
|
|
2022-04-14 23:50:10 +02:00
|
|
|
from zerver.actions.message_send import get_active_presence_idle_user_ids
|
2020-06-11 00:54:34 +02:00
|
|
|
from zerver.lib.test_classes import ZulipTestCase
|
|
|
|
from zerver.models import (
|
|
|
|
Message,
|
|
|
|
UserPresence,
|
|
|
|
UserProfile,
|
|
|
|
bulk_get_huddle_user_ids,
|
|
|
|
get_huddle_user_ids,
|
|
|
|
)
|
2016-03-24 20:24:01 +01:00
|
|
|
|
2017-08-05 22:10:34 +02:00
|
|
|
|
2017-09-05 20:50:25 +02:00
|
|
|
class MissedMessageTest(ZulipTestCase):
|
2017-11-05 10:51:25 +01:00
|
|
|
def test_presence_idle_user_ids(self) -> None:
|
2017-09-05 20:50:25 +02:00
|
|
|
UserPresence.objects.all().delete()
|
|
|
|
|
2021-02-12 08:20:45 +01:00
|
|
|
sender = self.example_user("cordelia")
|
2017-09-05 20:50:25 +02:00
|
|
|
realm = sender.realm
|
2021-02-12 08:20:45 +01:00
|
|
|
hamlet = self.example_user("hamlet")
|
|
|
|
othello = self.example_user("othello")
|
notifications: Calculate PMs/mentions settings like other settings.
Previously, we checked for the `enable_offline_email_notifications` and
`enable_offline_push_notifications` settings (which determine whether the
user will receive notifications for PMs and mentions) just before sending
notifications. This has a few problem:
1. We do not have access to all the user settings in the notification
handlers (`handle_missedmessage_emails` and `handle_push_notifications`),
and therefore, we cannot correctly determine whether the notification should
be sent. Checks like the following which existed previously, will, for
example, incorrectly not send notifications even when stream email
notifications are enabled-
```
if not receives_offline_email_notifications(user_profile):
return
```
With this commit, we simply do not enqueue notifications if the "offline"
settings are disabled, which fixes that bug.
Additionally, this also fixes a bug with the "online push notifications"
feature, which was, if someone were to:
* turn off notifications for PMs and mentions (`enable_offline_push_notifications`)
* turn on stream push notifications (`enable_stream_push_notifications`)
* turn on "online push" (`enable_online_push_notifications`)
then, they would still receive notifications for PMs when online.
This isn't how the "online push enabled" feature is supposed to work;
it should only act as a wrapper around the other notification settings.
The buggy code was this in `handle_push_notifications`:
```
if not (
receives_offline_push_notifications(user_profile)
or receives_online_push_notifications(user_profile)
):
return
// send notifications
```
This commit removes that code, and extends our `notification_data.py` logic
to cover this case, along with tests.
2. The name for these settings is slightly misleading. They essentially
talk about "what to send notifications for" (PMs and mentions), and not
"when to send notifications" (offline). This commit improves this condition
by restricting the use of this term only to the database field, and using
clearer names everywhere else. This distinction will be important to have
non-confusing code when we implement multiple options for notifications
in the future as dropdown (never/when offline/when offline or online, etc).
3. We should ideally re-check all notification settings just before the
notifications are sent. This is especially important for email notifications,
which may be sent after a long time after the message was sent. We will
in the future add code to thoroughly re-check settings before sending
notifications in a clean manner, but temporarily not re-checking isn't
a terrible scenario either.
2021-07-14 15:34:01 +02:00
|
|
|
|
|
|
|
hamlet_notifications_data = self.create_user_notifications_data_object(user_id=hamlet.id)
|
|
|
|
othello_notifications_data = self.create_user_notifications_data_object(user_id=othello.id)
|
2017-09-05 20:50:25 +02:00
|
|
|
|
2022-12-20 10:22:06 +01:00
|
|
|
def assert_active_presence_idle_user_ids(user_ids: List[int]) -> None:
|
2017-10-07 17:59:19 +02:00
|
|
|
presence_idle_user_ids = get_active_presence_idle_user_ids(
|
2017-09-05 20:50:25 +02:00
|
|
|
realm=realm,
|
|
|
|
sender_id=sender.id,
|
2022-12-20 10:55:03 +01:00
|
|
|
user_notifications_data_list=[
|
|
|
|
hamlet_notifications_data,
|
|
|
|
othello_notifications_data,
|
notifications: Calculate PMs/mentions settings like other settings.
Previously, we checked for the `enable_offline_email_notifications` and
`enable_offline_push_notifications` settings (which determine whether the
user will receive notifications for PMs and mentions) just before sending
notifications. This has a few problem:
1. We do not have access to all the user settings in the notification
handlers (`handle_missedmessage_emails` and `handle_push_notifications`),
and therefore, we cannot correctly determine whether the notification should
be sent. Checks like the following which existed previously, will, for
example, incorrectly not send notifications even when stream email
notifications are enabled-
```
if not receives_offline_email_notifications(user_profile):
return
```
With this commit, we simply do not enqueue notifications if the "offline"
settings are disabled, which fixes that bug.
Additionally, this also fixes a bug with the "online push notifications"
feature, which was, if someone were to:
* turn off notifications for PMs and mentions (`enable_offline_push_notifications`)
* turn on stream push notifications (`enable_stream_push_notifications`)
* turn on "online push" (`enable_online_push_notifications`)
then, they would still receive notifications for PMs when online.
This isn't how the "online push enabled" feature is supposed to work;
it should only act as a wrapper around the other notification settings.
The buggy code was this in `handle_push_notifications`:
```
if not (
receives_offline_push_notifications(user_profile)
or receives_online_push_notifications(user_profile)
):
return
// send notifications
```
This commit removes that code, and extends our `notification_data.py` logic
to cover this case, along with tests.
2. The name for these settings is slightly misleading. They essentially
talk about "what to send notifications for" (PMs and mentions), and not
"when to send notifications" (offline). This commit improves this condition
by restricting the use of this term only to the database field, and using
clearer names everywhere else. This distinction will be important to have
non-confusing code when we implement multiple options for notifications
in the future as dropdown (never/when offline/when offline or online, etc).
3. We should ideally re-check all notification settings just before the
notifications are sent. This is especially important for email notifications,
which may be sent after a long time after the message was sent. We will
in the future add code to thoroughly re-check settings before sending
notifications in a clean manner, but temporarily not re-checking isn't
a terrible scenario either.
2021-07-14 15:34:01 +02:00
|
|
|
],
|
2017-09-05 20:50:25 +02:00
|
|
|
)
|
2017-10-07 17:59:19 +02:00
|
|
|
self.assertEqual(sorted(user_ids), sorted(presence_idle_user_ids))
|
2017-09-05 20:50:25 +02:00
|
|
|
|
2020-02-08 18:46:27 +01:00
|
|
|
def set_presence(user: UserProfile, client_name: str, ago: int) -> None:
|
2017-09-05 20:50:25 +02:00
|
|
|
when = timezone_now() - datetime.timedelta(seconds=ago)
|
2020-06-11 16:03:47 +02:00
|
|
|
UserPresence.objects.update_or_create(
|
2020-02-08 18:46:27 +01:00
|
|
|
user_profile_id=user.id,
|
2020-06-11 16:03:47 +02:00
|
|
|
defaults=dict(
|
|
|
|
realm_id=user.realm_id, last_active_time=when, last_connected_time=when
|
|
|
|
),
|
2017-09-05 20:50:25 +02:00
|
|
|
)
|
|
|
|
|
notifications: Calculate PMs/mentions settings like other settings.
Previously, we checked for the `enable_offline_email_notifications` and
`enable_offline_push_notifications` settings (which determine whether the
user will receive notifications for PMs and mentions) just before sending
notifications. This has a few problem:
1. We do not have access to all the user settings in the notification
handlers (`handle_missedmessage_emails` and `handle_push_notifications`),
and therefore, we cannot correctly determine whether the notification should
be sent. Checks like the following which existed previously, will, for
example, incorrectly not send notifications even when stream email
notifications are enabled-
```
if not receives_offline_email_notifications(user_profile):
return
```
With this commit, we simply do not enqueue notifications if the "offline"
settings are disabled, which fixes that bug.
Additionally, this also fixes a bug with the "online push notifications"
feature, which was, if someone were to:
* turn off notifications for PMs and mentions (`enable_offline_push_notifications`)
* turn on stream push notifications (`enable_stream_push_notifications`)
* turn on "online push" (`enable_online_push_notifications`)
then, they would still receive notifications for PMs when online.
This isn't how the "online push enabled" feature is supposed to work;
it should only act as a wrapper around the other notification settings.
The buggy code was this in `handle_push_notifications`:
```
if not (
receives_offline_push_notifications(user_profile)
or receives_online_push_notifications(user_profile)
):
return
// send notifications
```
This commit removes that code, and extends our `notification_data.py` logic
to cover this case, along with tests.
2. The name for these settings is slightly misleading. They essentially
talk about "what to send notifications for" (PMs and mentions), and not
"when to send notifications" (offline). This commit improves this condition
by restricting the use of this term only to the database field, and using
clearer names everywhere else. This distinction will be important to have
non-confusing code when we implement multiple options for notifications
in the future as dropdown (never/when offline/when offline or online, etc).
3. We should ideally re-check all notification settings just before the
notifications are sent. This is especially important for email notifications,
which may be sent after a long time after the message was sent. We will
in the future add code to thoroughly re-check settings before sending
notifications in a clean manner, but temporarily not re-checking isn't
a terrible scenario either.
2021-07-14 15:34:01 +02:00
|
|
|
hamlet_notifications_data.pm_push_notify = True
|
|
|
|
othello_notifications_data.pm_push_notify = True
|
2022-12-20 10:22:06 +01:00
|
|
|
assert_active_presence_idle_user_ids([hamlet.id, othello.id])
|
2017-09-05 20:50:25 +02:00
|
|
|
|
2021-06-18 14:16:16 +02:00
|
|
|
# We have already thoroughly tested the `is_notifiable` function elsewhere,
|
|
|
|
# so we needn't test all cases here. This test exists mainly to avoid a bug
|
|
|
|
# which existed earlier with `get_active_presence_idle_user_ids` only looking
|
|
|
|
# at `private_message` and the `mentioned` flag, not stream level notifications.
|
|
|
|
# Simulate Hamlet has turned on notifications for the stream, and test that he's
|
|
|
|
# in the list.
|
notifications: Calculate PMs/mentions settings like other settings.
Previously, we checked for the `enable_offline_email_notifications` and
`enable_offline_push_notifications` settings (which determine whether the
user will receive notifications for PMs and mentions) just before sending
notifications. This has a few problem:
1. We do not have access to all the user settings in the notification
handlers (`handle_missedmessage_emails` and `handle_push_notifications`),
and therefore, we cannot correctly determine whether the notification should
be sent. Checks like the following which existed previously, will, for
example, incorrectly not send notifications even when stream email
notifications are enabled-
```
if not receives_offline_email_notifications(user_profile):
return
```
With this commit, we simply do not enqueue notifications if the "offline"
settings are disabled, which fixes that bug.
Additionally, this also fixes a bug with the "online push notifications"
feature, which was, if someone were to:
* turn off notifications for PMs and mentions (`enable_offline_push_notifications`)
* turn on stream push notifications (`enable_stream_push_notifications`)
* turn on "online push" (`enable_online_push_notifications`)
then, they would still receive notifications for PMs when online.
This isn't how the "online push enabled" feature is supposed to work;
it should only act as a wrapper around the other notification settings.
The buggy code was this in `handle_push_notifications`:
```
if not (
receives_offline_push_notifications(user_profile)
or receives_online_push_notifications(user_profile)
):
return
// send notifications
```
This commit removes that code, and extends our `notification_data.py` logic
to cover this case, along with tests.
2. The name for these settings is slightly misleading. They essentially
talk about "what to send notifications for" (PMs and mentions), and not
"when to send notifications" (offline). This commit improves this condition
by restricting the use of this term only to the database field, and using
clearer names everywhere else. This distinction will be important to have
non-confusing code when we implement multiple options for notifications
in the future as dropdown (never/when offline/when offline or online, etc).
3. We should ideally re-check all notification settings just before the
notifications are sent. This is especially important for email notifications,
which may be sent after a long time after the message was sent. We will
in the future add code to thoroughly re-check settings before sending
notifications in a clean manner, but temporarily not re-checking isn't
a terrible scenario either.
2021-07-14 15:34:01 +02:00
|
|
|
hamlet_notifications_data.pm_push_notify = False
|
|
|
|
othello_notifications_data.pm_push_notify = False
|
|
|
|
hamlet_notifications_data.stream_email_notify = True
|
2022-12-20 10:22:06 +01:00
|
|
|
assert_active_presence_idle_user_ids([hamlet.id])
|
2021-06-18 14:16:16 +02:00
|
|
|
|
2022-12-20 10:55:03 +01:00
|
|
|
# Hamlet is idle (and is supposed to receive stream notifications), so he should be in the list.
|
2021-02-12 08:20:45 +01:00
|
|
|
set_presence(hamlet, "iPhone", ago=5000)
|
2022-12-20 10:22:06 +01:00
|
|
|
assert_active_presence_idle_user_ids([hamlet.id])
|
2017-09-05 20:50:25 +02:00
|
|
|
|
2021-06-18 14:16:16 +02:00
|
|
|
# If Hamlet is active, don't include him in the `presence_idle` list.
|
2021-05-14 00:16:30 +02:00
|
|
|
set_presence(hamlet, "website", ago=15)
|
2022-12-20 10:22:06 +01:00
|
|
|
assert_active_presence_idle_user_ids([])
|
2017-09-05 20:50:25 +02:00
|
|
|
|
2021-06-18 14:16:16 +02:00
|
|
|
# Hamlet is active now, so only Othello should be in the list for a huddle
|
|
|
|
# message.
|
2022-12-20 10:55:03 +01:00
|
|
|
hamlet_notifications_data.stream_email_notify = False
|
notifications: Calculate PMs/mentions settings like other settings.
Previously, we checked for the `enable_offline_email_notifications` and
`enable_offline_push_notifications` settings (which determine whether the
user will receive notifications for PMs and mentions) just before sending
notifications. This has a few problem:
1. We do not have access to all the user settings in the notification
handlers (`handle_missedmessage_emails` and `handle_push_notifications`),
and therefore, we cannot correctly determine whether the notification should
be sent. Checks like the following which existed previously, will, for
example, incorrectly not send notifications even when stream email
notifications are enabled-
```
if not receives_offline_email_notifications(user_profile):
return
```
With this commit, we simply do not enqueue notifications if the "offline"
settings are disabled, which fixes that bug.
Additionally, this also fixes a bug with the "online push notifications"
feature, which was, if someone were to:
* turn off notifications for PMs and mentions (`enable_offline_push_notifications`)
* turn on stream push notifications (`enable_stream_push_notifications`)
* turn on "online push" (`enable_online_push_notifications`)
then, they would still receive notifications for PMs when online.
This isn't how the "online push enabled" feature is supposed to work;
it should only act as a wrapper around the other notification settings.
The buggy code was this in `handle_push_notifications`:
```
if not (
receives_offline_push_notifications(user_profile)
or receives_online_push_notifications(user_profile)
):
return
// send notifications
```
This commit removes that code, and extends our `notification_data.py` logic
to cover this case, along with tests.
2. The name for these settings is slightly misleading. They essentially
talk about "what to send notifications for" (PMs and mentions), and not
"when to send notifications" (offline). This commit improves this condition
by restricting the use of this term only to the database field, and using
clearer names everywhere else. This distinction will be important to have
non-confusing code when we implement multiple options for notifications
in the future as dropdown (never/when offline/when offline or online, etc).
3. We should ideally re-check all notification settings just before the
notifications are sent. This is especially important for email notifications,
which may be sent after a long time after the message was sent. We will
in the future add code to thoroughly re-check settings before sending
notifications in a clean manner, but temporarily not re-checking isn't
a terrible scenario either.
2021-07-14 15:34:01 +02:00
|
|
|
hamlet_notifications_data.pm_push_notify = False
|
|
|
|
othello_notifications_data.pm_push_notify = True
|
2022-12-20 10:22:06 +01:00
|
|
|
assert_active_presence_idle_user_ids([othello.id])
|
2017-09-05 20:50:25 +02:00
|
|
|
|
2021-02-12 08:19:30 +01:00
|
|
|
|
2019-08-13 23:05:47 +02:00
|
|
|
class TestBulkGetHuddleUserIds(ZulipTestCase):
|
|
|
|
def test_bulk_get_huddle_user_ids(self) -> None:
|
2021-02-12 08:20:45 +01:00
|
|
|
hamlet = self.example_user("hamlet")
|
|
|
|
cordelia = self.example_user("cordelia")
|
|
|
|
othello = self.example_user("othello")
|
|
|
|
iago = self.example_user("iago")
|
2019-08-13 23:05:47 +02:00
|
|
|
message_ids = [
|
2021-02-12 08:20:45 +01:00
|
|
|
self.send_huddle_message(hamlet, [cordelia, othello], "test"),
|
|
|
|
self.send_huddle_message(cordelia, [hamlet, othello, iago], "test"),
|
2019-08-13 23:05:47 +02:00
|
|
|
]
|
|
|
|
|
|
|
|
messages = Message.objects.filter(id__in=message_ids).order_by("id")
|
|
|
|
first_huddle_recipient = messages[0].recipient
|
2023-07-16 18:29:58 +02:00
|
|
|
first_huddle_user_ids = set(get_huddle_user_ids(first_huddle_recipient))
|
2019-08-13 23:05:47 +02:00
|
|
|
second_huddle_recipient = messages[1].recipient
|
2023-07-16 18:29:58 +02:00
|
|
|
second_huddle_user_ids = set(get_huddle_user_ids(second_huddle_recipient))
|
2019-08-13 23:05:47 +02:00
|
|
|
|
2021-02-12 08:19:30 +01:00
|
|
|
huddle_user_ids = bulk_get_huddle_user_ids(
|
2023-07-16 19:08:55 +02:00
|
|
|
[first_huddle_recipient.id, second_huddle_recipient.id]
|
2021-02-12 08:19:30 +01:00
|
|
|
)
|
2019-08-13 23:05:47 +02:00
|
|
|
self.assertEqual(huddle_user_ids[first_huddle_recipient.id], first_huddle_user_ids)
|
|
|
|
self.assertEqual(huddle_user_ids[second_huddle_recipient.id], second_huddle_user_ids)
|
|
|
|
|
|
|
|
def test_bulk_get_huddle_user_ids_empty_list(self) -> None:
|
|
|
|
self.assertEqual(bulk_get_huddle_user_ids([]), {})
|