push_notif: Don't clear notif if stream notif enabled.

If a message was edited to remove a user mention, we don't
remove the push_notification for the user if the user has
stream_push_notfications enabled.
This commit is contained in:
Aman Agrawal
2020-06-27 15:41:12 +05:30
committed by Tim Abbott
parent 28166c14e8
commit 5f82e1a984
2 changed files with 26 additions and 8 deletions

View File

@@ -4149,7 +4149,9 @@ def do_mark_stream_messages_as_read(user_profile: UserProfile,
None, event_time, increment=min(1, count)) None, event_time, increment=min(1, count))
return count return count
def do_update_mobile_push_notification(message: Message, prior_mention_user_ids: Set[int]) -> None: def do_update_mobile_push_notification(message: Message,
prior_mention_user_ids: Set[int],
stream_push_user_ids: Set[int]) -> None:
# Called during the message edit code path to remove mobile push # Called during the message edit code path to remove mobile push
# notifications for users who are no longer mentioned following # notifications for users who are no longer mentioned following
# the edit. See #15428 for details. # the edit. See #15428 for details.
@@ -4158,15 +4160,10 @@ def do_update_mobile_push_notification(message: Message, prior_mention_user_ids:
# in a sent notification if a message was edited to mention a # in a sent notification if a message was edited to mention a
# group rather than a user (or vise versa), though it is likely # group rather than a user (or vise versa), though it is likely
# not worth the effort to do such a change. # not worth the effort to do such a change.
#
# This implementation is subtly incorrect, in that it will remove
# the push notification for a user who was mentioned (and then the
# mention removed) in a stream that they have configured to
# receive push notifications by default. That is likely worth fixing.
if not message.is_stream_message(): if not message.is_stream_message():
return return
remove_notify_users = prior_mention_user_ids - message.mentions_user_ids remove_notify_users = prior_mention_user_ids - message.mentions_user_ids - stream_push_user_ids
do_clear_mobile_push_notifications_for_ids(list(remove_notify_users), [message.id]) do_clear_mobile_push_notifications_for_ids(list(remove_notify_users), [message.id])
def do_clear_mobile_push_notifications_for_ids(user_profile_ids: List[int], def do_clear_mobile_push_notifications_for_ids(user_profile_ids: List[int],
@@ -4535,7 +4532,7 @@ def do_update_message(user_profile: UserProfile, message: Message,
else: else:
event['wildcard_mention_user_ids'] = [] event['wildcard_mention_user_ids'] = []
do_update_mobile_push_notification(message, prior_mention_user_ids) do_update_mobile_push_notification(message, prior_mention_user_ids, info['stream_push_user_ids'])
if topic_name is not None or new_stream is not None: if topic_name is not None or new_stream is not None:
orig_topic_name = message.topic_name() orig_topic_name = message.topic_name()

View File

@@ -531,3 +531,24 @@ class EditMessageSideEffectsTest(ZulipTestCase):
) )
self.assertEqual(get_apns_badge_count(group_mentioned_user), 0) self.assertEqual(get_apns_badge_count(group_mentioned_user), 0)
def test_not_clear_notification_when_mention_removed_but_stream_notified(self) -> None:
mentioned_user = self.example_user('iago')
mentioned_user.enable_stream_push_notifications = True
mentioned_user.save()
self.assertEqual(get_apns_badge_count(mentioned_user), 0)
with mock.patch('zerver.lib.push_notifications.push_notifications_enabled', return_value=True):
message_id = self._login_and_send_original_stream_message(
content="@**Iago**",
)
self.assertEqual(get_apns_badge_count(mentioned_user), 1)
self._get_queued_data_for_message_update(
message_id=message_id,
content="Removed mention"
)
self.assertEqual(get_apns_badge_count(mentioned_user), 1)