Skip to content

Commit d77d227

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, where the previous narrow was not fully loaded. Tests amended. To reproduce this (now fixed) bug, * Go to stream A which has been fully loaded. (By pressing `down`) found_newest is set to True * Go to stream B and press `down` but B is still not fully loaded. Hence, found_newest is set to False * Recieving message on stream A now won't add it to the Stream A view.
1 parent 1fc17d5 commit d77d227

File tree

3 files changed

+18
-15
lines changed

3 files changed

+18
-15
lines changed

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: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,6 @@ def _narrow_to(self, button: Any, anchor: Optional[int],
186186
if already_narrowed:
187187
return
188188

189-
190189
# store the steam id in the model (required for get_message_ids...)
191190
if hasattr(button, 'stream_id'): # FIXME Include in set_narrow?
192191
self.model.stream_id = button.stream_id

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)