diff --git a/static/js/admin.js b/static/js/admin.js index f7159130788f5c..7585da3da81b07 100644 --- a/static/js/admin.js +++ b/static/js/admin.js @@ -88,6 +88,7 @@ function get_realm_level_notification_settings(options) { options.notification_settings = all_notifications_settings.settings; options.show_push_notifications_tooltip = all_notifications_settings.show_push_notifications_tooltip; + options.realm_name_in_notifications_values = settings_config.realm_name_in_notifications_values; } export function build_page() { diff --git a/static/js/realm_user_settings_defaults.ts b/static/js/realm_user_settings_defaults.ts index c3cfcb258e7125..881d56b18a2513 100644 --- a/static/js/realm_user_settings_defaults.ts +++ b/static/js/realm_user_settings_defaults.ts @@ -29,7 +29,7 @@ export type RealmDefaultSettings = { notification_sound: string; pm_content_in_desktop_notifications: boolean; presence_enabled: boolean; - realm_name_in_notifications: boolean; + realm_name_in_notifications: number; starred_message_counts: boolean; translate_emoticons: boolean; twenty_four_hour_time: boolean; diff --git a/static/js/settings.js b/static/js/settings.js index 8951c6750bf085..fd6f01880152e0 100644 --- a/static/js/settings.js +++ b/static/js/settings.js @@ -92,6 +92,7 @@ export function build_page() { notification_settings: settings_config.all_notifications(user_settings).settings, email_notifications_batching_period_values: settings_config.email_notifications_batching_period_values, + realm_name_in_notifications_values: settings_config.realm_name_in_notifications_values, desktop_icon_count_display_values: settings_config.desktop_icon_count_display_values, show_push_notifications_tooltip: settings_config.all_notifications(user_settings).show_push_notifications_tooltip, diff --git a/static/js/settings_config.ts b/static/js/settings_config.ts index 4121a62501af5c..afc5dfebfefeff 100644 --- a/static/js/settings_config.ts +++ b/static/js/settings_config.ts @@ -662,10 +662,13 @@ export const email_notifications_batching_period_values = [ }, ]; -const email_message_notification_settings = [ - "message_content_in_email_notifications", - "realm_name_in_notifications", -]; +const email_message_notification_checkbox_settings = ["message_content_in_email_notifications"]; + +const email_message_notification_dropdown_settings = ["realm_name_in_notifications"]; + +const email_message_notification_settings = email_message_notification_checkbox_settings.concat( + email_message_notification_dropdown_settings, +); const other_email_settings = [ "enable_digest_emails", @@ -734,7 +737,7 @@ export interface AllNotifications { settings: { desktop_notification_settings: string[]; mobile_notification_settings: string[]; - email_message_notification_settings: string[]; + email_message_notification_checkbox_settings: string[]; other_email_settings: string[]; }; show_push_notifications_tooltip: { @@ -763,7 +766,7 @@ export const all_notifications = (settings_object: Settings): AllNotifications = settings: { desktop_notification_settings, mobile_notification_settings, - email_message_notification_settings, + email_message_notification_checkbox_settings, other_email_settings, }, show_push_notifications_tooltip: { @@ -772,6 +775,21 @@ export const all_notifications = (settings_object: Settings): AllNotifications = }, }); +export const realm_name_in_notifications_values = { + automatic: { + code: 1, + description: $t({defaultMessage: "Automatic"}), + }, + yes: { + code: 2, + description: $t({defaultMessage: "Yes"}), + }, + no: { + code: 3, + description: $t({defaultMessage: "No"}), + }, +}; + export const desktop_icon_count_display_values = { messages: { code: 1, diff --git a/static/js/settings_notifications.js b/static/js/settings_notifications.js index e79fec1db16f62..f82f2ffd40e408 100644 --- a/static/js/settings_notifications.js +++ b/static/js/settings_notifications.js @@ -150,6 +150,11 @@ export function set_up(settings_panel) { settings_object.email_notifications_batching_period_seconds, ); + const $realm_name_in_notifications_dropdown = $container.find( + ".setting_realm_name_in_notifications", + ); + $realm_name_in_notifications_dropdown.val(settings_object.realm_name_in_notifications); + set_enable_digest_emails_visibility(settings_panel); if (for_realm_settings) { @@ -236,6 +241,10 @@ export function update_page(settings_panel) { $container.find(`.setting_${CSS.escape(setting)}`).val(settings_object[setting]); break; } + case "realm_name_in_notifications": { + $container.find(`.setting_${CSS.escape(setting)}`).val(settings_object[setting]); + break; + } default: { $container .find(`.${CSS.escape(setting)}`) diff --git a/static/js/user_settings.ts b/static/js/user_settings.ts index 513404cad59dbc..fedaff5ef2ae0e 100644 --- a/static/js/user_settings.ts +++ b/static/js/user_settings.ts @@ -35,7 +35,7 @@ export type UserSettings = (StreamNotificationSettings & PmNotificationSettings) notification_sound: string; pm_content_in_desktop_notifications: boolean; presence_enabled: boolean; - realm_name_in_notifications: boolean; + realm_name_in_notifications: number; user_list_style: number; starred_message_counts: boolean; translate_emoticons: boolean; diff --git a/static/templates/settings/notification_settings.hbs b/static/templates/settings/notification_settings.hbs index 7582af23fd0c69..996e2e0061b723 100644 --- a/static/templates/settings/notification_settings.hbs +++ b/static/templates/settings/notification_settings.hbs @@ -147,7 +147,14 @@ - {{#each notification_settings.email_message_notification_settings}} +
+ + +
+ + {{#each notification_settings.email_message_notification_checkbox_settings}} {{> settings_checkbox setting_name=this is_checked=(lookup ../settings_object this) diff --git a/templates/zerver/emails/missed_message.subject.txt b/templates/zerver/emails/missed_message.subject.txt index a474574fb727ee..5c2c54edcf69e6 100644 --- a/templates/zerver/emails/missed_message.subject.txt +++ b/templates/zerver/emails/missed_message.subject.txt @@ -16,5 +16,5 @@ {% else %} {% trans %}New messages{% endtrans %} {% endif %} -{% if realm_name_in_notifications %} [{{ realm_str }}] +{% if realm_name_allowed_in_missedmessage_emails_subject %} [{{ realm_str }}] {% endif %} diff --git a/zerver/lib/email_notifications.py b/zerver/lib/email_notifications.py index 9aca51e2b0c599..c33ee60b1107fa 100644 --- a/zerver/lib/email_notifications.py +++ b/zerver/lib/email_notifications.py @@ -371,6 +371,23 @@ def message_content_allowed_in_missedmessage_emails(user_profile: UserProfile) - ) +def realm_name_allowed_in_missedmessage_emails_subject(user_profile: UserProfile) -> bool: + # if 'user_profile.realm_name_in_notifications' set to 'automatic', + # return true only if user is part of multiple realms. + if ( + user_profile.realm_name_in_notifications + == UserProfile.REALM_NAME_IN_NOTIFICATIONS_AUTOMATIC + ): + realms_count = UserProfile.objects.filter( + delivery_email=user_profile.delivery_email, + is_active=True, + is_bot=False, + realm__deactivated=False, + ).count() + return realms_count > 1 + return user_profile.realm_name_in_notifications == UserProfile.REALM_NAME_IN_NOTIFICATIONS_YES + + @statsd_increment("missed_message_reminders") def do_send_missedmessage_events_reply_in_zulip( user_profile: UserProfile, missed_messages: List[Dict[str, Any]], message_count: int @@ -402,7 +419,9 @@ def do_send_missedmessage_events_reply_in_zulip( name=user_profile.full_name, message_count=message_count, unsubscribe_link=unsubscribe_link, - realm_name_in_notifications=user_profile.realm_name_in_notifications, + realm_name_allowed_in_missedmessage_emails_subject=realm_name_allowed_in_missedmessage_emails_subject( + user_profile + ), ) mentioned_user_group_name = get_mentioned_user_group_name(missed_messages, user_profile) diff --git a/zerver/migrations/0423_alter_realmuserdefault_realm_name_in_notifications_and_more.py b/zerver/migrations/0423_alter_realmuserdefault_realm_name_in_notifications_and_more.py new file mode 100644 index 00000000000000..3b72db444e8715 --- /dev/null +++ b/zerver/migrations/0423_alter_realmuserdefault_realm_name_in_notifications_and_more.py @@ -0,0 +1,72 @@ +# Generated by Django 4.1.5 on 2023-01-11 20:21 + +from django.db import migrations, models +from django.db.backends.postgresql.schema import BaseDatabaseSchemaEditor +from django.db.migrations.state import StateApps + +# We include a copy of this structure as it was at the time this +# migration was merged, since future should not impact the migration. +REALM_NAME_IN_NOTIFICATIONS_AUTOMATIC = 1 +REALM_NAME_IN_NOTIFICATIONS_YES = 2 +REALM_NAME_IN_NOTIFICATIONS_NO = 3 +REALM_NAME_IN_NOTIFICATIONS_CHOICES = [ + REALM_NAME_IN_NOTIFICATIONS_AUTOMATIC, + REALM_NAME_IN_NOTIFICATIONS_YES, + REALM_NAME_IN_NOTIFICATIONS_NO, +] + +# The value of 'realm_name_in_notifications' for those users +# who have manually changed the value of 'realm_name_in_notifications' as 'true' +# should be updated as 'Yes', not 'Automatic' +def update_realm_name_in_notifications_new_values( + apps: StateApps, schema_editor: BaseDatabaseSchemaEditor +) -> None: + UserProfile = apps.get_model("zerver", "UserProfile") + userprofiles = UserProfile.objects.filter(realm_name_in_notifications=True) + userprofiles.update(realm_name_in_notifications_new=REALM_NAME_IN_NOTIFICATIONS_YES) + + +def reverse_code(apps: StateApps, schema_editor: BaseDatabaseSchemaEditor) -> None: + UserProfile = apps.get_model("zerver", "UserProfile") + userprofiles = UserProfile.objects.filter( + realm_name_in_notifications_new=REALM_NAME_IN_NOTIFICATIONS_YES + ) + userprofiles.update(realm_name_in_notifications=True) + + +class Migration(migrations.Migration): + + dependencies = [ + ("zerver", "0422_multiuseinvite_status"), + ] + + operations = [ + migrations.RemoveField( + model_name="realmuserdefault", + name="realm_name_in_notifications", + ), + migrations.AddField( + model_name="realmuserdefault", + name="realm_name_in_notifications", + field=models.PositiveSmallIntegerField(default=REALM_NAME_IN_NOTIFICATIONS_AUTOMATIC), + ), + migrations.AddField( + model_name="userprofile", + name="realm_name_in_notifications_new", + field=models.PositiveSmallIntegerField(default=REALM_NAME_IN_NOTIFICATIONS_AUTOMATIC), + ), + migrations.RunPython( + update_realm_name_in_notifications_new_values, + reverse_code=reverse_code, + elidable=True, + ), + migrations.RemoveField( + model_name="userprofile", + name="realm_name_in_notifications", + ), + migrations.RenameField( + model_name="userprofile", + old_name="realm_name_in_notifications_new", + new_name="realm_name_in_notifications", + ), + ] diff --git a/zerver/models.py b/zerver/models.py index 9d900924f367c2..d0920ead6e5230 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -1596,9 +1596,20 @@ class UserBaseSettings(models.Model): enable_digest_emails = models.BooleanField(default=True) enable_login_emails = models.BooleanField(default=True) enable_marketing_emails = models.BooleanField(default=True) - realm_name_in_notifications = models.BooleanField(default=False) presence_enabled = models.BooleanField(default=True) + REALM_NAME_IN_NOTIFICATIONS_AUTOMATIC = 1 + REALM_NAME_IN_NOTIFICATIONS_YES = 2 + REALM_NAME_IN_NOTIFICATIONS_NO = 3 + REALM_NAME_IN_NOTIFICATIONS_CHOICES = [ + REALM_NAME_IN_NOTIFICATIONS_AUTOMATIC, + REALM_NAME_IN_NOTIFICATIONS_YES, + REALM_NAME_IN_NOTIFICATIONS_NO, + ] + realm_name_in_notifications = models.PositiveSmallIntegerField( + default=REALM_NAME_IN_NOTIFICATIONS_AUTOMATIC + ) + # Whether or not the user wants to sync their drafts. enable_drafts_synchronization = models.BooleanField(default=True) @@ -1647,7 +1658,7 @@ class UserBaseSettings(models.Model): notification_sound=str, pm_content_in_desktop_notifications=bool, presence_enabled=bool, - realm_name_in_notifications=bool, + realm_name_in_notifications=int, wildcard_mentions_notify=bool, ) diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 31070289fb4f31..5731cc6cc73b9b 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -9035,9 +9035,17 @@ paths: in: query description: | Include organization name in subject of message notification emails. + + - 1 - Automatic + - 2 - Yes + - 3 - No schema: - type: boolean - example: true + type: integer + enum: + - 1 + - 2 + - 3 + example: 1 - name: presence_enabled in: query description: | @@ -10983,9 +10991,13 @@ paths: - 2 - Private messages and mentions - 3 - None realm_name_in_notifications: - type: boolean + type: integer description: | Include organization name in subject of message notification emails. + + - 1 - Automatic + - 2 - Yes + - 3 - No presence_enabled: type: boolean description: | @@ -11377,7 +11389,7 @@ paths: client capability and access the `user_settings` object instead. realm_name_in_notifications: deprecated: true - type: boolean + type: integer description: | Present if `update_global_notifications` is present in `fetch_event_types` and only for clients that did not include `user_settings_object` in their @@ -12884,9 +12896,13 @@ paths: - 2 - Private messages and mentions - 3 - None realm_name_in_notifications: - type: boolean + type: integer description: | Include organization name in subject of message notification emails. + + - 1 - Automatic + - 2 - Yes + - 3 - No presence_enabled: type: boolean description: | @@ -14041,11 +14057,19 @@ paths: description: | Include organization name in subject of message notification emails. + - 1 - Automatic + - 2 - Yes + - 3 - No + **Changes**: Before Zulip 5.0 (feature level 80), this setting was managed by the `PATCH /settings/notifications` endpoint. schema: - type: boolean - example: true + type: integer + enum: + - 1 + - 2 + - 3 + example: 1 - name: presence_enabled in: query description: | diff --git a/zerver/tests/test_email_notifications.py b/zerver/tests/test_email_notifications.py index 9ea8bb645da55b..2ea6bd865b13b8 100644 --- a/zerver/tests/test_email_notifications.py +++ b/zerver/tests/test_email_notifications.py @@ -24,6 +24,7 @@ fix_spoilers_in_html, followup_day2_email_delay, handle_missedmessage_emails, + realm_name_allowed_in_missedmessage_emails_subject, relative_to_full_url, ) from zerver.lib.send_email import FromAddress, deliver_scheduled_emails, send_custom_email @@ -491,7 +492,7 @@ def _test_cases( self.assertEqual(msg.extra_headers["List-Id"], "Zulip Dev ") def _realm_name_in_missed_message_email_subject( - self, realm_name_in_notifications: bool + self, realm_name_allowed_in_notifications: bool ) -> None: msg_id = self.send_personal_message( self.example_user("othello"), @@ -501,7 +502,7 @@ def _realm_name_in_missed_message_email_subject( verify_body_include = ["Extremely personal message!"] email_subject = "PMs with Othello, the Moor of Venice" - if realm_name_in_notifications: + if realm_name_allowed_in_notifications: email_subject = "PMs with Othello, the Moor of Venice [Zulip Dev]" self._test_cases(msg_id, verify_body_include, email_subject, False) @@ -989,18 +990,70 @@ def test_wildcard_over_stream_mention_priority(self) -> None: for text in expected_email_include: self.assertIn(text, self.normalize_string(mail.outbox[0].body)) - def test_realm_name_in_notifications(self) -> None: - # Test with realm_name_in_notifications for hamlet disabled. - self._realm_name_in_missed_message_email_subject(False) + def test_realm_name_allowed_in_missedmessage_emails_subject(self) -> None: + user = self.example_user("hamlet") + + # Test with 'realm_name_in_notification' set to 'Yes' + do_change_user_setting( + user, + "realm_name_in_notifications", + UserProfile.REALM_NAME_IN_NOTIFICATIONS_YES, + acting_user=None, + ) + self.assertTrue(realm_name_allowed_in_missedmessage_emails_subject(user)) - # Enable realm_name_in_notifications for hamlet and test again. + # Test with 'realm_name_in_notification' set to 'No' + do_change_user_setting( + user, + "realm_name_in_notifications", + UserProfile.REALM_NAME_IN_NOTIFICATIONS_NO, + acting_user=None, + ) + self.assertFalse(realm_name_allowed_in_missedmessage_emails_subject(user)) + + # Test with 'realm_name_in_notification' set to 'Automatic' + do_change_user_setting( + user, + "realm_name_in_notifications", + UserProfile.REALM_NAME_IN_NOTIFICATIONS_AUTOMATIC, + acting_user=None, + ) + # Case 1: if user is part of a single realm, then realm_name is not present in notifications. + self.assertFalse(realm_name_allowed_in_missedmessage_emails_subject(user)) + + # Case 2: if user is part of multiple realms, then realm_name should be present in notifications. + # Welcome Bot is a cross realm user (bot). + # realm = get_realm("zulip") + # internal_realm = get_realm(settings.SYSTEM_BOT_REALM) + # bot = get_system_bot(settings.WELCOME_BOT, internal_realm.id) + + # # Verification for cross realm bot. + # cross_realm_bot = get_user_by_id_in_realm_including_cross_realm(bot.id, realm) + # self.assertEqual(cross_realm_bot.email, bot.email) + # self.assertEqual(cross_realm_bot.id, bot.id) + + # self.assertTrue(realm_name_allowed_in_missedmessage_emails_subject(cross_realm_bot)) + + def test_realm_name_in_notifications(self) -> None: hamlet = self.example_user("hamlet") - hamlet.realm_name_in_notifications = True - hamlet.save(update_fields=["realm_name_in_notifications"]) - # Empty the test outbox - mail.outbox = [] - self._realm_name_in_missed_message_email_subject(True) + # Test with realm_name_in_notifications for hamlet disabled. + with mock.patch( + "zerver.lib.email_notifications.realm_name_allowed_in_missedmessage_emails_subject", + return_value=False, + ): + is_allowed = realm_name_allowed_in_missedmessage_emails_subject(hamlet) + self._realm_name_in_missed_message_email_subject(is_allowed) + + # Test with realm_name_in_notifications for hamlet disabled. + with mock.patch( + "zerver.lib.email_notifications.realm_name_allowed_in_missedmessage_emails_subject", + return_value=False, + ): + is_allowed = realm_name_allowed_in_missedmessage_emails_subject(hamlet) + # Empty the test outbox + mail.outbox = [] + self._realm_name_in_missed_message_email_subject(is_allowed) def test_message_content_disabled_in_missed_message_notifications(self) -> None: # Test when user disabled message content in email notifications. diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index 415c9d5d1dbf1d..69e60527c8744f 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -1802,6 +1802,7 @@ def test_change_notification_settings(self) -> None: "notification_sound", "desktop_icon_count_display", "presence_enabled", + "realm_name_in_notifications", ]: # These settings are tested in their own tests. continue @@ -1891,6 +1892,27 @@ def test_change_desktop_icon_count_display(self) -> None: check_user_settings_update("events[0]", events[0]) check_update_global_notifications("events[1]", events[1], 1) + def test_change_realm_name_in_notifications(self) -> None: + notification_setting = "realm_name_in_notifications" + + events = self.verify_action( + lambda: do_change_user_setting( + self.user_profile, notification_setting, 3, acting_user=self.user_profile + ), + num_events=2, + ) + check_user_settings_update("events[0]", events[0]) + check_update_global_notifications("events[1]", events[1], 3) + + events = self.verify_action( + lambda: do_change_user_setting( + self.user_profile, notification_setting, 2, acting_user=self.user_profile + ), + num_events=2, + ) + check_user_settings_update("events[0]", events[0]) + check_update_global_notifications("events[1]", events[1], 2) + def test_realm_update_org_type(self) -> None: realm = self.user_profile.realm @@ -2665,6 +2687,7 @@ def do_set_realm_user_default_setting_test(self, name: str) -> None: desktop_icon_count_display=[1, 2, 3], notification_sound=["zulip", "ding"], email_notifications_batching_period_seconds=[120, 300], + realm_name_in_notifications=UserProfile.REALM_NAME_IN_NOTIFICATIONS_CHOICES, ) vals = test_values.get(name) diff --git a/zerver/tests/test_realm.py b/zerver/tests/test_realm.py index 5818ca88d455c9..5461b5a32700dd 100644 --- a/zerver/tests/test_realm.py +++ b/zerver/tests/test_realm.py @@ -1243,6 +1243,7 @@ def do_test_realm_default_setting_update_api(self, name: str) -> None: desktop_icon_count_display=[1, 2, 3], notification_sound=["zulip", "ding"], email_notifications_batching_period_seconds=[120, 300], + realm_name_in_notifications=UserProfile.REALM_NAME_IN_NOTIFICATIONS_CHOICES, ) vals = test_values.get(name) diff --git a/zerver/tests/test_settings.py b/zerver/tests/test_settings.py index 25da2ab209ea72..a91acbae82cd25 100644 --- a/zerver/tests/test_settings.py +++ b/zerver/tests/test_settings.py @@ -362,6 +362,7 @@ def do_test_change_user_setting(self, setting_name: str) -> None: email_notifications_batching_period_seconds=100, notification_sound="ding", desktop_icon_count_display=2, + realm_name_in_notifications=2, ) self.login("hamlet") diff --git a/zerver/views/realm.py b/zerver/views/realm.py index c50298d5799349..47a1b2b366d3d6 100644 --- a/zerver/views/realm.py +++ b/zerver/views/realm.py @@ -427,7 +427,9 @@ def update_realm_user_settings_defaults( desktop_icon_count_display: Optional[int] = REQ( json_validator=check_int_in(UserProfile.DESKTOP_ICON_COUNT_DISPLAY_CHOICES), default=None ), - realm_name_in_notifications: Optional[bool] = REQ(json_validator=check_bool, default=None), + realm_name_in_notifications: Optional[int] = REQ( + json_validator=check_int_in(UserProfile.REALM_NAME_IN_NOTIFICATIONS_CHOICES), default=None + ), presence_enabled: Optional[bool] = REQ(json_validator=check_bool, default=None), enter_sends: Optional[bool] = REQ(json_validator=check_bool, default=None), enable_drafts_synchronization: Optional[bool] = REQ(json_validator=check_bool, default=None), diff --git a/zerver/views/user_settings.py b/zerver/views/user_settings.py index c903205750a712..7050c9b16194e7 100644 --- a/zerver/views/user_settings.py +++ b/zerver/views/user_settings.py @@ -211,7 +211,9 @@ def json_change_settings( desktop_icon_count_display: Optional[int] = REQ( json_validator=check_int_in(UserProfile.DESKTOP_ICON_COUNT_DISPLAY_CHOICES), default=None ), - realm_name_in_notifications: Optional[bool] = REQ(json_validator=check_bool, default=None), + realm_name_in_notifications: Optional[int] = REQ( + json_validator=check_int_in(UserProfile.REALM_NAME_IN_NOTIFICATIONS_CHOICES), default=None + ), presence_enabled: Optional[bool] = REQ(json_validator=check_bool, default=None), enter_sends: Optional[bool] = REQ(json_validator=check_bool, default=None), send_private_typing_notifications: Optional[bool] = REQ(