Skip to content

Commit 85379bd

Browse files
committed
model: refactor: Group _handle_* and associated methods at end of Model.
Underscore prefixes added to update_rendered_view and update_topic_index, with updated docstrings to indicate their internal usage in _handle_* methods. Tests amended.
1 parent 8fdb607 commit 85379bd

File tree

2 files changed

+47
-42
lines changed

2 files changed

+47
-42
lines changed

tests/model/test_model.py

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -648,7 +648,7 @@ def test__stream_info_from_subscriptions(self, initial_data, streams,
648648
def test__handle_message_event_with_Falsey_log(self, mocker,
649649
model, message_fixture):
650650
model.found_newest = True
651-
mocker.patch('zulipterminal.model.Model.update_topic_index')
651+
mocker.patch('zulipterminal.model.Model._update_topic_index')
652652
index_msg = mocker.patch('zulipterminal.model.index_messages',
653653
return_value={})
654654
model.msg_list = mocker.Mock(log=[])
@@ -669,7 +669,7 @@ def test__handle_message_event_with_Falsey_log(self, mocker,
669669
def test__handle_message_event_with_valid_log(self, mocker,
670670
model, message_fixture):
671671
model.found_newest = True
672-
mocker.patch('zulipterminal.model.Model.update_topic_index')
672+
mocker.patch('zulipterminal.model.Model._update_topic_index')
673673
index_msg = mocker.patch('zulipterminal.model.index_messages',
674674
return_value={})
675675
model.msg_list = mocker.Mock(log=[mocker.Mock()])
@@ -692,7 +692,7 @@ def test__handle_message_event_with_valid_log(self, mocker,
692692
def test__handle_message_event_with_flags(self, mocker,
693693
model, message_fixture):
694694
model.found_newest = True
695-
mocker.patch('zulipterminal.model.Model.update_topic_index')
695+
mocker.patch('zulipterminal.model.Model._update_topic_index')
696696
index_msg = mocker.patch('zulipterminal.model.index_messages',
697697
return_value={})
698698
model.msg_list = mocker.Mock()
@@ -757,7 +757,7 @@ def test__handle_message_event_with_flags(self, mocker,
757757
def test__handle_message_event(self, mocker, user_profile, response,
758758
narrow, recipients, model, log):
759759
model.found_newest = True
760-
mocker.patch('zulipterminal.model.Model.update_topic_index')
760+
mocker.patch('zulipterminal.model.Model._update_topic_index')
761761
index_msg = mocker.patch('zulipterminal.model.index_messages',
762762
return_value={})
763763
create_msg_box_list = mocker.patch('zulipterminal.model.'
@@ -794,14 +794,14 @@ def test__handle_message_event(self, mocker, user_profile, response,
794794
('TOPIC1', [], ['TOPIC1'])
795795
], ids=['reorder_topic3', 'topic1_discussion_continues', 'new_topic4',
796796
'first_topic_1'])
797-
def test_update_topic_index(self, topic_name, topic_order_intial,
798-
topic_order_final, model):
797+
def test__update_topic_index(self, topic_name, topic_order_intial,
798+
topic_order_final, model):
799799
model.index = {
800800
'topics': {
801801
86: topic_order_intial,
802802
}
803803
}
804-
model.update_topic_index(86, topic_name)
804+
model._update_topic_index(86, topic_name)
805805
assert model.index['topics'][86] == topic_order_final
806806

807807
# TODO: Ideally message_fixture would use standardized ids?
@@ -972,12 +972,12 @@ def test__handle_update_message_event(self, mocker, model,
972972
},
973973
'edited_messages': set()
974974
}
975-
mocker.patch('zulipterminal.model.Model.update_rendered_view')
975+
mocker.patch('zulipterminal.model.Model._update_rendered_view')
976976

977977
model._handle_update_message_event(response)
978978

979979
assert model.index == new_index
980-
assert model.update_rendered_view.call_count == update_call_count
980+
assert model._update_rendered_view.call_count == update_call_count
981981

982982
@pytest.mark.parametrize('subject, narrow, new_log_len', [
983983
('foo', [['stream', 'boo'], ['topic', 'foo']], 2),
@@ -988,8 +988,8 @@ def test__handle_update_message_event(self, mocker, model,
988988
'msgbox_removed_due_to_topic_narrow_mismatch',
989989
'msgbox_updated_in_all_messages_narrow',
990990
])
991-
def test_update_rendered_view(self, mocker, model, subject, narrow,
992-
new_log_len, msg_id=1):
991+
def test__update_rendered_view(self, mocker, model, subject, narrow,
992+
new_log_len, msg_id=1):
993993
msg_w = mocker.Mock()
994994
other_msg_w = mocker.Mock()
995995
msg_w.original_widget.message = {'id': msg_id, 'subject': subject}
@@ -1001,7 +1001,7 @@ def test_update_rendered_view(self, mocker, model, subject, narrow,
10011001
cmbl = mocker.patch('zulipterminal.model.create_msg_box_list',
10021002
return_value=[new_msg_w])
10031003

1004-
model.update_rendered_view(msg_id)
1004+
model._update_rendered_view(msg_id)
10051005

10061006
# If there are 2 msgs and first one is updated, next one is updated too
10071007
if new_log_len == 2:
@@ -1018,9 +1018,9 @@ def test_update_rendered_view(self, mocker, model, subject, narrow,
10181018
'previous_topic_narrow_empty_so_change_narrow',
10191019
'same_all_messages_narrow',
10201020
])
1021-
def test_update_rendered_view_change_narrow(self, mocker, model, subject,
1022-
narrow, narrow_changed,
1023-
msg_id=1):
1021+
def test__update_rendered_view_change_narrow(self, mocker, model, subject,
1022+
narrow, narrow_changed,
1023+
msg_id=1):
10241024
msg_w = mocker.Mock()
10251025
other_msg_w = mocker.Mock()
10261026
msg_w.original_widget.message = {'id': msg_id, 'subject': subject}
@@ -1031,7 +1031,7 @@ def test_update_rendered_view_change_narrow(self, mocker, model, subject,
10311031
cmbl = mocker.patch('zulipterminal.model.create_msg_box_list',
10321032
return_value=[new_msg_w])
10331033

1034-
model.update_rendered_view(msg_id)
1034+
model._update_rendered_view(msg_id)
10351035

10361036
assert model.controller.narrow_to_topic.called == narrow_changed
10371037
assert model.controller.update_screen.called
@@ -1152,13 +1152,13 @@ def test__handle_reaction_event_remove_reaction(self, mocker, model,
11521152
def test_update_star_status_no_index(self, mocker, model):
11531153
model.index = dict(messages={}) # Not indexed
11541154
event = dict(messages=[1], flag='starred', all=False, operation='add')
1155-
mocker.patch('zulipterminal.model.Model.update_rendered_view')
1155+
mocker.patch('zulipterminal.model.Model._update_rendered_view')
11561156
set_count = mocker.patch('zulipterminal.model.set_count')
11571157

11581158
model._handle_update_message_flags_event(event)
11591159

11601160
assert model.index == dict(messages={})
1161-
model.update_rendered_view.assert_not_called()
1161+
model._update_rendered_view.assert_not_called()
11621162
set_count.assert_not_called()
11631163

11641164
def test_update_star_status_invalid_operation(self, mocker, model):
@@ -1170,11 +1170,11 @@ def test_update_star_status_invalid_operation(self, mocker, model):
11701170
'operation': 'OTHER', # not 'add' or 'remove'
11711171
'all': False,
11721172
}
1173-
mocker.patch('zulipterminal.model.Model.update_rendered_view')
1173+
mocker.patch('zulipterminal.model.Model._update_rendered_view')
11741174
set_count = mocker.patch('zulipterminal.model.set_count')
11751175
with pytest.raises(RuntimeError):
11761176
model._handle_update_message_flags_event(event)
1177-
model.update_rendered_view.assert_not_called()
1177+
model._update_rendered_view.assert_not_called()
11781178
set_count.assert_not_called()
11791179

11801180
@pytest.mark.parametrize('event_message_ids, indexed_ids', [
@@ -1208,15 +1208,15 @@ def test_update_star_status(self, mocker, model, event_op,
12081208
'operation': event_op,
12091209
'all': False,
12101210
}
1211-
mocker.patch('zulipterminal.model.Model.update_rendered_view')
1211+
mocker.patch('zulipterminal.model.Model._update_rendered_view')
12121212
set_count = mocker.patch('zulipterminal.model.set_count')
12131213

12141214
model._handle_update_message_flags_event(event)
12151215

12161216
changed_ids = set(indexed_ids) & set(event_message_ids)
12171217
for changed_id in changed_ids:
12181218
assert model.index['messages'][changed_id]['flags'] == flags_after
1219-
(model.update_rendered_view.
1219+
(model._update_rendered_view.
12201220
assert_has_calls([mocker.call(changed_id)
12211221
for changed_id in changed_ids]))
12221222

@@ -1258,7 +1258,7 @@ def test_update_read_status(self, mocker, model, event_op,
12581258
'all': False,
12591259
}
12601260

1261-
mocker.patch('zulipterminal.model.Model.update_rendered_view')
1261+
mocker.patch('zulipterminal.model.Model._update_rendered_view')
12621262
set_count = mocker.patch('zulipterminal.model.set_count')
12631263

12641264
model._handle_update_message_flags_event(event)
@@ -1268,10 +1268,10 @@ def test_update_read_status(self, mocker, model, event_op,
12681268
assert model.index['messages'][changed_id]['flags'] == flags_after
12691269

12701270
if event_op == 'add':
1271-
model.update_rendered_view.assert_has_calls(
1271+
model._update_rendered_view.assert_has_calls(
12721272
[mocker.call(changed_id)])
12731273
elif event_op == 'remove':
1274-
model.update_rendered_view.assert_not_called()
1274+
model._update_rendered_view.assert_not_called()
12751275

12761276
for unchanged_id in (set(indexed_ids) - set(event_message_ids)):
12771277
assert (model.index['messages'][unchanged_id]['flags'] ==

zulipterminal/model.py

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -602,6 +602,16 @@ def _group_info_from_realm_user_groups(self,
602602
user_group_names.sort(key=str.lower)
603603
return user_group_names
604604

605+
def toggle_stream_muted_status(self, stream_id: int) -> bool:
606+
request = [{
607+
'stream_id': stream_id,
608+
'property': 'is_muted',
609+
'value': not self.is_muted_stream(stream_id)
610+
# True for muting and False for unmuting.
611+
}]
612+
response = self.client.update_subscription_settings(request)
613+
return response['result'] == 'success'
614+
605615
def _handle_subscription_event(self, event: Event) -> None:
606616
"""
607617
Handle changes in subscription (eg. muting/unmuting streams)
@@ -624,16 +634,6 @@ def _handle_subscription_event(self, event: Event) -> None:
624634

625635
self.controller.update_screen()
626636

627-
def toggle_stream_muted_status(self, stream_id: int) -> bool:
628-
request = [{
629-
'stream_id': stream_id,
630-
'property': 'is_muted',
631-
'value': not self.is_muted_stream(stream_id)
632-
# True for muting and False for unmuting.
633-
}]
634-
response = self.client.update_subscription_settings(request)
635-
return response['result'] == 'success'
636-
637637
def _handle_typing_event(self, event: Event) -> None:
638638
"""
639639
Handle typing notifications (in private messages)
@@ -693,7 +693,8 @@ def _handle_message_event(self, event: Event) -> None:
693693
response['flags'] = event.get('flags', [])
694694
# We need to update the topic order in index, unconditionally.
695695
if response['type'] == 'stream':
696-
self.update_topic_index(response['stream_id'], response['subject'])
696+
self._update_topic_index(response['stream_id'],
697+
response['subject'])
697698
# If the topic view is toggled for incoming message's
698699
# recipient stream, then we re-arrange topic buttons
699700
# with most recent at the top.
@@ -755,9 +756,10 @@ def _handle_message_event(self, event: Event) -> None:
755756
set_count([response['id']], self.controller, 1)
756757
self.controller.update_screen()
757758

758-
def update_topic_index(self, stream_id: int, topic_name: str) -> None:
759+
def _update_topic_index(self, stream_id: int, topic_name: str) -> None:
759760
"""
760761
Update topic order in index based on incoming message.
762+
Helper method called by _handle_message_event
761763
"""
762764
topic_index = self.index['topics'][stream_id]
763765
for topic_iterator, topic in enumerate(topic_index):
@@ -781,15 +783,15 @@ def _handle_update_message_event(self, response: Event) -> None:
781783
if 'rendered_content' in response:
782784
message['content'] = response['rendered_content']
783785
self.index['messages'][message_id] = message
784-
self.update_rendered_view(message_id)
786+
self._update_rendered_view(message_id)
785787

786788
# 'subject' is not present in update event if
787789
# the response didn't have a 'subject' update.
788790
if 'subject' in response:
789791
for msg_id in response['message_ids']:
790792
self.index['messages'][msg_id]['subject']\
791793
= response['subject']
792-
self.update_rendered_view(msg_id)
794+
self._update_rendered_view(msg_id)
793795

794796
def _handle_reaction_event(self, response: Event) -> None:
795797
"""
@@ -818,7 +820,7 @@ def _handle_reaction_event(self, response: Event) -> None:
818820
message['reactions'].remove(reaction)
819821

820822
self.index['messages'][message_id] = message
821-
self.update_rendered_view(message_id)
823+
self._update_rendered_view(message_id)
822824

823825
def _handle_update_message_flags_event(self, event: Event) -> None:
824826
"""
@@ -849,13 +851,16 @@ def _handle_update_message_flags_event(self, event: Event) -> None:
849851
raise RuntimeError(event, msg['flags'])
850852

851853
self.index['messages'][message_id] = msg
852-
self.update_rendered_view(message_id)
854+
self._update_rendered_view(message_id)
853855

854856
if event['operation'] == 'add' and flag_to_change == 'read':
855857
set_count(list(message_ids_to_mark & indexed_message_ids),
856858
self.controller, -1)
857859

858-
def update_rendered_view(self, msg_id: int) -> None:
860+
def _update_rendered_view(self, msg_id: int) -> None:
861+
"""
862+
Helper method called by various _handle_* methods
863+
"""
859864
# Update new content in the rendered view
860865
for msg_w in self.msg_list.log:
861866
msg_box = msg_w.original_widget

0 commit comments

Comments
 (0)