Commit Graph

19 Commits

Author SHA1 Message Date
Abhijeet Prasad Bodas ba0dd70904 user_topics: Introduce visibility policy enum subclass in models.py.
This commit gives more readable code than using the `VISIBILITY_POLICY`
prefix.
This commit does not alter the database schema.
2023-03-14 17:50:45 -07:00
Kartik Srivastava ce5d13f9b2 message_send: Handle notifications for UNMUTED topic in a muted stream.
This commit adds 'visibility_policy' as a
parameter to user_allows_notifications_in_StreamTopic
function.

This adds logic inside the user_allows_notifications_in_StreamTopic
function, to not return False when a stream is muted
but the topic is UNMUTED.

Adds a method `user_id_to_visibility_policy_dict`
to 'StreamTopicTarget' class to fetch
(user_id => visibility_policy) in single db query.

Co-authored-by: Kartik Srivastava <kaushiksri0908@gmail.com>
Co-authored-by: Prakhar Pratyush <prakhar841301@gmail.com>
2023-03-06 19:15:45 -08:00
Abhijeet Prasad Bodas 9bc49ea4be message_send: Extract out stream notification user-set calculation logic.
This will allow us to re-use this logic later, when we add support for
re-checking notification settings just before sending email/push
notifications to the user.

Also, since this is essentially part of the notifiability logic,
this better belongs to `notification_data.py` and this change will
hopefully reduce the reading complexity of the message-send codepath.
2023-02-10 22:47:30 -08:00
Anders Kaseorg df001db1a9 black: Reformat with Black 23.
Black 23 enforces some slightly more specific rules about empty line
counts and redundant parenthesis removal, but the result is still
compatible with Black 22.

(This does not actually upgrade our Python environment to Black 23
yet.)

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2023-02-02 10:40:13 -08:00
Prakhar Pratyush 1a400b21e7 notifications: Fix missed message email notifications of welcome bot.
A missed message email notification, where the message is the welcome
message sent by the welcome bot on account creation, get sent when
the user somehow not focuses the browser tab during account creation.

No missed message email or push notifications should be sent for the
messages generated by the welcome bot.

'internal_send_private_message' accepts a parameter
'disable_external_notifications' and is set to 'True' when the sender
is 'welcome bot'.

A check is introduced in `trivially_should_not_notify`, not to notify
if `disable_external_notifications` is true.

TestCases are updated to include the `disable_external_notifications`
check in the early (False) return patterns of `is_push_notifiable` and
`is_email_notifiable`.

One query reduced for both `test_create_user_with_multiple_streams`
and `test_register`.
Reason: When welcome bot sends message after user creation
`do_send_messages` calls `get_active_presence_idle_user_ids`,
`user_ids` in `get_active_presence_idle_user_ids` remains empty if
`disable_external_notifications` is true because `is_notifiable` returns
false.
`get_active_presence_idle_user_ids` calls `filter_presence_idle_user_ids`
and since the `user_ids` is empty, the query inside the function doesn't
get executed.

MissedMessageHookTest updated.

Fixes: #22884
2023-01-24 11:16:21 -08:00
Prakhar Pratyush 4595b5d132 notifications: Add separate function for `is_notifiable` trivial checks.
A separate function named `trivially_should_not_notify` is added which
extracts the common checks from `get_push_notification_trigger` and
`get_email_notification_trigger` which are users' notification settings
independent and thus don't depend on what type of notification (email/push)
it is.
2023-01-24 11:16:14 -08:00
Zixuan James Li d8101de34d notifications: Move user group mentions helpers together.
This refactored `get_mentioned_user_group_name` from
`zerver/lib/email_notifications.py` to
`zerver/lib/notification_data.py` just after
`get_user_group_mentions_data` to indicate the logical
similarity between them.

Signed-off-by: Zixuan James Li <359101898@qq.com>
2022-04-27 16:43:54 -07:00
Abhijeet Prasad Bodas 15e8717847 notifications: Don't enqueue notifications for bots.
This replaces the temporary (and testless) fix in
24b1439e93 with a more permanent
fix.

Instead of checking if the user is a bot just before
sending the notifications, we now just don't enqueue
notifications for bots. This is done by sending a list
of bot IDs to the event_queue code, just like other
lists which are used for creating NotificationData objects.

