Navigation Menu

Skip to content

Commit

Permalink
message send: Rename push_notify_user_ids -> online_push_user_ids.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
abhijeetbodas2001 authored and timabbott committed May 26, 2021
1 parent f3eea72 commit ddd123f
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 13 deletions.
2 changes: 1 addition & 1 deletion docs/subsystems/notifications.md
Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions zerver/lib/actions.py
Expand Up @@ -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]
Expand Down Expand Up @@ -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"],
)

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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"],
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion zerver/lib/message.py
Expand Up @@ -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]
Expand Down
2 changes: 1 addition & 1 deletion zerver/tests/test_users.py
Expand Up @@ -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(),
Expand Down
16 changes: 12 additions & 4 deletions zerver/tornado/event_queue.py
Expand Up @@ -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"]
Expand All @@ -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):
Expand All @@ -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

Expand Down Expand Up @@ -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)

Expand Down

0 comments on commit ddd123f

Please sign in to comment.