From 482cabefffe926b7e7c354e5e9d2d3bae3516d99 Mon Sep 17 00:00:00 2001 From: Zeeshan <equbalzeeshan@gmail.com> Date: Wed, 9 Jun 2021 18:17:16 +0530 Subject: [PATCH] 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 #993. --- tests/model/test_model.py | 94 +++++++++++++++++++++++++++++++++++++-- zulipterminal/model.py | 13 ++++++ 2 files changed, 103 insertions(+), 4 deletions(-) diff --git a/tests/model/test_model.py b/tests/model/test_model.py index c7bd9a2a4f..a7373a43da 100644 --- a/tests/model/test_model.py +++ b/tests/model/test_model.py @@ -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( { @@ -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( { @@ -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( { @@ -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", ), ], ) @@ -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() diff --git a/zulipterminal/model.py b/zulipterminal/model.py index aaac129b9f..f140783329 100644 --- a/zulipterminal/model.py +++ b/zulipterminal/model.py @@ -1172,6 +1172,17 @@ 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 @@ -1179,6 +1190,8 @@ def _handle_delete_message_event(self, event: Event) -> None: 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)