Skip to content

Commit

Permalink
model: Add support for handling delete_message events.
Browse files Browse the repository at this point in the history 
This adds a new event action in model that looks for delete_message
events, potentially handling it by removing the message completely
from the index, and then updates the rendered_view to handle the
event dynamically without needing to switch narrows. It also looks
out for messages being unread before being deleted and updates the
count appropriately.

Tests added.
Fixes one check-box of zulip#993.
  • Loading branch information
@zee-bit
zee-bit committed Jun 8, 2021
1 parent 8f3b193 commit 68d97ba
Show file tree
Hide file tree
Showing 2 changed files with 221 additions and 0 deletions.
163 changes: 163 additions & 0 deletions tests/model/test_model.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ def test_register_initial_desired_events(self, mocker, initial_data):
event_types = [
"message",
"update_message",
"delete_message",
"reaction",
"subscription",
"typing",
Expand Down Expand Up @@ -1599,6 +1600,168 @@ def _set_topics_to_old_and_new(event):
view.left_panel.show_topic_view.assert_called_once_with(stream_button)
model.controller.update_screen.assert_called_once_with()

@pytest.mark.parametrize(
"event, to_vary_in_index, to_vary_in_message, expected_set_count_called,"
"expected_star_count_called",
[
case(
{
"message_id": 537286,
"message_type": "stream",
"stream_id": 205,
"topic": "Test",
},
{
"all_msg_ids": {537286, 537287, 537288},
"topic_msg_ids": {205: {"Test": {537286}}},
},
{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(
{
"message_id": 537287,
"message_type": "private",
"sender_id": 5140,
},
{
"private_msg_ids": {537287},
"private_msg_ids_by_user_ids": {(5140, 5179): {537287}},
},
{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}},
},
{537286: {"flags": ["read", "starred"]}},
False,
True,
id="starred_private_msg_deleted_from_starred+private_msgs",
),
case(
{
"message_id": 537288,
"message_type": "private",
"sender_id": 5140,
},
{
"private_msg_ids": {537287, 537288},
"private_msg_ids_by_user_ids": {
(5140, 5179): {537287},
(5179, 5140, 5180): {537288},
},
},
{537288: {"flags": ["read", "wildcard_mentioned"]}},
False,
False,
id="read+wildcard_mentioned_group_msg_deleted_from_private_msgs",
),
],
)
def test__handle_delete_message_event(
self,
mocker,
model,
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"]
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()
else:
stream_id, topic = event["stream_id"], event["topic"]
assert message_id not in model.index["stream_msg_ids_by_stream_id"].values()
assert message_id not in model.index["topic_msg_ids"][stream_id][topic]
model._update_rendered_view.assert_called_once_with(message_id)

@pytest.mark.parametrize(
"subject, narrow, new_log_len",
[
Expand Down
58 changes: 58 additions & 0 deletions zulipterminal/model.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ def __init__(self, controller: Any) -> None:
[
("message", self._handle_message_event),
("update_message", self._handle_update_message_event),
("delete_message", self._handle_delete_message_event),
("reaction", self._handle_reaction_event),
("subscription", self._handle_subscription_event),
("typing", self._handle_typing_event),
Expand Down Expand Up @@ -1160,6 +1161,58 @@ def _update_topic_index(self, stream_id: int, topic_name: str) -> None:
# Update the index.
self.index["topics"][stream_id] = topic_list

def _handle_delete_message_event(self, event: Event) -> None:
"""
Handles message delete event.
TODO: Handle bulk_message_deletion when we support that
"""
assert event["type"] == "delete_message"

message_id = event["message_id"]
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)
sender_id = event["sender_id"]
private_msg_set = self.index["private_msg_ids_by_user_ids"]
for user_id_set, msg_ids in private_msg_set.items():
if sender_id in user_id_set and message_id in msg_ids:
private_msg_set[user_id_set].discard(message_id)
else:
stream_id, topic = event["stream_id"], event["topic"]
stream_msg_ids = self.index["stream_msg_ids_by_stream_id"].get(
stream_id, None
)
if stream_msg_ids:
stream_msg_ids.discard(message_id)
topic_msg_ids = self.index["topic_msg_ids"][stream_id].get(topic, None)
if topic_msg_ids:
topic_msg_ids.discard(message_id)

self._update_rendered_view(message_id)

def _handle_update_message_event(self, event: Event) -> None:
"""
Handle updated (edited) messages (changed content/subject)
Expand Down Expand Up @@ -1327,6 +1380,11 @@ def _update_rendered_view(self, msg_id: int) -> None:
for msg_w in view.message_view.log:
msg_box = msg_w.original_widget
if msg_box.message["id"] == msg_id:
# Remove the message if deleted
if msg_id not in self.index["messages"]:
view.message_view.log.remove(msg_w)
self.controller.update_screen()
return
# Remove the message if it no longer belongs in the current
# narrow.
if (
Expand Down

0 comments on commit 68d97ba

Please sign in to comment.