From ddd123f133612beb8c591ba7a7115eec4d0aee39 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Bodas Date: Mon, 24 May 2021 09:27:59 +0530 Subject: [PATCH] message send: Rename `push_notify_user_ids` -> `online_push_user_ids`. The old name `push_notify_user_ids` was misleading, because it does not contain user ids which should be notified for the current message, but rather user ids who have the online push notifications setting enabled. When the Tornado server is restarted during an upgrade, if server has old events with the `push_notify_user_ids` fields, the server will throw error after this rename. Hence, we need to explicitly handle such cases while processing the event. --- docs/subsystems/notifications.md | 2 +- zerver/lib/actions.py | 12 ++++++------ zerver/lib/message.py | 2 +- zerver/tests/test_users.py | 2 +- zerver/tornado/event_queue.py | 16 ++++++++++++---- 5 files changed, 21 insertions(+), 13 deletions(-) diff --git a/docs/subsystems/notifications.md b/docs/subsystems/notifications.md index 480bce01c7930..a66ae396f2faa 100644 --- a/docs/subsystems/notifications.md +++ b/docs/subsystems/notifications.md @@ -32,7 +32,7 @@ as follows: table's `flags` structure, which is in turn passed into `send_event` for each user receiving the message. * Data about user configuration relevant to the message, such as - `push_notify_user_ids` and `stream_notify_user_ids`, are included + `online_push_user_ids` and `stream_notify_user_ids`, are included alongside `flags` in the per-user data structure. * The `presence_idle_user_ids` set, containing the subset of recipient users who are mentioned, are PM recipients, have alert diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index 66b612afb1fb4..65c3bdbb7964c 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -1420,7 +1420,7 @@ def render_incoming_message( class RecipientInfoResult(TypedDict): active_user_ids: Set[int] - push_notify_user_ids: Set[int] + online_push_user_ids: Set[int] stream_email_user_ids: Set[int] stream_push_user_ids: Set[int] wildcard_mention_user_ids: Set[int] @@ -1587,7 +1587,7 @@ def is_service_bot(row: Dict[str, Any]) -> bool: return row["is_bot"] and (row["bot_type"] in UserProfile.SERVICE_BOT_TYPES) active_user_ids = get_ids_for(lambda r: True) - push_notify_user_ids = get_ids_for( + online_push_user_ids = get_ids_for( lambda r: r["enable_online_push_notifications"], ) @@ -1618,7 +1618,7 @@ def is_service_bot(row: Dict[str, Any]) -> bool: info: RecipientInfoResult = dict( active_user_ids=active_user_ids, - push_notify_user_ids=push_notify_user_ids, + online_push_user_ids=online_push_user_ids, stream_push_user_ids=stream_push_user_ids, stream_email_user_ids=stream_email_user_ids, wildcard_mention_user_ids=wildcard_mention_user_ids, @@ -1811,7 +1811,7 @@ def build_message_send_dict( mention_data=mention_data, message=message, active_user_ids=info["active_user_ids"], - push_notify_user_ids=info["push_notify_user_ids"], + online_push_user_ids=info["online_push_user_ids"], stream_push_user_ids=info["stream_push_user_ids"], stream_email_user_ids=info["stream_email_user_ids"], um_eligible_user_ids=info["um_eligible_user_ids"], @@ -1961,7 +1961,7 @@ def do_send_messages( dict( id=user_id, flags=user_flags.get(user_id, []), - always_push_notify=(user_id in send_request.push_notify_user_ids), + always_push_notify=(user_id in send_request.online_push_user_ids), stream_push_notify=(user_id in send_request.stream_push_user_ids), stream_email_notify=(user_id in send_request.stream_email_user_ids), wildcard_mention_notify=(user_id in send_request.wildcard_mention_user_ids), @@ -5789,7 +5789,7 @@ def do_update_message( possible_wildcard_mention=mention_data.message_has_wildcards(), ) - event["push_notify_user_ids"] = list(info["push_notify_user_ids"]) + event["online_push_user_ids"] = list(info["online_push_user_ids"]) event["stream_push_user_ids"] = list(info["stream_push_user_ids"]) event["stream_email_user_ids"] = list(info["stream_email_user_ids"]) event["prior_mention_user_ids"] = list(prior_mention_user_ids) diff --git a/zerver/lib/message.py b/zerver/lib/message.py index a57721bda665c..0ce599e258b35 100644 --- a/zerver/lib/message.py +++ b/zerver/lib/message.py @@ -95,7 +95,7 @@ class SendMessageRequest: realm: Realm mention_data: MentionData active_user_ids: Set[int] - push_notify_user_ids: Set[int] + online_push_user_ids: Set[int] stream_push_user_ids: Set[int] stream_email_user_ids: Set[int] um_eligible_user_ids: Set[int] diff --git a/zerver/tests/test_users.py b/zerver/tests/test_users.py index 146f930e41bf0..023dff0fd9cb4 100644 --- a/zerver/tests/test_users.py +++ b/zerver/tests/test_users.py @@ -1520,7 +1520,7 @@ def test_stream_recipient_info(self) -> None: expected_info = dict( active_user_ids=all_user_ids, - push_notify_user_ids=set(), + online_push_user_ids=set(), stream_push_user_ids=set(), stream_email_user_ids=set(), wildcard_mention_user_ids=set(), diff --git a/zerver/tornado/event_queue.py b/zerver/tornado/event_queue.py index 71cddcb3b6655..8e7026d13b10b 100644 --- a/zerver/tornado/event_queue.py +++ b/zerver/tornado/event_queue.py @@ -1102,7 +1102,15 @@ def process_message_update_event( stream_push_user_ids = set(event_template.pop("stream_push_user_ids", [])) stream_email_user_ids = set(event_template.pop("stream_email_user_ids", [])) wildcard_mention_user_ids = set(event_template.pop("wildcard_mention_user_ids", [])) - push_notify_user_ids = set(event_template.pop("push_notify_user_ids", [])) + + # TODO/compatibility: Translation code for the rename of + # `push_notify_user_ids` to `online_push_user_ids`. Remove this + # when one can no longer directly upgrade from 4.x to master. + online_push_user_ids = set() + if "online_push_user_ids" in event_template: + online_push_user_ids = set(event_template.pop("online_push_user_ids")) + elif "push_notify_user_ids" in event_template: + online_push_user_ids = set(event_template.pop("push_notify_user_ids")) stream_name = event_template.get("stream_name") message_id = event_template["message_id"] @@ -1128,7 +1136,7 @@ def process_message_update_event( presence_idle_user_ids=presence_idle_user_ids, stream_push_user_ids=stream_push_user_ids, stream_email_user_ids=stream_email_user_ids, - push_notify_user_ids=push_notify_user_ids, + online_push_user_ids=online_push_user_ids, ) for client in get_client_descriptors_for_user(user_profile_id): @@ -1148,7 +1156,7 @@ def maybe_enqueue_notifications_for_message_update( presence_idle_user_ids: Set[int], stream_push_user_ids: Set[int], stream_email_user_ids: Set[int], - push_notify_user_ids: Set[int], + online_push_user_ids: Set[int], ) -> None: private_message = stream_name is None @@ -1187,7 +1195,7 @@ def maybe_enqueue_notifications_for_message_update( # We can have newly mentioned people in an updated message. mentioned = user_profile_id in mention_user_ids - always_push_notify = user_profile_id in push_notify_user_ids + always_push_notify = user_profile_id in online_push_user_ids idle = (user_profile_id in presence_idle_user_ids) or receiver_is_off_zulip(user_profile_id)