notifications: Remove support for unbatched push removal events.

We remove support for the old clients which required an event for
each message to clear notification.

This is justified since it has been around 1.5 years since we started
supporting the bulk operation (and so essentially nobody is using a
mobile app version so old that it doesn't support the batched
approach) and the unbatched approach has a maintenance and reliability
cost.
This commit is contained in:
Aman Agrawal
2020-06-30 11:54:37 +05:30
committed by Tim Abbott
parent 988e765e7c
commit 5b7917da5f
3 changed files with 12 additions and 32 deletions

View File

@@ -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,

View File

@@ -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))

View File

@@ -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