diff --git a/zerver/lib/email_notifications.py b/zerver/lib/email_notifications.py index 29884a3ff6..74a6c60279 100644 --- a/zerver/lib/email_notifications.py +++ b/zerver/lib/email_notifications.py @@ -573,20 +573,6 @@ def do_send_missedmessage_events_reply_in_zulip( with override_language(user_profile.default_language): from_name: str = _("Zulip notifications") from_address = FromAddress.NOREPLY - if len(senders) == 1 and settings.SEND_MISSED_MESSAGE_EMAILS_AS_USER: - # If this setting is enabled, you can reply to the Zulip - # message notification emails directly back to the original sender. - # However, one must ensure the Zulip server is in the SPF - # record for the domain, or there will be spam/deliverability - # problems. - # - # Also, this setting is not really compatible with - # EMAIL_ADDRESS_VISIBILITY_ADMINS. - sender = senders[0] - from_name, from_address = (sender.full_name, sender.email) - context.update( - reply_to_zulip=False, - ) email_dict = { "template_prefix": "zerver/emails/missed_message", diff --git a/zerver/tests/test_message_notification_emails.py b/zerver/tests/test_message_notification_emails.py index 5749601c62..13d5ca85e4 100644 --- a/zerver/tests/test_message_notification_emails.py +++ b/zerver/tests/test_message_notification_emails.py @@ -104,14 +104,12 @@ class TestMessageNotificationEmails(ZulipTestCase): msg_id: int, verify_body_include: List[str], email_subject: str, - send_as_user: bool, verify_html_body: bool = False, show_message_content: bool = True, verify_body_does_not_include: Sequence[str] = [], trigger: str = "", mentioned_user_group_id: Optional[int] = None, ) -> None: - othello = self.example_user("othello") hamlet = self.example_user("hamlet") tokens = self._get_tokens() with patch("zerver.lib.email_mirror.generate_missed_message_token", side_effect=tokens): @@ -135,8 +133,6 @@ class TestMessageNotificationEmails(ZulipTestCase): assert isinstance(msg, EmailMultiAlternatives) from_email = str(Address(display_name="Zulip notifications", addr_spec=FromAddress.NOREPLY)) self.assert_length(mail.outbox, 1) - if send_as_user: - from_email = f'"{othello.full_name}" <{othello.email}>' self.assertEqual(self.email_envelope_from(msg), settings.NOREPLY_EMAIL_ADDRESS) self.assertEqual(self.email_display_from(msg), from_email) self.assertEqual(msg.subject, email_subject) @@ -168,10 +164,10 @@ class TestMessageNotificationEmails(ZulipTestCase): if realm_name_in_notifications: email_subject = "DMs with Othello, the Moor of Venice [Zulip Dev]" - self._test_cases(msg_id, verify_body_include, email_subject, False) + self._test_cases(msg_id, verify_body_include, email_subject) def _extra_context_in_missed_stream_messages_mention( - self, send_as_user: bool, show_message_content: bool = True + self, show_message_content: bool = True ) -> None: for i in range(11): self.send_stream_message( @@ -214,14 +210,13 @@ class TestMessageNotificationEmails(ZulipTestCase): msg_id, verify_body_include, email_subject, - send_as_user, show_message_content=show_message_content, verify_body_does_not_include=verify_body_does_not_include, trigger=NotificationTriggers.MENTION, ) def _extra_context_in_missed_stream_messages_topic_wildcard_mention_in_followed_topic( - self, send_as_user: bool, show_message_content: bool = True + self, show_message_content: bool = True ) -> None: for i in range(1, 6): self.send_stream_message(self.example_user("othello"), "Denmark", content=str(i)) @@ -255,14 +250,13 @@ class TestMessageNotificationEmails(ZulipTestCase): msg_id, verify_body_include, email_subject, - send_as_user, show_message_content=show_message_content, verify_body_does_not_include=verify_body_does_not_include, trigger=NotificationTriggers.TOPIC_WILDCARD_MENTION_IN_FOLLOWED_TOPIC, ) def _extra_context_in_missed_stream_messages_stream_wildcard_mention_in_followed_topic( - self, send_as_user: bool, show_message_content: bool = True + self, show_message_content: bool = True ) -> None: for i in range(1, 6): self.send_stream_message(self.example_user("othello"), "Denmark", content=str(i)) @@ -297,14 +291,13 @@ class TestMessageNotificationEmails(ZulipTestCase): msg_id, verify_body_include, email_subject, - send_as_user, show_message_content=show_message_content, verify_body_does_not_include=verify_body_does_not_include, trigger=NotificationTriggers.STREAM_WILDCARD_MENTION_IN_FOLLOWED_TOPIC, ) def _extra_context_in_missed_stream_messages_topic_wildcard_mention( - self, send_as_user: bool, show_message_content: bool = True + self, show_message_content: bool = True ) -> None: for i in range(1, 6): self.send_stream_message(self.example_user("othello"), "Denmark", content=str(i)) @@ -338,14 +331,13 @@ class TestMessageNotificationEmails(ZulipTestCase): msg_id, verify_body_include, email_subject, - send_as_user, show_message_content=show_message_content, verify_body_does_not_include=verify_body_does_not_include, trigger=NotificationTriggers.TOPIC_WILDCARD_MENTION, ) def _extra_context_in_missed_stream_messages_stream_wildcard_mention( - self, send_as_user: bool, show_message_content: bool = True + self, show_message_content: bool = True ) -> None: for i in range(1, 6): self.send_stream_message(self.example_user("othello"), "Denmark", content=str(i)) @@ -380,13 +372,12 @@ class TestMessageNotificationEmails(ZulipTestCase): msg_id, verify_body_include, email_subject, - send_as_user, show_message_content=show_message_content, verify_body_does_not_include=verify_body_does_not_include, trigger=NotificationTriggers.STREAM_WILDCARD_MENTION, ) - def _extra_context_in_missed_stream_messages_email_notify(self, send_as_user: bool) -> None: + def _extra_context_in_missed_stream_messages_email_notify(self) -> None: for i in range(11): self.send_stream_message(self.example_user("othello"), "Denmark", content=str(i)) self.send_stream_message(self.example_user("othello"), "Denmark", "11", topic_name="test2") @@ -400,12 +391,11 @@ class TestMessageNotificationEmails(ZulipTestCase): msg_id, verify_body_include, email_subject, - send_as_user, trigger=NotificationTriggers.STREAM_EMAIL, ) def _extra_context_in_missed_stream_messages_mention_two_senders( - self, send_as_user: bool + self, ) -> None: cordelia = self.example_user("cordelia") self.subscribe(cordelia, "Denmark") @@ -424,11 +414,10 @@ class TestMessageNotificationEmails(ZulipTestCase): msg_id, verify_body_include, email_subject, - send_as_user, trigger=NotificationTriggers.MENTION, ) - def _resolved_topic_missed_stream_messages_thread_friendly(self, send_as_user: bool) -> None: + def _resolved_topic_missed_stream_messages_thread_friendly(self) -> None: topic_name = "threading and so forth" othello_user = self.example_user("othello") msg_id = -1 @@ -451,13 +440,11 @@ class TestMessageNotificationEmails(ZulipTestCase): msg_id, verify_body_include, email_subject, - send_as_user, trigger=NotificationTriggers.STREAM_EMAIL, ) def _extra_context_in_missed_personal_messages( self, - send_as_user: bool, show_message_content: bool = True, message_content_disabled_by_user: bool = False, message_content_disabled_by_realm: bool = False, @@ -499,12 +486,11 @@ class TestMessageNotificationEmails(ZulipTestCase): msg_id, verify_body_include, email_subject, - send_as_user, show_message_content=show_message_content, verify_body_does_not_include=verify_body_does_not_include, ) - def _reply_to_email_in_missed_personal_messages(self, send_as_user: bool) -> None: + def _reply_to_email_in_missed_personal_messages(self) -> None: msg_id = self.send_personal_message( self.example_user("othello"), self.example_user("hamlet"), @@ -512,9 +498,9 @@ class TestMessageNotificationEmails(ZulipTestCase): ) verify_body_include = ["Reply to this email directly, or view it in Zulip Dev Zulip"] email_subject = "DMs with Othello, the Moor of Venice" - self._test_cases(msg_id, verify_body_include, email_subject, send_as_user) + self._test_cases(msg_id, verify_body_include, email_subject) - def _reply_warning_in_missed_personal_messages(self, send_as_user: bool) -> None: + def _reply_warning_in_missed_personal_messages(self) -> None: msg_id = self.send_personal_message( self.example_user("othello"), self.example_user("hamlet"), @@ -522,10 +508,10 @@ class TestMessageNotificationEmails(ZulipTestCase): ) verify_body_include = ["Do not reply to this email."] email_subject = "DMs with Othello, the Moor of Venice" - self._test_cases(msg_id, verify_body_include, email_subject, send_as_user) + self._test_cases(msg_id, verify_body_include, email_subject) def _extra_context_in_missed_huddle_messages_two_others( - self, send_as_user: bool, show_message_content: bool = True + self, show_message_content: bool = True ) -> None: msg_id = self.send_huddle_message( self.example_user("othello"), @@ -561,12 +547,11 @@ class TestMessageNotificationEmails(ZulipTestCase): msg_id, verify_body_include, email_subject, - send_as_user, show_message_content=show_message_content, verify_body_does_not_include=verify_body_does_not_include, ) - def _extra_context_in_missed_huddle_messages_three_others(self, send_as_user: bool) -> None: + def _extra_context_in_missed_huddle_messages_three_others(self) -> None: msg_id = self.send_huddle_message( self.example_user("othello"), [ @@ -581,9 +566,9 @@ class TestMessageNotificationEmails(ZulipTestCase): email_subject = ( "Group DMs with Cordelia, Lear's daughter, Iago, and Othello, the Moor of Venice" ) - self._test_cases(msg_id, verify_body_include, email_subject, send_as_user) + self._test_cases(msg_id, verify_body_include, email_subject) - def _extra_context_in_missed_huddle_messages_many_others(self, send_as_user: bool) -> None: + def _extra_context_in_missed_huddle_messages_many_others(self) -> None: msg_id = self.send_huddle_message( self.example_user("othello"), [ @@ -597,9 +582,9 @@ class TestMessageNotificationEmails(ZulipTestCase): verify_body_include = ["Othello, the Moor of Venice: > Group personal message! -- Reply"] email_subject = "Group DMs with Cordelia, Lear's daughter, Iago, and 2 others" - self._test_cases(msg_id, verify_body_include, email_subject, send_as_user) + self._test_cases(msg_id, verify_body_include, email_subject) - def _deleted_message_in_missed_stream_messages(self, send_as_user: bool) -> None: + def _deleted_message_in_missed_stream_messages(self) -> None: msg_id = self.send_stream_message( self.example_user("othello"), "denmark", "@**King Hamlet** to be deleted" ) @@ -613,7 +598,7 @@ class TestMessageNotificationEmails(ZulipTestCase): ) self.assert_length(mail.outbox, 0) - def _deleted_message_in_missed_personal_messages(self, send_as_user: bool) -> None: + def _deleted_message_in_missed_personal_messages(self) -> None: msg_id = self.send_personal_message( self.example_user("othello"), self.example_user("hamlet"), @@ -629,7 +614,7 @@ class TestMessageNotificationEmails(ZulipTestCase): ) self.assert_length(mail.outbox, 0) - def _deleted_message_in_missed_huddle_messages(self, send_as_user: bool) -> None: + def _deleted_message_in_missed_huddle_messages(self) -> None: msg_id = self.send_huddle_message( self.example_user("othello"), [ @@ -999,155 +984,85 @@ class TestMessageNotificationEmails(ZulipTestCase): False, acting_user=None, ) - self._extra_context_in_missed_stream_messages_mention(False, show_message_content=False) + self._extra_context_in_missed_stream_messages_mention(show_message_content=False) mail.outbox = [] self._extra_context_in_missed_stream_messages_topic_wildcard_mention_in_followed_topic( - False, show_message_content=False + show_message_content=False ) mail.outbox = [] self._extra_context_in_missed_stream_messages_stream_wildcard_mention_in_followed_topic( - False, show_message_content=False + show_message_content=False ) mail.outbox = [] self._extra_context_in_missed_stream_messages_topic_wildcard_mention( - False, show_message_content=False + show_message_content=False ) mail.outbox = [] self._extra_context_in_missed_stream_messages_stream_wildcard_mention( - False, show_message_content=False + show_message_content=False ) mail.outbox = [] self._extra_context_in_missed_personal_messages( - False, show_message_content=False, message_content_disabled_by_user=True + show_message_content=False, message_content_disabled_by_user=True ) mail.outbox = [] - self._extra_context_in_missed_huddle_messages_two_others(False, show_message_content=False) - - @override_settings(SEND_MISSED_MESSAGE_EMAILS_AS_USER=True) - def test_extra_context_in_missed_stream_messages_as_user(self) -> None: - self._extra_context_in_missed_stream_messages_mention(True) + self._extra_context_in_missed_huddle_messages_two_others(show_message_content=False) def test_extra_context_in_missed_stream_messages(self) -> None: - self._extra_context_in_missed_stream_messages_mention(False) - - @override_settings(SEND_MISSED_MESSAGE_EMAILS_AS_USER=True) - def test_extra_context_in_missed_stream_messages_as_user_topic_wildcard_in_followed_topic( - self, - ) -> None: - self._extra_context_in_missed_stream_messages_topic_wildcard_mention_in_followed_topic(True) + self._extra_context_in_missed_stream_messages_mention() def test_extra_context_in_missed_stream_messages_topic_wildcard_in_followed_topic( self, ) -> None: - self._extra_context_in_missed_stream_messages_topic_wildcard_mention_in_followed_topic( - False - ) - - @override_settings(SEND_MISSED_MESSAGE_EMAILS_AS_USER=True) - def test_extra_context_in_missed_stream_messages_as_user_stream_wildcard_in_followed_topic( - self, - ) -> None: - self._extra_context_in_missed_stream_messages_stream_wildcard_mention_in_followed_topic( - True - ) + self._extra_context_in_missed_stream_messages_topic_wildcard_mention_in_followed_topic() def test_extra_context_in_missed_stream_messages_stream_wildcard_in_followed_topic( self, ) -> None: - self._extra_context_in_missed_stream_messages_stream_wildcard_mention_in_followed_topic( - False - ) - - @override_settings(SEND_MISSED_MESSAGE_EMAILS_AS_USER=True) - def test_extra_context_in_missed_stream_messages_as_user_topic_wildcard(self) -> None: - self._extra_context_in_missed_stream_messages_topic_wildcard_mention(True) + self._extra_context_in_missed_stream_messages_stream_wildcard_mention_in_followed_topic() def test_extra_context_in_missed_stream_messages_topic_wildcard(self) -> None: - self._extra_context_in_missed_stream_messages_topic_wildcard_mention(False) - - @override_settings(SEND_MISSED_MESSAGE_EMAILS_AS_USER=True) - def test_extra_context_in_missed_stream_messages_as_user_stream_wildcard(self) -> None: - self._extra_context_in_missed_stream_messages_stream_wildcard_mention(True) + self._extra_context_in_missed_stream_messages_topic_wildcard_mention() def test_extra_context_in_missed_stream_messages_stream_wildcard(self) -> None: - self._extra_context_in_missed_stream_messages_stream_wildcard_mention(False) - - @override_settings(SEND_MISSED_MESSAGE_EMAILS_AS_USER=True) - def test_extra_context_in_missed_stream_messages_as_user_two_senders(self) -> None: - self._extra_context_in_missed_stream_messages_mention_two_senders(True) + self._extra_context_in_missed_stream_messages_stream_wildcard_mention() def test_extra_context_in_missed_stream_messages_two_senders(self) -> None: - self._extra_context_in_missed_stream_messages_mention_two_senders(False) + self._extra_context_in_missed_stream_messages_mention_two_senders() def test_reply_to_email_in_missed_personal_messages(self) -> None: - self._reply_to_email_in_missed_personal_messages(False) - - @override_settings(SEND_MISSED_MESSAGE_EMAILS_AS_USER=True) - def test_extra_context_in_missed_stream_messages_email_notify_as_user(self) -> None: - self._extra_context_in_missed_stream_messages_email_notify(True) + self._reply_to_email_in_missed_personal_messages() def test_extra_context_in_missed_stream_messages_email_notify(self) -> None: - self._extra_context_in_missed_stream_messages_email_notify(False) - - @override_settings(SEND_MISSED_MESSAGE_EMAILS_AS_USER=True) - def test_resolved_topic_missed_stream_messages_thread_friendly_as_user(self) -> None: - self._resolved_topic_missed_stream_messages_thread_friendly(True) + self._extra_context_in_missed_stream_messages_email_notify() def test_resolved_topic_missed_stream_messages_thread_friendly(self) -> None: - self._resolved_topic_missed_stream_messages_thread_friendly(False) + self._resolved_topic_missed_stream_messages_thread_friendly() @override_settings(EMAIL_GATEWAY_PATTERN="") def test_reply_warning_in_missed_personal_messages(self) -> None: - self._reply_warning_in_missed_personal_messages(False) - - @override_settings(SEND_MISSED_MESSAGE_EMAILS_AS_USER=True) - def test_extra_context_in_missed_personal_messages_as_user(self) -> None: - self._extra_context_in_missed_personal_messages(True) + self._reply_warning_in_missed_personal_messages() def test_extra_context_in_missed_personal_messages(self) -> None: - self._extra_context_in_missed_personal_messages(False) - - @override_settings(SEND_MISSED_MESSAGE_EMAILS_AS_USER=True) - def test_extra_context_in_missed_huddle_messages_two_others_as_user(self) -> None: - self._extra_context_in_missed_huddle_messages_two_others(True) + self._extra_context_in_missed_personal_messages() def test_extra_context_in_missed_huddle_messages_two_others(self) -> None: - self._extra_context_in_missed_huddle_messages_two_others(False) - - @override_settings(SEND_MISSED_MESSAGE_EMAILS_AS_USER=True) - def test_extra_context_in_missed_huddle_messages_three_others_as_user(self) -> None: - self._extra_context_in_missed_huddle_messages_three_others(True) + self._extra_context_in_missed_huddle_messages_two_others() def test_extra_context_in_missed_huddle_messages_three_others(self) -> None: - self._extra_context_in_missed_huddle_messages_three_others(False) - - @override_settings(SEND_MISSED_MESSAGE_EMAILS_AS_USER=True) - def test_extra_context_in_missed_huddle_messages_many_others_as_user(self) -> None: - self._extra_context_in_missed_huddle_messages_many_others(True) + self._extra_context_in_missed_huddle_messages_three_others() def test_extra_context_in_missed_huddle_messages_many_others(self) -> None: - self._extra_context_in_missed_huddle_messages_many_others(False) - - @override_settings(SEND_MISSED_MESSAGE_EMAILS_AS_USER=True) - def test_deleted_message_in_missed_stream_messages_as_user(self) -> None: - self._deleted_message_in_missed_stream_messages(True) + self._extra_context_in_missed_huddle_messages_many_others() def test_deleted_message_in_missed_stream_messages(self) -> None: - self._deleted_message_in_missed_stream_messages(False) - - @override_settings(SEND_MISSED_MESSAGE_EMAILS_AS_USER=True) - def test_deleted_message_in_missed_personal_messages_as_user(self) -> None: - self._deleted_message_in_missed_personal_messages(True) + self._deleted_message_in_missed_stream_messages() def test_deleted_message_in_missed_personal_messages(self) -> None: - self._deleted_message_in_missed_personal_messages(False) - - @override_settings(SEND_MISSED_MESSAGE_EMAILS_AS_USER=True) - def test_deleted_message_in_missed_huddle_messages_as_user(self) -> None: - self._deleted_message_in_missed_huddle_messages(True) + self._deleted_message_in_missed_personal_messages() def test_deleted_message_in_missed_huddle_messages(self) -> None: - self._deleted_message_in_missed_huddle_messages(False) + self._deleted_message_in_missed_huddle_messages() def test_realm_message_content_allowed_in_email_notifications(self) -> None: user = self.example_user("hamlet") @@ -1162,7 +1077,7 @@ class TestMessageNotificationEmails(ZulipTestCase): user, "message_content_in_email_notifications", True, acting_user=None ) mail.outbox = [] - self._extra_context_in_missed_personal_messages(False, show_message_content=True) + self._extra_context_in_missed_personal_messages(show_message_content=True) # Emails don't have missed message content when message content is disabled by the user do_change_user_setting( @@ -1170,7 +1085,7 @@ class TestMessageNotificationEmails(ZulipTestCase): ) mail.outbox = [] self._extra_context_in_missed_personal_messages( - False, show_message_content=False, message_content_disabled_by_user=True + show_message_content=False, message_content_disabled_by_user=True ) # When message content is not allowed at realm level @@ -1184,7 +1099,7 @@ class TestMessageNotificationEmails(ZulipTestCase): ) mail.outbox = [] self._extra_context_in_missed_personal_messages( - False, show_message_content=False, message_content_disabled_by_realm=True + show_message_content=False, message_content_disabled_by_realm=True ) do_change_user_setting( @@ -1192,7 +1107,6 @@ class TestMessageNotificationEmails(ZulipTestCase): ) mail.outbox = [] self._extra_context_in_missed_personal_messages( - False, show_message_content=False, message_content_disabled_by_user=True, message_content_disabled_by_realm=True, @@ -1219,7 +1133,6 @@ class TestMessageNotificationEmails(ZulipTestCase): msg_id, verify_body_include, email_subject, - send_as_user=False, verify_html_body=True, ) @@ -1240,7 +1153,6 @@ class TestMessageNotificationEmails(ZulipTestCase): msg_id, verify_body_include, email_subject, - send_as_user=False, verify_html_body=True, ) @@ -1260,7 +1172,6 @@ class TestMessageNotificationEmails(ZulipTestCase): msg_id, verify_body_include, email_subject, - send_as_user=False, verify_html_body=True, ) @@ -1277,7 +1188,7 @@ class TestMessageNotificationEmails(ZulipTestCase): f"view it in Zulip Dev Zulip: http://zulip.testserver/#narrow/dm/{cordelia.id}-{encoded_name}" ] email_subject = "DMs with Cordelia, Lear's daughter" - self._test_cases(msg_id, verify_body_include, email_subject, send_as_user=False) + self._test_cases(msg_id, verify_body_include, email_subject) def test_sender_name_in_missed_message(self) -> None: hamlet = self.example_user("hamlet") @@ -1588,12 +1499,10 @@ class TestMessageNotificationEmails(ZulipTestCase): verify_body_include = ["header text", "Open Zulip to see the spoiler content"] verify_body_does_not_include = ["secret-text"] email_subject = "#Denmark > test" - send_as_user = False self._test_cases( msg_id, verify_body_include, email_subject, - send_as_user, trigger=NotificationTriggers.MENTION, verify_body_does_not_include=verify_body_does_not_include, ) @@ -1623,9 +1532,7 @@ class TestMessageNotificationEmails(ZulipTestCase): ) verify_body_include = ["view it in Zulip Dev Zulip"] email_subject = "DMs with Othello, the Moor of Venice" - self._test_cases( - msg_id, verify_body_include, email_subject, send_as_user=False, verify_html_body=True - ) + self._test_cases(msg_id, verify_body_include, email_subject, verify_html_body=True) def test_long_term_idle_user_missed_message(self) -> None: hamlet = self.example_user("hamlet") diff --git a/zproject/default_settings.py b/zproject/default_settings.py index 12c6b86281..3e6f0cf689 100644 --- a/zproject/default_settings.py +++ b/zproject/default_settings.py @@ -470,11 +470,7 @@ ADMINS = (("Zulip Administrator", ZULIP_ADMINISTRATOR),) # From address for welcome emails. WELCOME_EMAIL_SENDER: Optional[Dict[str, str]] = None -# Whether we should use users' own email addresses as the from -# address when sending missed-message emails. Off by default -# because some transactional email providers reject sending such -# emails since they can look like spam. -SEND_MISSED_MESSAGE_EMAILS_AS_USER = False + # Whether to send periodic digests of activity. SEND_DIGEST_EMAILS = True