From eb377a88726456c76c4f8c8de2fba1a0ab07171b Mon Sep 17 00:00:00 2001 From: Lauryn Menard Date: Mon, 12 Sep 2022 17:15:00 +0200 Subject: [PATCH] read_receipts: Exclude muted users from read receipts. Removes IDs of users who have muted or been muted by the current user from the list of user IDs returned by the read receipts endpoint. Fixes #22909. --- templates/zerver/api/changelog.md | 6 +++ version.py | 2 +- zerver/openapi/zulip.yaml | 12 ++++- zerver/tests/test_read_receipts.py | 79 ++++++++++++++++++++++++++++++ zerver/views/read_receipts.py | 11 +++++ 5 files changed, 107 insertions(+), 3 deletions(-) diff --git a/templates/zerver/api/changelog.md b/templates/zerver/api/changelog.md index a2b4d71d9f..8bea4cfbc4 100644 --- a/templates/zerver/api/changelog.md +++ b/templates/zerver/api/changelog.md @@ -20,6 +20,12 @@ format used by the Zulip server that they are interacting with. ## Changes in Zulip 6.0 +**Feature level 144** + +* [`GET /messages/{message_id}/read_receipts`](/api/get-read-receipts): + The `user_ids` array returned by the server no longer includes IDs + of users who have been muted by or have muted the current user. + **Feature level 143** * `PATCH /realm`: The `disallow_disposable_email_addresses`, diff --git a/version.py b/version.py index d0fd1fe1d3..15d8cb169d 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 = 143 +API_FEATURE_LEVEL = 144 # 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/zulip.yaml b/zerver/openapi/zulip.yaml index 713c9220ad..cdabcd5e93 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -5903,8 +5903,16 @@ paths: The IDs of users who have disabled sending read receipts (`send_read_receipts=false`) will never appear in the response, - nor will the message's sender. The current user's ID will, if - they have marked the read target message as read. + nor will the message's sender. Additionally, the IDs of any users + who have been muted by the current user or who have muted the + current user will not be included in the response. + + The current user's ID will appear if they have marked the target + message as read. + + **Changes**: Prior to Zulip 6.0 (feature level 143), the IDs of + users who have been muted by or have muted the current user were + included in the response. items: type: integer example: diff --git a/zerver/tests/test_read_receipts.py b/zerver/tests/test_read_receipts.py index 9761e0274c..9a1ed3a5b1 100644 --- a/zerver/tests/test_read_receipts.py +++ b/zerver/tests/test_read_receipts.py @@ -1,10 +1,12 @@ import orjson +from django.utils.timezone import now as timezone_now from zerver.actions.create_user import do_reactivate_user from zerver.actions.realm_settings import do_set_realm_property from zerver.actions.user_settings import do_change_user_setting from zerver.actions.users import do_deactivate_user from zerver.lib.test_classes import ZulipTestCase +from zerver.lib.user_mutes import add_user_mute, get_mute_object from zerver.models import UserMessage, UserProfile @@ -206,3 +208,80 @@ class TestReadReceipts(ZulipTestCase): result = self.client_get(f"/json/messages/{message_id}/read_receipts") self.assert_json_success(result) self.assertIn(hamlet.id, result.json()["user_ids"]) + + def test_filter_muted_users(self) -> None: + hamlet = self.example_user("hamlet") + cordelia = self.example_user("cordelia") + othello = self.example_user("othello") + iago = self.example_user("iago") + + # Hamlet mutes Cordelia + add_user_mute(hamlet, cordelia, date_muted=timezone_now()) + # Cordelia mutes Othello + add_user_mute(cordelia, othello, date_muted=timezone_now()) + + # Iago sends a message + message_id = self.send_stream_message(iago, "Verona", "read receipts") + + # Mark message as read for users. + self.mark_message_read(hamlet, message_id) + self.mark_message_read(cordelia, message_id) + self.mark_message_read(othello, message_id) + + # Login as Iago and make sure all three users are in read receipts. + self.login("iago") + result = self.client_get(f"/json/messages/{message_id}/read_receipts") + response_dict = self.assert_json_success(result) + self.assert_length(response_dict["user_ids"], 3) + self.assertTrue(hamlet.id in response_dict["user_ids"]) + self.assertTrue(cordelia.id in response_dict["user_ids"]) + self.assertTrue(othello.id in response_dict["user_ids"]) + + # Login as Hamlet and make sure Cordelia is not in read receipts. + self.login("hamlet") + result = self.client_get(f"/json/messages/{message_id}/read_receipts") + response_dict = self.assert_json_success(result) + self.assert_length(response_dict["user_ids"], 2) + self.assertTrue(hamlet.id in response_dict["user_ids"]) + self.assertFalse(cordelia.id in response_dict["user_ids"]) + self.assertTrue(othello.id in response_dict["user_ids"]) + + # Login as Othello and make sure Cordelia is not in in read receipts. + self.login("othello") + result = self.client_get(f"/json/messages/{message_id}/read_receipts") + response_dict = self.assert_json_success(result) + self.assert_length(response_dict["user_ids"], 2) + self.assertTrue(hamlet.id in response_dict["user_ids"]) + self.assertFalse(cordelia.id in response_dict["user_ids"]) + self.assertTrue(othello.id in response_dict["user_ids"]) + + # Login as Cordelia and make sure Hamlet and Othello are not in read receipts. + self.login("cordelia") + result = self.client_get(f"/json/messages/{message_id}/read_receipts") + response_dict = self.assert_json_success(result) + self.assert_length(response_dict["user_ids"], 1) + self.assertFalse(hamlet.id in response_dict["user_ids"]) + self.assertTrue(cordelia.id in response_dict["user_ids"]) + self.assertFalse(othello.id in response_dict["user_ids"]) + + # Cordelia unmutes Othello + mute_object = get_mute_object(cordelia, othello) + assert mute_object is not None + mute_object.delete() + + # Now Othello should appear in her read receipts, but not Hamlet. + result = self.client_get(f"/json/messages/{message_id}/read_receipts") + response_dict = self.assert_json_success(result) + self.assert_length(response_dict["user_ids"], 2) + self.assertFalse(hamlet.id in response_dict["user_ids"]) + self.assertTrue(cordelia.id in response_dict["user_ids"]) + self.assertTrue(othello.id in response_dict["user_ids"]) + + # Login as Othello and make sure all three users are in read receipts. + self.login("othello") + result = self.client_get(f"/json/messages/{message_id}/read_receipts") + response_dict = self.assert_json_success(result) + self.assert_length(response_dict["user_ids"], 3) + self.assertTrue(hamlet.id in response_dict["user_ids"]) + self.assertTrue(cordelia.id in response_dict["user_ids"]) + self.assertTrue(othello.id in response_dict["user_ids"]) diff --git a/zerver/views/read_receipts.py b/zerver/views/read_receipts.py index 606cd432b1..01f0757354 100644 --- a/zerver/views/read_receipts.py +++ b/zerver/views/read_receipts.py @@ -29,6 +29,15 @@ def read_receipts( # read the message before sending it, and showing the sender as # having read their own message is likely to be confusing. # + # * Users who have muted the current user are not included, since + # the current user could infer that they have been muted by + # said users by noting that the muters immediately read every + # message that the current user sends to mutually subscribed + # streams. + # + # * Users muted by the current user are also not included, as this + # is consistent with other aspects of how muting works. + # # * Deactivated users are excluded. While in theory someone # could be interested in the information, not including them # is a cleaner policy, and usually read receipts are only of @@ -66,6 +75,8 @@ def read_receipts( user_profile__send_read_receipts=True, ) .exclude(user_profile_id=message.sender_id) + .exclude(user_profile__muter__muted_user_id=user_profile.id) + .exclude(user_profile__muted__user_profile_id=user_profile.id) .extra( where=[UserMessage.where_read()], )