Skip to content

Commit 47bede5

Browse files
committed
model: Store found_newest for each narrow.
Currently, found_newest is not stored per-narrow. This leads to message loading bugs when switching between narrows, and a narrow is not fully read. Tests amended.
1 parent 7af7e26 commit 47bede5

File tree

4 files changed

+28
-16
lines changed

4 files changed

+28
-16
lines changed

tests/core/test_core.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ def test_narrow_to_stream(self, mocker, controller,
8181
'color': '#ffffff',
8282
}
8383
}
84+
controller.model.found_newest = {}
8485
controller.model.muted_streams = []
8586
controller.model.muted_topics = []
8687
controller.narrow_to_stream(stream_button)
@@ -107,6 +108,7 @@ def test_narrow_to_topic(self, mocker, controller,
107108
'color': '#ffffff',
108109
}
109110
}
111+
controller.model.found_newest = {}
110112
controller.model.muted_streams = []
111113
controller.model.muted_topics = []
112114
controller.narrow_to_topic(msg_box)
@@ -131,6 +133,7 @@ def test_narrow_to_user(self, mocker, controller, user_button, index_user):
131133
'user_id': user_button.user_id
132134
}
133135
}
136+
controller.model.found_newest = {}
134137
controller.narrow_to_user(user_button)
135138
assert controller.model.narrow == [["pm_with", user_button.email]]
136139
controller.model.msg_view.clear.assert_called_once_with()
@@ -151,6 +154,7 @@ def test_show_all_messages(self, mocker, controller, index_all_messages):
151154
'color': '#ffffff',
152155
}
153156
}
157+
controller.model.found_newest = {}
154158
controller.model.muted_streams = []
155159
controller.model.muted_topics = []
156160

@@ -170,6 +174,7 @@ def test_show_all_pm(self, mocker, controller, index_user):
170174
controller.model.msg_view = mocker.patch('urwid.SimpleFocusListWalker')
171175
controller.model.msg_list = mocker.patch('urwid.ListBox')
172176
controller.model.user_email = "some@email"
177+
controller.model.found_newest = {}
173178

174179
controller.show_all_pm('')
175180

@@ -192,6 +197,7 @@ def test_show_all_starred(self, mocker, controller, index_all_starred):
192197
'color': '#ffffff',
193198
}
194199
}
200+
controller.model.found_newest = {}
195201
controller.model.msg_view = mocker.patch('urwid.SimpleFocusListWalker')
196202
controller.model.msg_list = mocker.patch('urwid.ListBox')
197203

@@ -216,6 +222,7 @@ def test_show_all_mentions(self, mocker, controller, index_all_mentions):
216222
'color': '#ffffff',
217223
}
218224
}
225+
controller.model.found_newest = {}
219226
controller.model.msg_view = mocker.patch('urwid.SimpleFocusListWalker')
220227
controller.model.msg_list = mocker.patch('urwid.ListBox')
221228

