From b71c5746abd3aa67ec60d924f2e43f1c4fa9f59b Mon Sep 17 00:00:00 2001 From: Tim Abbott Date: Thu, 23 Nov 2023 13:10:26 -0800 Subject: [PATCH] notifications: Rename push_notifications_enabled for clarity. This doesn't actually check if push notifications are working, just whether there is configuration for them. --- api_docs/changelog.md | 7 ++++++ zerver/lib/events.py | 4 ++-- zerver/lib/push_notifications.py | 8 +++---- zerver/openapi/zulip.yaml | 9 ++++--- zerver/tests/test_auth_backends.py | 2 +- .../tests/test_message_edit_notifications.py | 6 ++--- zerver/tests/test_message_flags.py | 8 +++---- zerver/tests/test_push_notifications.py | 24 +++++++++---------- zerver/views/auth.py | 4 ++-- 9 files changed, 41 insertions(+), 31 deletions(-) diff --git a/api_docs/changelog.md b/api_docs/changelog.md index da8e04e688..e5116837af 100644 --- a/api_docs/changelog.md +++ b/api_docs/changelog.md @@ -20,6 +20,13 @@ format used by the Zulip server that they are interacting with. ## Changes in Zulip 8.0 +**Feature level 231** + +* [`GET /server_settings`](/api/get-server-settings): Renamed +`push_notifications_enabled` to `push_notifications_configured` + as it doesn't actually check if push notifications are working, + just whether there is configuration for them. + **Feature level 230** * [`GET /events`](/api/get-events): Added `has_trigger` field in diff --git a/zerver/lib/events.py b/zerver/lib/events.py index 23d837ae73..14f92efd12 100644 --- a/zerver/lib/events.py +++ b/zerver/lib/events.py @@ -39,7 +39,7 @@ from zerver.lib.muted_users import get_user_mutes from zerver.lib.narrow import check_narrow_for_events, read_stop_words from zerver.lib.narrow_helpers import NarrowTerm from zerver.lib.presence import get_presence_for_user, get_presences_for_realm -from zerver.lib.push_notifications import push_notifications_enabled +from zerver.lib.push_notifications import push_notifications_configured from zerver.lib.realm_icon import realm_icon_url from zerver.lib.realm_logo import get_realm_logo_source, get_realm_logo_url from zerver.lib.scheduled_messages import get_undelivered_scheduled_messages @@ -346,7 +346,7 @@ def fetch_initial_state_data( ] = settings.EVENT_QUEUE_LONGPOLL_TIMEOUT_SECONDS # TODO: Should these have the realm prefix replaced with server_? - state["realm_push_notifications_enabled"] = push_notifications_enabled() + state["realm_push_notifications_enabled"] = push_notifications_configured() state["realm_default_external_accounts"] = get_default_external_accounts() server_default_jitsi_server_url = ( diff --git a/zerver/lib/push_notifications.py b/zerver/lib/push_notifications.py index 4d0043a95a..8184f12bd7 100644 --- a/zerver/lib/push_notifications.py +++ b/zerver/lib/push_notifications.py @@ -710,7 +710,7 @@ def clear_push_device_tokens(user_profile_id: int) -> None: # -def push_notifications_enabled() -> bool: +def push_notifications_configured() -> bool: """True just if this server has configured a way to send push notifications.""" if ( uses_notification_bouncer() @@ -735,7 +735,7 @@ def push_notifications_enabled() -> bool: def initialize_push_notifications() -> None: - if not push_notifications_enabled(): + if not push_notifications_configured(): if settings.DEVELOPMENT and not settings.TEST_SUITE: # nocoverage # Avoid unnecessary spam on development environment startup return @@ -1037,7 +1037,7 @@ def handle_remove_push_notification(user_profile_id: int, message_ids: List[int] mobile app, when the message is read on the server, to remove the message from the notification. """ - if not push_notifications_enabled(): + if not push_notifications_configured(): return user_profile = get_user_profile_by_id(user_profile_id) @@ -1116,7 +1116,7 @@ def handle_push_notification(user_profile_id: int, missed_message: Dict[str, Any missed_message is the event received by the zerver.worker.queue_processors.PushNotificationWorker.consume function. """ - if not push_notifications_enabled(): + if not push_notifications_configured(): return user_profile = get_user_profile_by_id(user_profile_id) diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 676f5eef95..313370b099 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -15624,10 +15624,13 @@ paths: This will be `""` if unavailable. **Changes**: New in Zulip 5.0 (feature level 88). - push_notifications_enabled: + push_notifications_configured: type: boolean description: | - Whether mobile/push notifications are enabled. + Whether mobile/push notifications are configured. + + **Changes**: In Zulip 8.0 (feature level 231), renamed + `push_notifications_enabled` to `push_notifications_configured`. is_incompatible: type: boolean description: | @@ -15698,7 +15701,7 @@ paths: }, "zulip_version": "5.0-dev-1650-gc3fd37755f", "zulip_merge_base": "5.0-dev-1646-gea6b21cd8c", - "push_notifications_enabled": false, + "push_notifications_configured": false, "msg": "", "is_incompatible": false, "email_auth_enabled": true, diff --git a/zerver/tests/test_auth_backends.py b/zerver/tests/test_auth_backends.py index d15736f7d7..aef1cb7313 100644 --- a/zerver/tests/test_auth_backends.py +++ b/zerver/tests/test_auth_backends.py @@ -5248,7 +5248,7 @@ class FetchAuthBackends(ZulipTestCase): ("zulip_version", check_string), ("zulip_merge_base", check_string), ("zulip_feature_level", check_int), - ("push_notifications_enabled", check_bool), + ("push_notifications_configured", check_bool), ("realm_web_public_access_enabled", check_bool), ("msg", check_string), ("result", check_string), diff --git a/zerver/tests/test_message_edit_notifications.py b/zerver/tests/test_message_edit_notifications.py index 77c18a21ae..8f87ec692d 100644 --- a/zerver/tests/test_message_edit_notifications.py +++ b/zerver/tests/test_message_edit_notifications.py @@ -620,7 +620,7 @@ class EditMessageSideEffectsTest(ZulipTestCase): # push notifications or message notification emails. self.assert_length(info["queue_messages"], 0) - @mock.patch("zerver.lib.push_notifications.push_notifications_enabled", return_value=True) + @mock.patch("zerver.lib.push_notifications.push_notifications_configured", return_value=True) def test_clear_notification_when_mention_removed( self, mock_push_notifications: mock.MagicMock ) -> None: @@ -640,7 +640,7 @@ class EditMessageSideEffectsTest(ZulipTestCase): self.assertEqual(get_apns_badge_count(mentioned_user), 0) self.assertEqual(get_apns_badge_count_future(mentioned_user), 0) - @mock.patch("zerver.lib.push_notifications.push_notifications_enabled", return_value=True) + @mock.patch("zerver.lib.push_notifications.push_notifications_configured", return_value=True) def test_clear_notification_when_group_mention_removed( self, mock_push_notifications: mock.MagicMock ) -> None: @@ -663,7 +663,7 @@ class EditMessageSideEffectsTest(ZulipTestCase): self.assertEqual(get_apns_badge_count(group_mentioned_user), 0) self.assertEqual(get_apns_badge_count_future(group_mentioned_user), 0) - @mock.patch("zerver.lib.push_notifications.push_notifications_enabled", return_value=True) + @mock.patch("zerver.lib.push_notifications.push_notifications_configured", return_value=True) def test_not_clear_notification_when_mention_removed_but_stream_notified( self, mock_push_notifications: mock.MagicMock ) -> None: diff --git a/zerver/tests/test_message_flags.py b/zerver/tests/test_message_flags.py index 9d10962079..a7d46655a3 100644 --- a/zerver/tests/test_message_flags.py +++ b/zerver/tests/test_message_flags.py @@ -171,8 +171,8 @@ class UnreadCountTests(ZulipTestCase): def setUp(self) -> None: super().setUp() with mock.patch( - "zerver.lib.push_notifications.push_notifications_enabled", return_value=True - ) as mock_push_notifications_enabled: + "zerver.lib.push_notifications.push_notifications_configured", return_value=True + ) as mock_push_notifications_configured: self.unread_msg_ids = [ self.send_personal_message( self.example_user("iago"), self.example_user("hamlet"), "hello" @@ -181,7 +181,7 @@ class UnreadCountTests(ZulipTestCase): self.example_user("iago"), self.example_user("hamlet"), "hello2" ), ] - mock_push_notifications_enabled.assert_called() + mock_push_notifications_configured.assert_called() # Sending a new message results in unread UserMessages being created # for users other than sender. @@ -580,7 +580,7 @@ class PushNotificationMarkReadFlowsTest(ZulipTestCase): .values_list("message_id", flat=True) ) - @mock.patch("zerver.lib.push_notifications.push_notifications_enabled", return_value=True) + @mock.patch("zerver.lib.push_notifications.push_notifications_configured", return_value=True) def test_track_active_mobile_push_notifications( self, mock_push_notifications: mock.MagicMock ) -> None: diff --git a/zerver/tests/test_push_notifications.py b/zerver/tests/test_push_notifications.py index a6ed4d7cee..da4ddca7c0 100644 --- a/zerver/tests/test_push_notifications.py +++ b/zerver/tests/test_push_notifications.py @@ -1784,7 +1784,7 @@ class HandlePushNotificationTest(PushNotificationTest): with self.assertRaises(PushNotificationBouncerRetryLaterError): handle_push_notification(self.user_profile.id, missed_message) - @mock.patch("zerver.lib.push_notifications.push_notifications_enabled", return_value=True) + @mock.patch("zerver.lib.push_notifications.push_notifications_configured", return_value=True) def test_read_message(self, mock_push_notifications: mock.MagicMock) -> None: user_profile = self.example_user("hamlet") message = self.get_message( @@ -1850,7 +1850,7 @@ class HandlePushNotificationTest(PushNotificationTest): with mock.patch( "zerver.lib.push_notifications.uses_notification_bouncer" ) as mock_check, mock.patch("logging.error") as mock_logging_error, mock.patch( - "zerver.lib.push_notifications.push_notifications_enabled", return_value=True + "zerver.lib.push_notifications.push_notifications_configured", return_value=True ) as mock_push_notifications: handle_push_notification(user_profile.id, missed_message) mock_push_notifications.assert_called_once() @@ -1882,7 +1882,7 @@ class HandlePushNotificationTest(PushNotificationTest): with mock.patch( "zerver.lib.push_notifications.uses_notification_bouncer" ) as mock_check, self.assertLogs(level="INFO") as mock_logging_info, mock.patch( - "zerver.lib.push_notifications.push_notifications_enabled", return_value=True + "zerver.lib.push_notifications.push_notifications_configured", return_value=True ) as mock_push_notifications: handle_push_notification(user_profile.id, missed_message) mock_push_notifications.assert_called_once() @@ -1975,7 +1975,7 @@ class HandlePushNotificationTest(PushNotificationTest): "zerver.lib.push_notifications.send_android_push_notification", return_value=len(android_devices) - 1, ) as mock_send_android, mock.patch( - "zerver.lib.push_notifications.push_notifications_enabled", return_value=True + "zerver.lib.push_notifications.push_notifications_configured", return_value=True ) as mock_push_notifications: handle_push_notification(self.user_profile.id, missed_message) user_identity = UserPushIdentityCompat(user_id=self.user_profile.id) @@ -2062,7 +2062,7 @@ class HandlePushNotificationTest(PushNotificationTest): ) with mock.patch( - "zerver.lib.push_notifications.push_notifications_enabled", return_value=True + "zerver.lib.push_notifications.push_notifications_configured", return_value=True ) as mock_push_notifications, mock.patch( # Simulate the send...push_notification functions returning a number of successes # lesser than the number of devices, so that we can verify correct CountStat counting. @@ -2129,7 +2129,7 @@ class HandlePushNotificationTest(PushNotificationTest): message_id = self.send_stream_message(sender, "public_stream", "test") missed_message = {"message_id": message_id} with self.assertLogs("zerver.lib.push_notifications", level="ERROR") as logger, mock.patch( - "zerver.lib.push_notifications.push_notifications_enabled", return_value=True + "zerver.lib.push_notifications.push_notifications_configured", return_value=True ) as mock_push_notifications: handle_push_notification(self.user_profile.id, missed_message) self.assertEqual( @@ -2151,7 +2151,7 @@ class HandlePushNotificationTest(PushNotificationTest): self.subscribe(sender, "public_stream") message_id = self.send_stream_message(sender, "public_stream", "test") with mock.patch( - "zerver.lib.push_notifications.push_notifications_enabled", return_value=True + "zerver.lib.push_notifications.push_notifications_configured", return_value=True ) as mock_push_notifications, mock.patch( "zerver.lib.push_notifications.send_android_push_notification", return_value=1 ) as mock_send_android, mock.patch( @@ -2209,7 +2209,7 @@ class HandlePushNotificationTest(PushNotificationTest): ) as mock_send_android, mock.patch( "zerver.lib.push_notifications.logger.error" ) as mock_logger, mock.patch( - "zerver.lib.push_notifications.push_notifications_enabled", return_value=True + "zerver.lib.push_notifications.push_notifications_configured", return_value=True ) as mock_push_notifications: handle_push_notification(self.user_profile.id, missed_message) mock_logger.assert_not_called() @@ -2218,7 +2218,7 @@ class HandlePushNotificationTest(PushNotificationTest): mock_send_android.assert_called_with(user_identity, android_devices, {"gcm": True}, {}) mock_push_notifications.assert_called_once() - @mock.patch("zerver.lib.push_notifications.push_notifications_enabled", return_value=True) + @mock.patch("zerver.lib.push_notifications.push_notifications_configured", return_value=True) def test_user_push_soft_reactivate_soft_deactivated_user( self, mock_push_notifications: mock.MagicMock ) -> None: @@ -2342,7 +2342,7 @@ class HandlePushNotificationTest(PushNotificationTest): self.expect_to_stay_long_term_idle(self.user_profile, send_group_mention) @mock.patch("zerver.lib.push_notifications.logger.info") - @mock.patch("zerver.lib.push_notifications.push_notifications_enabled", return_value=True) + @mock.patch("zerver.lib.push_notifications.push_notifications_configured", return_value=True) def test_user_push_notification_already_active( self, mock_push_notifications: mock.MagicMock, mock_info: mock.MagicMock ) -> None: @@ -2517,7 +2517,7 @@ class TestAPNs(PushNotificationTest): ) self.assertEqual(modernize_apns_payload(payload), payload) - @mock.patch("zerver.lib.push_notifications.push_notifications_enabled", return_value=True) + @mock.patch("zerver.lib.push_notifications.push_notifications_configured", return_value=True) def test_apns_badge_count(self, mock_push_notifications: mock.MagicMock) -> None: user_profile = self.example_user("othello") # Test APNs badge count for personal messages. @@ -2585,7 +2585,7 @@ class TestGetAPNsPayload(PushNotificationTest): } self.assertDictEqual(payload, expected) - @mock.patch("zerver.lib.push_notifications.push_notifications_enabled", return_value=True) + @mock.patch("zerver.lib.push_notifications.push_notifications_configured", return_value=True) def test_get_message_payload_apns_huddle_message( self, mock_push_notifications: mock.MagicMock ) -> None: diff --git a/zerver/views/auth.py b/zerver/views/auth.py index 46ae4439c3..299b1aa5fb 100644 --- a/zerver/views/auth.py +++ b/zerver/views/auth.py @@ -57,7 +57,7 @@ from zerver.lib.exceptions import ( UserDeactivatedError, ) from zerver.lib.mobile_auth_otp import otp_encrypt_api_key -from zerver.lib.push_notifications import push_notifications_enabled +from zerver.lib.push_notifications import push_notifications_configured from zerver.lib.pysa import mark_sanitized from zerver.lib.realm_icon import realm_icon_url from zerver.lib.request import REQ, RequestNotes, has_request_variables @@ -1109,7 +1109,7 @@ def api_get_server_settings(request: HttpRequest) -> HttpResponse: zulip_version=ZULIP_VERSION, zulip_merge_base=ZULIP_MERGE_BASE, zulip_feature_level=API_FEATURE_LEVEL, - push_notifications_enabled=push_notifications_enabled(), + push_notifications_configured=push_notifications_configured(), is_incompatible=check_server_incompatibility(request), ) context = zulip_default_context(request)