From 5e5538886f9d9ecb01ae3da473db1f863268796a Mon Sep 17 00:00:00 2001 From: Prakhar Pratyush Date: Wed, 17 May 2023 19:31:16 +0530 Subject: [PATCH] settings: Add email notifications for the followed topics. This commit makes it possible for users to control the email notifications for messages sent to followed topics via a global notification setting. Although there is no support for configuring this setting through the UI yet. Add five new fields to the UserBaseSettings class for the "followed topic notifications" feature, similar to stream notifications. But this commit consists only of the implementation of email notifications. --- api_docs/changelog.md | 7 +++ templates/zerver/emails/missed_message.html | 2 + .../zerver/emails/missed_message.subject.txt | 2 +- templates/zerver/emails/missed_message.txt | 2 + zerver/actions/message_edit.py | 1 + zerver/actions/message_send.py | 29 +++++++++ zerver/lib/email_notifications.py | 7 ++- zerver/lib/message.py | 2 + zerver/lib/notification_data.py | 14 ++++- zerver/lib/stream_subscription.py | 10 ++- zerver/lib/test_classes.py | 1 + .../0331_scheduledmessagenotificationemail.py | 1 + .../0453_followed_topic_notifications.py | 62 +++++++++++++++++++ zerver/models.py | 10 +++ zerver/openapi/zulip.yaml | 30 +++++++++ zerver/tests/test_email_notifications.py | 20 ++++++ zerver/tests/test_event_queue.py | 50 +++++++++++++++ zerver/tests/test_events.py | 21 +++++-- zerver/tests/test_notification_data.py | 11 ++++ zerver/tests/test_soft_deactivation.py | 1 + zerver/tests/test_users.py | 34 ++++++++++ zerver/tornado/event_queue.py | 11 +++- zerver/views/realm.py | 3 + zerver/views/user_settings.py | 3 + 24 files changed, 324 insertions(+), 10 deletions(-) create mode 100644 zerver/migrations/0453_followed_topic_notifications.py diff --git a/api_docs/changelog.md b/api_docs/changelog.md index 10b9919ef7..20b9a70b51 100644 --- a/api_docs/changelog.md +++ b/api_docs/changelog.md @@ -20,6 +20,13 @@ format used by the Zulip server that they are interacting with. ## Changes in Zulip 8.0 +**Feature level 189** + +* [`PATCH /realm/user_settings_defaults`](/api/update-realm-user-settings-defaults), + [`POST /register`](/api/register-queue), [`PATCH /settings`](/api/update-settings): + Added `enable_followed_topic_email_notifications` field to control + email notifications for messages sent to followed topics. + **Feature level 188** * [`POST /users/me/muted_users/{muted_user_id}`](/api/mute-user), diff --git a/templates/zerver/emails/missed_message.html b/templates/zerver/emails/missed_message.html index 53dbfe45fb..37adba30e1 100644 --- a/templates/zerver/emails/missed_message.html +++ b/templates/zerver/emails/missed_message.html @@ -33,6 +33,8 @@ {% trans %}You are receiving this because everyone was mentioned in #{{ stream_name }}.{% endtrans %}
{% elif stream_email_notify %} {% trans %}You are receiving this because you have email notifications enabled for #{{ stream_name }}.{% endtrans %}
+ {% elif followed_topic_email_notify %} + {% trans %}You are receiving this because you have email notifications enabled for topics you follow.{% endtrans %}
{% endif %} {% if reply_to_zulip %} {% trans notif_url=realm_uri + "/#settings/notifications" %}Reply to this email directly, view it in {{ realm_name }} Zulip, or manage email preferences.{% endtrans %} diff --git a/templates/zerver/emails/missed_message.subject.txt b/templates/zerver/emails/missed_message.subject.txt index ac2326559c..6ad8e8c3db 100644 --- a/templates/zerver/emails/missed_message.subject.txt +++ b/templates/zerver/emails/missed_message.subject.txt @@ -1,7 +1,7 @@ {% if show_message_content %} {% if group_pm %} {% trans %}Group DMs with {{ huddle_display_name }}{% endtrans %} {% elif private_message %} {% trans %}DMs with {{ sender_str }}{% endtrans %} - {% elif stream_email_notify or mention %} + {% elif stream_email_notify or mention or followed_topic_email_notify %} {# Some email clients, like Gmail Web (as of 2022), will auto-thread emails that share a subject and recipients, but will disregard diff --git a/templates/zerver/emails/missed_message.txt b/templates/zerver/emails/missed_message.txt index 56bc427c5c..b0319647e9 100644 --- a/templates/zerver/emails/missed_message.txt +++ b/templates/zerver/emails/missed_message.txt @@ -29,6 +29,8 @@ See {{ alert_notif_url }} for more details. {% trans %}You are receiving this because everyone was mentioned in #{{ stream_name }}.{% endtrans %} {% elif stream_email_notify %} {% trans %}You are receiving this because you have email notifications enabled for #{{ stream_name }}.{% endtrans %} +{% elif followed_topic_email_notify %} +{% trans %}You are receiving this because you have email notifications enabled for topics you follow.{% endtrans %} {% endif %} {% if reply_to_zulip %} diff --git a/zerver/actions/message_edit.py b/zerver/actions/message_edit.py index 1a7d19eb4a..fc74427a25 100644 --- a/zerver/actions/message_edit.py +++ b/zerver/actions/message_edit.py @@ -473,6 +473,7 @@ def do_update_message( event["pm_mention_email_disabled_user_ids"] = list(info.pm_mention_email_disabled_user_ids) event["stream_push_user_ids"] = list(info.stream_push_user_ids) event["stream_email_user_ids"] = list(info.stream_email_user_ids) + event["followed_topic_email_user_ids"] = list(info.followed_topic_email_user_ids) event["muted_sender_user_ids"] = list(info.muted_sender_user_ids) event["prior_mention_user_ids"] = list(prior_mention_user_ids) event["presence_idle_user_ids"] = filter_presence_idle_user_ids(info.active_user_ids) diff --git a/zerver/actions/message_send.py b/zerver/actions/message_send.py index a7be30d2d8..01fbcd3b0a 100644 --- a/zerver/actions/message_send.py +++ b/zerver/actions/message_send.py @@ -166,6 +166,7 @@ class RecipientInfoResult: stream_email_user_ids: Set[int] stream_push_user_ids: Set[int] wildcard_mention_user_ids: Set[int] + followed_topic_email_user_ids: Set[int] muted_sender_user_ids: Set[int] um_eligible_user_ids: Set[int] long_term_idle_user_ids: Set[int] @@ -196,6 +197,7 @@ def get_recipient_info( stream_push_user_ids: Set[int] = set() stream_email_user_ids: Set[int] = set() wildcard_mention_user_ids: Set[int] = set() + followed_topic_email_user_ids: Set[int] = set() muted_sender_user_ids: Set[int] = get_muting_users(sender_id) if recipient.type == Recipient.PERSONAL: @@ -214,6 +216,7 @@ def get_recipient_info( get_subscriptions_for_send_message( realm_id=realm_id, stream_id=stream_topic.stream_id, + topic_name=stream_topic.topic_name, possible_wildcard_mention=possible_wildcard_mention, possibly_mentioned_user_ids=possibly_mentioned_user_ids, ) @@ -223,12 +226,16 @@ def get_recipient_info( ), user_profile_push_notifications=F("user_profile__enable_stream_push_notifications"), user_profile_wildcard_mentions_notify=F("user_profile__wildcard_mentions_notify"), + followed_topic_email_notifications=F( + "user_profile__enable_followed_topic_email_notifications" + ), ) .values( "user_profile_id", "push_notifications", "email_notifications", "wildcard_mentions_notify", + "followed_topic_email_notifications", "user_profile_email_notifications", "user_profile_push_notifications", "user_profile_wildcard_mentions_notify", @@ -257,6 +264,21 @@ def get_recipient_info( stream_push_user_ids = notification_recipients("push_notifications") stream_email_user_ids = notification_recipients("email_notifications") + def followed_topic_notification_recipients(setting: str) -> Set[int]: + return { + row["user_profile_id"] + for row in subscription_rows + if user_id_to_visibility_policy.get( + row["user_profile_id"], UserTopic.VisibilityPolicy.INHERIT + ) + == UserTopic.VisibilityPolicy.FOLLOWED + and row["followed_topic_" + setting] + } + + followed_topic_email_user_ids = followed_topic_notification_recipients( + "email_notifications" + ) + if possible_wildcard_mention: # We calculate `wildcard_mention_user_ids` only if there's a possible # wildcard mention in the message. This is important so as to avoid @@ -381,6 +403,7 @@ def get_recipient_info( stream_push_user_ids=stream_push_user_ids, stream_email_user_ids=stream_email_user_ids, wildcard_mention_user_ids=wildcard_mention_user_ids, + followed_topic_email_user_ids=followed_topic_email_user_ids, muted_sender_user_ids=muted_sender_user_ids, um_eligible_user_ids=um_eligible_user_ids, long_term_idle_user_ids=long_term_idle_user_ids, @@ -564,6 +587,7 @@ def build_message_send_dict( pm_mention_push_disabled_user_ids=info.pm_mention_push_disabled_user_ids, stream_push_user_ids=info.stream_push_user_ids, stream_email_user_ids=info.stream_email_user_ids, + followed_topic_email_user_ids=info.followed_topic_email_user_ids, muted_sender_user_ids=info.muted_sender_user_ids, um_eligible_user_ids=info.um_eligible_user_ids, long_term_idle_user_ids=info.long_term_idle_user_ids, @@ -588,6 +612,7 @@ def create_user_messages( stream_push_user_ids: AbstractSet[int], stream_email_user_ids: AbstractSet[int], mentioned_user_ids: AbstractSet[int], + followed_topic_email_user_ids: AbstractSet[int], mark_as_read_user_ids: Set[int], limit_unread_user_ids: Optional[Set[int]], scheduled_message_to_self: bool, @@ -648,6 +673,7 @@ def create_user_messages( user_profile_id in long_term_idle_user_ids and user_profile_id not in stream_push_user_ids and user_profile_id not in stream_email_user_ids + and user_profile_id not in followed_topic_email_user_ids and is_stream_message and int(flags) == 0 ): @@ -761,6 +787,7 @@ def do_send_messages( stream_push_user_ids=send_request.stream_push_user_ids, stream_email_user_ids=send_request.stream_email_user_ids, mentioned_user_ids=mentioned_user_ids, + followed_topic_email_user_ids=send_request.followed_topic_email_user_ids, mark_as_read_user_ids=mark_as_read_user_ids, limit_unread_user_ids=send_request.limit_unread_user_ids, scheduled_message_to_self=scheduled_message_to_self, @@ -861,6 +888,7 @@ def do_send_messages( stream_push_user_ids=send_request.stream_push_user_ids, stream_email_user_ids=send_request.stream_email_user_ids, wildcard_mention_user_ids=send_request.wildcard_mention_user_ids, + followed_topic_email_user_ids=send_request.followed_topic_email_user_ids, muted_sender_user_ids=send_request.muted_sender_user_ids, all_bot_user_ids=send_request.all_bot_user_ids, ) @@ -886,6 +914,7 @@ def do_send_messages( stream_push_user_ids=list(send_request.stream_push_user_ids), stream_email_user_ids=list(send_request.stream_email_user_ids), wildcard_mention_user_ids=list(send_request.wildcard_mention_user_ids), + followed_topic_email_user_ids=list(send_request.followed_topic_email_user_ids), muted_sender_user_ids=list(send_request.muted_sender_user_ids), all_bot_user_ids=list(send_request.all_bot_user_ids), disable_external_notifications=send_request.disable_external_notifications, diff --git a/zerver/lib/email_notifications.py b/zerver/lib/email_notifications.py index b6d186912f..4fb9e55f37 100644 --- a/zerver/lib/email_notifications.py +++ b/zerver/lib/email_notifications.py @@ -457,6 +457,7 @@ def do_send_missedmessage_events_reply_in_zulip( personal_mentioned=personal_mentioned, wildcard_mentioned="wildcard_mentioned" in unique_triggers, stream_email_notify="stream_email_notify" in unique_triggers, + followed_topic_email_notify="followed_topic_email_notify" in unique_triggers, mention_count=triggers.count("mentioned") + triggers.count("wildcard_mentioned"), mentioned_user_group_name=mentioned_user_group_name, ) @@ -508,7 +509,11 @@ def do_send_missedmessage_events_reply_in_zulip( context.update(huddle_display_name=huddle_display_name) elif missed_messages[0]["message"].recipient.type == Recipient.PERSONAL: context.update(private_message=True) - elif context["mention"] or context["stream_email_notify"]: + elif ( + context["mention"] + or context["stream_email_notify"] + or context["followed_topic_email_notify"] + ): # Keep only the senders who actually mentioned the user if context["mention"]: senders = list( diff --git a/zerver/lib/message.py b/zerver/lib/message.py index f7bc7b521a..e53d1c8303 100644 --- a/zerver/lib/message.py +++ b/zerver/lib/message.py @@ -156,6 +156,8 @@ class SendMessageRequest: pm_mention_email_disabled_user_ids: Set[int] stream_push_user_ids: Set[int] stream_email_user_ids: Set[int] + # IDs of users who have followed the topic the message is being sent to, and have the followed topic email notifications setting ON. + followed_topic_email_user_ids: Set[int] muted_sender_user_ids: Set[int] um_eligible_user_ids: Set[int] long_term_idle_user_ids: Set[int] diff --git a/zerver/lib/notification_data.py b/zerver/lib/notification_data.py index 3946cbf939..901d2e0447 100644 --- a/zerver/lib/notification_data.py +++ b/zerver/lib/notification_data.py @@ -19,15 +19,20 @@ class UserMessageNotificationsData: wildcard_mention_push_notify: bool stream_push_notify: bool stream_email_notify: bool + followed_topic_email_notify: bool sender_is_muted: bool disable_external_notifications: bool def __post_init__(self) -> None: # Check that there's no dubious data. if self.pm_email_notify or self.pm_push_notify: - assert not (self.stream_email_notify or self.stream_push_notify) + assert not ( + self.stream_email_notify + or self.stream_push_notify + or self.followed_topic_email_notify + ) - if self.stream_email_notify or self.stream_push_notify: + if self.stream_email_notify or self.stream_push_notify or self.followed_topic_email_notify: assert not (self.pm_email_notify or self.pm_push_notify) @classmethod @@ -44,6 +49,7 @@ class UserMessageNotificationsData: stream_push_user_ids: Set[int], stream_email_user_ids: Set[int], wildcard_mention_user_ids: Set[int], + followed_topic_email_user_ids: Set[int], muted_sender_user_ids: Set[int], all_bot_user_ids: Set[int], ) -> "UserMessageNotificationsData": @@ -60,6 +66,7 @@ class UserMessageNotificationsData: online_push_enabled=False, stream_push_notify=False, stream_email_notify=False, + followed_topic_email_notify=False, sender_is_muted=False, disable_external_notifications=False, ) @@ -97,6 +104,7 @@ class UserMessageNotificationsData: online_push_enabled=(user_id in online_push_user_ids), stream_push_notify=(user_id in stream_push_user_ids), stream_email_notify=(user_id in stream_email_user_ids), + followed_topic_email_notify=(user_id in followed_topic_email_user_ids), sender_is_muted=(user_id in muted_sender_user_ids), disable_external_notifications=disable_external_notifications, ) @@ -168,6 +176,8 @@ class UserMessageNotificationsData: return NotificationTriggers.MENTION elif self.wildcard_mention_email_notify: return NotificationTriggers.WILDCARD_MENTION + elif self.followed_topic_email_notify: + return NotificationTriggers.FOLLOWED_TOPIC_EMAIL elif self.stream_email_notify: return NotificationTriggers.STREAM_EMAIL else: diff --git a/zerver/lib/stream_subscription.py b/zerver/lib/stream_subscription.py index a54dbd406d..86a725687a 100644 --- a/zerver/lib/stream_subscription.py +++ b/zerver/lib/stream_subscription.py @@ -7,7 +7,7 @@ from typing import AbstractSet, Any, Collection, Dict, List, Optional, Set from django.db.models import Q, QuerySet from django_stubs_ext import ValuesQuerySet -from zerver.models import AlertWord, Realm, Recipient, Stream, Subscription, UserProfile +from zerver.models import AlertWord, Realm, Recipient, Stream, Subscription, UserProfile, UserTopic @dataclass @@ -305,6 +305,7 @@ def get_subscriptions_for_send_message( *, realm_id: int, stream_id: int, + topic_name: str, possible_wildcard_mention: bool, possibly_mentioned_user_ids: AbstractSet[int], ) -> QuerySet[Subscription]: @@ -356,5 +357,12 @@ def get_subscriptions_for_send_message( "user_profile_id" ) ) + | Q( + user_profile_id__in=UserTopic.objects.filter( + stream_id=stream_id, + topic_name__iexact=topic_name, + visibility_policy=UserTopic.VisibilityPolicy.FOLLOWED, + ).values_list("user_profile_id") + ) ) return query diff --git a/zerver/lib/test_classes.py b/zerver/lib/test_classes.py index a68ea5f9d8..e43b8a21c9 100644 --- a/zerver/lib/test_classes.py +++ b/zerver/lib/test_classes.py @@ -1729,6 +1729,7 @@ Output: wildcard_mention_push_notify=kwargs.get("wildcard_mention_push_notify", False), stream_email_notify=kwargs.get("stream_email_notify", False), stream_push_notify=kwargs.get("stream_push_notify", False), + followed_topic_email_notify=kwargs.get("followed_topic_email_notify", False), sender_is_muted=kwargs.get("sender_is_muted", False), disable_external_notifications=kwargs.get("disable_external_notifications", False), ) diff --git a/zerver/migrations/0331_scheduledmessagenotificationemail.py b/zerver/migrations/0331_scheduledmessagenotificationemail.py index d75fe9703c..1051f54d75 100644 --- a/zerver/migrations/0331_scheduledmessagenotificationemail.py +++ b/zerver/migrations/0331_scheduledmessagenotificationemail.py @@ -28,6 +28,7 @@ class Migration(migrations.Migration): ("mentioned", "Mention"), ("wildcard_mentioned", "Wildcard mention"), ("stream_email_notify", "Stream notifications enabled"), + ("followed_topic_email_notify", "Followed topic notifications enabled"), ] ), ), diff --git a/zerver/migrations/0453_followed_topic_notifications.py b/zerver/migrations/0453_followed_topic_notifications.py new file mode 100644 index 0000000000..38fe3ea731 --- /dev/null +++ b/zerver/migrations/0453_followed_topic_notifications.py @@ -0,0 +1,62 @@ +# Generated by Django 4.2.1 on 2023-05-24 11:09 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("zerver", "0452_realmauditlog_extra_data_json"), + ] + + operations = [ + migrations.AddField( + model_name="realmuserdefault", + name="enable_followed_topic_audible_notifications", + field=models.BooleanField(default=True), + ), + migrations.AddField( + model_name="realmuserdefault", + name="enable_followed_topic_desktop_notifications", + field=models.BooleanField(default=True), + ), + migrations.AddField( + model_name="realmuserdefault", + name="enable_followed_topic_email_notifications", + field=models.BooleanField(default=True), + ), + migrations.AddField( + model_name="realmuserdefault", + name="enable_followed_topic_push_notifications", + field=models.BooleanField(default=True), + ), + migrations.AddField( + model_name="realmuserdefault", + name="enable_followed_topic_wildcard_mentions_notify", + field=models.BooleanField(default=True), + ), + migrations.AddField( + model_name="userprofile", + name="enable_followed_topic_audible_notifications", + field=models.BooleanField(default=True), + ), + migrations.AddField( + model_name="userprofile", + name="enable_followed_topic_desktop_notifications", + field=models.BooleanField(default=True), + ), + migrations.AddField( + model_name="userprofile", + name="enable_followed_topic_email_notifications", + field=models.BooleanField(default=True), + ), + migrations.AddField( + model_name="userprofile", + name="enable_followed_topic_push_notifications", + field=models.BooleanField(default=True), + ), + migrations.AddField( + model_name="userprofile", + name="enable_followed_topic_wildcard_mentions_notify", + field=models.BooleanField(default=True), + ), + ] diff --git a/zerver/models.py b/zerver/models.py index aaefe73326..12e6a1aa8e 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -1589,6 +1589,13 @@ class UserBaseSettings(models.Model): notification_sound = models.CharField(max_length=20, default="zulip") wildcard_mentions_notify = models.BooleanField(default=True) + # Followed Topics notifications. + enable_followed_topic_desktop_notifications = models.BooleanField(default=True) + enable_followed_topic_email_notifications = models.BooleanField(default=True) + enable_followed_topic_push_notifications = models.BooleanField(default=True) + enable_followed_topic_audible_notifications = models.BooleanField(default=True) + enable_followed_topic_wildcard_mentions_notify = models.BooleanField(default=True) + # PM + @-mention notifications. enable_desktop_notifications = models.BooleanField(default=True) pm_content_in_desktop_notifications = models.BooleanField(default=True) @@ -1716,6 +1723,7 @@ class UserBaseSettings(models.Model): modern_notification_settings: Dict[str, Any] = dict( # Add new notification settings here. + enable_followed_topic_email_notifications=bool, ) notification_setting_types = { @@ -4276,6 +4284,7 @@ class NotificationTriggers: WILDCARD_MENTION = "wildcard_mentioned" STREAM_PUSH = "stream_push_notify" STREAM_EMAIL = "stream_email_notify" + FOLLOWED_TOPIC_EMAIL = "followed_topic_email_notify" class ScheduledMessageNotificationEmail(models.Model): @@ -4293,6 +4302,7 @@ class ScheduledMessageNotificationEmail(models.Model): (NotificationTriggers.MENTION, "Mention"), (NotificationTriggers.WILDCARD_MENTION, "Wildcard mention"), (NotificationTriggers.STREAM_EMAIL, "Stream notifications enabled"), + (NotificationTriggers.FOLLOWED_TOPIC_EMAIL, "Followed topic notifications enabled"), ] trigger = models.TextField(choices=EMAIL_NOTIFICATION_TRIGGER_CHOICES) diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 0a691246fa..31c8777f30 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -9748,6 +9748,15 @@ paths: schema: type: boolean example: true + - name: enable_followed_topic_email_notifications + in: query + description: | + Enable email notifications for messages sent to followed topics. + + **Changes**: New in Zulip 8.0 (feature level 189). + schema: + type: boolean + example: true - name: email_notifications_batching_period_seconds in: query description: | @@ -11840,6 +11849,12 @@ paths: description: | Enable audible desktop notifications for private messages and @-mentions. + enable_followed_topic_email_notifications: + type: boolean + description: | + Enable email notifications for messages sent to followed topics. + + **Changes**: New in Zulip 8.0 (feature level 189). email_notifications_batching_period_seconds: type: integer description: | @@ -13900,6 +13915,12 @@ paths: description: | Enable mobile notification for private messages and @-mentions received when the user is online. + enable_followed_topic_email_notifications: + type: boolean + description: | + Enable email notifications for messages sent to followed topics. + + **Changes**: New in Zulip 8.0 (feature level 189). enable_digest_emails: type: boolean description: | @@ -15033,6 +15054,15 @@ paths: schema: type: boolean example: true + - name: enable_followed_topic_email_notifications + in: query + description: | + Enable email notifications for messages sent to followed topics. + + **Changes**: New in Zulip 8.0 (feature level 189). + schema: + type: boolean + example: true - name: enable_digest_emails in: query description: | diff --git a/zerver/tests/test_email_notifications.py b/zerver/tests/test_email_notifications.py index 452f410c08..9701e5e08c 100644 --- a/zerver/tests/test_email_notifications.py +++ b/zerver/tests/test_email_notifications.py @@ -1851,6 +1851,26 @@ class TestMissedMessages(ZulipTestCase): ], ) + def test_followed_topic_missed_message(self) -> None: + hamlet = self.example_user("hamlet") + othello = self.example_user("othello") + msg_id = self.send_stream_message(othello, "Denmark") + + handle_missedmessage_emails( + hamlet.id, + [ + {"message_id": msg_id, "trigger": "followed_topic_email_notify"}, + ], + ) + self.assert_length(mail.outbox, 1) + email_subject = mail.outbox[0].subject + email_body = mail.outbox[0].body + self.assertEqual("#Denmark > test", email_subject) + self.assertIn( + "You are receiving this because you have email notifications enabled for topics you follow.", + email_body, + ) + class TestFollowupEmailDelay(ZulipTestCase): def test_get_onboarding_email_schedule(self) -> None: diff --git a/zerver/tests/test_event_queue.py b/zerver/tests/test_event_queue.py index ae62e95081..24a4aed921 100644 --- a/zerver/tests/test_event_queue.py +++ b/zerver/tests/test_event_queue.py @@ -760,6 +760,56 @@ class MissedMessageHookTest(ZulipTestCase): already_notified={"email_notified": False, "push_notified": False}, ) + def test_followed_topic_email_notify(self) -> None: + # By default, messages sent in followed topics should send email notifications. + do_set_user_topic_visibility_policy( + self.user_profile, + get_stream("Denmark", self.user_profile.realm), + "followed_topic_test", + visibility_policy=UserTopic.VisibilityPolicy.FOLLOWED, + ) + msg_id = self.send_stream_message( + self.iago, "Denmark", content="what's up everyone?", topic_name="followed_topic_test" + ) + with mock.patch("zerver.tornado.event_queue.maybe_enqueue_notifications") as mock_enqueue: + missedmessage_hook(self.user_profile.id, self.client_descriptor, True) + mock_enqueue.assert_called_once() + args_dict = mock_enqueue.call_args_list[0][1] + + self.assert_maybe_enqueue_notifications_call_args( + args_dict=args_dict, + message_id=msg_id, + user_id=self.user_profile.id, + followed_topic_email_notify=True, + already_notified={"email_notified": True, "push_notified": False}, + ) + + def test_followed_topic_email_notify_global_setting(self) -> None: + do_change_user_setting( + self.user_profile, "enable_followed_topic_email_notifications", False, acting_user=None + ) + do_set_user_topic_visibility_policy( + self.user_profile, + get_stream("Denmark", self.user_profile.realm), + "followed_topic_test", + visibility_policy=UserTopic.VisibilityPolicy.FOLLOWED, + ) + msg_id = self.send_stream_message( + self.iago, "Denmark", content="what's up everyone?", topic_name="followed_topic_test" + ) + with mock.patch("zerver.tornado.event_queue.maybe_enqueue_notifications") as mock_enqueue: + missedmessage_hook(self.user_profile.id, self.client_descriptor, True) + mock_enqueue.assert_called_once() + args_dict = mock_enqueue.call_args_list[0][1] + + self.assert_maybe_enqueue_notifications_call_args( + args_dict=args_dict, + message_id=msg_id, + user_id=self.user_profile.id, + followed_topic_email_notify=False, + already_notified={"email_notified": False, "push_notified": False}, + ) + def test_muted_sender(self) -> None: do_mute_user(self.user_profile, self.iago) msg_id = self.send_personal_message(self.iago, self.user_profile) diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index 712a4c4568..dfb3dbe973 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -1946,6 +1946,17 @@ class NormalActionsTest(BaseAction): self.user_profile, notification_setting, False, acting_user=self.user_profile ) + num_events = 2 + is_modern_notification_setting = ( + notification_setting in self.user_profile.modern_notification_settings + ) + if is_modern_notification_setting: + # The legacy event format is not sent for modern_notification_settings + # as it exists only for backwards-compatibility with + # clients that don't support the new user_settings event type. + # We only send the legacy event for settings added before Feature level 89. + num_events = 1 + for setting_value in [True, False]: events = self.verify_action( lambda: do_change_user_setting( @@ -1954,10 +1965,11 @@ class NormalActionsTest(BaseAction): setting_value, acting_user=self.user_profile, ), - num_events=2, + num_events=num_events, ) check_user_settings_update("events[0]", events[0]) - check_update_global_notifications("events[1]", events[1], setting_value) + if not is_modern_notification_setting: + check_update_global_notifications("events[1]", events[1], setting_value) # Also test with notification_settings_null=True events = self.verify_action( @@ -1969,10 +1981,11 @@ class NormalActionsTest(BaseAction): ), notification_settings_null=True, state_change_expected=False, - num_events=2, + num_events=num_events, ) check_user_settings_update("events[0]", events[0]) - check_update_global_notifications("events[1]", events[1], setting_value) + if not is_modern_notification_setting: + check_update_global_notifications("events[1]", events[1], setting_value) def test_change_presence_enabled(self) -> None: presence_enabled_setting = "presence_enabled" diff --git a/zerver/tests/test_notification_data.py b/zerver/tests/test_notification_data.py index f4f7dfa59a..4d52326bd0 100644 --- a/zerver/tests/test_notification_data.py +++ b/zerver/tests/test_notification_data.py @@ -182,6 +182,16 @@ class TestNotificationData(ZulipTestCase): ) 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 user_data = self.create_user_notifications_data_object( user_id=user_id, pm_email_notify=True @@ -267,6 +277,7 @@ class TestNotificationData(ZulipTestCase): stream_email_user_ids=set(), stream_push_user_ids=set(), wildcard_mention_user_ids=set(), + followed_topic_email_user_ids=set(), ) self.assertEqual(user_data.is_notifiable(acting_user_id=1000, idle=True), notifiable) diff --git a/zerver/tests/test_soft_deactivation.py b/zerver/tests/test_soft_deactivation.py index 12be027883..90684267fe 100644 --- a/zerver/tests/test_soft_deactivation.py +++ b/zerver/tests/test_soft_deactivation.py @@ -631,6 +631,7 @@ class SoftDeactivationMessageTest(ZulipTestCase): get_subscriptions_for_send_message( realm_id=realm_id, stream_id=stream_id, + topic_name=topic_name, possible_wildcard_mention=possible_wildcard_mention, possibly_mentioned_user_ids=possibly_mentioned_user_ids, ) diff --git a/zerver/tests/test_users.py b/zerver/tests/test_users.py index acc31ed048..6acc335fb1 100644 --- a/zerver/tests/test_users.py +++ b/zerver/tests/test_users.py @@ -1763,6 +1763,7 @@ class RecipientInfoTest(ZulipTestCase): stream_push_user_ids=set(), stream_email_user_ids=set(), wildcard_mention_user_ids=set(), + followed_topic_email_user_ids=set(), muted_sender_user_ids=set(), um_eligible_user_ids=all_user_ids, long_term_idle_user_ids=set(), @@ -1979,6 +1980,39 @@ class RecipientInfoTest(ZulipTestCase): self.assertEqual(info.default_bot_user_ids, {normal_bot.id}) self.assertEqual(info.all_bot_user_ids, {normal_bot.id, service_bot.id}) + # Now Hamlet follows the topic with the 'followed_topic_email_notifications' + # global setting enabled by default. + do_set_user_topic_visibility_policy( + hamlet, + stream, + topic_name, + visibility_policy=UserTopic.VisibilityPolicy.FOLLOWED, + ) + + info = get_recipient_info( + realm_id=realm.id, + recipient=recipient, + sender_id=hamlet.id, + stream_topic=stream_topic, + ) + self.assertEqual(info.followed_topic_email_user_ids, {hamlet.id}) + + # Omit Hamlet from followed_topic_email_user_ids + do_change_user_setting( + hamlet, + "enable_followed_topic_email_notifications", + False, + acting_user=None, + ) + + info = get_recipient_info( + realm_id=realm.id, + recipient=recipient, + sender_id=hamlet.id, + stream_topic=stream_topic, + ) + self.assertEqual(info.followed_topic_email_user_ids, set()) + def test_get_recipient_info_invalid_recipient_type(self) -> None: hamlet = self.example_user("hamlet") realm = hamlet.realm diff --git a/zerver/tornado/event_queue.py b/zerver/tornado/event_queue.py index ef666c89e6..cfbefd08c6 100644 --- a/zerver/tornado/event_queue.py +++ b/zerver/tornado/event_queue.py @@ -780,6 +780,7 @@ def missedmessage_hook( wildcard_mention_email_notify=internal_data.get("wildcard_mention_email_notify", False), stream_push_notify=internal_data.get("stream_push_notify", False), stream_email_notify=internal_data.get("stream_email_notify", False), + followed_topic_email_notify=internal_data.get("followed_topic_email_notify", False), # Since one is by definition idle, we don't need to check online_push_enabled online_push_enabled=False, disable_external_notifications=internal_data.get( @@ -933,6 +934,7 @@ def process_message_event( stream_push_user_ids = set(event_template.get("stream_push_user_ids", [])) stream_email_user_ids = set(event_template.get("stream_email_user_ids", [])) wildcard_mention_user_ids = set(event_template.get("wildcard_mention_user_ids", [])) + followed_topic_email_user_ids = set(event_template.get("followed_topic_email_user_ids", [])) muted_sender_user_ids = set(event_template.get("muted_sender_user_ids", [])) all_bot_user_ids = set(event_template.get("all_bot_user_ids", [])) disable_external_notifications = event_template.get("disable_external_notifications", False) @@ -983,6 +985,7 @@ def process_message_event( stream_push_user_ids=stream_push_user_ids, stream_email_user_ids=stream_email_user_ids, wildcard_mention_user_ids=wildcard_mention_user_ids, + followed_topic_email_user_ids=followed_topic_email_user_ids, muted_sender_user_ids=muted_sender_user_ids, all_bot_user_ids=all_bot_user_ids, ) @@ -1134,6 +1137,7 @@ def process_message_update_event( stream_push_user_ids = set(event_template.pop("stream_push_user_ids", [])) stream_email_user_ids = set(event_template.pop("stream_email_user_ids", [])) wildcard_mention_user_ids = set(event_template.pop("wildcard_mention_user_ids", [])) + followed_topic_email_user_ids = set(event_template.pop("followed_topic_email_user_ids", [])) muted_sender_user_ids = set(event_template.pop("muted_sender_user_ids", [])) all_bot_user_ids = set(event_template.pop("all_bot_user_ids", [])) disable_external_notifications = event_template.pop("disable_external_notifications", False) @@ -1194,6 +1198,7 @@ def process_message_update_event( stream_push_user_ids=stream_push_user_ids, stream_email_user_ids=stream_email_user_ids, wildcard_mention_user_ids=wildcard_mention_user_ids, + followed_topic_email_user_ids=followed_topic_email_user_ids, muted_sender_user_ids=muted_sender_user_ids, all_bot_user_ids=all_bot_user_ids, ) @@ -1266,7 +1271,11 @@ def maybe_enqueue_notifications_for_message_update( # without extending the UserMessage data model. return - if user_notifications_data.stream_push_notify or user_notifications_data.stream_email_notify: + if ( + user_notifications_data.stream_push_notify + or user_notifications_data.stream_email_notify + or user_notifications_data.followed_topic_email_notify + ): # Currently we assume that if this flag is set to True, then # the user already was notified about the earlier message, # so we short circuit. We may handle this more rigorously diff --git a/zerver/views/realm.py b/zerver/views/realm.py index 80fc40dae7..619b172a3c 100644 --- a/zerver/views/realm.py +++ b/zerver/views/realm.py @@ -426,6 +426,9 @@ def update_realm_user_settings_defaults( json_validator=check_bool, default=None ), wildcard_mentions_notify: Optional[bool] = REQ(json_validator=check_bool, default=None), + enable_followed_topic_email_notifications: Optional[bool] = REQ( + json_validator=check_bool, default=None + ), notification_sound: Optional[str] = REQ(default=None), enable_desktop_notifications: Optional[bool] = REQ(json_validator=check_bool, default=None), enable_sounds: Optional[bool] = REQ(json_validator=check_bool, default=None), diff --git a/zerver/views/user_settings.py b/zerver/views/user_settings.py index 371b33b193..2c96603f39 100644 --- a/zerver/views/user_settings.py +++ b/zerver/views/user_settings.py @@ -193,6 +193,9 @@ def json_change_settings( json_validator=check_bool, default=None ), wildcard_mentions_notify: Optional[bool] = REQ(json_validator=check_bool, default=None), + enable_followed_topic_email_notifications: Optional[bool] = REQ( + json_validator=check_bool, default=None + ), notification_sound: Optional[str] = REQ(default=None), enable_desktop_notifications: Optional[bool] = REQ(json_validator=check_bool, default=None), enable_sounds: Optional[bool] = REQ(json_validator=check_bool, default=None),