model: Handle read events for indexed messages.

This commit primarily adds handling of updates to the status flags on
messages for 'read' events, in addition to the previously-present
'starred' events, in update_message_flag_status.

We previously sent a message to the server indicating a particular
message had been read locally, also marking it read and updating
unread-counts locally. Now that we handle the 'read'-flag events, we can
also handle remotely-read messages, from other clients.

For now, the count updating is moved from the server-message method to
update_message_flag_status.

NOTE: One current limitation to updating remotely-read messages is that
we only handle messages we have full content of in the index.

Star-flag update tests amended.

New tests added for updating the read-flag.

Author: kaustubh-nair

tests/model/test_model.py

@@ -1150,13 +1150,15 @@ def test_update_reaction_remove_reaction(self, mocker, model, response,
 
     def test_update_star_status_no_index(self, mocker, model):
         model.index = dict(messages={})  # Not indexed
-        event = dict(messages=[1], flag='starred', all=False)
+        event = dict(messages=[1], flag='starred', all=False, operation='add')
         mocker.patch('zulipterminal.model.Model.update_rendered_view')
+        set_count = mocker.patch('zulipterminal.model.set_count')
 
         model.update_message_flag_status(event)
 
         assert model.index == dict(messages={})
         model.update_rendered_view.assert_not_called()
+        set_count.assert_not_called()
 
     def test_update_star_status_invalid_operation(self, mocker, model):
         model.index = dict(messages={1: {'flags': None}})  # Minimal
@@ -1168,9 +1170,11 @@ def test_update_star_status_invalid_operation(self, mocker, model):
             'all': False,
         }
         mocker.patch('zulipterminal.model.Model.update_rendered_view')
+        set_count = mocker.patch('zulipterminal.model.set_count')
         with pytest.raises(RuntimeError):
             model.update_message_flag_status(event)
         model.update_rendered_view.assert_not_called()
+        set_count.assert_not_called()
 
     @pytest.mark.parametrize('event_message_ids, indexed_ids', [
         ([1], [1]),
@@ -1204,19 +1208,80 @@ def test_update_star_status(self, mocker, model, event_op,
             'all': False,
         }
         mocker.patch('zulipterminal.model.Model.update_rendered_view')
+        set_count = mocker.patch('zulipterminal.model.set_count')
 
         model.update_message_flag_status(event)
 
         changed_ids = set(indexed_ids) & set(event_message_ids)
         for changed_id in changed_ids:
             assert model.index['messages'][changed_id]['flags'] == flags_after
         (model.update_rendered_view.
-         has_calls([mocker.call(changed_id) for changed_id in changed_ids]))
+         assert_has_calls([mocker.call(changed_id)
+                          for changed_id in changed_ids]))
 
         for unchanged_id in (set(indexed_ids) - set(event_message_ids)):
             assert (model.index['messages'][unchanged_id]['flags'] ==
                     flags_before)
 
+        set_count.assert_not_called()
+
+    @pytest.mark.parametrize('event_message_ids, indexed_ids', [
+        ([1], [1]),
+        ([1, 2], [1]),
+        ([1, 2], [1, 2]),
+        ([1], [1, 2]),
+        ([], [1, 2]),
+        ([1, 2], []),
+    ])
+    @pytest.mark.parametrize('event_op, flags_before, flags_after', [
+        ('add', [], ['read']),
+        ('add', ['read'], ['read']),
+        ('add', ['starred'], ['starred', 'read']),
+        ('add', ['read', 'starred'], ['read', 'starred']),
+        ('remove', [], []),
+        ('remove', ['read'], ['read']),   # msg cannot be marked 'unread'
+        ('remove', ['starred'], ['starred']),
+        ('remove', ['starred', 'read'], ['starred', 'read']),
+        ('remove', ['read', 'starred'], ['read', 'starred']),
+    ])
+    def test_update_read_status(self, mocker, model, event_op,
+                                event_message_ids, indexed_ids,
+                                flags_before, flags_after):
+        model.index = dict(messages={msg_id: {'flags': flags_before}
+                                     for msg_id in indexed_ids})
+        event = {
+            'messages': event_message_ids,
+            'type': 'update_message_flags',
+            'flag': 'read',
+            'operation': event_op,
+            'all': False,
+        }
+
+        mocker.patch('zulipterminal.model.Model.update_rendered_view')
+        set_count = mocker.patch('zulipterminal.model.set_count')
+
+        model.update_message_flag_status(event)
+
+        changed_ids = set(indexed_ids) & set(event_message_ids)
+        for changed_id in changed_ids:
+            assert model.index['messages'][changed_id]['flags'] == flags_after
+
+            if event_op == 'add':
+                model.update_rendered_view.assert_has_calls(
+                        [mocker.call(changed_id)])
+            elif event_op == 'remove':
+                model.update_rendered_view.assert_not_called()
+
+        for unchanged_id in (set(indexed_ids) - set(event_message_ids)):
+            assert (model.index['messages'][unchanged_id]['flags'] ==
+                    flags_before)
+
+        if event_op == 'add':
+            set_count.assert_called_once_with(list(changed_ids),
+                                              self.controller, -1)
+        elif event_op == 'remove':
+            set_count.assert_not_called()
+
     @pytest.mark.parametrize('narrow, event, called', [
         # Not in PM Narrow
         ([], {}, False),

zulipterminal/model.py

@@ -289,7 +289,6 @@ def mark_message_ids_as_read(self, id_list: List[int]) -> None:
             'flag': 'read',
             'op': 'add',
         })
-        set_count(id_list, self.controller, -1)  # FIXME Update?
 
     def send_private_message(self, recipients: str,
                              content: str) -> bool:
@@ -815,9 +814,11 @@ def update_message_flag_status(self, event: Event) -> None:
         if event['all']:  # FIXME Should handle eventually
             return
 
-        # TODO: Expand from 'starred' to also support 'read' flag changes?
         flag_to_change = event['flag']
-        if flag_to_change != 'starred':
+        if flag_to_change not in {'starred', 'read'}:
+            return
+
+        if flag_to_change == 'read' and event['operation'] == 'remove':
             return
 
         indexed_message_ids = set(self.index['messages'])
@@ -837,6 +838,10 @@ def update_message_flag_status(self, event: Event) -> None:
             self.index['messages'][message_id] = msg
             self.update_rendered_view(message_id)
 
+        if event['operation'] == 'add' and flag_to_change == 'read':
+            set_count(list(message_ids_to_mark & indexed_message_ids),
+                      self.controller, -1)
+
     def update_rendered_view(self, msg_id: int) -> None:
         # Update new content in the rendered view
         for msg_w in self.msg_list.log: