From 8f461b3c0369e0e8e630181c768a65853aa25e65 Mon Sep 17 00:00:00 2001 From: Mateusz Mandera Date: Tue, 9 Jul 2024 01:15:12 +0200 Subject: [PATCH] email_mirror: Change default topic name if subject ends up empty. If the email subject is something like `Fwd:`, it gets stripped to an empty string, activating the "(no topic)" override. This however leads to failure if the organization enables the setting forcing every message to have a topic. Such emails should still go through, so we should just change the topic value used. --- tools/lib/capitalization.py | 1 + zerver/lib/email_mirror.py | 10 ++++++++-- zerver/tests/test_email_mirror.py | 8 ++++---- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/tools/lib/capitalization.py b/tools/lib/capitalization.py index 8fe97f93a1..9194dea3ac 100644 --- a/tools/lib/capitalization.py +++ b/tools/lib/capitalization.py @@ -73,6 +73,7 @@ IGNORED_PHRASES = [ r"and", r"bot", r"e\.g\.", + r"email", r"enabled", r"signups", # Placeholders diff --git a/zerver/lib/email_mirror.py b/zerver/lib/email_mirror.py index 61cdf2b512..fadbd09c6d 100644 --- a/zerver/lib/email_mirror.py +++ b/zerver/lib/email_mirror.py @@ -409,11 +409,17 @@ def is_forwarded(subject: str) -> bool: def process_stream_message(to: str, message: EmailMessage) -> None: subject_header = message.get("Subject", "") - subject = strip_from_subject(subject_header) or "(no topic)" + subject = strip_from_subject(subject_header) # We don't want to reject email messages with disallowed characters in the Subject, # so we just remove them to make it a valid Zulip topic name. - subject = "".join([char for char in subject if is_character_printable(char)]) or "(no topic)" + subject = "".join([char for char in subject if is_character_printable(char)]) + + # If the subject gets stripped to the empty string, we need to set some + # default value for the message topic. We can't use the usual + # "(no topic)" as that value is not permitted if the realm enforces + # that all messages must have a topic. + subject = subject or _("Email with no subject") stream, options = decode_stream_email_address(to) # Don't remove quotations if message is forwarded, unless otherwise specified: diff --git a/zerver/tests/test_email_mirror.py b/zerver/tests/test_email_mirror.py index 09f9f793b9..f9c48e6c0c 100644 --- a/zerver/tests/test_email_mirror.py +++ b/zerver/tests/test_email_mirror.py @@ -326,7 +326,7 @@ class TestStreamEmailMessagesSuccess(ZulipTestCase): self.assertEqual(message.content, "TestStreamEmailMessages body") self.assert_message_stream_name(message, stream.name) - self.assertEqual(message.topic_name(), "(no topic)") + self.assertEqual(message.topic_name(), "Email with no subject") def test_receive_stream_email_messages_subject_with_nonprintable_chars( self, @@ -358,7 +358,7 @@ class TestStreamEmailMessagesSuccess(ZulipTestCase): process_message(incoming_valid_message) message = most_recent_message(user_profile) - self.assertEqual(message.topic_name(), "(no topic)") + self.assertEqual(message.topic_name(), "Email with no subject") def test_receive_private_stream_email_messages_success(self) -> None: user_profile = self.example_user("hamlet") @@ -1617,12 +1617,12 @@ class TestStreamEmailMessagesSubjectStripping(ZulipTestCase): message = most_recent_message(user_profile) self.assertEqual("Test", message.topic_name()) - # If after stripping we get an empty subject, it should get set to (no topic) + # If after stripping we get an empty subject, it should get set to Email with no subject del incoming_valid_message["Subject"] incoming_valid_message["Subject"] = "Re: Fwd: Re: " process_message(incoming_valid_message) message = most_recent_message(user_profile) - self.assertEqual("(no topic)", message.topic_name()) + self.assertEqual("Email with no subject", message.topic_name()) def test_strip_from_subject(self) -> None: subject_list = orjson.loads(self.fixture_data("subjects.json", type="email"))