Credit @andersk for the test code in `test_notification_data.py`.
2022-01-03 09:55:06 -08:00
Abhijeet Prasad Bodas ac70a2d2e1 notifications: Fix unnecessary wildcard mention notifications.
This fixes a bug where email notifications were sent for wildcard
mentions even if the `enable_offline_email_notifications` setting was
turned off.
This was because the `notification_data` class incorrectly considered
`wildcard_mentions_notify` as an indeoendent setting, instead of a wrapper
around `enable_offline_email_notifications` and `enable_offline_push_notifications`.

Also add a test for this case.
2021-08-13 09:48:18 -07:00
Abhijeet Prasad Bodas de04f0ad67 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-28 13:55:25 -07:00
Abhijeet Prasad Bodas c3319a5231 notification_data: Create common source for trigger strings.
This reduces loose strings in the codebase, and allows us to not worry
about the exact naming (`stream_email_enabled` or `stream_emails_enabled`?)
and tense (`mentioned` or `mention`?).

Ideally this new class should have been in `lib/notification_data.py`,
which is our file for things like this. But, the next commit requires
using this data in `models.py`, and importing from `notification_data.py`
to `models.py` causes recursive imports.
2021-07-13 17:16:32 -07:00
Abhijeet Prasad Bodas 07d6ab9753 notification_data: Add `get_user_group_mentions_data` function.
We will use this later to display which user group was mentioned
in push and email notifications.

`mentioned_user_group_ids` is kept as a List (not Set) to ensure proper
test coverage of the function, since it depends on the order of iteration,
and we cannot change the order of iteration for a set (which we'll need
to do for proper testing).

Part of #13080.
2021-07-05 14:23:59 -07:00
Abhijeet Prasad Bodas 733e0ae75e notification_data: Rename `sender_id` -> `acting_user_id`.
This better shows the situation for message edits, where we use the same
class.
2021-06-25 08:54:00 -07:00
Abhijeet Prasad Bodas 1cf1d147aa event_queue: Move notification trigger logic to notification_data.
This removes some complexity from the event_queue module.
To avoid code duplication, we reduce the `is_notifiable` methods to
internally just call the `trigger` methods and check their return value.
2021-06-24 09:35:18 -07:00
Abhijeet Prasad Bodas dedc39f139 notifications_data: Rename `id` -> `user_id`.
We also make this a mandatory named argument for our test helper
for clarity.
2021-06-24 17:34:50 +05:30
Abhijeet Prasad Bodas 5c483e3b58 get_active_presence_idle_user_ids: Check notifiability more thoroughly.
* Have the `get_active_presence_idle_user_ids` function look at all the
user data, not just `private_message` and `mentioned`.
* Fix a couple of incorrect `missedmessage_hook` tests, which did not
catch the earlier behaviour.
* Add some comments to the tests for this function for clarity.
* Add a helper to create `UserMessageNotificationsData` objects from the
user ID lists. This will later help us deduplicate code in the event_queue
logic.

This fixes a bug which earlier existed, that if a user turned on stream
notifications, and received a message in that stream which did not mention
them, they wouldn't be in the `presence_idle_users` list, and hence would
never get notifications for that message.

Note that, after this commit, users might still not get notifications in
the above scenarios in some cases, because the downstream logic in the
notification queue consumers sometimes erroneously skips sending
notifications for stream messages.
2021-06-21 10:52:59 -07:00
Abhijeet Prasad Bodas aeb2ad5f46 notification_data: Annotate `flags` with `Collection`.
Since `flags` here could be iterated through multiple times
(to check for push/email notifiability), we use `Collection`.
Inspired by 871e73ab8f.

The other change here in the `event_queue` code is prep for using
the `UserMessageNotificationsData` class there.
2021-06-21 10:52:59 -07:00
Abhijeet Prasad Bodas 8182632d7e notification_data: Add methods to determine notifiability.
We will later consistently use these functions to check for notifiable
messages in the message send and event_queue code.

We have these functions accept the `sender_id` so that we can avoid the
`private_message = message["type"] == "private" and user_id != sender_id`
wizardy.
2021-06-21 10:52:59 -07:00
Abhijeet Prasad Bodas 951b49c048 message send: Introduce dataclass to wrap user-notifications variables.
We will in later commits, extend this class to contain methods
to determine if a message is notifiable or not, but for now
we only turn it into a dict and pass it on.
2021-06-15 12:30:31 -07:00