From 1291e7000b86f167d8f29d1cbef047a4704cc53f Mon Sep 17 00:00:00 2001 From: Kartik Srivastava Date: Sat, 26 Feb 2022 02:18:56 +0530 Subject: [PATCH] user_topic: Add user_topic event. We now send a new user_topic event while muting and unmuting topics. fetch_initial_state_data now returns an additional user_topics array to the client that will maintain the user-topic relationship data. This will support any future addition of new features to modify the relationship between a user-topic pair. This commit adds the relevent backend code and schema for the new event. --- templates/zerver/api/changelog.md | 12 +++ version.py | 2 +- zerver/actions/message_edit.py | 12 +++ zerver/actions/user_topics.py | 41 +++++++- zerver/lib/event_schema.py | 13 +++ zerver/lib/events.py | 27 +++++- zerver/lib/user_topics.py | 7 ++ .../0402_alter_usertopic_visibility_policy.py | 26 +++++ zerver/models.py | 4 + zerver/openapi/zulip.yaml | 97 +++++++++++++++++++ zerver/tests/test_event_system.py | 3 +- zerver/tests/test_events.py | 17 +++- zerver/tests/test_home.py | 7 +- zerver/tornado/event_queue.py | 12 ++- 14 files changed, 264 insertions(+), 16 deletions(-) create mode 100644 zerver/migrations/0402_alter_usertopic_visibility_policy.py diff --git a/templates/zerver/api/changelog.md b/templates/zerver/api/changelog.md index af0db64a42..5e901ed895 100644 --- a/templates/zerver/api/changelog.md +++ b/templates/zerver/api/changelog.md @@ -20,6 +20,18 @@ format used by the Zulip server that they are interacting with. ## Changes in Zulip 6.0 +**Feature level 134** + +* [`GET /events`](/api/get-events): Added `user_topic` event type + which is sent when a topic is muted or unmuted. This generalizes and + replaces the previous `muted_topics` array, which will no longer be + sent if `user_topic` was included in `event_types` when registering + the queue. +* [`POST /register`](/api/register-queue): Added `user_topics` array + to the response. This generalizes and replaces the previous + `muted_topics` array, which will no longer be sent if `user_topic` + is included in `fetch_event_types`. + **Feature level 133** * [`POST /register`](/api/register-queue), `PATCH /realm`: Removed diff --git a/version.py b/version.py index f50d19d56d..e9e42ba49c 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 = 133 +API_FEATURE_LEVEL = 134 # 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/message_edit.py b/zerver/actions/message_edit.py index 69c51582ea..26d611ce4a 100644 --- a/zerver/actions/message_edit.py +++ b/zerver/actions/message_edit.py @@ -61,6 +61,7 @@ from zerver.models import ( Stream, UserMessage, UserProfile, + UserTopic, get_stream_by_id_in_realm, get_system_bot, ) @@ -804,6 +805,17 @@ def do_update_message( # immediate succession; this is correct only because # muted_topics events always send the full set of topics. remove_topic_mute(muting_user, stream_being_edited.id, orig_topic_name) + + date_unmuted = timezone_now() + user_topic_event: Dict[str, Any] = { + "type": "user_topic", + "stream_id": stream_being_edited.id, + "topic_name": orig_topic_name, + "last_updated": datetime_to_timestamp(date_unmuted), + "visibility_policy": UserTopic.VISIBILITY_POLICY_INHERIT, + } + send_event(user_profile.realm, user_topic_event, [user_profile.id]) + do_mute_topic( muting_user, new_stream if new_stream is not None else stream_being_edited, diff --git a/zerver/actions/user_topics.py b/zerver/actions/user_topics.py index ec00e5db7a..f1fda71907 100644 --- a/zerver/actions/user_topics.py +++ b/zerver/actions/user_topics.py @@ -1,10 +1,11 @@ import datetime -from typing import Optional +from typing import Any, Dict, Optional from django.utils.timezone import now as timezone_now from django.utils.translation import gettext as _ from zerver.lib.exceptions import JsonableError +from zerver.lib.timestamp import datetime_to_timestamp from zerver.lib.user_topics import add_topic_mute, get_topic_mutes, remove_topic_mute from zerver.models import Stream, UserProfile, UserTopic from zerver.tornado.django_api import send_event @@ -28,8 +29,22 @@ def do_mute_topic( date_muted, ignore_duplicate=ignore_duplicate, ) - event = dict(type="muted_topics", muted_topics=get_topic_mutes(user_profile)) - send_event(user_profile.realm, event, [user_profile.id]) + + # This first muted_topics event is deprecated and will be removed + # once clients are migrated to handle the user_topic event type + # instead. + muted_topics_event = dict(type="muted_topics", muted_topics=get_topic_mutes(user_profile)) + send_event(user_profile.realm, muted_topics_event, [user_profile.id]) + + user_topic_event: Dict[str, Any] = { + "type": "user_topic", + "stream_id": stream.id, + "topic_name": topic, + "last_updated": datetime_to_timestamp(date_muted), + "visibility_policy": UserTopic.MUTED, + } + + send_event(user_profile.realm, user_topic_event, [user_profile.id]) def do_unmute_topic(user_profile: UserProfile, stream: Stream, topic: str) -> None: @@ -40,5 +55,21 @@ def do_unmute_topic(user_profile: UserProfile, stream: Stream, topic: str) -> No remove_topic_mute(user_profile, stream.id, topic) except UserTopic.DoesNotExist: raise JsonableError(_("Topic is not muted")) - event = dict(type="muted_topics", muted_topics=get_topic_mutes(user_profile)) - send_event(user_profile.realm, event, [user_profile.id]) + + # This first muted_topics event is deprecated and will be removed + # once clients are migrated to handle the user_topic event type + # instead. + muted_topics_event = dict(type="muted_topics", muted_topics=get_topic_mutes(user_profile)) + send_event(user_profile.realm, muted_topics_event, [user_profile.id]) + + date_unmuted = timezone_now() + + user_topic_event: Dict[str, Any] = { + "type": "user_topic", + "stream_id": stream.id, + "topic_name": topic, + "last_updated": datetime_to_timestamp(date_unmuted), + "visibility_policy": UserTopic.VISIBILITY_POLICY_INHERIT, + } + + send_event(user_profile.realm, user_topic_event, [user_profile.id]) diff --git a/zerver/lib/event_schema.py b/zerver/lib/event_schema.py index 65002ab157..c81328937e 100644 --- a/zerver/lib/event_schema.py +++ b/zerver/lib/event_schema.py @@ -343,6 +343,19 @@ muted_topics_event = event_dict_type( ) check_muted_topics = make_checker(muted_topics_event) +user_topic_event = DictType( + required_keys=[ + ("id", int), + ("type", Equals("user_topic")), + ("stream_id", int), + ("topic_name", str), + ("last_updated", int), + ("visibility_policy", int), + ] +) + +check_user_topic = make_checker(user_topic_event) + muted_user_type = DictType( required_keys=[ ("id", int), diff --git a/zerver/lib/events.py b/zerver/lib/events.py index aa70e4dec9..c0d78f4b64 100644 --- a/zerver/lib/events.py +++ b/zerver/lib/events.py @@ -49,7 +49,7 @@ from zerver.lib.topic import TOPIC_NAME from zerver.lib.user_groups import user_groups_in_realm_serialized from zerver.lib.user_mutes import get_user_mutes from zerver.lib.user_status import get_user_info_dict -from zerver.lib.user_topics import get_topic_mutes +from zerver.lib.user_topics import get_topic_mutes, get_user_topics from zerver.lib.users import get_cross_realm_dicts, get_raw_user_data, is_administrator_role from zerver.models import ( MAX_TOPIC_NAME_LENGTH, @@ -63,6 +63,7 @@ from zerver.models import ( UserMessage, UserProfile, UserStatus, + UserTopic, custom_profile_fields_for_realm, get_default_stream_groups, get_realm_domains, @@ -192,7 +193,13 @@ def fetch_initial_state_data( state["drafts"] = user_draft_dicts if want("muted_topics"): - state["muted_topics"] = [] if user_profile is None else get_topic_mutes(user_profile) + # Suppress muted_topics data for clients that explicitly + # support user_topic. This allows clients to request both the + # user_topic and muted_topics, and receive the duplicate + # muted_topics data only from older servers that don't yet + # support user_topic. + if event_types is None or not want("user_topic"): + state["muted_topics"] = [] if user_profile is None else get_topic_mutes(user_profile) if want("muted_users"): state["muted_users"] = [] if user_profile is None else get_user_mutes(user_profile) @@ -584,6 +591,9 @@ def fetch_initial_state_data( # We require creating an account to access statuses. state["user_status"] = {} if user_profile is None else get_user_info_dict(realm_id=realm.id) + if want("user_topic"): + state["user_topics"] = [] if user_profile is None else get_user_topics(user_profile) + if want("video_calls"): state["has_zoom_token"] = settings_user.zoom_token is not None @@ -1319,6 +1329,19 @@ def apply_event( user_status.pop(user_id_str, None) state["user_status"] = user_status + elif event["type"] == "user_topic": + if event["visibility_policy"] == UserTopic.VISIBILITY_POLICY_INHERIT: + user_topics_state = state["user_topics"] + for i in range(len(user_topics_state)): + if ( + user_topics_state[i]["stream_id"] == event["stream_id"] + and user_topics_state[i]["topic_name"] == event["topic_name"] + ): + del user_topics_state[i] + break + else: + fields = ["stream_id", "topic_name", "visibility_policy", "last_updated"] + state["user_topics"].append({x: event[x] for x in fields}) elif event["type"] == "has_zoom_token": state["has_zoom_token"] = event["value"] else: diff --git a/zerver/lib/user_topics.py b/zerver/lib/user_topics.py index a38a77793d..11723155f6 100644 --- a/zerver/lib/user_topics.py +++ b/zerver/lib/user_topics.py @@ -15,12 +15,15 @@ from zerver.models import UserProfile, UserTopic, get_stream def get_user_topics( user_profile: UserProfile, include_deactivated: bool = False, + include_stream_name: bool = False, visibility_policy: Optional[int] = None, ) -> List[UserTopicDict]: """ Fetches UserTopic objects associated with the target user. * include_deactivated: Whether to include those associated with deactivated streams. + * include_stream_name: Whether to include stream names in the + returned dictionaries. * visibility_policy: If specified, returns only UserTopic objects with the specified visibility_policy value. """ @@ -40,6 +43,9 @@ def get_user_topics( result = [] for row in rows: + if not include_stream_name: + del row["stream__name"] + row["last_updated"] = datetime_to_timestamp(row["last_updated"]) result.append(row) @@ -53,6 +59,7 @@ def get_topic_mutes( user_topics = get_user_topics( user_profile=user_profile, include_deactivated=include_deactivated, + include_stream_name=True, visibility_policy=UserTopic.MUTED, ) diff --git a/zerver/migrations/0402_alter_usertopic_visibility_policy.py b/zerver/migrations/0402_alter_usertopic_visibility_policy.py new file mode 100644 index 0000000000..4279326488 --- /dev/null +++ b/zerver/migrations/0402_alter_usertopic_visibility_policy.py @@ -0,0 +1,26 @@ +# Generated by Django 4.0.6 on 2022-08-01 20:40 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("zerver", "0401_migrate_old_realm_reactivation_links"), + ] + + operations = [ + migrations.AlterField( + model_name="usertopic", + name="visibility_policy", + field=models.SmallIntegerField( + choices=[ + (1, "Muted topic"), + (2, "Unmuted topic in muted stream"), + (3, "Followed topic"), + (0, "User's default policy for the stream."), + ], + default=1, + ), + ), + ] diff --git a/zerver/models.py b/zerver/models.py index 85cca21d52..9a2626cbe4 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -2545,6 +2545,9 @@ class UserTopic(models.Model): # Implicitly, if a UserTopic does not exist, the (user, topic) # pair should have normal behavior for that (user, stream) pair. + # We use this in our code to represent the condition in the comment above. + VISIBILITY_POLICY_INHERIT = 0 + # A normal muted topic. No notifications and unreads hidden. MUTED = 1 @@ -2559,6 +2562,7 @@ class UserTopic(models.Model): (MUTED, "Muted topic"), (UNMUTED, "Unmuted topic in muted stream"), (FOLLOWED, "Followed topic"), + (VISIBILITY_POLICY_INHERIT, "User's default policy for the stream."), ) visibility_policy: int = models.SmallIntegerField( diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 032450ef72..1cf2651fea 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -1886,6 +1886,7 @@ paths: - muted_topics muted_topics: type: array + deprecated: true description: | Array of tuples, where each tuple describes a muted topic. The first element of the tuple is the stream name in which the topic @@ -1893,6 +1894,12 @@ paths: and the third element is an integer UNIX timestamp representing when the topic was muted. + **Changes**: Deprecated in Zulip 6.0 (feature level + 134). Starting with this version, clients that explicitly + requested the replacement `user_topic` event type when + registering their event queue will not receive this legacy + event type. + **Changes**: Before Zulip 3.0 (feature level 1), the `muted_topics` array objects were 2-item tuples and did not include the timestamp information for when the topic was muted. @@ -1910,6 +1917,57 @@ paths: [["Denmark", "topic", 1594825442]], "id": 0, } + - type: object + description: | + Event sent to a user's clients when the user mutes/unmutes + a topic, or otherwise modifies their personal per-topic + configuration. + + **Changes**: New in Zulip 6.0 (feature level 134). Previously, + clients were notified about changes in muted topic + configuration via the `muted_topics` event type. + properties: + id: + $ref: "#/components/schemas/EventIdSchema" + type: + allOf: + - $ref: "#/components/schemas/EventTypeSchema" + - enum: + - user_topic + stream_id: + type: integer + description: | + The ID of the stream to which the topic belongs. + topic_name: + type: string + description: | + The name of the topic. + last_updated: + type: integer + description: | + An integer UNIX timestamp representing when the user-topic + relationship was last changed. + visibility_policy: + type: integer + description: | + An integer indicating the user's visibility + preferences for the topic, such as whether the topic + is muted. + + - 0 = None. Used in events to indicate that the user no + longer has a special visibility policy for this + topic (for example, the user just unmuted it). + - 1 = Muted. Used to record muted topics. + additionalProperties: false + example: + { + "id": 1, + "type": "user_topic", + "stream_id": 1, + "topic_name": "topic", + "last_updated": 1594825442, + "visibility_policy": 1, + } - type: object description: | Event sent to a user's clients when that user's set of @@ -9332,6 +9390,7 @@ paths: this always had value 10000. muted_topics: type: array + deprecated: true description: | Present if `muted_topics` is present in `fetch_event_types`. @@ -9341,6 +9400,11 @@ paths: and the third element is an integer UNIX timestamp representing when the topic was muted. + **Changes**: Deprecated in Zulip 6.0 (feature level 134). Starting + with this version, `muted_topics` will only be present in the + response if the `user_topic` object, which generalizes and replaces + this field, is not explicitly requested via `fetch_event_types`. + **Changes**: Before Zulip 3.0 (feature level 1), the `muted_topics` array objects were 2-item tuples and did not include the timestamp information for when the topic was muted. @@ -10192,6 +10256,39 @@ paths: read messages. **Changes**: New in Zulip 5.0 (feature level 105). + user_topics: + type: array + description: | + Present if `user_topic` is present in `fetch_event_types`. + + **Changes**: New in Zulip 6.0 (feature level 134), deprecating and + replacing the previous `muted_topics` structure. + items: + type: object + description: | + Object describing the user's configuration for a given topic. + additionalProperties: false + properties: + stream_id: + type: integer + description: | + The ID of the stream to which the topic belongs. + topic_name: + type: string + description: | + The name of the topic. + last_updated: + type: integer + description: | + An integer UNIX timestamp representing when the user-topic + relationship was changed. + visibility_policy: + type: integer + description: | + An integer indicating the user's visibility configuration for + the topic. + + - 1 = Muted. Used to record [muted topics](/help/mute-a-topic). has_zoom_token: type: boolean description: | diff --git a/zerver/tests/test_event_system.py b/zerver/tests/test_event_system.py index 4de2c57eb3..982314056a 100644 --- a/zerver/tests/test_event_system.py +++ b/zerver/tests/test_event_system.py @@ -1051,7 +1051,7 @@ class FetchQueriesTest(ZulipTestCase): with mock.patch("zerver.lib.events.always_want") as want_mock: fetch_initial_state_data(user) - self.assert_length(queries, 36) + self.assert_length(queries, 37) expected_counts = dict( alert_words=1, @@ -1086,6 +1086,7 @@ class FetchQueriesTest(ZulipTestCase): update_message_flags=5, user_settings=0, user_status=1, + user_topic=1, video_calls=0, giphy=0, ) diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index 8f9a0de9b9..49b4f69448 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -181,6 +181,7 @@ from zerver.lib.event_schema import ( check_user_group_update, check_user_settings_update, check_user_status, + check_user_topic, ) from zerver.lib.events import ( RestartEventException, @@ -1332,11 +1333,23 @@ class NormalActionsTest(BaseAction): def test_muted_topics_events(self) -> None: stream = get_stream("Denmark", self.user_profile.realm) - events = self.verify_action(lambda: do_mute_topic(self.user_profile, stream, "topic")) + events = self.verify_action( + lambda: do_mute_topic(self.user_profile, stream, "topic"), num_events=2 + ) check_muted_topics("events[0]", events[0]) + check_user_topic("events[1]", events[1]) - events = self.verify_action(lambda: do_unmute_topic(self.user_profile, stream, "topic")) + events = self.verify_action( + lambda: do_unmute_topic(self.user_profile, stream, "topic"), num_events=2 + ) check_muted_topics("events[0]", events[0]) + check_user_topic("events[1]", events[1]) + + events = self.verify_action( + lambda: do_mute_topic(self.user_profile, stream, "topic"), + event_types=["muted_topics", "user_topic"], + ) + check_user_topic("events[0]", events[0]) def test_muted_users_events(self) -> None: muted_user = self.example_user("othello") diff --git a/zerver/tests/test_home.py b/zerver/tests/test_home.py index d2500bfcee..b1e376c022 100644 --- a/zerver/tests/test_home.py +++ b/zerver/tests/test_home.py @@ -213,6 +213,7 @@ class HomeTest(ZulipTestCase): "user_id", "user_settings", "user_status", + "user_topics", "warn_no_email", "webpack_public_path", "zulip_feature_level", @@ -250,7 +251,7 @@ class HomeTest(ZulipTestCase): set(result["Cache-Control"].split(", ")), {"must-revalidate", "no-store", "no-cache"} ) - self.assert_length(queries, 46) + self.assert_length(queries, 47) self.assert_length(cache_mock.call_args_list, 5) html = result.content.decode() @@ -394,7 +395,7 @@ class HomeTest(ZulipTestCase): result = self._get_home_page() self.check_rendered_logged_in_app(result) self.assert_length(cache_mock.call_args_list, 6) - self.assert_length(queries, 43) + self.assert_length(queries, 44) def test_num_queries_with_streams(self) -> None: main_user = self.example_user("hamlet") @@ -425,7 +426,7 @@ class HomeTest(ZulipTestCase): with queries_captured() as queries2: result = self._get_home_page() - self.assert_length(queries2, 41) + self.assert_length(queries2, 42) # Do a sanity check that our new streams were in the payload. html = result.content.decode() diff --git a/zerver/tornado/event_queue.py b/zerver/tornado/event_queue.py index 138ee6240b..54cfa0addb 100644 --- a/zerver/tornado/event_queue.py +++ b/zerver/tornado/event_queue.py @@ -206,8 +206,16 @@ class ClientDescriptor: return False def accepts_event(self, event: Mapping[str, Any]) -> bool: - if self.event_types is not None and event["type"] not in self.event_types: - return False + if self.event_types is not None: + if event["type"] not in self.event_types: + return False + if event["type"] == "muted_topics" and "user_topic" in self.event_types: + # Suppress muted_topics events for clients that + # support user_topic. This allows clients to request + # both the user_topic and muted_topics event types, + # and receive the duplicate muted_topics data only on + # older servers that don't support user_topic. + return False if event["type"] == "message": return self.narrow_filter(event) if event["type"] == "typing" and "stream_id" in event: