-
-
Notifications
You must be signed in to change notification settings - Fork 256
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.
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
[WIP] Support muted users #1546
base: main
Are you sure you want to change the base?
Changes from 1 commit
68ab15e
6f1fcce
a7d0d10
3937ee2
10ad48b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -190,6 +190,36 @@ def test_init_muted_topics( | |
|
||
assert model._muted_topics == locally_processed_data | ||
|
||
@pytest.mark.parametrize( | ||
"server_response, ids, zulip_feature_level", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nits:
|
||
[ | ||
( | ||
[ | ||
{"id": 32323, "timestamp": 1726810359}, | ||
{"id": 37372, "timestamp": 214214214}, | ||
], | ||
{32323, 37372}, | ||
48, | ||
), | ||
([], set(), 0), | ||
], | ||
ids=[ | ||
"zulip_feature_level:48", | ||
"zulip_feature_level:0", | ||
], | ||
Comment on lines
+206
to
+209
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In other tests we've also handled an edge case near the ZFL when it's enabled; given the setup, it wouldn't be much to add, though there should be no significant difference. |
||
) | ||
def test_init_muted_users( | ||
self, mocker, initial_data, server_response, ids, zulip_feature_level | ||
): | ||
Comment on lines
+211
to
+213
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for adding this test 👍 |
||
mocker.patch(MODEL + ".get_messages", return_value="") | ||
initial_data["zulip_feature_level"] = zulip_feature_level | ||
initial_data["muted_users"] = server_response | ||
self.client.register = mocker.Mock(return_value=initial_data) | ||
|
||
model = Model(self.controller) | ||
|
||
assert model._muted_users == ids | ||
|
||
def test_init_InvalidAPIKey_response(self, mocker, initial_data): | ||
# Both network calls indicate the same response | ||
mocker.patch(MODEL + ".get_messages", return_value="Invalid API key") | ||
|
@@ -262,6 +292,7 @@ def test_register_initial_desired_events(self, mocker, initial_data): | |
"realm_emoji", | ||
"custom_profile_fields", | ||
"zulip_version", | ||
"muted_users", | ||
] | ||
model.client.register.assert_called_once_with( | ||
event_types=event_types, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -145,6 +145,7 @@ def __init__(self, controller: Any) -> None: | |
# zulip_version and zulip_feature_level are always returned in | ||
# POST /register from Feature level 3. | ||
"zulip_version", | ||
"muted_users", | ||
] | ||
|
||
# Events desired with their corresponding callback | ||
|
@@ -208,6 +209,11 @@ def __init__(self, controller: Any) -> None: | |
) | ||
for stream_name, topic, *date_muted in muted_topics | ||
} | ||
# NOTE: muted_users also contains timestamps, but we only store the user IDs | ||
# muted_users was added in ZFL 48, Zulip 4.0 | ||
self._muted_users: Set[int] = set() | ||
if self.server_feature_level >= 48: | ||
self._update_muted_users(self.initial_data["muted_users"]) | ||
|
||
groups = self.initial_data["realm_user_groups"] | ||
self.user_group_by_id: Dict[int, Dict[str, Any]] = {} | ||
|
@@ -1204,6 +1210,9 @@ def get_user_info(self, user_id: int) -> Optional[TidiedUserInfo]: | |
|
||
return user_info | ||
|
||
def _update_muted_users(self, muted_users: List[Dict[int, int]]) -> None: | ||
self._muted_users = {muted_user["id"] for muted_user in muted_users} | ||
Comment on lines
+1217
to
+1218
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can see this is good preparation for the next commit, though you don't use the MutedUser type here, since you define it later - and that may be clearer. For just this one commit, we don't need this function, so it could be inline in the earlier conditional - and then refactor it next with the new type, before then reusing the type and method in what is now the next commit (with the event). However, pick the commit flow that you feel works well - I see this was broadly similar to the previous PR right now? |
||
|
||
def _update_users_data_from_initial_data(self) -> None: | ||
# Dict which stores the active/idle status of users (by email) | ||
presences = self.initial_data["presences"] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that the co-author tags aren't showing in github since they're not quite formatted correctly.