mirror of https://github.com/zulip/zulip.git
notifications: Fix soft-deactivated users don't get push notifications.
Fixes the urgent part of #10397. It was discovered that soft-deactivated users don't get mobile push notifications for messages on private streams that they have configured to send push notifications. Reason: `handle_push_notification` calls `access_message`, and that logic assumes that a user who is a recipient of a message has an associated UserMessage row. Those UserMessage rows are created lazily for soft-deactivated users, so they might not exist (yet) until the user comes back. Solution: Ensure that userMessage row is created for stream_push_user_ids and stream_email_user_ids in create_user_messages.
This commit is contained in:
parent
d152b84ccc
commit
6bfa29b8e6
|
@ -1283,6 +1283,8 @@ def do_send_messages(messages_maybe_none: Sequence[Optional[MutableMapping[str,
|
|||
message=message['message'],
|
||||
um_eligible_user_ids=message['um_eligible_user_ids'],
|
||||
long_term_idle_user_ids=message['long_term_idle_user_ids'],
|
||||
stream_push_user_ids = message['stream_push_user_ids'],
|
||||
stream_email_user_ids = message['stream_email_user_ids'],
|
||||
mentioned_user_ids=mentioned_user_ids,
|
||||
)
|
||||
|
||||
|
@ -1437,9 +1439,10 @@ class UserMessageLite:
|
|||
def create_user_messages(message: Message,
|
||||
um_eligible_user_ids: Set[int],
|
||||
long_term_idle_user_ids: Set[int],
|
||||
stream_push_user_ids: Set[int],
|
||||
stream_email_user_ids: Set[int],
|
||||
mentioned_user_ids: Set[int]) -> List[UserMessageLite]:
|
||||
ums_to_create = []
|
||||
|
||||
for user_profile_id in um_eligible_user_ids:
|
||||
um = UserMessageLite(
|
||||
user_profile_id=user_profile_id,
|
||||
|
@ -1465,9 +1468,27 @@ def create_user_messages(message: Message,
|
|||
if message.recipient.type in [Recipient.HUDDLE, Recipient.PERSONAL]:
|
||||
um.flags |= UserMessage.flags.is_private
|
||||
|
||||
# For long_term_idle (aka soft-deactivated) users, we are allowed
|
||||
# to optimize by lazily not creating UserMessage rows that would
|
||||
# have the default 0 flag set (since the soft-reactivation logic
|
||||
# knows how to create those when the user comes back). We need to
|
||||
# create the UserMessage rows for these long_term_idle users
|
||||
# non-lazily in a few cases:
|
||||
#
|
||||
# * There are nonzero flags (e.g. the user was mentioned), since
|
||||
# that case is rare and this saves a lot of complexity in
|
||||
# soft-reactivation.
|
||||
#
|
||||
# * If the user is going to be notified (e.g. they get push/email
|
||||
# notifications for every message on a stream), since in that
|
||||
# case the notifications code will call `access_message` on the
|
||||
# message to re-verify permissions, and for private streams,
|
||||
# will get an error if the UserMessage row doesn't exist yet.
|
||||
user_messages = []
|
||||
for um in ums_to_create:
|
||||
if (um.user_profile_id in long_term_idle_user_ids and
|
||||
um.user_profile_id not in stream_push_user_ids and
|
||||
um.user_profile_id not in stream_email_user_ids and
|
||||
message.is_stream_message() and
|
||||
int(um.flags) == 0):
|
||||
continue
|
||||
|
|
|
@ -61,6 +61,7 @@ from zerver.lib.test_helpers import (
|
|||
most_recent_message,
|
||||
most_recent_usermessage,
|
||||
queries_captured,
|
||||
get_subscription,
|
||||
)
|
||||
|
||||
from zerver.lib.test_classes import (
|
||||
|
@ -3583,6 +3584,21 @@ class SoftDeactivationMessageTest(ZulipTestCase):
|
|||
assert_um_count(cordelia, general_user_msg_count + 1)
|
||||
assert_last_um_content(cordelia, message)
|
||||
|
||||
# Test that sending a message to a stream with soft deactivated user
|
||||
# and push/email notifications on creates a UserMessage row for the
|
||||
# deactivated user.
|
||||
sub = get_subscription(stream_name, long_term_idle_user)
|
||||
sub.push_notifications = True
|
||||
sub.save()
|
||||
general_user_msg_count = len(get_user_messages(cordelia))
|
||||
soft_deactivated_user_msg_count = len(get_user_messages(long_term_idle_user))
|
||||
message = 'Test private stream message'
|
||||
send_stream_message(message)
|
||||
assert_um_count(long_term_idle_user, soft_deactivated_user_msg_count + 1)
|
||||
assert_last_um_content(long_term_idle_user, message)
|
||||
sub.push_notifications = False
|
||||
sub.save()
|
||||
|
||||
# Test sending a private message to soft deactivated user creates
|
||||
# UserMessage row.
|
||||
soft_deactivated_user_msg_count = len(get_user_messages(long_term_idle_user))
|
||||
|
|
Loading…
Reference in New Issue