Skip to content

Commit

Permalink
model: Handle updating narrows depending on flags for delete_msg event.
Browse files Browse the repository at this point in the history 
This commit extends the previous commit that added rudimentary
support for handling delete_message event, and allows updating the
special narrows such as mentioned/starred that were depending on
message flags. This commit could be considered as a bugfix over the
previous one. It also handles updating the unread count if the
message was earlier unread before being deleted.

Tests added and amended.
Fixes one check-box in zulip#993.
  • Loading branch information
@zee-bit
zee-bit committed Jun 9, 2021
1 parent 33b601d commit 482cabe
Show file tree
Hide file tree
Showing 2 changed files with 103 additions and 4 deletions.
94 changes: 90 additions & 4 deletions tests/model/test_model.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1601,7 +1601,8 @@ def _set_topics_to_old_and_new(event):
model.controller.update_screen.assert_called_once_with()

@pytest.mark.parametrize(
"event, to_vary_in_index",
"event, to_vary_in_index, to_vary_in_message, expected_set_count_called,"
"expected_star_count_called",
[
case(
{
Expand All @@ -1614,7 +1615,58 @@ def _set_topics_to_old_and_new(event):
"all_msg_ids": {537286, 537287, 537288},
"topic_msg_ids": {205: {"Test": {537286}}},
},
id="stream_msg_deleted_from_all_msg",
{537286: {"flags": ["read"]}},
False,
False,
id="read_stream_msg_deleted_from_all_msg",
),
case(
{
"message_id": 537286,
"message_type": "stream",
"stream_id": 205,
"topic": "Test",
},
{
"mentioned_msg_ids": {537286},
"topic_msg_ids": {205: {"Test": {537286}}},
},
{537286: {"flags": ["read", "mentioned"]}},
False,
False,
id="read+mentioned_stream_msg_deleted_from_mentions",
),
case(
{
"message_id": 537286,
"message_type": "stream",
"stream_id": 205,
"topic": "Test",
},
{
"starred_msg_ids": {537286},
"topic_msg_ids": {205: {"Test": {537286}}},
},
{537286: {"flags": ["read", "starred"]}},
False,
True,
id="read+starred_stream_msg_deleted_from_starred",
),
case(
{
"message_id": 537286,
"message_type": "stream",
"stream_id": 205,
"topic": "Test",
},
{
"mentioned_msg_ids": {537286},
"topic_msg_ids": {205: {"Test": {537286}}},
},
{537286: {"flags": ["wildcard_mentioned"]}},
True,
False,
id="unread+wildcard_mentioned_stream_msg_deleted_from_mentioned",
),
case(
{
Expand All @@ -1626,7 +1678,26 @@ def _set_topics_to_old_and_new(event):
"private_msg_ids": {537287},
"private_msg_ids_by_user_ids": {(5140, 5179): {537287}},
},
id="rivate_msg_deleted_from_private_msgs",
{537287: {"flags": []}},
True,
False,
id="unread_private_msg_deleted_from_private_msgs",
),
case(
{
"message_id": 537287,
"message_type": "private",
"sender_id": 5140,
},
{
"private_msg_ids": {537287, 537288},
"starred_msg_ids": {537287},
"private_msg_ids_by_user_ids": {(5140, 5179): {537287}},
},
{537287: {"flags": ["read", "starred"]}},
False,
True,
id="starred_private_msg_deleted_from_starred+private_msgs",
),
case(
{
Expand All @@ -1641,7 +1712,10 @@ def _set_topics_to_old_and_new(event):
(5179, 5140, 5180): {537288},
},
},
id="group_msg_deleted_from_private_msgs",
{537288: {"flags": ["read", "wildcard_mentioned"]}},
False,
False,
id="read+wildcard_mentioned_group_msg_deleted_from_private_msgs",
),
],
)
Expand All @@ -1652,23 +1726,35 @@ def test__handle_delete_message_event(
empty_index,
event,
to_vary_in_index,
to_vary_in_message,
expected_set_count_called,
expected_star_count_called,
):
event["type"] = "delete_message"
message_id = event["message_id"]

model.index = empty_index
model.index.update(to_vary_in_index)
model.index["messages"].update(to_vary_in_message)

mocker.patch(MODEL + "._update_rendered_view")
set_count = mocker.patch("zulipterminal.model.set_count")
self.controller.view.message_view = mocker.Mock(log=[])

assert message_id in model.index["messages"].keys()

model._handle_delete_message_event(event)

if expected_set_count_called:
set_count.assert_called_once_with([message_id], self.controller, -1)
if expected_star_count_called:
self.controller.view.starred_button.update_count.assert_called

assert message_id not in model.index["messages"]
assert message_id not in model.index["all_msg_ids"]
assert message_id not in model.index["edited_messages"]
assert message_id not in model.index["mentioned_msg_ids"]
assert message_id not in model.index["starred_msg_ids"]
if event["message_type"] == "private":
assert message_id not in model.index["private_msg_ids"]
assert message_id not in model.index["private_msg_ids_by_user_ids"].values()
Expand Down
13 changes: 13 additions & 0 deletions zulipterminal/model.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1172,13 +1172,26 @@ def _handle_delete_message_event(self, event: Event) -> None:
indexed_message = self.index["messages"].get(message_id, None)

if indexed_message:
# Update unread_count if message was unread before being deleted
# We need to do this before removing the message from index.
if "read" not in indexed_message["flags"]:
set_count([message_id], self.controller, -1)
# Update starred_count if message was starred before being deleted
if "starred" in indexed_message["flags"]:
self.index["starred_msg_ids"].discard(message_id)
self.controller.view.starred_button.update_count(
self.controller.view.starred_button.count - 1
)

# Remove all traces of the message from index if present and
# update the rendered view.
# FIXME?: Do we need to archive the message instead of completely
# erasing from index?
self.index["messages"].pop(message_id, None)
self.index["all_msg_ids"].discard(message_id)
self.index["edited_messages"].discard(message_id)
if {"mentioned", "wildcard_mentioned"} & set(indexed_message["flags"]):
self.index["mentioned_msg_ids"].discard(message_id)

if event["message_type"] == "private":
self.index["private_msg_ids"].discard(message_id)
Expand Down

0 comments on commit 482cabe

Please sign in to comment.