mirror of
				https://github.com/zulip/zulip.git
				synced 2025-10-31 20:13:46 +00:00 
			
		
		
		
	email: Apply message content visibility settings to digest too.
Fixes #33190.
This commit is contained in:
		| @@ -1,6 +1,7 @@ | ||||
| {% extends "zerver/emails/email_base_messages.html" %} | ||||
|  | ||||
| {% block content %} | ||||
|     {% if show_message_content %} | ||||
|     {% if hot_conversations %} | ||||
|         {% for convo in hot_conversations %} | ||||
|         <div class='messages'> | ||||
| @@ -28,6 +29,24 @@ | ||||
|  | ||||
|     <p class="digest_paragraph">{{ new_channels.html|display_list(1000)|safe }}.</p> | ||||
|     {% endif %} | ||||
|     {% else %} | ||||
|     <p class="digest_paragraph"> | ||||
|         {% if new_messages_count > 0 and new_streams_count > 0 %} | ||||
|             {% trans %}You have {{ new_messages_count }} new messages, and there are {{ new_streams_count }} new channels in <a href="{{ realm_url }}">{{ realm_name }}</a>.{% endtrans %} | ||||
|         {% elif new_messages_count > 0 and new_streams_count == 0 %} | ||||
|             {% trans %}You have {{ new_messages_count }} new messages in <a href="{{ realm_url }}">{{ realm_name }}</a>.{% endtrans %} | ||||
|         {% else  %} | ||||
|             {% trans %}There are {{ new_streams_count }} new channels in <a href="{{ realm_url }}">{{ realm_name }}</a>.{% endtrans %} | ||||
|         {% endif %} | ||||
|     </p> | ||||
|     <p class="digest_paragraph"> | ||||
|         {% if message_content_disabled_by_realm %} | ||||
|         {% trans help_url=realm_url + "/help/hide-message-content-in-emails" %}This email does not include message content because your organization <a class="content_disabled_help_link" href="{{ help_url }}">hides message content</a> in email notifications.{% endtrans %} | ||||
|         {% elif message_content_disabled_by_user %} | ||||
|         {% trans help_url=realm_url + "/help/email-notifications#hide-message-content" %}This email does not include message content because you have chosen to <a class="content_disabled_help_link" href="{{ help_url }}">hide message content</a> in email notifications.{% endtrans %} | ||||
|         {% endif %} | ||||
|     </p> | ||||
|     {% endif %} | ||||
|     <p class="digest_paragraph">{% trans %}<a href="{{ realm_url }}">Log in</a> to Zulip to catch up.{% endtrans %}</p> | ||||
| {% endblock %} | ||||
|  | ||||
|   | ||||
| @@ -1,3 +1,4 @@ | ||||
| {% 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 sender_block in recipient_block.senders %} | ||||
| @@ -8,6 +9,26 @@ | ||||
| {% if new_channels.plain %}** {% trans %}New channels{% endtrans %} ** | ||||
|     {{ new_channels.plain|display_list(1000) }}. | ||||
| {% endif %} | ||||
| {% else %} | ||||
| {% if new_messages_count > 0 and new_streams_count > 0 %} | ||||
| {% trans %}You have {{ new_messages_count }} new messages, and there are {{ new_streams_count }} new channels in {{ realm_name }}.{% endtrans %} | ||||
| {% elif new_messages_count > 0 and new_streams_count == 0 %} | ||||
| {% trans %}You have {{ new_messages_count }} new messages in {{ realm_name }}.{% endtrans %} | ||||
| {% else %} | ||||
| {% trans %}There are {{ new_streams_count }} new channels in {{ realm_name }}.{% endtrans %} | ||||
| {% endif %} | ||||
|  | ||||
| {% if message_content_disabled_by_realm %} | ||||
| {% trans hide_content_url=realm_url + "/help/hide-message-content-in-emails" %} | ||||
| This email does not include message content because your organization hides message content in email notifications. See {{ hide_content_url }} for more details. | ||||
| {% endtrans %} | ||||
| {% elif message_content_disabled_by_user %} | ||||
| {% trans help_url=realm_url + "/help/email-notifications#hide-message-content" %} | ||||
| This email does not include message content because you have chosen to hide message content in email notifications. See {{ help_url }} for more details. | ||||
| {% endtrans %} | ||||
| {% endif %} | ||||
| {% endif %} | ||||
|  | ||||
| {% trans organization_url=realm_url %}Log in to Zulip to catch up: {{ organization_url }}.{% endtrans %} | ||||
|  | ||||
| -- | ||||
|   | ||||
| @@ -14,7 +14,10 @@ from django.utils.translation import gettext as _ | ||||
|  | ||||
| from confirmation.models import one_click_unsubscribe_link | ||||
| from zerver.context_processors import common_context | ||||
| from zerver.lib.email_notifications import build_message_list | ||||
| from zerver.lib.email_notifications import ( | ||||
|     build_message_list, | ||||
|     message_content_allowed_in_missedmessage_emails, | ||||
| ) | ||||
| from zerver.lib.logging_util import log_to_file | ||||
| from zerver.lib.message import get_last_message_id | ||||
| from zerver.lib.queue import queue_json_publish_rollback_unsafe | ||||
| @@ -28,6 +31,7 @@ from zerver.models import ( | ||||
|     Stream, | ||||
|     Subscription, | ||||
|     UserActivityInterval, | ||||
|     UserMessage, | ||||
|     UserProfile, | ||||
| ) | ||||
| from zerver.models.realm_audit_logs import AuditLogEventType | ||||
| @@ -268,8 +272,21 @@ def gather_new_streams( | ||||
|     return len(new_streams), {"html": channels_html, "plain": channels_plain} | ||||
|  | ||||
|  | ||||
| def enough_traffic(hot_conversations: int, new_streams: int) -> bool: | ||||
|     return bool(hot_conversations or new_streams) | ||||
| def get_new_messages_count(user: UserProfile, threshold: datetime) -> int: | ||||
|     count = UserMessage.objects.filter( | ||||
|         user_profile=user, message__date_sent__gte=threshold, message__sender__is_bot=False | ||||
|     ).count() | ||||
|     return count | ||||
|  | ||||
|  | ||||
| def enough_traffic( | ||||
|     hot_conversations: int, new_streams: int, new_messages: int, show_message_content: bool | ||||
| ) -> bool: | ||||
|     if new_streams > 0: | ||||
|         return True | ||||
|     if not show_message_content: | ||||
|         return new_messages > 0 | ||||
|     return hot_conversations > 0 | ||||
|  | ||||
|  | ||||
| def get_user_stream_map(user_ids: list[int], cutoff_date: datetime) -> dict[int, set[int]]: | ||||
| @@ -350,33 +367,46 @@ def bulk_get_digest_context( | ||||
|     user_stream_map = get_user_stream_map(user_ids, cutoff_date) | ||||
|  | ||||
|     for user in users: | ||||
|         stream_ids = user_stream_map[user.id] | ||||
|  | ||||
|         recent_topics = [] | ||||
|         for stream_id in stream_ids: | ||||
|             recent_topics += get_recent_topics(realm.id, stream_id, cutoff_date) | ||||
|  | ||||
|         hot_topics = get_hot_topics(recent_topics, stream_ids) | ||||
|  | ||||
|         context = common_context(user) | ||||
|  | ||||
|         # Start building email template data. | ||||
|         unsubscribe_link = one_click_unsubscribe_link(user, "digest") | ||||
|         context.update(unsubscribe_link=unsubscribe_link) | ||||
|  | ||||
|         # Get context data for hot conversations. | ||||
|         context["hot_conversations"] = [ | ||||
|             hot_topic.teaser_data(user, stream_id_map) for hot_topic in hot_topics | ||||
|         ] | ||||
|  | ||||
|         # Gather new streams. | ||||
|         new_streams_count, new_streams = gather_new_streams( | ||||
|             realm=realm, | ||||
|             recently_created_streams=recently_created_streams, | ||||
|             can_access_public=user.can_access_public_streams(), | ||||
|         ) | ||||
|         context["new_channels"] = new_streams | ||||
|  | ||||
|         context["new_streams_count"] = new_streams_count | ||||
|         context[ | ||||
|             "message_content_disabled_by_realm" | ||||
|         ] = not realm.message_content_allowed_in_email_notifications | ||||
|         context[ | ||||
|             "message_content_disabled_by_user" | ||||
|         ] = not user.message_content_in_email_notifications | ||||
|  | ||||
|         if not message_content_allowed_in_missedmessage_emails(user): | ||||
|             # Count new messages when message content is hidden in email notifications. | ||||
|             context["new_messages_count"] = get_new_messages_count(user, cutoff_date) | ||||
|             context["hot_conversations"] = [] | ||||
|             context["show_message_content"] = False | ||||
|         else: | ||||
|             # Otherwise, get context data for hot conversations. | ||||
|             stream_ids = user_stream_map[user.id] | ||||
|             recent_topics = [] | ||||
|             for stream_id in stream_ids: | ||||
|                 recent_topics += get_recent_topics(realm.id, stream_id, cutoff_date) | ||||
|             hot_topics = get_hot_topics(recent_topics, stream_ids) | ||||
|  | ||||
|             context["hot_conversations"] = [ | ||||
|                 hot_topic.teaser_data(user, stream_id_map) for hot_topic in hot_topics | ||||
|             ] | ||||
|             context["new_channels"] = new_streams | ||||
|             context["new_messages_count"] = 0 | ||||
|             context["show_message_content"] = True | ||||
|  | ||||
|         yield user, context | ||||
|  | ||||
| @@ -400,7 +430,12 @@ def bulk_handle_digest_email(user_ids: list[int], cutoff: float) -> None: | ||||
|  | ||||
|     for user, context in bulk_get_digest_context(users, cutoff): | ||||
|         # We don't want to send emails containing almost no information. | ||||
|         if not enough_traffic(len(context["hot_conversations"]), context["new_streams_count"]): | ||||
|         if not enough_traffic( | ||||
|             len(context["hot_conversations"]), | ||||
|             context["new_streams_count"], | ||||
|             context["new_messages_count"], | ||||
|             context["show_message_content"], | ||||
|         ): | ||||
|             continue | ||||
|  | ||||
|         digest_users.append(user) | ||||
|   | ||||
| @@ -18,6 +18,7 @@ from zerver.lib.digest import ( | ||||
|     enqueue_emails, | ||||
|     gather_new_streams, | ||||
|     get_hot_topics, | ||||
|     get_new_messages_count, | ||||
|     get_recent_topics, | ||||
|     get_recently_created_streams, | ||||
|     get_user_stream_map, | ||||
| @@ -129,9 +130,13 @@ class TestDigestEmailMessages(ZulipTestCase): | ||||
|             set(emailed_user_ids), {user_id for user_id in user_ids if user_id != hamlet.id} | ||||
|         ) | ||||
|  | ||||
|     @mock.patch("zerver.lib.digest.get_new_messages_count") | ||||
|     @mock.patch("zerver.lib.digest.send_future_email") | ||||
|     def test_enough_traffic(self, mock_send_future_email: mock.MagicMock) -> None: | ||||
|     def test_enough_traffic( | ||||
|         self, mock_send_future_email: mock.MagicMock, mock_get_new_messages_count: mock.MagicMock | ||||
|     ) -> None: | ||||
|         othello = self.example_user("othello") | ||||
|         realm = othello.realm | ||||
|         self.subscribe(othello, "Verona") | ||||
|  | ||||
|         in_the_future = timezone_now().timestamp() + 60 | ||||
| @@ -144,7 +149,19 @@ class TestDigestEmailMessages(ZulipTestCase): | ||||
|         ) as enough_traffic_mock: | ||||
|             bulk_handle_digest_email([othello.id], in_the_future) | ||||
|             mock_send_future_email.assert_called() | ||||
|             enough_traffic_mock.assert_called_once_with(0, 0) | ||||
|             enough_traffic_mock.assert_called_once_with(0, 0, 0, True) | ||||
|  | ||||
|         do_set_realm_property( | ||||
|             realm, "message_content_allowed_in_email_notifications", False, acting_user=None | ||||
|         ) | ||||
|         mock_get_new_messages_count.return_value = 10 | ||||
|  | ||||
|         with mock.patch( | ||||
|             "zerver.lib.digest.enough_traffic", return_value=True | ||||
|         ) as enough_traffic_mock: | ||||
|             bulk_handle_digest_email([othello.id], in_the_future) | ||||
|             mock_send_future_email.assert_called() | ||||
|             enough_traffic_mock.assert_called_once_with(0, 0, 10, False) | ||||
|  | ||||
|     @mock.patch("zerver.lib.digest.enough_traffic") | ||||
|     @mock.patch("zerver.lib.digest.send_future_email") | ||||
| @@ -540,6 +557,78 @@ class TestDigestEmailMessages(ZulipTestCase): | ||||
|         self.assertEqual(stream_count, 0) | ||||
|         self.assertEqual(stream_info["html"], []) | ||||
|  | ||||
|     def test_get_new_messages_count(self) -> None: | ||||
|         Message.objects.all().delete() | ||||
|         othello = self.example_user("othello") | ||||
|         self.subscribe(othello, "Verona") | ||||
|         cutoff = timezone_now() - timedelta(days=5) | ||||
|  | ||||
|         senders = ["hamlet", "cordelia", "default_bot"] | ||||
|         # Exclude messages sent by bots from digests, as only human messages should be considered. | ||||
|         new_messages_count = len(self.simulate_stream_conversation("Verona", senders)) - 1 | ||||
|  | ||||
|         self.assertEqual( | ||||
|             get_new_messages_count(othello, cutoff), | ||||
|             new_messages_count, | ||||
|         ) | ||||
|  | ||||
|     @mock.patch("zerver.lib.digest.send_future_email") | ||||
|     def test_email_digest_when_message_content_is_disabled( | ||||
|         self, mock_send_future_email: mock.MagicMock | ||||
|     ) -> None: | ||||
|         Message.objects.all().delete() | ||||
|         Stream.objects.all().delete() | ||||
|         realm = get_realm("zulip") | ||||
|         othello = self.example_user("othello") | ||||
|         cutoff_date = timezone_now() - timedelta(days=5) | ||||
|         cutoff = time.mktime(cutoff_date.timetuple()) | ||||
|  | ||||
|         stream = create_stream_if_needed(realm, "New stream")[0] | ||||
|         # Make the stream appear to be older. | ||||
|         stream.date_created = timezone_now() - timedelta(days=7) | ||||
|         stream.save() | ||||
|  | ||||
|         self.subscribe(othello, "New stream") | ||||
|  | ||||
|         do_set_realm_property( | ||||
|             realm, "message_content_allowed_in_email_notifications", False, acting_user=None | ||||
|         ) | ||||
|  | ||||
|         senders = ["hamlet", "cordelia", "default_bot"] | ||||
|         # Exclude messages sent by bots from digests, as only human messages should be considered. | ||||
|         new_messages_count = len(self.simulate_stream_conversation("New stream", senders)) - 1 | ||||
|  | ||||
|         # When this test is run in isolation, one additional query is run which | ||||
|         # is equivalent to | ||||
|         # ContentType.objects.get(app_label='zerver', model='userprofile') | ||||
|         # This code is run when we call `confirmation.models.create_confirmation_link`. | ||||
|         # To trigger this, we call the one_click_unsubscribe_link function below. | ||||
|         one_click_unsubscribe_link(othello, "digest") | ||||
|         # Clear the LRU cache on the stream topics | ||||
|         get_recent_topics.cache_clear() | ||||
|         with self.assert_database_query_count(8): | ||||
|             bulk_handle_digest_email([othello.id], cutoff) | ||||
|  | ||||
|         self.assertEqual(mock_send_future_email.call_count, 1) | ||||
|         kwargs = mock_send_future_email.call_args[1] | ||||
|         self.assertEqual(kwargs["context"]["show_message_content"], False) | ||||
|         self.assertEqual(kwargs["context"]["message_content_disabled_by_realm"], True) | ||||
|         self.assertEqual(kwargs["context"]["new_streams_count"], 0) | ||||
|         self.assertEqual(kwargs["context"]["new_messages_count"], new_messages_count) | ||||
|  | ||||
|         stream.date_created = timezone_now() - timedelta(days=3) | ||||
|         stream.save() | ||||
|  | ||||
|         with self.assert_database_query_count(8): | ||||
|             bulk_handle_digest_email([othello.id], cutoff) | ||||
|  | ||||
|         self.assertEqual(mock_send_future_email.call_count, 2) | ||||
|         kwargs = mock_send_future_email.call_args[1] | ||||
|         self.assertEqual(kwargs["context"]["show_message_content"], False) | ||||
|         self.assertEqual(kwargs["context"]["message_content_disabled_by_realm"], True) | ||||
|         self.assertEqual(kwargs["context"]["new_streams_count"], 1) | ||||
|         self.assertEqual(kwargs["context"]["new_messages_count"], new_messages_count) | ||||
|  | ||||
|     def simulate_stream_conversation(self, stream: str, senders: list[str]) -> list[int]: | ||||
|         message_ids = []  # List[int] | ||||
|         for sender_name in senders: | ||||
|   | ||||
		Reference in New Issue
	
	Block a user