@@ -287,6 +294,7 @@ def test_search_message(self, initial_narrow, final_narrow,
287294
controller.model.index = {'search': {500}} # Any initial search index
288295
controller.model.msg_view = []
289296
controller.model.narrow = initial_narrow
297+
controller.model.found_newest = {}
290298

291299
def set_msg_ids(*args, **kwargs):
292300
controller.model.index['search'].update(msg_ids)

tests/model/test_model.py

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ def test_init(self, model, initial_data, user_profile):
4444
assert model.msg_view is None
4545
assert model.msg_list is None
4646
assert model.narrow == []
47-
assert model.found_newest is False
47+
assert isinstance(model.found_newest, dict)
4848
assert model.stream_id == -1
4949
assert model.stream_dict == {}
5050
assert model.recipients == frozenset()
@@ -467,7 +467,7 @@ def test_success_get_messages(self, mocker, messages_successful_response,
467467
anchor = messages_successful_response['anchor']
468468
if anchor < 10000000000000000:
469469
assert model.index['pointer'][repr(model.narrow)] == anchor
470-
assert model.found_newest is True
470+
assert model.found_newest[repr(model.narrow)] is True
471471

472472
def test_get_message_false_first_anchor(
473473
self, mocker, messages_successful_response, index_all_messages,
@@ -502,9 +502,10 @@ def test_get_message_false_first_anchor(
502502
assert model.index['pointer'][repr(model.narrow)] == 0
503503

504504
# TEST `query_range` < no of messages received
505-
model.found_newest = False # RESET model.found_newest value
505+
# RESET model.found_newest value
506+
model.found_newest[repr(model.narrow)] = False
506507
model.get_messages(num_after=0, num_before=0, anchor=0)
507-
assert model.found_newest is False
508+
assert model.found_newest[repr(model.narrow)] is False
508509

509510
# FIXME This only tests the case where the get_messages is in __init__
510511
def test_fail_get_messages(self, mocker, error_response,
@@ -652,7 +653,7 @@ def test__stream_info_from_subscriptions(self, initial_data, streams,
652653

653654
def test__handle_message_event_with_Falsey_log(self, mocker,
654655
model, message_fixture):
655-
model.found_newest = True
656+
model.found_newest[repr([])] = True
656657
mocker.patch('zulipterminal.model.Model._update_topic_index')
657658
index_msg = mocker.patch('zulipterminal.model.index_messages',
658659
return_value={})
@@ -673,7 +674,7 @@ def test__handle_message_event_with_Falsey_log(self, mocker,
673674

674675
def test__handle_message_event_with_valid_log(self, mocker,
675676
model, message_fixture):
676-
model.found_newest = True
677+
model.found_newest[repr([])] = True
677678
mocker.patch('zulipterminal.model.Model._update_topic_index')
678679
index_msg = mocker.patch('zulipterminal.model.index_messages',
679680
return_value={})
@@ -696,7 +697,7 @@ def test__handle_message_event_with_valid_log(self, mocker,
696697

697698
def test__handle_message_event_with_flags(self, mocker,
698699
model, message_fixture):
699-
model.found_newest = True
700+
model.found_newest[repr([])] = True
700701
mocker.patch('zulipterminal.model.Model._update_topic_index')
701702
index_msg = mocker.patch('zulipterminal.model.index_messages',
702703
return_value={})
@@ -761,7 +762,7 @@ def test__handle_message_event_with_flags(self, mocker,
761762
'mentioned_msg_in_mentioned_msg_narrow'])
762763
def test__handle_message_event(self, mocker, user_profile, response,
763764
narrow, recipients, model, log):
764-
model.found_newest = True
765+
model.found_newest[repr(narrow)] = True
765766
mocker.patch('zulipterminal.model.Model._update_topic_index')
766767
index_msg = mocker.patch('zulipterminal.model.index_messages',
767768
return_value={})
@@ -782,7 +783,7 @@ def test__handle_message_event(self, mocker, user_profile, response,
782783
assert model.msg_list.log == log
783784
set_count.assert_called_once_with([response['id']], self.controller, 1)
784785

785-
model.found_newest = False
786+
model.found_newest[repr(narrow)] = False
786787
model.notify_user.assert_called_once_with(response)
787788
model._handle_message_event(event)
788789
# LOG REMAINS THE SAME IF UPDATE IS FALSE

zulipterminal/core.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ def search_messages(self, text: str) -> None:
159159
self.model.index['search'].clear()
160160
self.model.set_search_narrow(text)
161161

162-
self.model.found_newest = False
162+
self.model.found_newest[repr(self.model.narrow)] = False
163163
self.model.get_messages(num_after=0, num_before=30, anchor=10000000000)
164164
msg_id_list = self.model.get_message_ids_in_current_narrow()
165165

@@ -187,7 +187,7 @@ def _narrow_to(self, button: Any, anchor: Optional[int],
187187
if already_narrowed:
188188
return
189189

190-
self.model.found_newest = False
190+
self.model.found_newest[repr(narrow)] = False
191191

192192
# store the steam id in the model (required for get_message_ids...)
193193
if hasattr(button, 'stream_id'): # FIXME Include in set_narrow?

zulipterminal/model.py

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ def __init__(self, controller: Any) -> None:
6969
self.msg_view = None # type: Any
7070
self.msg_list = None # type: Any
7171
self.narrow = [] # type: List[Any]
72-
self.found_newest = False
72+
self.found_newest = {} # type: Dict[str, bool]
7373
self.stream_id = -1
7474
self.recipients = frozenset() # type: FrozenSet[Any]
7575
self.index = initial_index
@@ -349,15 +349,17 @@ def get_messages(self, *,
349349
response = self.client.get_messages(message_filters=request)
350350
if response['result'] == 'success':
351351
self.index = index_messages(response['messages'], self, self.index)
352+
narrow_str = repr(self.narrow)
352353
if first_anchor and response['anchor'] != 10000000000000000:
353-
self.index['pointer'][repr(self.narrow)] = response['anchor']
354+
self.index['pointer'][narrow_str] = response['anchor']
354355
if 'found_newest' in response:
355-
self.found_newest = response['found_newest']
356+
self.found_newest[narrow_str] = response['found_newest']
356357
else:
357358
# Older versions of the server does not contain the
358359
# 'found_newest' flag. Instead, we use this logic:
359360
query_range = num_after + num_before + 1
360-
self.found_newest = len(response['messages']) < query_range
361+
self.found_newest[narrow_str] = (len(response['messages'])
362+
< query_range)
361363
return ""
362364
return response['msg']
363365

@@ -741,7 +743,8 @@ def _handle_message_event(self, event: Event) -> None:
741743
if 'read' not in message['flags']:
742744
set_count([message['id']], self.controller, 1)
743745

744-
if hasattr(self.controller, 'view') and self.found_newest:
746+
if(hasattr(self.controller, 'view')
747+
and self.found_newest[repr(self.narrow)]):
745748
if self.msg_list.log:
746749
last_message = self.msg_list.log[-1].original_widget.message
747750
else:

0 commit comments

Comments
 (0)