From 43136abb70664586762bfd49ecccde638ecb3fcf Mon Sep 17 00:00:00 2001 From: Prakhar Pratyush Date: Mon, 22 Apr 2024 16:14:22 +0530 Subject: [PATCH] push_notification: Use 'get_mentioned_user_group' function. This commit updates the code in 'handle_push_notifications' to use a common lib function 'get_mentioned_user_group'. The code logically does the same thing in both the places, hence the change. --- zerver/lib/email_notifications.py | 8 ++++++-- zerver/lib/notification_data.py | 33 +++++++++++++++++++++---------- zerver/lib/push_notifications.py | 18 ++++++++--------- 3 files changed, 37 insertions(+), 22 deletions(-) diff --git a/zerver/lib/email_notifications.py b/zerver/lib/email_notifications.py index b52b013cea..1202faf633 100644 --- a/zerver/lib/email_notifications.py +++ b/zerver/lib/email_notifications.py @@ -25,7 +25,7 @@ from confirmation.models import one_click_unsubscribe_link from zerver.lib.display_recipient import get_display_recipient from zerver.lib.markdown.fenced_code import FENCE_RE from zerver.lib.message import bulk_access_messages -from zerver.lib.notification_data import get_mentioned_user_group_name +from zerver.lib.notification_data import get_mentioned_user_group 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 @@ -413,7 +413,11 @@ def do_send_missedmessage_events_reply_in_zulip( ), ) - mentioned_user_group_name = get_mentioned_user_group_name(missed_messages, user_profile) + mentioned_user_group_name = None + mentioned_user_group = get_mentioned_user_group(missed_messages, user_profile) + if mentioned_user_group is not None: + mentioned_user_group_name = mentioned_user_group.name + triggers = [message["trigger"] for message in missed_messages] unique_triggers = set(triggers) diff --git a/zerver/lib/notification_data.py b/zerver/lib/notification_data.py index d80174f6d9..95f669ba36 100644 --- a/zerver/lib/notification_data.py +++ b/zerver/lib/notification_data.py @@ -313,9 +313,16 @@ def get_user_group_mentions_data( return mentioned_user_groups_map -def get_mentioned_user_group_name( +@dataclass +class MentionedUserGroup: + id: int + name: str + members_count: int + + +def get_mentioned_user_group( messages: List[Dict[str, Any]], user_profile: UserProfile -) -> Optional[str]: +) -> Optional[MentionedUserGroup]: """Returns the user group name to display in the email notification if user group(s) are mentioned. @@ -325,7 +332,7 @@ def get_mentioned_user_group_name( """ for message in messages: if ( - message["mentioned_user_group_id"] is None + message.get("mentioned_user_group_id") is None and message["trigger"] == NotificationTriggers.MENTION ): # The user has also been personally mentioned, so that gets prioritized. @@ -335,21 +342,27 @@ def get_mentioned_user_group_name( mentioned_user_group_ids = [ message["mentioned_user_group_id"] for message in messages - if message["mentioned_user_group_id"] is not None + if message.get("mentioned_user_group_id") is not None ] + if len(mentioned_user_group_ids) == 0: + return None + # We now want to calculate the name of the smallest user group mentioned among # all these messages. smallest_user_group_size = math.inf - smallest_user_group_name = None for user_group_id in mentioned_user_group_ids: current_user_group = UserGroup.objects.get(id=user_group_id, realm=user_profile.realm) - current_user_group_size = len(get_user_group_member_ids(current_user_group)) + current_mentioned_user_group = MentionedUserGroup( + id=current_user_group.id, + name=current_user_group.name, + members_count=len(get_user_group_member_ids(current_user_group)), + ) - if current_user_group_size < smallest_user_group_size: + if current_mentioned_user_group.members_count < smallest_user_group_size: # If multiple user groups are mentioned, we prefer the # user group with the least members. - smallest_user_group_size = current_user_group_size - smallest_user_group_name = current_user_group.name + smallest_user_group_size = current_mentioned_user_group.members_count + smallest_mentioned_user_group = current_mentioned_user_group - return smallest_user_group_name + return smallest_mentioned_user_group diff --git a/zerver/lib/push_notifications.py b/zerver/lib/push_notifications.py index 738914e1c6..58e08210ef 100644 --- a/zerver/lib/push_notifications.py +++ b/zerver/lib/push_notifications.py @@ -43,6 +43,7 @@ from zerver.lib.display_recipient import get_display_recipient from zerver.lib.emoji_utils import hex_codepoint_to_emoji from zerver.lib.exceptions import ErrorCode, JsonableError from zerver.lib.message import access_message_and_usermessage, huddle_users +from zerver.lib.notification_data import get_mentioned_user_group from zerver.lib.outgoing_http import OutgoingSession from zerver.lib.remote_server import ( record_push_notifications_recently_working, @@ -62,7 +63,6 @@ from zerver.models import ( Realm, Recipient, Stream, - UserGroup, UserMessage, UserProfile, ) @@ -1351,17 +1351,15 @@ def handle_push_notification(user_profile_id: int, missed_message: Dict[str, Any if trigger == "private_message": trigger = NotificationTriggers.DIRECT_MESSAGE # nocoverage - mentioned_user_group_name = None - # mentioned_user_group_id will be None if the user is personally mentioned + # mentioned_user_group will be None if the user is personally mentioned # regardless whether they are a member of the mentioned user group in the # message or not. - mentioned_user_group_id = missed_message.get("mentioned_user_group_id") - - if mentioned_user_group_id is not None: - user_group = UserGroup.objects.get( - id=mentioned_user_group_id, realm_id=user_profile.realm_id - ) - mentioned_user_group_name = user_group.name + mentioned_user_group_id = None + mentioned_user_group_name = None + mentioned_user_group = get_mentioned_user_group([missed_message], user_profile) + if mentioned_user_group is not None: + mentioned_user_group_id = mentioned_user_group.id + mentioned_user_group_name = mentioned_user_group.name # Soft reactivate if pushing to a long_term_idle user that is personally mentioned soft_reactivate_if_personal_notification(user_profile, {trigger}, mentioned_user_group_name)