Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support muted users #1546

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Support muted users #1546

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
68ab15e
model: Add muted_users to intial_data_to_fetch.
rsashank Jul 30, 2023
6f1fcce
model/api_types: Handle muted_user event.
rsashank Aug 9, 2023
a7d0d10
model: Add is_user_muted method.
rsashank Sep 20, 2024
3937ee2
messages: Changes made in MessageBox for muted_messages.
rsashank Sep 20, 2024
10ad48b
views: Remove muted_users from user_list.
rsashank Sep 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions tests/core/test_core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ def test_narrow_to_user(
controller.view.message_view = mocker.patch("urwid.ListBox")
controller.model.user_id = 5140
controller.model.user_email = "some@email"
controller.model._muted_users = set()
controller.model.user_dict = {
user_email: {
"user_id": user_id,
Expand Down Expand Up @@ -267,6 +268,7 @@ def test_narrow_to_all_messages(
controller.view.message_view = mocker.patch("urwid.ListBox")
controller.model.user_email = "some@email"
controller.model.user_id = 1
controller.model._muted_users = set()
controller.model.stream_dict = {
205: {
"color": "#ffffff",
Expand Down Expand Up @@ -295,6 +297,7 @@ def test_narrow_to_all_pm(
controller.view.message_view = mocker.patch("urwid.ListBox")
controller.model.user_id = 1
controller.model.user_email = "some@email"
controller.model._muted_users = set()

controller.narrow_to_all_pm() # FIXME: Add id narrowing test

Expand All @@ -316,6 +319,7 @@ def test_narrow_to_all_starred(
# FIXME: Expand upon is_muted_topic().
mocker.patch(MODEL + ".is_muted_topic", return_value=False)
controller.model.user_email = "some@email"
controller.model._muted_users = set()
controller.model.stream_dict = {
205: {
"color": "#ffffff",
Expand Down Expand Up @@ -343,6 +347,7 @@ def test_narrow_to_all_mentions(
mocker.patch(MODEL + ".is_muted_topic", return_value=False)
controller.model.user_email = "some@email"
controller.model.user_id = 1
controller.model._muted_users = set()
controller.model.stream_dict = {
205: {
"color": "#ffffff",
Expand Down
6 changes: 6 additions & 0 deletions tests/ui_tools/test_messages.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -776,6 +776,7 @@ def test_main_view(self, mocker, message, last_message):
"reactions": [],
"sender_full_name": "Alice",
"timestamp": 1532103879,
"sender_id": 32900,
}
],
)
Expand Down Expand Up @@ -814,6 +815,7 @@ def test_main_view_renders_slash_me(self, mocker, message, content, is_me_messag
"reactions": [],
"sender_full_name": "Alice",
"timestamp": 1532103879,
"sender_id": 32900,
}
],
)
Expand Down Expand Up @@ -867,6 +869,7 @@ def test_main_view_generates_stream_header(
"reactions": [],
"sender_full_name": "Alice",
"timestamp": 1532103879,
"sender_id": 32900,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Linting easy fix?

},
],
)
Expand Down Expand Up @@ -1057,6 +1060,7 @@ def test_msg_generates_search_and_header_bar(
"reactions": [],
"sender_full_name": "alice",
"timestamp": 1532103879,
"sender_id": 32900,
}
],
)
Expand Down Expand Up @@ -1117,6 +1121,8 @@ def test_main_view_content_header_without_header(

msg_box = MessageBox(this_msg, self.model, last_msg)

self.model.is_muted_user.return_value = False

Comment on lines +1124 to +1125
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I expect this adjusts the tests to pass, but doesn't test the new behavior? That is, this doesn't handle any muted-user cases, as well as any cases with mixed/combined messages of muted and not-muted? (before vs current)

expected_header[2] = output_date_time
if current_year > 2018:
expected_header[2] = "2018 - " + expected_header[2]
Expand Down
17 changes: 15 additions & 2 deletions zulipterminal/ui_tools/messages.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,9 @@ def __init__(self, message: Message, model: "Model", last_message: Any) -> None:
else:
self.recipients_names = ", ".join(
[
recipient["full_name"]
"Muted user"
if self.model.is_muted_user(recipient["id"])
else recipient["full_name"]
for recipient in self.message["display_recipient"]
if recipient["email"] != self.model.user_email
Comment on lines -99 to 103
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be inclined to add parentheses here for the new code, to make this clearer.

]
Expand Down Expand Up @@ -687,7 +689,12 @@ def main_view(self) -> List[Any]:
text: Dict[str, urwid_MarkupTuple] = {key: (None, " ") for key in text_keys}

if any(different[key] for key in ("recipients", "author", "24h")):
text["author"] = ("msg_sender", message["this"]["author"])
text["author"] = (
"msg_sender",
"Muted user"
if self.model.is_muted_user(self.message["sender_id"])
else message["this"]["author"],
)
Comment on lines 691 to +697
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this only work for non-adjacent muted user messages?

That is, does this cover the case where a muted user sends multiple messages in succession?


# TODO: Refactor to use user ids for look up instead of emails.
email = self.message.get("sender_email", "")
Expand Down Expand Up @@ -729,10 +736,16 @@ def main_view(self) -> List[Any]:
"/me", f"<strong>{self.message['sender_full_name']}</strong>", 1
)

muted_message_text = "This message was hidden because you have muted the sender"

# Transform raw message content into markup (As needed by urwid.Text)
content, self.message_links, self.time_mentions = self.transform_content(
self.message["content"], self.model.server_url
)

if self.model.is_muted_user(self.message["sender_id"]):
content = (None, muted_message_text)
Comment on lines +746 to +747
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're checking if it's a muted user every time, then it seems reasonable to place the check earlier, and only check for muting once?

Comment on lines +739 to +747
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd keep the muted_message_text close to the conditional.

I was going to suggest the transform_content could be skipped, but it's also used for extracting eg. the links, so we should always do that - that may be worth a comment for future readers :)

It'd be preferable to simplify the whole content-showing section, but I suspect that may need careful work, possibly with more tests, so for later.


self.content.set_text(content)

if self.message["id"] in self.model.index["edited_messages"]:
Expand Down