From 0364d0c8cacae8884309dbf33b2c0f2f0af345ee Mon Sep 17 00:00:00 2001 From: Sahil Batra Date: Sat, 24 Jul 2021 23:21:25 +0530 Subject: [PATCH] events: Add "user_settings_object" to client_capabilities. This commit adds "user_settings_object" field to client_capabilities which will be used to determine if the client needs 'update_display_settings' and 'update_global_notifications' event. --- templates/zerver/api/changelog.md | 6 ++++++ zerver/lib/events.py | 2 ++ zerver/lib/home.py | 1 + zerver/openapi/zulip.yaml | 17 +++++++++++++++-- zerver/tests/test_events.py | 27 +++++++++++++++++++++++++++ zerver/tornado/django_api.py | 2 ++ zerver/tornado/event_queue.py | 12 ++++++++++++ zerver/tornado/views.py | 4 ++++ zerver/views/events_register.py | 1 + 9 files changed, 70 insertions(+), 2 deletions(-) diff --git a/templates/zerver/api/changelog.md b/templates/zerver/api/changelog.md index 319ad7f2b0..370e7d5b7d 100644 --- a/templates/zerver/api/changelog.md +++ b/templates/zerver/api/changelog.md @@ -17,6 +17,12 @@ below features are supported. `user_settings`. The previous `update_display_settings` and `update_global_notifications` event types are still supported for backwards compatibility, but will be removed in future. +* [`POST /register`](/api/register-queue): Added the new + `user_settings_object` property to supported `client_capabilities`. +* [`GET /events`](/api/get-events): `update_display_settings` and + `update_global_notifications` are sent only when `user_settings_object` + is not included in the `client_capabilities` when registering the + event queue. **Feature level 88** diff --git a/zerver/lib/events.py b/zerver/lib/events.py index 84999e8e4f..9ac5baf699 100644 --- a/zerver/lib/events.py +++ b/zerver/lib/events.py @@ -1217,6 +1217,7 @@ def do_events_register( "user_avatar_url_field_optional", False ) stream_typing_notifications = client_capabilities.get("stream_typing_notifications", False) + user_settings_object = client_capabilities.get("user_settings_object", False) if user_profile.realm.email_address_visibility != Realm.EMAIL_ADDRESS_VISIBILITY_EVERYONE: # If real email addresses are not available to the user, their @@ -1248,6 +1249,7 @@ def do_events_register( narrow=narrow, bulk_message_deletion=bulk_message_deletion, stream_typing_notifications=stream_typing_notifications, + user_settings_object=user_settings_object, ) if queue_id is None: diff --git a/zerver/lib/home.py b/zerver/lib/home.py index 2c3f4d5dc5..bc7f79cdc2 100644 --- a/zerver/lib/home.py +++ b/zerver/lib/home.py @@ -135,6 +135,7 @@ def build_page_params_for_home_page_load( "bulk_message_deletion": True, "user_avatar_url_field_optional": True, "stream_typing_notifications": False, # Set this to True when frontend support is implemented. + "user_settings_object": False, # Set this to True when frontend support is implemented. } if user_profile is not None: diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 44c04a6a06..f5b73b3f2f 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -242,7 +242,9 @@ paths: - type: object description: | Event sent to a user's clients when that user's display settings - have changed. + have changed with an additional rule that it is only sent to + clients that did not include `user_settings_object` in their + `client_capabilities` when registering the event queue. **Changes**: Deprecated in Zulip 5.0 (feature level 89), replaced by the `user_settings` event type. @@ -283,7 +285,10 @@ paths: - type: object description: | Event sent to a user's clients when that user's [notification - settings](/api/update-settings) have changed. + settings](/api/update-settings) have changed with an additional + rule that it is only sent to clients that did not include + `user_settings_object` in their `client_capabilities` when + registering the event queue. **Changes**: Deprecated in Zulip 5.0 (feature level 89), replaced by the `user_settings` event type. @@ -8260,6 +8265,14 @@ paths: New in Zulip 4.0 (feature level 58). This capability is for backwards-compatibility; it will be required in a future server release. + + * `user_settings_object`: Boolean for whether the client supports the modern + `user_settings` event type. If False, the server will additionally send the + legacy `update_display_settings` and `update_global_notifications` event + types for backwards-compatibility with clients that predate this API migration. + + New in Zulip 4.0 (feature level 89). This capability is for + backwards-compatibility; it will be removed in a future server release. content: application/json: schema: diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index e39665f9bb..f1666c56a1 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -241,6 +241,7 @@ class BaseAction(ZulipTestCase): num_events: int = 1, bulk_message_deletion: bool = True, stream_typing_notifications: bool = True, + user_settings_object: bool = False, ) -> List[Dict[str, Any]]: """ Make sure we have a clean slate of client descriptors for these tests. @@ -267,6 +268,7 @@ class BaseAction(ZulipTestCase): narrow=[], bulk_message_deletion=bulk_message_deletion, stream_typing_notifications=stream_typing_notifications, + user_settings_object=user_settings_object, ) ) @@ -2057,6 +2059,31 @@ class NormalActionsTest(BaseAction): with self.assertRaises(RestartEventException): self.verify_action(lambda: send_restart_events(immediate=True)) + def test_display_setting_event_not_sent(self) -> None: + events = self.verify_action( + lambda: do_set_user_display_setting( + self.user_profile, + "default_view", + "all_messages", + ), + state_change_expected=True, + user_settings_object=True, + ) + check_user_settings_update("events[0]", events[0]) + + def test_notification_setting_event_not_sent(self) -> None: + events = self.verify_action( + lambda: do_change_notification_settings( + self.user_profile, + "enable_sounds", + False, + acting_user=self.user_profile, + ), + state_change_expected=True, + user_settings_object=True, + ) + check_user_settings_update("events[0]", events[0]) + class RealmPropertyActionTest(BaseAction): def do_set_realm_property_test(self, name: str) -> None: diff --git a/zerver/tornado/django_api.py b/zerver/tornado/django_api.py index fafb24e10c..b83eaa5b7b 100644 --- a/zerver/tornado/django_api.py +++ b/zerver/tornado/django_api.py @@ -75,6 +75,7 @@ def request_event_queue( narrow: Iterable[Sequence[str]] = [], bulk_message_deletion: bool = False, stream_typing_notifications: bool = False, + user_settings_object: bool = False, ) -> Optional[str]: if not settings.USING_TORNADO: @@ -95,6 +96,7 @@ def request_event_queue( "lifespan_secs": queue_lifespan_secs, "bulk_message_deletion": orjson.dumps(bulk_message_deletion), "stream_typing_notifications": orjson.dumps(stream_typing_notifications), + "user_settings_object": orjson.dumps(user_settings_object), } if event_types is not None: diff --git a/zerver/tornado/event_queue.py b/zerver/tornado/event_queue.py index 5e54b40d8e..02c5cb75a8 100644 --- a/zerver/tornado/event_queue.py +++ b/zerver/tornado/event_queue.py @@ -95,6 +95,7 @@ class ClientDescriptor: narrow: Collection[Sequence[str]] = [], bulk_message_deletion: bool = False, stream_typing_notifications: bool = False, + user_settings_object: bool = False, ) -> None: # These objects are serialized on shutdown and restored on restart. # If fields are added or semantics are changed, temporary code must be @@ -117,6 +118,7 @@ class ClientDescriptor: self.narrow_filter = build_narrow_filter(narrow) self.bulk_message_deletion = bulk_message_deletion self.stream_typing_notifications = stream_typing_notifications + self.user_settings_object = user_settings_object # Default for lifespan_secs is DEFAULT_EVENT_QUEUE_TIMEOUT_SECS; # but users can set it as high as MAX_QUEUE_TIMEOUT_SECS. @@ -143,6 +145,7 @@ class ClientDescriptor: client_type_name=self.client_type_name, bulk_message_deletion=self.bulk_message_deletion, stream_typing_notifications=self.stream_typing_notifications, + user_settings_object=self.user_settings_object, ) def __repr__(self) -> str: @@ -174,6 +177,7 @@ class ClientDescriptor: d.get("narrow", []), d.get("bulk_message_deletion", False), d.get("stream_typing_notifications", False), + d.get("user_settings_object", False), ) ret.last_connection_time = d["last_connection_time"] return ret @@ -217,6 +221,14 @@ class ClientDescriptor: # delivered if the stream_typing_notifications # client_capability is enabled, for backwards compatibility. return self.stream_typing_notifications + if self.user_settings_object and event["type"] in [ + "update_display_settings", + "update_global_notifications", + ]: + # 'update_display_settings' and 'update_global_notifications' + # events are sent only if user_settings_object is False, + # otherwise only 'user_settings' event is sent. + return False return True # TODO: Refactor so we don't need this function diff --git a/zerver/tornado/views.py b/zerver/tornado/views.py index defdf00d5c..30e82fc14a 100644 --- a/zerver/tornado/views.py +++ b/zerver/tornado/views.py @@ -103,6 +103,9 @@ def get_events_backend( stream_typing_notifications: bool = REQ( default=False, json_validator=check_bool, intentionally_undocumented=True ), + user_settings_object: bool = REQ( + default=False, json_validator=check_bool, intentionally_undocumented=True + ), ) -> HttpResponse: if all_public_streams and not user_profile.can_access_public_streams(): raise JsonableError(_("User not authorized for this query")) @@ -147,6 +150,7 @@ def get_events_backend( narrow=narrow, bulk_message_deletion=bulk_message_deletion, stream_typing_notifications=stream_typing_notifications, + user_settings_object=user_settings_object, ) result = fetch_events(events_query) diff --git a/zerver/views/events_register.py b/zerver/views/events_register.py index ff1c3c9cb3..8a630cd086 100644 --- a/zerver/views/events_register.py +++ b/zerver/views/events_register.py @@ -54,6 +54,7 @@ def events_register_backend( ("bulk_message_deletion", check_bool), ("user_avatar_url_field_optional", check_bool), ("stream_typing_notifications", check_bool), + ("user_settings_object", check_bool), ], value_validator=check_bool, ),