From c15d066bf52ccde0435568d71b665d518c6c4e72 Mon Sep 17 00:00:00 2001 From: Josh Klar Date: Thu, 27 Oct 2022 15:25:31 -0700 Subject: [PATCH] email-notifs: Use bracketed prefix to indicate a resolved topic. Some email clients (notably, Gmail Web) support automatically threading emails together if recipients and subjects match[1]. Manual testing indicated that prefixing a subject with "[bracketed content]" does not break this threading behavior, but the added checkmark in a resolved topic's title does. Before sending an email notification, determine whether the topic is resolved, and pass this information to the Jinja template to properly format a threadable email subject. Fixes: #22538 [1]: https://support.google.com/mail/answer/5900 --- templates/zerver/emails/missed_message.subject.txt | 12 +++++++++++- zerver/lib/email_notifications.py | 4 +++- zerver/lib/topic.py | 14 ++++++++++++++ zerver/tests/test_email_notifications.py | 7 +++++++ 4 files changed, 35 insertions(+), 2 deletions(-) diff --git a/templates/zerver/emails/missed_message.subject.txt b/templates/zerver/emails/missed_message.subject.txt index 2ee788166e..a474574fb7 100644 --- a/templates/zerver/emails/missed_message.subject.txt +++ b/templates/zerver/emails/missed_message.subject.txt @@ -1,7 +1,17 @@ {% if show_message_content %} {% if group_pm %} {% trans %}Group PMs with {{ huddle_display_name }}{% endtrans %} {% elif private_message %} {% trans %}PMs with {{ sender_str }}{% endtrans %} - {% elif stream_email_notify or mention %} #{{ stream_name }} > {{ topic_name }} + {% elif stream_email_notify or mention %} + {# + Some email clients, like Gmail Web (as of 2022), will auto-thread + emails that share a subject and recipients, but will disregard + [Bracketed Prefixes] in the style of old-school mailing lists. We take + advantage of this to retain thread continuity in email notifications + even after a topic is resolved. + #} + {% if topic_resolved %}{% trans %}[resolved] #{{ stream_name }} > {{ topic_name }}{% endtrans %} + {% else %}#{{ stream_name }} > {{ topic_name }} + {% endif %} {% endif %} {% else %} {% trans %}New messages{% endtrans %} diff --git a/zerver/lib/email_notifications.py b/zerver/lib/email_notifications.py index d2ca64b2ba..9aca51e2b0 100644 --- a/zerver/lib/email_notifications.py +++ b/zerver/lib/email_notifications.py @@ -27,6 +27,7 @@ from zerver.lib.notification_data import get_mentioned_user_group_name from zerver.lib.queue import queue_json_publish from zerver.lib.send_email import FromAddress, send_future_email from zerver.lib.soft_deactivation import soft_reactivate_if_personal_notification +from zerver.lib.topic import get_topic_resolution_and_bare_name from zerver.lib.types import DisplayRecipientT from zerver.lib.url_encoding import ( huddle_narrow_url, @@ -481,10 +482,11 @@ def do_send_missedmessage_events_reply_in_zulip( ) message = missed_messages[0]["message"] stream = Stream.objects.only("id", "name").get(id=message.recipient.type_id) - topic_name = message.topic_name() + topic_resolved, topic_name = get_topic_resolution_and_bare_name(message.topic_name()) context.update( stream_name=stream.name, topic_name=topic_name, + topic_resolved=topic_resolved, ) else: raise AssertionError("Invalid messages!") diff --git a/zerver/lib/topic.py b/zerver/lib/topic.py index e020f66925..82e13f44cf 100644 --- a/zerver/lib/topic.py +++ b/zerver/lib/topic.py @@ -270,3 +270,17 @@ def get_topic_history_for_stream( cursor.close() return generate_topic_history_from_db_rows(rows) + + +def get_topic_resolution_and_bare_name(stored_name: str) -> Tuple[bool, str]: + """ + Resolved topics are denoted only by a title change, not by a boolean toggle in a database column. This + method inspects the topic name and returns a tuple of: + + - Whether the topic has been resolved + - The topic name with the resolution prefix, if present in stored_name, removed + """ + if stored_name.startswith(RESOLVED_TOPIC_PREFIX): + return (True, stored_name[len(RESOLVED_TOPIC_PREFIX) :]) + + return (False, stored_name) diff --git a/zerver/tests/test_email_notifications.py b/zerver/tests/test_email_notifications.py index 3cc02e2e41..3b043bd4c1 100644 --- a/zerver/tests/test_email_notifications.py +++ b/zerver/tests/test_email_notifications.py @@ -1053,6 +1053,13 @@ class TestMissedMessages(ZulipTestCase): 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) + + def test_resolved_topic_missed_stream_messages_thread_friendly(self) -> None: + self._resolved_topic_missed_stream_messages_thread_friendly(False) + @override_settings(EMAIL_GATEWAY_PATTERN="") def test_reply_warning_in_missed_personal_messages(self) -> None: self._reply_warning_in_missed_personal_messages(False)