From 885c3d65934d275d44caadb496d3b6bef6edba75 Mon Sep 17 00:00:00 2001 From: Lauryn Menard Date: Tue, 9 Aug 2022 20:37:07 +0200 Subject: [PATCH] subscriptions: Send update events for `is_muted` property. In Zulip 2.1.0, the `is_muted` stream subscription property was added and replaced the `in_home_view` property. But the server has still only been sending subscription update events with the `in_home_view` property. Updates `do_change_subscription_property` to send a subscription update event for both `is_muted` and `in_home_view`, so that clients can fully migrate away from using `in_home_view` allowing us to eventually remove it completely. --- templates/zerver/api/changelog.md | 14 +++++++-- version.py | 2 +- zerver/actions/streams.py | 28 ++++++++++++------ zerver/openapi/zulip.yaml | 25 ++++++++++++---- zerver/tests/test_events.py | 49 +++++++++++++++++++++++++++++++ zerver/tests/test_subs.py | 12 ++++++-- 6 files changed, 109 insertions(+), 21 deletions(-) diff --git a/templates/zerver/api/changelog.md b/templates/zerver/api/changelog.md index 6a0504e704..6dee09edca 100644 --- a/templates/zerver/api/changelog.md +++ b/templates/zerver/api/changelog.md @@ -20,6 +20,15 @@ format used by the Zulip server that they are interacting with. ## Changes in Zulip 6.0 +**Feature level 139** + +* [`GET /get-events`](/api/get-events): When a user mutes or unmutes + their subscription to a stream, a `subscription` update event + is now sent for the `is_muted` property and for the deprecated + `in_home_view` property to support clients fully migrating to use the + `is_muted` property. Prior to this feature level, only one event was + sent to clients with the deprecated `in_home_view` property. + **Feature level 138** * [`POST /register`](/api/register-queue), [`GET @@ -228,7 +237,7 @@ No changes; feature level used for Zulip 5.0 release. **Feature level 111** -* [`POST /subscriptions/properties`](/api/update-subscription-settings): +* [`POST /users/me/subscriptions/properties`](/api/update-subscription-settings): Removed `subscription_data` from response object, replacing it with `ignored_parameters_unsupported`. @@ -1137,7 +1146,8 @@ No changes; feature level used for Zulip 3.0 release. `demote_inactive_streams` display settings. * `enable_stream_sounds` was renamed to `enable_stream_audible_notifications`. -* Deprecated `in_home_view`, replacing it with the more readable +* [`POST /users/me/subscriptions/properties`](/api/update-subscription-settings): + Deprecated `in_home_view`, replacing it with the more readable `is_muted` (with the opposite meaning). * Custom profile fields: Added `EXTERNAL_ACCOUNT` field type. diff --git a/version.py b/version.py index bef5f2408d..96741e49bc 100644 --- a/version.py +++ b/version.py @@ -33,7 +33,7 @@ DESKTOP_WARNING_VERSION = "5.4.3" # Changes should be accompanied by documentation explaining what the # new level means in templates/zerver/api/changelog.md, as well as # "**Changes**" entries in the endpoint's documentation in `zulip.yaml`. -API_FEATURE_LEVEL = 138 +API_FEATURE_LEVEL = 139 # Bump the minor PROVISION_VERSION to indicate that folks should provision # only when going from an old version of the code to a newer version. Bump diff --git a/zerver/actions/streams.py b/zerver/actions/streams.py index ac5b88c82b..5879d50846 100644 --- a/zerver/actions/streams.py +++ b/zerver/actions/streams.py @@ -769,19 +769,14 @@ def do_change_subscription_property( acting_user: Optional[UserProfile], ) -> None: database_property_name = property_name - event_property_name = property_name database_value = value - event_value = value # For this property, is_muted is used in the database, but - # in_home_view in the API, since we haven't migrated the events - # API to the new name yet. + # in_home_view is still in the API, since we haven't fully + # migrated to the new name yet. if property_name == "in_home_view": database_property_name = "is_muted" database_value = not value - if property_name == "is_muted": - event_property_name = "in_home_view" - event_value = not value old_value = getattr(sub, database_property_name) setattr(sub, database_property_name, database_value) @@ -803,11 +798,26 @@ def do_change_subscription_property( ).decode(), ) + # This first in_home_view event is deprecated and will be removed + # once clients are migrated to handle the subscription update event + # with is_muted as the property name. + if database_property_name == "is_muted": + event_value = not database_value + in_home_view_event = dict( + type="subscription", + op="update", + property="in_home_view", + value=event_value, + stream_id=stream.id, + ) + + send_event(user_profile.realm, in_home_view_event, [user_profile.id]) + event = dict( type="subscription", op="update", - property=event_property_name, - value=event_value, + property=database_property_name, + value=database_value, stream_id=stream.id, ) send_event(user_profile.realm, event, [user_profile.id]) diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 7bef1389e8..ef9115db65 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -697,8 +697,9 @@ paths: description: | Event sent to a user's clients when a property of the user's subscription to a stream has been updated. This event is used - only for personal properties like `is_muted`; see the `stream` event - for global properties of a stream. + only for personal properties like `is_muted` or `pin_to_top`. + See the [stream update event](/api/get-events#stream-update) + for updates to global properties of a stream. properties: id: $ref: "#/components/schemas/EventIdSchema" @@ -718,13 +719,19 @@ paths: property: type: string description: | - The property of the subscription which has changed. See - [/users/me/subscriptions/properties GET](/api/update-subscription-settings) - for details on the various properties of a stream. + The property of the subscription which has changed. For details on the + various subscription properties that a user can change, see + [POST /users/me/subscriptions/properties](/api/update-subscription-settings). Clients should generally handle an unknown property received here without crashing, since that will naturally happen when connecting to a Zulip server running a new version that adds a new subscription property. + + **Changes**: As of Zulip 6.0 (feature level 139), updates to the `is_muted` + property or the deprecated `in_home_view` property will send two `subscription` + update events, one for each property, to support clients fully migrating to + use the `is_muted` property. Prior to this feature level, updates to either + property only sent one event with the deprecated `in_home_view` property. value: description: | The new value of the changed property. @@ -8773,8 +8780,14 @@ paths: - `"color"`: The hex value of the user's display color for the stream. - `"is_muted"`: Whether the stream is [muted](/help/mute-a-stream).
+ **Changes**: As of Zulip 6.0 (feature level 139), updating either + `"is_muted"` or `"in_home_view"` generates two [subscription update + events](/api/get-events#subscription-update), one for each property, + that are sent to clients. Prior to this feature level, updating either + property only generated a subscription update event for + `"in_home_view"`.
**Changes**: Prior to Zulip 2.1.0, this feature was represented - by the more confusingly named `in_home_view` (with the + by the more confusingly named `"in_home_view"` (with the opposite value: `in_home_view=!is_muted`); for backwards-compatibility, modern Zulip still accepts that property. diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index 49b4f69448..7ea1821d84 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -1481,6 +1481,55 @@ class NormalActionsTest(BaseAction): value=pinned, ) + def test_mute_and_unmute_stream(self) -> None: + stream = get_stream("Denmark", self.user_profile.realm) + sub = get_subscription(stream.name, self.user_profile) + + # While migrating events API from in_home_view to is_muted: + # First, test in_home_view sends 2 events: in_home_view and is_muted. + do_change_subscription_property( + self.user_profile, sub, stream, "in_home_view", False, acting_user=None + ) + + events = self.verify_action( + lambda: do_change_subscription_property( + self.user_profile, sub, stream, "in_home_view", True, acting_user=None + ), + num_events=2, + ) + check_subscription_update( + "events[0]", + events[0], + property="in_home_view", + value=True, + ) + check_subscription_update( + "events[1]", + events[1], + property="is_muted", + value=False, + ) + + # Then, test is_muted also sends both events, in the same order. + events = self.verify_action( + lambda: do_change_subscription_property( + self.user_profile, sub, stream, "is_muted", True, acting_user=None + ), + num_events=2, + ) + check_subscription_update( + "events[0]", + events[0], + property="in_home_view", + value=False, + ) + check_subscription_update( + "events[1]", + events[1], + property="is_muted", + value=True, + ) + def test_change_stream_notification_settings(self) -> None: for setting_name in ["email_notifications"]: stream = get_stream("Denmark", self.user_profile.realm) diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py index 9640e54607..374bd95b58 100644 --- a/zerver/tests/test_subs.py +++ b/zerver/tests/test_subs.py @@ -3016,7 +3016,7 @@ class SubscriptionPropertiesTest(ZulipTestCase): events: List[Mapping[str, Any]] = [] property_name = "is_muted" - with self.tornado_redirected_to_list(events, expected_num_events=1): + with self.tornado_redirected_to_list(events, expected_num_events=2): result = self.api_post( test_user, "/api/v1/users/me/subscriptions/properties", @@ -3035,6 +3035,8 @@ class SubscriptionPropertiesTest(ZulipTestCase): self.assert_json_success(result) self.assertEqual(events[0]["event"]["property"], "in_home_view") self.assertEqual(events[0]["event"]["value"], False) + self.assertEqual(events[1]["event"]["property"], "is_muted") + self.assertEqual(events[1]["event"]["value"], True) sub = Subscription.objects.get( recipient__type=Recipient.STREAM, recipient__type_id=subs[0]["stream_id"], @@ -3043,7 +3045,7 @@ class SubscriptionPropertiesTest(ZulipTestCase): self.assertEqual(sub.is_muted, True) legacy_property_name = "in_home_view" - with self.tornado_redirected_to_list(events, expected_num_events=1): + with self.tornado_redirected_to_list(events, expected_num_events=2): result = self.api_post( test_user, "/api/v1/users/me/subscriptions/properties", @@ -3062,6 +3064,8 @@ class SubscriptionPropertiesTest(ZulipTestCase): self.assert_json_success(result) self.assertEqual(events[0]["event"]["property"], "in_home_view") self.assertEqual(events[0]["event"]["value"], True) + self.assertEqual(events[1]["event"]["property"], "is_muted") + self.assertEqual(events[1]["event"]["value"], False) self.assert_json_success(result) sub = Subscription.objects.get( recipient__type=Recipient.STREAM, @@ -3070,7 +3074,7 @@ class SubscriptionPropertiesTest(ZulipTestCase): ) self.assertEqual(sub.is_muted, False) - with self.tornado_redirected_to_list(events, expected_num_events=1): + with self.tornado_redirected_to_list(events, expected_num_events=2): result = self.api_post( test_user, "/api/v1/users/me/subscriptions/properties", @@ -3089,6 +3093,8 @@ class SubscriptionPropertiesTest(ZulipTestCase): self.assert_json_success(result) self.assertEqual(events[0]["event"]["property"], "in_home_view") self.assertEqual(events[0]["event"]["value"], False) + self.assertEqual(events[1]["event"]["property"], "is_muted") + self.assertEqual(events[1]["event"]["value"], True) sub = Subscription.objects.get( recipient__type=Recipient.STREAM,