Skip to content

Commit 8b4f797

Browse files
committed
model: Store _have_last_message for each narrow.
Currently, _have_last_message 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`) _have_last_message is set to True * Go to stream B and press `down` but B is still not fully loaded. Hence, _have_last_message is set to False * Recieving message on stream A now won't add it to the Stream A view.
1 parent e26ed11 commit 8b4f797

File tree

2 files changed

+18
-15
lines changed

2 files changed

+18
-15
lines changed

tests/model/test_model.py

Lines changed: 9 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._have_last_message is False
47+
assert isinstance(model._have_last_message, 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._have_last_message is True
470+
assert model._have_last_message[repr(model.narrow)] is True
471471

472472
def test_get_message_false_first_anchor(
473473
self, mocker, messages_successful_response, index_all_messages,
@@ -503,9 +503,9 @@ def test_get_message_false_first_anchor(
503503

504504
# TEST `query_range` < no of messages received
505505
# RESET model._have_last_message value
506-
model._have_last_message = False
506+
model._have_last_message[repr(model.narrow)] = False
507507
model.get_messages(num_after=0, num_before=0, anchor=0)
508-
assert model._have_last_message is False
508+
assert model._have_last_message[repr(model.narrow)] is False
509509

510510
# FIXME This only tests the case where the get_messages is in __init__
511511
def test_fail_get_messages(self, mocker, error_response,
@@ -653,7 +653,7 @@ def test__stream_info_from_subscriptions(self, initial_data, streams,
653653

654654
def test__handle_message_event_with_Falsey_log(self, mocker,
655655
model, message_fixture):
656-
model._have_last_message = True
656+
model._have_last_message[repr([])] = True
657657
mocker.patch('zulipterminal.model.Model._update_topic_index')
658658
index_msg = mocker.patch('zulipterminal.model.index_messages',
659659
return_value={})
@@ -674,7 +674,7 @@ def test__handle_message_event_with_Falsey_log(self, mocker,
674674

675675
def test__handle_message_event_with_valid_log(self, mocker,
676676
model, message_fixture):
677-
model._have_last_message = True
677+
model._have_last_message[repr([])] = True
678678
mocker.patch('zulipterminal.model.Model._update_topic_index')
679679
index_msg = mocker.patch('zulipterminal.model.index_messages',
680680
return_value={})
@@ -697,7 +697,7 @@ def test__handle_message_event_with_valid_log(self, mocker,
697697

698698
def test__handle_message_event_with_flags(self, mocker,
699699
model, message_fixture):
700-
model._have_last_message = True
700+
model._have_last_message[repr([])] = True
701701
mocker.patch('zulipterminal.model.Model._update_topic_index')
702702
index_msg = mocker.patch('zulipterminal.model.index_messages',
703703
return_value={})
@@ -762,7 +762,7 @@ def test__handle_message_event_with_flags(self, mocker,
762762
'mentioned_msg_in_mentioned_msg_narrow'])
763763
def test__handle_message_event(self, mocker, user_profile, response,
764764
narrow, recipients, model, log):
765-
model._have_last_message = True
765+
model._have_last_message[repr(narrow)] = True
766766
mocker.patch('zulipterminal.model.Model._update_topic_index')
767767
index_msg = mocker.patch('zulipterminal.model.index_messages',
768768
return_value={})
@@ -783,7 +783,7 @@ def test__handle_message_event(self, mocker, user_profile, response,
783783
assert model.msg_list.log == log
784784
set_count.assert_called_once_with([response['id']], self.controller, 1)
785785

786-
model._have_last_message = False
786+
model._have_last_message[repr(narrow)] = False
787787
model.notify_user.assert_called_once_with(response)
788788
model._handle_message_event(event)
789789
# LOG REMAINS THE SAME IF UPDATE IS FALSE

zulipterminal/model.py

Lines changed: 9 additions & 6 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._have_last_message = False
72+
self._have_last_message = {} # type: Dict[str, bool]
7373
self.stream_id = -1
7474
self.recipients = frozenset() # type: FrozenSet[Any]
7575
self.index = initial_index
@@ -349,16 +349,18 @@ 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._have_last_message = response['found_newest']
356+
self._have_last_message[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._have_last_message = (len(response['messages'])
361-
< query_range)
361+
num_messages = len(response['messages'])
362+
self._have_last_message[narrow_str] = (num_messages
363+
< query_range)
362364
return ""
363365
return response['msg']
364366

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

745-
if hasattr(self.controller, 'view') and self._have_last_message:
747+
if (hasattr(self.controller, 'view')
748+
and self._have_last_message[repr(self.narrow)]):
746749
if self.msg_list.log:
747750
last_message = self.msg_list.log[-1].original_widget.message
748751
else:

0 commit comments

Comments
 (0)