diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index c4dfc1e20e..25749b6621 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -4169,7 +4169,10 @@ def do_update_mobile_push_notification(message: Message, def do_clear_mobile_push_notifications_for_ids(user_profile_ids: List[int], message_ids: List[int]) -> None: - # This functions supports clearing notifications for several users + if len(message_ids) == 0: + return + + # This function supports clearing notifications for several users # only for the message-edit use case where we'll have a single message_id. assert len(user_profile_ids) == 1 or len(message_ids) == 1 @@ -4180,28 +4183,16 @@ def do_clear_mobile_push_notifications_for_ids(user_profile_ids: List[int], ).extra( where=[UserMessage.where_active_push_notification()], ).values_list('user_profile_id', 'message_id')) + for (user_id, message_id) in notifications_to_update: messages_by_user[user_id].append(message_id) - num_detached = settings.MAX_UNBATCHED_REMOVE_NOTIFICATIONS - 1 for user_profile_id in user_profile_ids: - filtered_message_ids = messages_by_user[user_profile_id] - for message_id in filtered_message_ids[:num_detached]: - # Older clients (all clients older than 2019-02-13) will only - # see the first message ID in a given notification-message. - # To help them out, send a few of these separately. - queue_json_publish("missedmessage_mobile_notifications", { - "type": "remove", - "user_profile_id": user_profile_id, - "message_ids": [message_id], - }) - - if filtered_message_ids[num_detached:]: - queue_json_publish("missedmessage_mobile_notifications", { - "type": "remove", - "user_profile_id": user_profile_id, - "message_ids": filtered_message_ids[num_detached:], - }) + queue_json_publish("missedmessage_mobile_notifications", { + "type": "remove", + "user_profile_id": user_profile_id, + "message_ids": messages_by_user[user_profile_id], + }) def do_update_message_flags(user_profile: UserProfile, client: Client, diff --git a/zerver/tests/test_push_notifications.py b/zerver/tests/test_push_notifications.py index 10862c6ec0..a973198b2e 100644 --- a/zerver/tests/test_push_notifications.py +++ b/zerver/tests/test_push_notifications.py @@ -1900,7 +1900,6 @@ class GCMSendTest(PushNotificationTest): class TestClearOnRead(ZulipTestCase): def test_mark_stream_as_read(self) -> None: n_msgs = 3 - max_unbatched = 2 hamlet = self.example_user("hamlet") hamlet.enable_stream_push_notifications = True @@ -1919,14 +1918,11 @@ class TestClearOnRead(ZulipTestCase): UserMessage.flags.active_mobile_push_notification)) with mock.patch("zerver.lib.actions.queue_json_publish") as mock_publish: - with override_settings(MAX_UNBATCHED_REMOVE_NOTIFICATIONS=max_unbatched): - do_mark_stream_messages_as_read(hamlet, self.client, stream) + do_mark_stream_messages_as_read(hamlet, self.client, stream) queue_items = [c[0][1] for c in mock_publish.call_args_list] groups = [item['message_ids'] for item in queue_items] - self.assertEqual(len(groups), min(len(message_ids), max_unbatched)) - for g in groups[:-1]: - self.assertEqual(len(g), 1) + self.assert_length(groups, 1) self.assertEqual(sum(len(g) for g in groups), len(message_ids)) self.assertEqual({id for g in groups for id in g}, set(message_ids)) diff --git a/zproject/default_settings.py b/zproject/default_settings.py index 05a3c8dca7..96896cf2f9 100644 --- a/zproject/default_settings.py +++ b/zproject/default_settings.py @@ -285,13 +285,6 @@ APNS_SANDBOX = True APNS_TOPIC = 'org.zulip.Zulip' ZULIP_IOS_APP_ID = 'org.zulip.Zulip' -# Max number of "remove notification" FCM/GCM messages to send separately -# in one burst; the rest are batched. Older clients ignore the batched -# portion, so only receive this many removals. Lower values mitigate -# server congestion and client battery use. To batch unconditionally, -# set to 1. -MAX_UNBATCHED_REMOVE_NOTIFICATIONS = 10 - # Limits related to the size of file uploads; last few in MB. DATA_UPLOAD_MAX_MEMORY_SIZE = 25 * 1024 * 1024 MAX_AVATAR_FILE_SIZE = 5