diff --git a/templates/zerver/emails/digest.html b/templates/zerver/emails/digest.html index 2e2a36461a..fc85347f64 100644 --- a/templates/zerver/emails/digest.html +++ b/templates/zerver/emails/digest.html @@ -5,7 +5,7 @@ {% if hot_conversations %} {% for convo in hot_conversations %}
- {% for recipient_block in convo.first_few_messages %} + {% with recipient_block=convo.first_few_messages %}
{{ recipient_block.header.html|safe }}
@@ -19,7 +19,7 @@
{% if convo.count > 0 %}

+ {{ convo.count }} more message{{ convo.count|pluralize }} by {{ convo.participants|display_list(4) }}.

{% endif %} - {% endfor %} + {% endwith %}
{% endfor %} {% endif %} diff --git a/templates/zerver/emails/digest.txt b/templates/zerver/emails/digest.txt index a01975642f..e2763fdf38 100644 --- a/templates/zerver/emails/digest.txt +++ b/templates/zerver/emails/digest.txt @@ -1,10 +1,10 @@ {% if show_message_content %} {% if hot_conversations %} -{% for convo in hot_conversations %}{% for recipient_block in convo.first_few_messages %}{{ recipient_block.header.plain }} +{% for convo in hot_conversations %}{% with recipient_block=convo.first_few_messages %}{{ recipient_block.header.plain }} {% for sender_block in recipient_block.senders %} {% for message_block in sender_block.content %}{{ message_block.plain }} {% endfor %}{% endfor %} -{% if convo.count > 0 %}+ {{ convo.count }} more message{{ convo.count|pluralize }} by {{ convo.participants|display_list(4) }}.{% endif %}{% endfor %}{% endfor %}{% endif %} +{% if convo.count > 0 %}+ {{ convo.count }} more message{{ convo.count|pluralize }} by {{ convo.participants|display_list(4) }}.{% endif %}{% endwith %}{% endfor %}{% endif %} {% if new_channels.plain %}** {% trans %}New channels{% endtrans %} ** {{ new_channels.plain|display_list(1000) }}. diff --git a/templates/zerver/emails/missed_message.html b/templates/zerver/emails/missed_message.html index 571331e2df..9c6e97813c 100644 --- a/templates/zerver/emails/missed_message.html +++ b/templates/zerver/emails/missed_message.html @@ -2,7 +2,7 @@ {% block content %} {% if show_message_content %} - {% for recipient_block in messages %} + {% with recipient_block=messages %} {% for sender_block in recipient_block.senders %}
{% for message_block in sender_block.content %} @@ -10,7 +10,7 @@ {% endfor %}
{% endfor %} - {% endfor %} + {% endwith %} {% else %}
{% if message_content_disabled_by_realm %} diff --git a/templates/zerver/emails/missed_message.txt b/templates/zerver/emails/missed_message.txt index 6020384312..3740fa51e3 100644 --- a/templates/zerver/emails/missed_message.txt +++ b/templates/zerver/emails/missed_message.txt @@ -1,11 +1,11 @@ {% if show_message_content %} -{% for recipient_block in messages %} +{% with recipient_block=messages %} {% for sender_block in recipient_block.senders %} {% for message_block in sender_block.content %} {{ message_block.plain }} {% endfor %} {% endfor %} -{% endfor %} +{% endwith %} {% else %} {% if message_content_disabled_by_realm %} {% trans hide_content_url=realm_url + "/help/hide-message-content-in-emails" %} diff --git a/zerver/lib/email_notifications.py b/zerver/lib/email_notifications.py index d1957866c6..382b8bd04a 100644 --- a/zerver/lib/email_notifications.py +++ b/zerver/lib/email_notifications.py @@ -221,12 +221,11 @@ def build_message_list( user: UserProfile, messages: list[Message], stream_id_map: dict[int, Stream] | None = None, # only needs id, name -) -> list[dict[str, Any]]: +) -> dict[str, Any]: """ Builds the message list object for the message notification email and digest email template. All `messages` share the same recipient (and topic). """ - messages_to_render: list[dict[str, Any]] = [] def sender_string(message: Message) -> str: if message.recipient.type == Recipient.STREAM: @@ -342,54 +341,37 @@ def build_message_list( "html": header_html, } - # # Collapse message list to - # [ - # { - # "header": { - # "plain":"header", - # "html":"htmlheader" - # } - # "senders":[ - # { - # "sender":"sender_name", - # "content":[ - # { - # "plain":"content", - # "html":"htmlcontent" - # } - # { - # "plain":"content", - # "html":"htmlcontent" - # } - # ] - # } - # ] - # }, - # ] + # Collapse message list to: + # { + # "header": {"plain": "header", "html": "htmlheader"}, + # "senders": [ + # { + # "sender": "sender_name", + # "content": [ + # {"plain": "content", "html": "htmlcontent"}, + # {"plain": "content", "html": "htmlcontent"}, + # ], + # } + # ], + # } assert len(messages) > 0 recipients = {(message.recipient_id, message.topic_name().lower()) for message in messages} assert len(recipients) == 1, f"Unexpectedly multiple recipients: {recipients!r}" - header = message_header(messages[0]) messages.sort(key=lambda message: message.date_sent) - for message in messages: - # If we want to collapse into the previous recipient block - if len(messages_to_render) > 0: - sender = sender_string(message) - sender_block = messages_to_render[-1]["senders"] + header = message_header(messages[0]) + sender_block = [build_sender_payload(messages[0])] + messages_to_render = {"header": header, "senders": sender_block} - # Same message sender, collapse again - if sender_block[-1]["sender"] == sender: - sender_block[-1]["content"].append(build_message_payload(message)) - else: - # Start a new sender block - sender_block.append(build_sender_payload(message)) + for message in messages[1:]: + sender = sender_string(message) + # Same message sender, collapse again + if sender_block[-1]["sender"] == sender: + sender_block[-1]["content"].append(build_message_payload(message)) else: - # New recipient and sender block - recipient_block = {"header": header, "senders": [build_sender_payload(message)]} - - messages_to_render.append(recipient_block) + # Start a new sender block + sender_block.append(build_sender_payload(message)) return messages_to_render diff --git a/zerver/tests/test_digest.py b/zerver/tests/test_digest.py index 0f35784090..a4f58891ca 100644 --- a/zerver/tests/test_digest.py +++ b/zerver/tests/test_digest.py @@ -75,7 +75,7 @@ class TestDigestEmailMessages(ZulipTestCase): self.assertEqual(set(hot_convo["participants"]), expected_participants) self.assertEqual(hot_convo["count"], 5 - 2) # 5 messages, but 2 shown - teaser_messages = hot_convo["first_few_messages"][0]["senders"] + teaser_messages = hot_convo["first_few_messages"]["senders"] self.assertIn("some content", teaser_messages[0]["content"][0]["plain"]) self.assertIn(teaser_messages[0]["sender"], expected_participants) @@ -276,7 +276,7 @@ class TestDigestEmailMessages(ZulipTestCase): self.assertEqual(set(hot_convo["participants"]), expected_participants) self.assertEqual(hot_convo["count"], 5 - 2) # 5 messages, but 2 shown - teaser_messages = hot_convo["first_few_messages"][0]["senders"] + teaser_messages = hot_convo["first_few_messages"]["senders"] self.assertIn("some content", teaser_messages[0]["content"][0]["plain"]) self.assertIn(teaser_messages[0]["sender"], expected_participants)