diff --git a/api_docs/changelog.md b/api_docs/changelog.md index bcbc0bceb9..32142d60d4 100644 --- a/api_docs/changelog.md +++ b/api_docs/changelog.md @@ -20,6 +20,11 @@ format used by the Zulip server that they are interacting with. ## Changes in Zulip 7.0 +**Feature level 170** + +* [`POST /user_topics`](/api/update-user-topic): + Added a new endpoint to update the personal preferences for a topic. + **Feature level 169** * [`PATCH /users/me/subscriptions/muted_topics`](/api/mute-topic): diff --git a/api_docs/include/rest-endpoints.md b/api_docs/include/rest-endpoints.md index e70655489b..5b9534f353 100644 --- a/api_docs/include/rest-endpoints.md +++ b/api_docs/include/rest-endpoints.md @@ -40,6 +40,7 @@ * [Archive a stream](/api/archive-stream) * [Get topics in a stream](/api/get-stream-topics) * [Topic muting](/api/mute-topic) +* [Update personal preferences for a topic](/api/update-user-topic) * [Delete a topic](/api/delete-topic) * [Add a default stream](/api/add-default-stream) * [Remove a default stream](/api/remove-default-stream) diff --git a/version.py b/version.py index 33cf268e6b..985b1ac0ef 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 api_docs/changelog.md, as well as "**Changes**" # entries in the endpoint's documentation in `zulip.yaml`. -API_FEATURE_LEVEL = 169 +API_FEATURE_LEVEL = 170 # 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/openapi/python_examples.py b/zerver/openapi/python_examples.py index 4ad9eadd1d..90c62388fc 100644 --- a/zerver/openapi/python_examples.py +++ b/zerver/openapi/python_examples.py @@ -728,6 +728,44 @@ def toggle_mute_topic(client: Client) -> None: validate_against_openapi_schema(result, "/users/me/subscriptions/muted_topics", "patch", "200") +@openapi_test_function("/user_topics:post") +def update_user_topic(client: Client) -> None: + stream_id = client.get_stream_id("Denmark")["stream_id"] + + # {code_example|start} + # Mute the topic "dinner" in the stream having id 'stream_id'. + request = { + "stream_id": stream_id, + "topic": "dinner", + "visibility_policy": 1, + } + result = client.call_endpoint( + url="user_topics", + method="POST", + request=request, + ) + # {code_example|end} + + validate_against_openapi_schema(result, "/user_topics", "post", "200") + + # {code_example|start} + # Remove mute from the topic "dinner" in the stream having id 'stream_id'. + request = { + "stream_id": stream_id, + "topic": "dinner", + "visibility_policy": 0, + } + + result = client.call_endpoint( + url="user_topics", + method="POST", + request=request, + ) + # {code_example|end} + + validate_against_openapi_schema(result, "/user_topics", "post", "200") + + @openapi_test_function("/users/me/muted_users/{muted_user_id}:post") def add_user_mute(client: Client) -> None: ensure_users([10], ["hamlet"]) @@ -1537,6 +1575,7 @@ def test_streams(client: Client, nonadmin_client: Client) -> None: get_subscribers(client) remove_subscriptions(client) toggle_mute_topic(client) + update_user_topic(client) update_subscription_settings(client) get_stream_topics(client, 1) delete_topic(client, 1, "test") diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 17d4957363..3a922265d3 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -8203,7 +8203,12 @@ paths: user is subscribed to. Muted topics are displayed faded in the Zulip UI, and are not included in the user's unread count totals. - **Changes**: Before Zulip 7.0 (feature level 169), this endpoint + **Changes**: Deprecated in Zulip 7.0 (feature level 170). Clients connecting + to the newer server should use the `/user_topics` endpoint instead. + It will be removed once clients have migrated to use the + `/user_topics` endpoint. + + Before Zulip 7.0 (feature level 169), this endpoint returned an error if asked to mute a topic that was already muted or asked to unmute a topic that had not previously been muted. x-curl-examples-parameters: @@ -8261,6 +8266,73 @@ paths: responses: "200": $ref: "#/components/responses/SimpleSuccess" + /user_topics: + post: + operationId: update-user-topic + summary: Update personal preferences for a topic + tags: ["streams"] + description: | + This endpoint is used to update the personal preferences for a topic, + such as the topic's visibility policy, which is used to implement + [mute a topic](/help/mute-a-topic) and related features. + + This endpoint can be used to update the visibility policy for the single + stream and topic pair indicated by the parameters for a user. + + **Changes**: New in Zulip 7.0 (feature level 170). + Previously, toggling the muting state for a topic was managed by the + `/users/me/subscriptions/muted_topics` endpoint, which this endpoint + is intended to replace. + parameters: + - name: stream_id + in: query + description: | + The ID of the stream to access. + schema: + type: integer + example: 1 + required: true + - name: topic + in: query + description: | + The topic for which the personal preferences needs to be updated. + Note that the request will succeed regardless of whether + any messages have been sent to the specified topic. + schema: + type: string + example: dinner + required: true + - name: visibility_policy + in: query + description: | + Controls which visibility policy to set. + + - 0 - INHERIT + - 1 - MUTED + - 2 - UNMUTED + + The visibility policy, when set to MUTED, mutes a topic; + when set to UNMUTED, it unmutes a topic in a muted stream; + and INHERIT is used to remove the visibility policy already set. + + MUTED topics are displayed faded in the Zulip UI, are not included + in the user's unread count totals, and the user doesn't receive any + notifications. + + An UNMUTED topic will remain visible even if the stream is muted. + In a stream that is not muted, a policy of UNMUTED has the same effect + as INHERIT. + schema: + type: integer + enum: + - 0 + - 1 + - 2 + example: 1 + required: true + responses: + "200": + $ref: "#/components/responses/SimpleSuccess" /users/me/muted_users/{muted_user_id}: post: operationId: mute-user diff --git a/zerver/tests/test_user_topics.py b/zerver/tests/test_user_topics.py index 4d7b3407dc..2de7132fb4 100644 --- a/zerver/tests/test_user_topics.py +++ b/zerver/tests/test_user_topics.py @@ -1,5 +1,5 @@ from datetime import datetime, timezone -from typing import Any, Dict, List +from typing import Any, Dict, List, Mapping import time_machine from django.utils.timezone import now as timezone_now @@ -14,7 +14,10 @@ from zerver.lib.user_topics import ( from zerver.models import UserProfile, UserTopic, get_stream -class MutedTopicsTests(ZulipTestCase): +class MutedTopicsTestsDeprecated(ZulipTestCase): + # Tests the deprecated URL: "/api/v1/users/me/subscriptions/muted_topics". + # It exists for backward compatibility and should be removed once + # we remove the deprecated URL. def test_get_deactivated_muted_topic(self) -> None: user = self.example_user("hamlet") self.login_user(user) @@ -229,6 +232,225 @@ class MutedTopicsTests(ZulipTestCase): self.assert_json_error(result, "Please choose one: 'stream' or 'stream_id'.") +class MutedTopicsTests(ZulipTestCase): + def test_get_deactivated_muted_topic(self) -> None: + user = self.example_user("hamlet") + self.login_user(user) + + stream = get_stream("Verona", user.realm) + + url = "/api/v1/user_topics" + data = { + "stream_id": stream.id, + "topic": "Verona3", + "visibility_policy": UserTopic.VisibilityPolicy.MUTED, + } + + mock_date_muted = datetime(2020, 1, 1, tzinfo=timezone.utc).timestamp() + with time_machine.travel(datetime(2020, 1, 1, tzinfo=timezone.utc), tick=False): + result = self.api_post(user, url, data) + self.assert_json_success(result) + + stream.deactivated = True + stream.save() + + self.assertNotIn((stream.name, "Verona3", mock_date_muted), get_topic_mutes(user)) + self.assertIn((stream.name, "Verona3", mock_date_muted), get_topic_mutes(user, True)) + + def test_user_ids_muting_topic(self) -> None: + hamlet = self.example_user("hamlet") + cordelia = self.example_user("cordelia") + realm = hamlet.realm + stream = get_stream("Verona", realm) + topic_name = "teST topic" + date_muted = datetime(2020, 1, 1, tzinfo=timezone.utc) + + stream_topic_target = StreamTopicTarget( + stream_id=stream.id, + topic_name=topic_name, + ) + + user_ids = stream_topic_target.user_ids_with_visibility_policy( + UserTopic.VisibilityPolicy.MUTED + ) + self.assertEqual(user_ids, set()) + + url = "/api/v1/user_topics" + + def set_topic_visibility_for_user(user: UserProfile, visibility_policy: int) -> None: + data = { + "stream_id": stream.id, + "topic": "test TOPIC", + "visibility_policy": visibility_policy, + } + with time_machine.travel(date_muted, tick=False): + result = self.api_post(user, url, data) + self.assert_json_success(result) + + set_topic_visibility_for_user(hamlet, UserTopic.VisibilityPolicy.MUTED) + set_topic_visibility_for_user(cordelia, UserTopic.VisibilityPolicy.UNMUTED) + user_ids = stream_topic_target.user_ids_with_visibility_policy( + UserTopic.VisibilityPolicy.MUTED + ) + self.assertEqual(user_ids, {hamlet.id}) + hamlet_date_muted = UserTopic.objects.filter( + user_profile=hamlet, visibility_policy=UserTopic.VisibilityPolicy.MUTED + )[0].last_updated + self.assertEqual(hamlet_date_muted, date_muted) + + set_topic_visibility_for_user(cordelia, UserTopic.VisibilityPolicy.MUTED) + user_ids = stream_topic_target.user_ids_with_visibility_policy( + UserTopic.VisibilityPolicy.MUTED + ) + self.assertEqual(user_ids, {hamlet.id, cordelia.id}) + cordelia_date_muted = UserTopic.objects.filter( + user_profile=cordelia, visibility_policy=UserTopic.VisibilityPolicy.MUTED + )[0].last_updated + self.assertEqual(cordelia_date_muted, date_muted) + + def test_add_muted_topic(self) -> None: + user = self.example_user("hamlet") + self.login_user(user) + + stream = get_stream("Verona", user.realm) + + url = "/api/v1/user_topics" + data = { + "stream_id": stream.id, + "topic": "Verona3", + "visibility_policy": UserTopic.VisibilityPolicy.MUTED, + } + + mock_date_muted = datetime(2020, 1, 1, tzinfo=timezone.utc).timestamp() + + events: List[Mapping[str, Any]] = [] + with self.tornado_redirected_to_list(events, expected_num_events=2): + with time_machine.travel(datetime(2020, 1, 1, tzinfo=timezone.utc), tick=False): + result = self.api_post(user, url, data) + self.assert_json_success(result) + + self.assertTrue( + topic_has_visibility_policy( + user, stream.id, "verona3", UserTopic.VisibilityPolicy.MUTED + ) + ) + # Verify if events are sent properly + user_topic_event: Dict[str, Any] = { + "type": "user_topic", + "stream_id": stream.id, + "topic_name": "Verona3", + "last_updated": mock_date_muted, + "visibility_policy": UserTopic.VisibilityPolicy.MUTED, + } + muted_topics_event = dict(type="muted_topics", muted_topics=get_topic_mutes(user)) + self.assertEqual(events[0]["event"], muted_topics_event) + self.assertEqual(events[1]["event"], user_topic_event) + + # Now check that no error is raised when attempted to mute + # an already muted topic. This should be case-insensitive. + user_topic_count = UserTopic.objects.count() + data["topic"] = "VERONA3" + with self.assertLogs(level="INFO") as info_logs: + result = self.api_post(user, url, data) + self.assert_json_success(result) + self.assertEqual( + info_logs.output[0], + f"INFO:root:User {user.id} tried to set visibility_policy to its current value of {UserTopic.VisibilityPolicy.MUTED}", + ) + # Verify that we didn't end up with duplicate UserTopic rows + # with the two different cases after the previous API call. + self.assertEqual(UserTopic.objects.count() - user_topic_count, 0) + + def test_remove_muted_topic(self) -> None: + user = self.example_user("hamlet") + realm = user.realm + self.login_user(user) + + stream = get_stream("Verona", realm) + + do_set_user_topic_visibility_policy( + user, + stream, + "Verona3", + visibility_policy=UserTopic.VisibilityPolicy.MUTED, + last_updated=datetime(2020, 1, 1, tzinfo=timezone.utc), + ) + self.assertTrue( + topic_has_visibility_policy( + user, stream.id, "verona3", UserTopic.VisibilityPolicy.MUTED + ) + ) + + url = "/api/v1/user_topics" + data = { + "stream_id": stream.id, + "topic": "Verona3", + "visibility_policy": UserTopic.VisibilityPolicy.INHERIT, + } + + mock_date_mute_removed = datetime(2020, 1, 1, tzinfo=timezone.utc).timestamp() + + events: List[Mapping[str, Any]] = [] + with self.tornado_redirected_to_list(events, expected_num_events=2): + with time_machine.travel(datetime(2020, 1, 1, tzinfo=timezone.utc), tick=False): + result = self.api_post(user, url, data) + self.assert_json_success(result) + + self.assertFalse( + topic_has_visibility_policy( + user, stream.id, "verona3", UserTopic.VisibilityPolicy.MUTED + ) + ) + # Verify if events are sent properly + user_topic_event: Dict[str, Any] = { + "type": "user_topic", + "stream_id": stream.id, + "topic_name": data["topic"], + "last_updated": mock_date_mute_removed, + "visibility_policy": UserTopic.VisibilityPolicy.INHERIT, + } + muted_topics_event = dict(type="muted_topics", muted_topics=get_topic_mutes(user)) + self.assertEqual(events[0]["event"], muted_topics_event) + self.assertEqual(events[1]["event"], user_topic_event) + + # Check that removing mute from a topic for which the user + # doesn't already have a visibility_policy doesn't cause an error. + with self.assertLogs(level="INFO") as info_logs: + result = self.api_post(user, url, data) + self.assert_json_success(result) + self.assertEqual( + info_logs.output[0], + f"INFO:root:User {user.id} tried to remove visibility_policy, which actually doesn't exist", + ) + + def test_muted_topic_add_invalid(self) -> None: + user = self.example_user("hamlet") + self.login_user(user) + + url = "/api/v1/user_topics" + data = { + "stream_id": 999999999, + "topic": "Verona3", + "visibility_policy": UserTopic.VisibilityPolicy.MUTED, + } + result = self.api_post(user, url, data) + self.assert_json_error(result, "Invalid stream ID") + + def test_muted_topic_remove_invalid(self) -> None: + user = self.example_user("hamlet") + self.login_user(user) + + url = "/api/v1/user_topics" + data = { + "stream_id": 999999999, + "topic": "Verona3", + "visibility_policy": UserTopic.VisibilityPolicy.INHERIT, + } + + result = self.api_post(user, url, data) + self.assert_json_error(result, "Invalid stream ID") + + class UnmutedTopicsTests(ZulipTestCase): def test_user_ids_unmuting_topic(self) -> None: hamlet = self.example_user("hamlet") @@ -277,3 +499,146 @@ class UnmutedTopicsTests(ZulipTestCase): user_profile=cordelia, visibility_policy=UserTopic.VisibilityPolicy.UNMUTED )[0].last_updated self.assertEqual(cordelia_date_unmuted, date_unmuted) + + def test_add_unmuted_topic(self) -> None: + user = self.example_user("hamlet") + self.login_user(user) + + stream = get_stream("Verona", user.realm) + + url = "/api/v1/user_topics" + data = { + "stream_id": stream.id, + "topic": "Verona3", + "visibility_policy": UserTopic.VisibilityPolicy.UNMUTED, + } + + mock_date_unmuted = datetime(2020, 1, 1, tzinfo=timezone.utc).timestamp() + + events: List[Mapping[str, Any]] = [] + with self.tornado_redirected_to_list(events, expected_num_events=2): + with time_machine.travel(datetime(2020, 1, 1, tzinfo=timezone.utc), tick=False): + result = self.api_post(user, url, data) + self.assert_json_success(result) + + self.assertTrue( + topic_has_visibility_policy( + user, stream.id, "verona3", UserTopic.VisibilityPolicy.UNMUTED + ) + ) + # Verify if events are sent properly + user_topic_event: Dict[str, Any] = { + "type": "user_topic", + "stream_id": stream.id, + "topic_name": "Verona3", + "last_updated": mock_date_unmuted, + "visibility_policy": UserTopic.VisibilityPolicy.UNMUTED, + } + muted_topics_event = dict(type="muted_topics", muted_topics=get_topic_mutes(user)) + self.assertEqual(events[0]["event"], muted_topics_event) + self.assertEqual(events[1]["event"], user_topic_event) + + # Now check that no error is raised when attempted to UNMUTE + # an already UNMUTED topic. This should be case-insensitive. + user_topic_count = UserTopic.objects.count() + data["topic"] = "VERONA3" + with self.assertLogs(level="INFO") as info_logs: + result = self.api_post(user, url, data) + self.assert_json_success(result) + self.assertEqual( + info_logs.output[0], + f"INFO:root:User {user.id} tried to set visibility_policy to its current value of {UserTopic.VisibilityPolicy.UNMUTED}", + ) + # Verify that we didn't end up with duplicate UserTopic rows + # with the two different cases after the previous API call. + self.assertEqual(UserTopic.objects.count() - user_topic_count, 0) + + def test_remove_unmuted_topic(self) -> None: + user = self.example_user("hamlet") + realm = user.realm + self.login_user(user) + + stream = get_stream("Verona", realm) + + do_set_user_topic_visibility_policy( + user, + stream, + "Verona3", + visibility_policy=UserTopic.VisibilityPolicy.UNMUTED, + last_updated=datetime(2020, 1, 1, tzinfo=timezone.utc), + ) + self.assertTrue( + topic_has_visibility_policy( + user, stream.id, "verona3", UserTopic.VisibilityPolicy.UNMUTED + ) + ) + + url = "/api/v1/user_topics" + data = { + "stream_id": stream.id, + "topic": "vEroNA3", + "visibility_policy": UserTopic.VisibilityPolicy.INHERIT, + } + + mock_date_unmute_removed = datetime(2020, 1, 1, tzinfo=timezone.utc).timestamp() + + events: List[Mapping[str, Any]] = [] + with self.tornado_redirected_to_list(events, expected_num_events=2): + with time_machine.travel(datetime(2020, 1, 1, tzinfo=timezone.utc), tick=False): + result = self.api_post(user, url, data) + self.assert_json_success(result) + + self.assertFalse( + topic_has_visibility_policy( + user, stream.id, "verona3", UserTopic.VisibilityPolicy.UNMUTED + ) + ) + # Verify if events are sent properly + user_topic_event: Dict[str, Any] = { + "type": "user_topic", + "stream_id": stream.id, + "topic_name": data["topic"], + "last_updated": mock_date_unmute_removed, + "visibility_policy": UserTopic.VisibilityPolicy.INHERIT, + } + muted_topics_event = dict(type="muted_topics", muted_topics=get_topic_mutes(user)) + self.assertEqual(events[0]["event"], muted_topics_event) + self.assertEqual(events[1]["event"], user_topic_event) + + # Check that removing UNMUTE from a topic for which the user + # doesn't already have a visibility_policy doesn't cause an error. + with self.assertLogs(level="INFO") as info_logs: + result = self.api_post(user, url, data) + self.assert_json_success(result) + self.assertEqual( + info_logs.output[0], + f"INFO:root:User {user.id} tried to remove visibility_policy, which actually doesn't exist", + ) + + def test_unmuted_topic_add_invalid(self) -> None: + user = self.example_user("hamlet") + self.login_user(user) + + url = "/api/v1/user_topics" + data = { + "stream_id": 999999999, + "topic": "Verona3", + "visibility_policy": UserTopic.VisibilityPolicy.UNMUTED, + } + + result = self.api_post(user, url, data) + self.assert_json_error(result, "Invalid stream ID") + + def test_unmuted_topic_remove_invalid(self) -> None: + user = self.example_user("hamlet") + self.login_user(user) + + url = "/api/v1/user_topics" + data = { + "stream_id": 999999999, + "topic": "Verona3", + "visibility_policy": UserTopic.VisibilityPolicy.INHERIT, + } + + result = self.api_post(user, url, data) + self.assert_json_error(result, "Invalid stream ID") diff --git a/zerver/views/user_topics.py b/zerver/views/user_topics.py index d86393c512..547f93c870 100644 --- a/zerver/views/user_topics.py +++ b/zerver/views/user_topics.py @@ -15,7 +15,7 @@ from zerver.lib.streams import ( access_stream_to_remove_visibility_policy_by_name, check_for_exactly_one_stream_arg, ) -from zerver.lib.validator import check_int, check_string_in +from zerver.lib.validator import check_int, check_int_in, check_string_in from zerver.models import UserProfile, UserTopic @@ -87,3 +87,23 @@ def update_muted_topic( topic_name=topic, ) return json_success(request) + + +@has_request_variables +def update_user_topic( + request: HttpRequest, + user_profile: UserProfile, + stream_id: int = REQ(json_validator=check_int), + topic: str = REQ(), + visibility_policy: int = REQ(json_validator=check_int_in(UserTopic.VisibilityPolicy.values)), +) -> HttpResponse: + if visibility_policy == UserTopic.VisibilityPolicy.INHERIT: + error = _("Invalid stream ID") + stream = access_stream_to_remove_visibility_policy_by_id(user_profile, stream_id, error) + else: + (stream, sub) = access_stream_by_id(user_profile, stream_id) + + do_set_user_topic_visibility_policy( + user_profile, stream, topic, visibility_policy=visibility_policy + ) + return json_success(request) diff --git a/zproject/urls.py b/zproject/urls.py index 5168892473..b5685ff2cd 100644 --- a/zproject/urls.py +++ b/zproject/urls.py @@ -192,7 +192,7 @@ from zerver.views.user_settings import ( regenerate_api_key, set_avatar_backend, ) -from zerver.views.user_topics import update_muted_topic +from zerver.views.user_topics import update_muted_topic, update_user_topic from zerver.views.users import ( add_bot_backend, avatar, @@ -476,7 +476,10 @@ v1_api_and_json_patterns = [ DELETE=remove_subscriptions_backend, ), # topic-muting -> zerver.views.user_topics + # (deprecated and will be removed once clients are migrated to use '/user_topics') rest_path("users/me/subscriptions/muted_topics", PATCH=update_muted_topic), + # used to update the personal preferences for a topic -> zerver.views.user_topics + rest_path("user_topics", POST=update_user_topic), # user-muting -> zerver.views.user_mutes rest_path("users/me/muted_users/", POST=mute_user, DELETE=unmute_user), # used to register for an event queue in tornado