From ae72777c77ecf808842ba56f38eb0caffc234635 Mon Sep 17 00:00:00 2001 From: Prakhar Pratyush Date: Sun, 15 Jan 2023 01:06:37 +0530 Subject: [PATCH] user_settings: Automate 'Include realm name in message email subject'. Currently, there is a checkbox setting for whether to "Include realm name in subject of message notification emails". This commit replaces the checkbox setting with a dropdown having values: Automatic [default], Always, Never. The Automatic option includes the realm name if, and only if, there are multiple Zulip realms associated with the user's email. Tests are added and(or) modified. Fixes: #19905. --- api_docs/changelog.md | 7 ++ .../zerver/emails/missed_message.subject.txt | 2 +- version.py | 2 +- web/src/admin.js | 2 + web/src/realm_user_settings_defaults.ts | 2 +- web/src/settings.js | 2 + web/src/settings_config.ts | 23 +++-- web/src/settings_notifications.js | 10 ++- web/src/user_settings.ts | 2 +- .../settings/notification_settings.hbs | 7 ++ zerver/lib/email_notifications.py | 26 +++++- ...and_migrate_realm_name_in_notifications.py | 89 +++++++++++++++++++ zerver/models.py | 15 +++- zerver/openapi/zulip.yaml | 84 ++++++++++++----- zerver/tests/test_email_notifications.py | 82 +++++++++++++++-- zerver/tests/test_events.py | 23 +++++ zerver/tests/test_realm.py | 1 + zerver/tests/test_settings.py | 1 + zerver/views/realm.py | 5 +- zerver/views/user_settings.py | 5 +- 20 files changed, 346 insertions(+), 44 deletions(-) create mode 100644 zerver/migrations/0432_alter_and_migrate_realm_name_in_notifications.py diff --git a/api_docs/changelog.md b/api_docs/changelog.md index c73a444a25..dc98cd0c87 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 7.0 +**Feature level 168** + +* [`PATCH /realm/user_settings_defaults`](/api/update-realm-user-settings-defaults), + [`POST /register`](/api/register-queue), + [`PATCH /settings`](/api/update-settings): Replaced the `realm_name_in_notifications` + boolean field with an integer field `realm_name_in_email_notifications_policy`. + **Feature level 167** * [All REST API endpoints](/api/rest-error-handling#ignored-parameters): diff --git a/templates/zerver/emails/missed_message.subject.txt b/templates/zerver/emails/missed_message.subject.txt index 649f35e9bd..ac2326559c 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 include_realm_name_in_missedmessage_emails_subject %} [{{ realm_str }}] {% endif %} diff --git a/version.py b/version.py index 6420979846..1473485994 100644 --- a/version.py +++ b/version.py @@ -33,7 +33,7 @@ DESKTOP_WARNING_VERSION = "5.4.3" # Changes should be accompanied by documentation explaining what the # new level means in api_docs/changelog.md, as well as "**Changes**" # entries in the endpoint's documentation in `zulip.yaml`. -API_FEATURE_LEVEL = 167 +API_FEATURE_LEVEL = 168 # Bump the minor PROVISION_VERSION to indicate that folks should provision # only when going from an old version of the code to a newer version. Bump diff --git a/web/src/admin.js b/web/src/admin.js index f1eb4849e9..ec30d1cdcd 100644 --- a/web/src/admin.js +++ b/web/src/admin.js @@ -181,6 +181,8 @@ 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_email_notifications_policy_values: + settings_config.realm_name_in_email_notifications_policy_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/web/src/realm_user_settings_defaults.ts b/web/src/realm_user_settings_defaults.ts index c3cfcb258e..8172b47512 100644 --- a/web/src/realm_user_settings_defaults.ts +++ b/web/src/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_email_notifications_policy: number; starred_message_counts: boolean; translate_emoticons: boolean; twenty_four_hour_time: boolean; diff --git a/web/src/settings.js b/web/src/settings.js index c19b7c0d48..a0940241a6 100644 --- a/web/src/settings.js +++ b/web/src/settings.js @@ -95,6 +95,8 @@ 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_email_notifications_policy_values: + settings_config.realm_name_in_email_notifications_policy_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/web/src/settings_config.ts b/web/src/settings_config.ts index 7cfc9f356b..deea0bc776 100644 --- a/web/src/settings_config.ts +++ b/web/src/settings_config.ts @@ -541,7 +541,7 @@ export const notification_settings_labels = { message_content_in_email_notifications: $t({ defaultMessage: "Include message content in message notification emails", }), - realm_name_in_notifications: $t({ + realm_name_in_email_notifications_policy: $t({ defaultMessage: "Include organization name in subject of message notification emails", }), }; @@ -677,10 +677,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", @@ -699,6 +696,7 @@ const other_notification_settings = [ ...mobile_notification_settings, ...email_notification_settings, "email_notifications_batching_period_seconds", + "realm_name_in_email_notifications_policy", "notification_sound", ]; @@ -790,6 +788,21 @@ export const all_notifications = (settings_object: Settings): AllNotifications = }, }); +export const realm_name_in_email_notifications_policy_values = { + automatic: { + code: 1, + description: $t({defaultMessage: "Automatic"}), + }, + always: { + code: 2, + description: $t({defaultMessage: "Always"}), + }, + never: { + code: 3, + description: $t({defaultMessage: "Never"}), + }, +}; + export const desktop_icon_count_display_values = { messages: { code: 1, diff --git a/web/src/settings_notifications.js b/web/src/settings_notifications.js index e79fec1db1..d11f30606a 100644 --- a/web/src/settings_notifications.js +++ b/web/src/settings_notifications.js @@ -150,6 +150,13 @@ export function set_up(settings_panel) { settings_object.email_notifications_batching_period_seconds, ); + const $realm_name_in_email_notifications_policy_dropdown = $container.find( + ".setting_realm_name_in_email_notifications_policy", + ); + $realm_name_in_email_notifications_policy_dropdown.val( + settings_object.realm_name_in_email_notifications_policy, + ); + set_enable_digest_emails_visibility(settings_panel); if (for_realm_settings) { @@ -232,7 +239,8 @@ export function update_page(settings_panel) { set_notification_batching_ui($container, settings_object[setting]); break; } - case "notification_sound": { + case "notification_sound": + case "realm_name_in_email_notifications_policy": { $container.find(`.setting_${CSS.escape(setting)}`).val(settings_object[setting]); break; } diff --git a/web/src/user_settings.ts b/web/src/user_settings.ts index 513404cad5..81172ec917 100644 --- a/web/src/user_settings.ts +++ b/web/src/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_email_notifications_policy: number; user_list_style: number; starred_message_counts: boolean; translate_emoticons: boolean; diff --git a/web/templates/settings/notification_settings.hbs b/web/templates/settings/notification_settings.hbs index 7d9902c1e9..41dcf39055 100644 --- a/web/templates/settings/notification_settings.hbs +++ b/web/templates/settings/notification_settings.hbs @@ -147,6 +147,13 @@ +
+ + +
+ {{#each notification_settings.email_message_notification_settings}} {{> settings_checkbox setting_name=this diff --git a/zerver/lib/email_notifications.py b/zerver/lib/email_notifications.py index 3c3c8a4eea..486964a47b 100644 --- a/zerver/lib/email_notifications.py +++ b/zerver/lib/email_notifications.py @@ -385,6 +385,28 @@ def message_content_allowed_in_missedmessage_emails(user_profile: UserProfile) - ) +def include_realm_name_in_missedmessage_emails_subject(user_profile: UserProfile) -> bool: + # Determines whether to include the realm name in the subject line + # of missedmessage email notifications, based on the user's + # realm_name_in_email_notifications_policy settings and whether the + # user's delivery_email is associated with other active realms. + if ( + user_profile.realm_name_in_email_notifications_policy + == UserProfile.REALM_NAME_IN_EMAIL_NOTIFICATIONS_POLICY_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_email_notifications_policy + == UserProfile.REALM_NAME_IN_EMAIL_NOTIFICATIONS_POLICY_ALWAYS + ) + + @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 @@ -417,7 +439,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, + include_realm_name_in_missedmessage_emails_subject=include_realm_name_in_missedmessage_emails_subject( + user_profile + ), ) mentioned_user_group_name = get_mentioned_user_group_name(missed_messages, user_profile) diff --git a/zerver/migrations/0432_alter_and_migrate_realm_name_in_notifications.py b/zerver/migrations/0432_alter_and_migrate_realm_name_in_notifications.py new file mode 100644 index 0000000000..d5d498538f --- /dev/null +++ b/zerver/migrations/0432_alter_and_migrate_realm_name_in_notifications.py @@ -0,0 +1,89 @@ +# Generated by Django 4.1.5 on 2023-01-11 20:21 + +from django.db import migrations, models +from django.db.backends.base.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_EMAIL_NOTIFICATIONS_POLICY_AUTOMATIC = 1 +REALM_NAME_IN_EMAIL_NOTIFICATIONS_POLICY_ALWAYS = 2 +REALM_NAME_IN_EMAIL_NOTIFICATIONS_POLICY_NEVER = 3 + + +# The value of 'realm_name_in_email_notifications_policy' for those users +# who have manually changed the value of 'realm_name_in_notifications' as 'true' +# should be updated as 'Always', not 'Automatic' +def update_realm_name_in_email_notifications_policy_values( + apps: StateApps, schema_editor: BaseDatabaseSchemaEditor +) -> None: + UserProfile = apps.get_model("zerver", "UserProfile") + UserProfile.objects.filter(realm_name_in_notifications=True).update( + realm_name_in_email_notifications_policy=REALM_NAME_IN_EMAIL_NOTIFICATIONS_POLICY_ALWAYS + ) + + +def reverse_code(apps: StateApps, schema_editor: BaseDatabaseSchemaEditor) -> None: + UserProfile = apps.get_model("zerver", "UserProfile") + UserProfile.objects.filter( + realm_name_in_email_notifications_policy=REALM_NAME_IN_EMAIL_NOTIFICATIONS_POLICY_ALWAYS + ).update(realm_name_in_notifications=True) + + +def update_realm_name_in_email_notifications_policy_values_for_realm_user_default( + apps: StateApps, schema_editor: BaseDatabaseSchemaEditor +) -> None: + RealmUserDefault = apps.get_model("zerver", "RealmUserDefault") + RealmUserDefault.objects.filter(realm_name_in_notifications=True).update( + realm_name_in_email_notifications_policy=REALM_NAME_IN_EMAIL_NOTIFICATIONS_POLICY_ALWAYS + ) + + +def reverse_code_for_realm_user_default( + apps: StateApps, schema_editor: BaseDatabaseSchemaEditor +) -> None: + RealmUserDefault = apps.get_model("zerver", "RealmUserDefault") + RealmUserDefault.objects.filter( + realm_name_in_email_notifications_policy=REALM_NAME_IN_EMAIL_NOTIFICATIONS_POLICY_ALWAYS + ).update(realm_name_in_notifications=True) + + +class Migration(migrations.Migration): + dependencies = [ + ("zerver", "0431_alter_archivedreaction_unique_together_and_more"), + ] + + operations = [ + migrations.AddField( + model_name="realmuserdefault", + name="realm_name_in_email_notifications_policy", + field=models.PositiveSmallIntegerField( + default=REALM_NAME_IN_EMAIL_NOTIFICATIONS_POLICY_AUTOMATIC + ), + ), + migrations.RunPython( + update_realm_name_in_email_notifications_policy_values_for_realm_user_default, + reverse_code=reverse_code_for_realm_user_default, + elidable=True, + ), + migrations.AddField( + model_name="userprofile", + name="realm_name_in_email_notifications_policy", + field=models.PositiveSmallIntegerField( + default=REALM_NAME_IN_EMAIL_NOTIFICATIONS_POLICY_AUTOMATIC + ), + ), + migrations.RunPython( + update_realm_name_in_email_notifications_policy_values, + reverse_code=reverse_code, + elidable=True, + ), + migrations.RemoveField( + model_name="realmuserdefault", + name="realm_name_in_notifications", + ), + migrations.RemoveField( + model_name="userprofile", + name="realm_name_in_notifications", + ), + ] diff --git a/zerver/models.py b/zerver/models.py index 76d9a3fec5..5d86241f56 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -1597,9 +1597,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_EMAIL_NOTIFICATIONS_POLICY_AUTOMATIC = 1 + REALM_NAME_IN_EMAIL_NOTIFICATIONS_POLICY_ALWAYS = 2 + REALM_NAME_IN_EMAIL_NOTIFICATIONS_POLICY_NEVER = 3 + REALM_NAME_IN_EMAIL_NOTIFICATIONS_POLICY_CHOICES = [ + REALM_NAME_IN_EMAIL_NOTIFICATIONS_POLICY_AUTOMATIC, + REALM_NAME_IN_EMAIL_NOTIFICATIONS_POLICY_ALWAYS, + REALM_NAME_IN_EMAIL_NOTIFICATIONS_POLICY_NEVER, + ] + realm_name_in_email_notifications_policy = models.PositiveSmallIntegerField( + default=REALM_NAME_IN_EMAIL_NOTIFICATIONS_POLICY_AUTOMATIC + ) + # Whether or not the user wants to sync their drafts. enable_drafts_synchronization = models.BooleanField(default=True) @@ -1671,7 +1682,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_email_notifications_policy=int, wildcard_mentions_notify=bool, ) diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 05efe1c863..244776d904 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -9126,13 +9126,25 @@ paths: - 2 - 3 example: 1 - - name: realm_name_in_notifications + - name: realm_name_in_email_notifications_policy in: query description: | - Include organization name in subject of message notification emails. + Whether to [include organization name in subject of message notification emails](/help/email-notifications#include-organization-name-in-subject-line). + + - 1 - Automatic + - 2 - Always + - 3 - Never + + **Changes**: New in Zulip 7.0 (feature level 168), replacing the + previous `realm_name_in_notifications` boolean; + `true` corresponded to `Always`, and `false` to `Never`. schema: - type: boolean - example: true + type: integer + enum: + - 1 + - 2 + - 3 + example: 1 - name: presence_enabled in: query description: | @@ -11106,10 +11118,18 @@ paths: - 1 - All unreads - 2 - Private messages and mentions - 3 - None - realm_name_in_notifications: - type: boolean + realm_name_in_email_notifications_policy: + type: integer description: | - Include organization name in subject of message notification emails. + Whether to [include organization name in subject of message notification emails](/help/email-notifications#include-organization-name-in-subject-line). + + - 1 - Automatic + - 2 - Always + - 3 - Never + + **Changes**: New in Zulip 7.0 (feature level 168), replacing the + previous `realm_name_in_notifications` boolean; + `true` corresponded to `Always`, and `false` to `Never`. presence_enabled: type: boolean description: | @@ -11501,9 +11521,9 @@ paths: **Changes**: Deprecated in Zulip 5.0 (feature level 89). Clients connecting to newer servers should declare the `user_settings_object` client capability and access the `user_settings` object instead. - realm_name_in_notifications: + realm_name_in_email_notifications_policy: 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 @@ -11513,8 +11533,12 @@ paths: See [PATCH /settings](/api/update-settings) for details on the meaning of this setting. - **Changes**: Deprecated in Zulip 5.0 (feature level 89). Clients - connecting to newer servers should declare the `user_settings_object` + **Changes**: Deprecated `realm_name_in_notifications` in Zulip 5.0 (feature level 89). + Replaced `realm_name_in_notifications` boolean with + `realm_name_in_email_notifications_policy` in Zulip 7.0 (feature level 168); + `true` corresponded to `Always`, and `false` to `Never`. + + Clients connecting to newer servers should declare the `user_settings_object` client capability and access the `user_settings` object instead. presence_enabled: deprecated: true @@ -13024,10 +13048,18 @@ paths: - 1 - All unreads - 2 - Private messages and mentions - 3 - None - realm_name_in_notifications: - type: boolean + realm_name_in_email_notifications_policy: + type: integer description: | - Include organization name in subject of message notification emails. + Whether to [include organization name in subject of message notification emails](/help/email-notifications#include-organization-name-in-subject-line). + + - 1 - Automatic + - 2 - Always + - 3 - Never + + **Changes**: New in Zulip 7.0 (feature level 168), replacing the + previous `realm_name_in_notifications` boolean; + `true` corresponded to `Always`, and `false` to `Never`. presence_enabled: type: boolean description: | @@ -14190,16 +14222,28 @@ paths: - 2 - 3 example: 1 - - name: realm_name_in_notifications + - name: realm_name_in_email_notifications_policy in: query description: | - Include organization name in subject of message notification emails. + Whether to [include organization name in subject of message notification emails](/help/email-notifications#include-organization-name-in-subject-line). - **Changes**: Before Zulip 5.0 (feature level 80), this setting was managed by - the `PATCH /settings/notifications` endpoint. + - 1 - Automatic + - 2 - Always + - 3 - Never + + **Changes**: New in Zulip 7.0 (feature level 168), replacing the + previous `realm_name_in_notifications` boolean; + `true` corresponded to `Always`, and `false` to `Never`. + + Before Zulip 5.0 (feature level 80), the previous `realm_name_in_notifications` + 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 495d8d638d..97b297ac33 100644 --- a/zerver/tests/test_email_notifications.py +++ b/zerver/tests/test_email_notifications.py @@ -16,6 +16,7 @@ from django.test import override_settings 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 @@ from zerver.lib.email_notifications import ( fix_spoilers_in_html, followup_day2_email_delay, handle_missedmessage_emails, + include_realm_name_in_missedmessage_emails_subject, relative_to_full_url, ) from zerver.lib.send_email import FromAddress, deliver_scheduled_emails, send_custom_email @@ -1001,18 +1003,80 @@ class TestMissedMessages(ZulipTestCase): 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_include_realm_name_in_missedmessage_emails_subject(self) -> None: + user = self.example_user("hamlet") - # Enable realm_name_in_notifications for hamlet and test again. + # Test with 'realm_name_in_notification_policy' set to 'Always' + do_change_user_setting( + user, + "realm_name_in_email_notifications_policy", + UserProfile.REALM_NAME_IN_EMAIL_NOTIFICATIONS_POLICY_ALWAYS, + acting_user=None, + ) + self.assertTrue(include_realm_name_in_missedmessage_emails_subject(user)) + + # Test with 'realm_name_in_notification_policy' set to 'Never' + do_change_user_setting( + user, + "realm_name_in_email_notifications_policy", + UserProfile.REALM_NAME_IN_EMAIL_NOTIFICATIONS_POLICY_NEVER, + acting_user=None, + ) + self.assertFalse(include_realm_name_in_missedmessage_emails_subject(user)) + + # Test with 'realm_name_in_notification_policy' set to 'Automatic' + do_change_user_setting( + user, + "realm_name_in_email_notifications_policy", + UserProfile.REALM_NAME_IN_EMAIL_NOTIFICATIONS_POLICY_AUTOMATIC, + acting_user=None, + ) + # Case 1: if user is part of a single realm, then realm_name is not present in notifications. + self.assertFalse(include_realm_name_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(include_realm_name_in_missedmessage_emails_subject(cross_realm_user)) + + def test_realm_name_in_email_notifications_policy(self) -> None: + # Test with realm_name_in_email_notifications_policy set to Never. hamlet = self.example_user("hamlet") - hamlet.realm_name_in_notifications = True - hamlet.save(update_fields=["realm_name_in_notifications"]) + hamlet.realm_name_in_email_notifications_policy = ( + UserProfile.REALM_NAME_IN_EMAIL_NOTIFICATIONS_POLICY_NEVER + ) + hamlet.save(update_fields=["realm_name_in_email_notifications_policy"]) + with mock.patch( + "zerver.lib.email_notifications.include_realm_name_in_missedmessage_emails_subject", + return_value=False, + ): + is_allowed = include_realm_name_in_missedmessage_emails_subject(hamlet) + self._realm_name_in_missed_message_email_subject(is_allowed) - # Empty the test outbox - mail.outbox = [] - self._realm_name_in_missed_message_email_subject(True) + # Test with realm_name_in_email_notifications_policy set to Always. + + # Note: We don't need to test separately for 'realm_name_in_email_notifications_policy' + # 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 + # 'include_realm_name_in_missedmessage_emails_subject' for Automatic, Always, Never. + hamlet = self.example_user("hamlet") + hamlet.realm_name_in_email_notifications_policy = ( + UserProfile.REALM_NAME_IN_EMAIL_NOTIFICATIONS_POLICY_ALWAYS + ) + hamlet.save(update_fields=["realm_name_in_email_notifications_policy"]) + with mock.patch( + "zerver.lib.email_notifications.include_realm_name_in_missedmessage_emails_subject", + return_value=True, + ): + is_allowed = include_realm_name_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 909e21a3d4..e633d3c297 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -1837,6 +1837,7 @@ class NormalActionsTest(BaseAction): "notification_sound", "desktop_icon_count_display", "presence_enabled", + "realm_name_in_email_notifications_policy", ]: # These settings are tested in their own tests. continue @@ -1926,6 +1927,27 @@ class NormalActionsTest(BaseAction): check_user_settings_update("events[0]", events[0]) check_update_global_notifications("events[1]", events[1], 1) + def test_change_realm_name_in_email_notifications_policy(self) -> None: + notification_setting = "realm_name_in_email_notifications_policy" + + 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 @@ -2690,6 +2712,7 @@ class RealmPropertyActionTest(BaseAction): notification_sound=["zulip", "ding"], email_notifications_batching_period_seconds=[120, 300], email_address_visibility=UserProfile.EMAIL_ADDRESS_VISIBILITY_TYPES, + realm_name_in_email_notifications_policy=UserProfile.REALM_NAME_IN_EMAIL_NOTIFICATIONS_POLICY_CHOICES, ) vals = test_values.get(name) diff --git a/zerver/tests/test_realm.py b/zerver/tests/test_realm.py index 2bf9b750d6..cc69b26c72 100644 --- a/zerver/tests/test_realm.py +++ b/zerver/tests/test_realm.py @@ -1244,6 +1244,7 @@ class RealmAPITest(ZulipTestCase): notification_sound=["zulip", "ding"], email_notifications_batching_period_seconds=[120, 300], email_address_visibility=UserProfile.EMAIL_ADDRESS_VISIBILITY_TYPES, + realm_name_in_email_notifications_policy=UserProfile.REALM_NAME_IN_EMAIL_NOTIFICATIONS_POLICY_CHOICES, ) vals = test_values.get(name) diff --git a/zerver/tests/test_settings.py b/zerver/tests/test_settings.py index 2b0f8dccca..addecd6eed 100644 --- a/zerver/tests/test_settings.py +++ b/zerver/tests/test_settings.py @@ -362,6 +362,7 @@ class ChangeSettingsTest(ZulipTestCase): notification_sound="ding", desktop_icon_count_display=2, email_address_visibility=3, + realm_name_in_email_notifications_policy=2, ) self.login("hamlet") diff --git a/zerver/views/realm.py b/zerver/views/realm.py index ac3c493e6b..be9f1e013b 100644 --- a/zerver/views/realm.py +++ b/zerver/views/realm.py @@ -453,7 +453,10 @@ 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_email_notifications_policy: Optional[int] = REQ( + json_validator=check_int_in(UserProfile.REALM_NAME_IN_EMAIL_NOTIFICATIONS_POLICY_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 b9d676c828..674ba15110 100644 --- a/zerver/views/user_settings.py +++ b/zerver/views/user_settings.py @@ -211,7 +211,10 @@ 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_email_notifications_policy: Optional[int] = REQ( + json_validator=check_int_in(UserProfile.REALM_NAME_IN_EMAIL_NOTIFICATIONS_POLICY_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(