mirror of
https://github.com/zulip/zulip.git
synced 2025-11-11 17:36:27 +00:00
push_notification: Return early if no device registered - w/o bouncer.
In 'send_push_notifications_legacy', when a user has no registered devices: * `uses_notification_bouncer()`=True: we log "Skipping..." and return. * `uses_notification_bouncer()`=False: we make some function calls, which effectively does nothing. It's better to have a common check (and log) early in the codepath. This commit makes that change.
This commit is contained in:
committed by
Tim Abbott
parent
fcb8956852
commit
d91a6be3f1
@@ -590,13 +590,7 @@ def send_notifications_to_bouncer(
|
|||||||
android_devices: Sequence[DeviceToken],
|
android_devices: Sequence[DeviceToken],
|
||||||
apple_devices: Sequence[DeviceToken],
|
apple_devices: Sequence[DeviceToken],
|
||||||
) -> None:
|
) -> None:
|
||||||
if len(android_devices) + len(apple_devices) == 0:
|
assert len(android_devices) + len(apple_devices) != 0
|
||||||
logger.info(
|
|
||||||
"Skipping contacting the bouncer for user %s because there are no registered devices",
|
|
||||||
user_profile.id,
|
|
||||||
)
|
|
||||||
|
|
||||||
return
|
|
||||||
|
|
||||||
post_data = {
|
post_data = {
|
||||||
"user_uuid": str(user_profile.uuid),
|
"user_uuid": str(user_profile.uuid),
|
||||||
@@ -1323,6 +1317,17 @@ def send_push_notifications_legacy(
|
|||||||
PushDeviceToken.objects.filter(user=user_profile, kind=PushDeviceToken.APNS).order_by("id")
|
PushDeviceToken.objects.filter(user=user_profile, kind=PushDeviceToken.APNS).order_by("id")
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# Added this nocoverage check for this commit. We plan to add a commit
|
||||||
|
# which will remove the mocking of 'send_push_notifications_legacy'
|
||||||
|
# in 'test_e2ee_push_notifications' - and this case will be covered
|
||||||
|
# there as a part of that broader effort.
|
||||||
|
if len(android_devices) + len(apple_devices) == 0: # nocoverage
|
||||||
|
logger.info(
|
||||||
|
"Skipping legacy push notifications for user %s because there are no registered devices",
|
||||||
|
user_profile.id,
|
||||||
|
)
|
||||||
|
return
|
||||||
|
|
||||||
if uses_notification_bouncer():
|
if uses_notification_bouncer():
|
||||||
send_notifications_to_bouncer(
|
send_notifications_to_bouncer(
|
||||||
user_profile, apns_payload, gcm_payload, gcm_options, android_devices, apple_devices
|
user_profile, apns_payload, gcm_payload, gcm_options, android_devices, apple_devices
|
||||||
|
|||||||
@@ -362,6 +362,9 @@ class HandlePushNotificationTest(PushNotificationTestCase):
|
|||||||
@mock.patch("zerver.lib.push_notifications.push_notifications_configured", return_value=True)
|
@mock.patch("zerver.lib.push_notifications.push_notifications_configured", return_value=True)
|
||||||
@override_settings(ZULIP_SERVICE_PUSH_NOTIFICATIONS=False, ZULIP_SERVICES=set())
|
@override_settings(ZULIP_SERVICE_PUSH_NOTIFICATIONS=False, ZULIP_SERVICES=set())
|
||||||
def test_read_message(self, mock_push_notifications: mock.MagicMock) -> None:
|
def test_read_message(self, mock_push_notifications: mock.MagicMock) -> None:
|
||||||
|
self.setup_apns_tokens()
|
||||||
|
self.setup_fcm_tokens()
|
||||||
|
|
||||||
user_profile = self.example_user("hamlet")
|
user_profile = self.example_user("hamlet")
|
||||||
message = self.get_message(
|
message = self.get_message(
|
||||||
Recipient.PERSONAL,
|
Recipient.PERSONAL,
|
||||||
|
|||||||
@@ -2138,23 +2138,6 @@ class TestGetGCMPayload(PushNotificationTestCase):
|
|||||||
|
|
||||||
|
|
||||||
class TestSendNotificationsToBouncer(PushNotificationTestCase):
|
class TestSendNotificationsToBouncer(PushNotificationTestCase):
|
||||||
def test_send_notifications_to_bouncer_when_no_devices(self) -> None:
|
|
||||||
user = self.example_user("hamlet")
|
|
||||||
|
|
||||||
with (
|
|
||||||
mock.patch("zerver.lib.remote_server.send_to_push_bouncer") as mock_send,
|
|
||||||
self.assertLogs("zerver.lib.push_notifications", level="INFO") as mock_logging_info,
|
|
||||||
):
|
|
||||||
send_notifications_to_bouncer(
|
|
||||||
user, {"apns": True}, {"gcm": True}, {}, android_devices=[], apple_devices=[]
|
|
||||||
)
|
|
||||||
|
|
||||||
self.assertIn(
|
|
||||||
f"INFO:zerver.lib.push_notifications:Skipping contacting the bouncer for user {user.id} because there are no registered devices",
|
|
||||||
mock_logging_info.output,
|
|
||||||
)
|
|
||||||
mock_send.assert_not_called()
|
|
||||||
|
|
||||||
@mock.patch("zerver.lib.remote_server.send_to_push_bouncer")
|
@mock.patch("zerver.lib.remote_server.send_to_push_bouncer")
|
||||||
def test_send_notifications_to_bouncer(self, mock_send: mock.MagicMock) -> None:
|
def test_send_notifications_to_bouncer(self, mock_send: mock.MagicMock) -> None:
|
||||||
user = self.example_user("hamlet")
|
user = self.example_user("hamlet")
|
||||||
|
|||||||
Reference in New Issue
Block a user