Skip to content

Commit 482cabe

Browse files
committed
model: Handle updating narrows depending on flags for delete_msg event.
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.
1 parent 33b601d commit 482cabe

File tree

2 files changed

+103
-4
lines changed

2 files changed

+103
-4
lines changed

tests/model/test_model.py

Lines changed: 90 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1601,7 +1601,8 @@ def _set_topics_to_old_and_new(event):
16011601
model.controller.update_screen.assert_called_once_with()
16021602

16031603
@pytest.mark.parametrize(
1604-
"event, to_vary_in_index",
1604+
"event, to_vary_in_index, to_vary_in_message, expected_set_count_called,"
1605+
"expected_star_count_called",
16051606
[
16061607
case(
16071608
{
@@ -1614,7 +1615,58 @@ def _set_topics_to_old_and_new(event):
16141615
"all_msg_ids": {537286, 537287, 537288},
16151616
"topic_msg_ids": {205: {"Test": {537286}}},
16161617
},
1617-
id="stream_msg_deleted_from_all_msg",
1618+
{537286: {"flags": ["read"]}},
1619+
False,
1620+
False,
1621+
id="read_stream_msg_deleted_from_all_msg",
1622+
),
1623+
case(
1624+
{
1625+
"message_id": 537286,
1626+
"message_type": "stream",
1627+
"stream_id": 205,
1628+
"topic": "Test",
1629+
},
1630+
{
1631+
"mentioned_msg_ids": {537286},
1632+
"topic_msg_ids": {205: {"Test": {537286}}},
1633+
},
1634+
{537286: {"flags": ["read", "mentioned"]}},
1635+
False,
1636+
False,
1637+
id="read+mentioned_stream_msg_deleted_from_mentions",
1638+
),
1639+
case(
1640+
{
1641+
"message_id": 537286,
1642+
"message_type": "stream",
1643+
"stream_id": 205,
1644+
"topic": "Test",
1645+
},
1646+
{
1647+
"starred_msg_ids": {537286},
1648+
"topic_msg_ids": {205: {"Test": {537286}}},
1649+
},
1650+
{537286: {"flags": ["read", "starred"]}},
1651+
False,
1652+
True,
1653+
id="read+starred_stream_msg_deleted_from_starred",
1654+
),
1655+
case(
1656+
{
1657+
"message_id": 537286,
1658+
"message_type": "stream",
1659+
"stream_id": 205,
1660+
"topic": "Test",
1661+
},
1662+
{
1663+
"mentioned_msg_ids": {537286},
1664+
"topic_msg_ids": {205: {"Test": {537286}}},
1665+
},
1666+
{537286: {"flags": ["wildcard_mentioned"]}},
1667+
True,
1668+
False,
1669+
id="unread+wildcard_mentioned_stream_msg_deleted_from_mentioned",
16181670
),
16191671
case(
16201672
{
@@ -1626,7 +1678,26 @@ def _set_topics_to_old_and_new(event):
16261678
"private_msg_ids": {537287},
16271679
"private_msg_ids_by_user_ids": {(5140, 5179): {537287}},
16281680
},
1629-
id="rivate_msg_deleted_from_private_msgs",
1681+
{537287: {"flags": []}},
1682+
True,
1683+
False,
1684+
id="unread_private_msg_deleted_from_private_msgs",
1685+
),
1686+
case(
1687+
{
1688+
"message_id": 537287,
1689+
"message_type": "private",
1690+
"sender_id": 5140,
1691+
},
1692+
{
1693+
"private_msg_ids": {537287, 537288},
1694+
"starred_msg_ids": {537287},
1695+
"private_msg_ids_by_user_ids": {(5140, 5179): {537287}},
1696+
},
1697+
{537287: {"flags": ["read", "starred"]}},
1698+
False,
1699+
True,
1700+
id="starred_private_msg_deleted_from_starred+private_msgs",
16301701
),
16311702
case(
16321703
{
@@ -1641,7 +1712,10 @@ def _set_topics_to_old_and_new(event):
16411712
(5179, 5140, 5180): {537288},
16421713
},
16431714
},
1644-
id="group_msg_deleted_from_private_msgs",
1715+
{537288: {"flags": ["read", "wildcard_mentioned"]}},
1716+
False,
1717+
False,
1718+
id="read+wildcard_mentioned_group_msg_deleted_from_private_msgs",
16451719
),
16461720
],
16471721
)
@@ -1652,23 +1726,35 @@ def test__handle_delete_message_event(
16521726
empty_index,
16531727
event,
16541728
to_vary_in_index,
1729+
to_vary_in_message,
1730+
expected_set_count_called,
1731+
expected_star_count_called,
16551732
):
16561733
event["type"] = "delete_message"
16571734
message_id = event["message_id"]
16581735

16591736
model.index = empty_index
16601737
model.index.update(to_vary_in_index)
1738+
model.index["messages"].update(to_vary_in_message)
16611739

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

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

16671746
model._handle_delete_message_event(event)
16681747

1748+
if expected_set_count_called:
1749+
set_count.assert_called_once_with([message_id], self.controller, -1)
1750+
if expected_star_count_called:
1751+
self.controller.view.starred_button.update_count.assert_called
1752+
16691753
assert message_id not in model.index["messages"]
16701754
assert message_id not in model.index["all_msg_ids"]
16711755
assert message_id not in model.index["edited_messages"]
1756+
assert message_id not in model.index["mentioned_msg_ids"]
1757+
assert message_id not in model.index["starred_msg_ids"]
16721758
if event["message_type"] == "private":
16731759
assert message_id not in model.index["private_msg_ids"]
16741760
assert message_id not in model.index["private_msg_ids_by_user_ids"].values()

zulipterminal/model.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1172,13 +1172,26 @@ def _handle_delete_message_event(self, event: Event) -> None:
11721172
indexed_message = self.index["messages"].get(message_id, None)
11731173

11741174
if indexed_message:
1175+
# Update unread_count if message was unread before being deleted
1176+
# We need to do this before removing the message from index.
1177+
if "read" not in indexed_message["flags"]:
1178+
set_count([message_id], self.controller, -1)
1179+
# Update starred_count if message was starred before being deleted
1180+
if "starred" in indexed_message["flags"]:
1181+
self.index["starred_msg_ids"].discard(message_id)
1182+
self.controller.view.starred_button.update_count(
1183+
self.controller.view.starred_button.count - 1
1184+
)
1185+
11751186
# Remove all traces of the message from index if present and
11761187
# update the rendered view.
11771188
# FIXME?: Do we need to archive the message instead of completely
11781189
# erasing from index?
11791190
self.index["messages"].pop(message_id, None)
11801191
self.index["all_msg_ids"].discard(message_id)
11811192
self.index["edited_messages"].discard(message_id)
1193+
if {"mentioned", "wildcard_mentioned"} & set(indexed_message["flags"]):
1194+
self.index["mentioned_msg_ids"].discard(message_id)
11821195

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

0 commit comments

Comments
 (0)