Skip to content

Commit

Permalink
bugfix: model: Update topic list on update message event.
Browse files Browse the repository at this point in the history
This updates the topic list when it is open, else resets topic cache
if present.

Tests amended.
  • Loading branch information
kaustubh-nair committed Nov 16, 2020
1 parent bd2351e commit 1cbaef1
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 15 deletions.
91 changes: 76 additions & 15 deletions tests/model/test_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -967,135 +967,196 @@ def test_notify_users_enabled(self, mocker, model, message_fixture,
model.notify_user(message_fixture)
assert notify.called == is_notify_called

@pytest.mark.parametrize('response, update_call_count, new_index', [
@pytest.mark.parametrize(['response', 'update_call_count', 'new_index',
'topic_view_enabled'], [
({ # Only subject of 1 message is updated.
'message_id': 1,
'subject': 'new subject',
'stream_id': 10,
'message_ids': [1],
}, 1, {
'messages': {
1: {
'id': 1,
'stream_id': 10,
'content': 'old content',
'subject': 'new subject'
},
2: {
'id': 2,
'stream_id': 10,
'content': 'old content',
'subject': 'old subject'
}},
'edited_messages': {1}
}),
'edited_messages': {1},
'topics': {10: []},
}, False),
({ # Subject of 2 messages is updated
'message_id': 1,
'subject': 'new subject',
'stream_id': 10,
'message_ids': [1, 2],
}, 2, {
'messages': {
1: {
'id': 1,
'stream_id': 10,
'content': 'old content',
'subject': 'new subject'
},
2: {
'id': 2,
'stream_id': 10,
'content': 'old content',
'subject': 'new subject'
}},
'edited_messages': {1}
}),
'edited_messages': {1},
'topics': {10: []},
}, False),
({ # Message content is updated
'message_id': 1,
'stream_id': 10,
'rendered_content': '<p>new content</p>',
}, 1, {
'messages': {
1: {
'id': 1,
'stream_id': 10,
'content': '<p>new content</p>',
'subject': 'old subject'
},
2: {
'id': 2,
'stream_id': 10,
'content': 'old content',
'subject': 'old subject'
}},
'edited_messages': {1}
}),
'edited_messages': {1},
'topics': {10: ['old subject']},
}, False),
({ # Both message content and subject is updated.
'message_id': 1,
'rendered_content': '<p>new content</p>',
'subject': 'new subject',
'stream_id': 10,
'message_ids': [1],
}, 2, {
'messages': {
1: {
'id': 1,
'stream_id': 10,
'content': '<p>new content</p>',
'subject': 'new subject'
},
2: {
'id': 2,
'stream_id': 10,
'content': 'old content',
'subject': 'old subject'
}},
'edited_messages': {1}
}),
'edited_messages': {1},
'topics': {10: []},
}, False),
({ # Some new type of update which we don't handle yet.
'message_id': 1,
'foo': 'boo',
}, 0, {
'messages': {
1: {
'id': 1,
'stream_id': 10,
'content': 'old content',
'subject': 'old subject'
},
2: {
'id': 2,
'stream_id': 10,
'content': 'old content',
'subject': 'old subject'
}},
'edited_messages': {1}
}),
'edited_messages': {1},
'topics': {10: ['old subject']},
}, False),
({ # message_id not present in index.
'message_id': 3,
'rendered_content': '<p>new content</p>',
'subject': 'new subject',
'stream_id': 10,
'message_ids': [3],
}, 0, {
'messages': {
1: {
'id': 1,
'stream_id': 10,
'content': 'old content',
'subject': 'old subject'
},
2: {
'id': 2,
'stream_id': 10,
'content': 'old content',
'subject': 'old subject'
}},
'edited_messages': set()
}),
'edited_messages': set(),
'topics': {10: ['old subject']},
}, False),
({ # Message content is updated and topic view is enabled.
'message_id': 1,
'rendered_content': '<p>new content</p>',
'subject': 'new subject',
'stream_id': 10,
'message_ids': [1],
}, 2, {
'messages': {
1: {
'id': 1,
'stream_id': 10,
'content': '<p>new content</p>',
'subject': 'new subject'
},
2: {
'id': 2,
'stream_id': 10,
'content': 'old content',
'subject': 'old subject'
}},
'edited_messages': {1},
# _fetch_topics_in_streams will update this.
'topics': {10: ['old subject']},
}, True),
])
def test__handle_update_message_event(self, mocker, model,
response, new_index,
update_call_count):
update_call_count,
topic_view_enabled):
model.index = {
'messages': {
message_id: {
'id': message_id,
'stream_id': 10,
'content': 'old content',
'subject': 'old subject',
} for message_id in [1, 2]
},
'edited_messages': set()
'edited_messages': set(),
'topics': {10: ['old subject']},
}
mocker.patch('zulipterminal.model.Model._update_rendered_view')
fetch_topics = mocker.patch(
'zulipterminal.model.Model._fetch_topics_in_streams')
(model.controller.view.left_panel.stream_is_in_topic_view.
return_value) = topic_view_enabled

model._handle_update_message_event(response)

assert model.index == new_index
assert model._update_rendered_view.call_count == update_call_count
if topic_view_enabled:
fetch_topics.assert_called_once_with([response['stream_id']])
stream_button = model.controller.view.topic_w.stream_button
(model.controller.view.left_panel.show_topics_view.
assert_called_once_with(stream_button))
model.controller.update_screen.assert_called_once_with()

@pytest.mark.parametrize('subject, narrow, new_log_len', [
('foo', [['stream', 'boo'], ['topic', 'foo']], 2),
Expand Down
12 changes: 12 additions & 0 deletions zulipterminal/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -1012,9 +1012,21 @@ def _handle_update_message_event(self, event: Event) -> None:
# the event didn't have a 'subject' update.
if 'subject' in event:
new_subject = event['subject']
stream_id = event['stream_id']
for msg_id in event['message_ids']:
self.index['messages'][msg_id]['subject'] = new_subject
self._update_rendered_view(msg_id)
if stream_id in self.index['topics']:
# If topic view is open, reload list else reset cache.
if (hasattr(self.controller, 'view')
and self.controller.view.left_panel.
stream_is_in_topic_view(message['stream_id'])):
self._fetch_topics_in_streams([stream_id])
self.controller.view.left_panel.show_topics_view(
self.controller.view.topic_w.stream_button)
self.controller.update_screen()
else:
self.index['topics'][stream_id] = []

def _handle_reaction_event(self, event: Event) -> None:
"""
Expand Down

0 comments on commit 1cbaef1

Please sign in to comment.