diff --git a/static/js/admin.js b/static/js/admin.js index f7159130788f5c..505f950cd784c2 100644 --- a/static/js/admin.js +++ b/static/js/admin.js @@ -176,6 +176,7 @@ export function build_page() { realm_user_settings_defaults.enable_stream_audible_notifications, email_notifications_batching_period_values: settings_config.email_notifications_batching_period_values, + realm_name_in_notifications_values: settings_config.realm_name_in_notifications_values, twenty_four_hour_time_values: settings_config.twenty_four_hour_time_values, create_web_public_stream_policy_values: settings_config.create_web_public_stream_policy_values, 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..d19c051f8f522c 100644 --- a/static/js/settings_config.ts +++ b/static/js/settings_config.ts @@ -662,10 +662,7 @@ 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_settings = ["message_content_in_email_notifications"]; const other_email_settings = [ "enable_digest_emails", @@ -682,6 +679,7 @@ const other_notification_settings = desktop_notification_settings.concat( mobile_notification_settings, email_notification_settings, ["email_notifications_batching_period_seconds"], + ["realm_name_in_notifications"], ["notification_sound"], ); @@ -772,6 +770,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..cef284db509d6d 100644 --- a/static/templates/settings/notification_settings.hbs +++ b/static/templates/settings/notification_settings.hbs @@ -147,6 +147,13 @@ +
+ + +
+ {{#each notification_settings.email_message_notification_settings}} {{> settings_checkbox setting_name=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..96a92689e3ac89 100644 --- a/zerver/tests/test_email_notifications.py +++ b/zerver/tests/test_email_notifications.py @@ -16,6 +16,7 @@ from django.utils.timezone import now as timezone_now from django_auth_ldap.config import LDAPSearch +from zerver.actions.create_user import do_create_user from zerver.actions.user_settings import do_change_user_setting from zerver.actions.users import do_change_user_role from zerver.lib.email_notifications import ( @@ -24,6 +25,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 @@ -989,18 +991,75 @@ 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_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)) + + # 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. + # Create and verify a cross realm user. + cross_realm_user = do_create_user( + user.delivery_email, None, get_realm("lear"), user.full_name, acting_user=None + ) + self.assertEqual(cross_realm_user.delivery_email, user.delivery_email) + + self.assertTrue(realm_name_allowed_in_missedmessage_emails_subject(cross_realm_user)) + def test_realm_name_in_notifications(self) -> None: + hamlet = self.example_user("hamlet") + # Test with realm_name_in_notifications for hamlet disabled. - self._realm_name_in_missed_message_email_subject(False) + 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 enabled. - # Enable realm_name_in_notifications for hamlet and test again. + # Note: We don't need to test separately for 'realm_name_allowed_in_missedmessage_emails_subject' + # set to 'Automatic'. + # Here, we are concerned about the subject after the mocked function returns True/False. + # We already have separate test to check the appropriate behaviour of + # 'realm_name_allowed_in_missedmessage_emails_subject' for Automatic, Yes, No. hamlet = self.example_user("hamlet") - hamlet.realm_name_in_notifications = True + hamlet.realm_name_in_notifications = UserProfile.REALM_NAME_IN_NOTIFICATIONS_YES hamlet.save(update_fields=["realm_name_in_notifications"]) - - # Empty the test outbox - mail.outbox = [] - self._realm_name_in_missed_message_email_subject(True) + with mock.patch( + "zerver.lib.email_notifications.realm_name_allowed_in_missedmessage_emails_subject", + return_value=True, + ): + 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(