From 39f13a3a7830262b3c7b7469e0475db37f8065aa Mon Sep 17 00:00:00 2001
From: "neiljp (Neil Pilgrim)" <github@kepier.clara.net>
Date: Sun, 14 Aug 2022 11:58:45 -0700
Subject: [PATCH] model: Skip muted streams and topics in getting next unread
 topic.

Logic originally developed by Aman Agrawal (amanagr) in #442.

Updated to take into account:
- already-added (slightly different) is_muted[stream|topic] methods
- migration of get_next_unread_topic into model (simplifying tests)

Also ensure the current behavior is maintained for now by explicitly sorting
the unread counts, which previous relied upon the initial and subsequent data
being ordered.

Tests added.

Co-authored by: Aman Agrawal <f2016561@pilani.bits-pilani.ac.in>
---
 tests/model/test_model.py | 78 ++++++++++++++++++++++++++++++++++++++-
 zulipterminal/model.py    | 16 +++++---
 2 files changed, 87 insertions(+), 7 deletions(-)

diff --git a/tests/model/test_model.py b/tests/model/test_model.py
index 010d298a2b..7fe09f0eac 100644
--- a/tests/model/test_model.py
+++ b/tests/model/test_model.py
@@ -3229,7 +3229,7 @@ def test_is_muted_topic(
                 {(1, "topic")},
                 (1, "topic"),
                 (1, "topic"),
-                id="unread_still_present_in_topic",  # TODO Should this be None?
+                id="unread_still_present_in_topic",  # TODO Should be None? (2 other cases)
             ),
             case(
                 {},
@@ -3238,6 +3238,66 @@ def test_is_muted_topic(
                 id="no_unreads_with_previous_topic_state",
             ),
             case({}, None, None, id="no_unreads_with_no_previous_topic_state"),
+            case(
+                {(1, "topic"), (2, "topic2"), (3, "topic3")},
+                (2, "topic2"),
+                (1, "topic"),
+                id="unread_present_before_previous_topic_skipping_muted_stream",
+            ),
+            case(
+                {(1, "topic"), (2, "topic2"), (3, "topic3"), (4, "topic4")},
+                (2, "topic2"),
+                (4, "topic4"),
+                id="unread_present_after_previous_topic_skipping_muted_stream",
+            ),
+            case(
+                {(3, "topic3")},
+                (2, "topic2"),
+                None,
+                id="unread_present_only_in_muted_stream",
+            ),
+            case(
+                {(3, "topic3")},
+                (3, "topic3"),
+                None,
+                id="unread_present_starting_in_muted_stream",  # TODO See other 2 cases
+            ),
+            case(
+                {(3, "topic3"), (4, "topic4")},
+                None,
+                (4, "topic4"),
+                id="unread_present_skipping_first_muted_stream_unread",
+            ),
+            case(
+                {(2, "topic2 muted")},
+                None,
+                None,
+                id="unread_present_only_in_muted_topic",
+            ),
+            case(
+                {(2, "topic2 muted")},
+                (2, "topic2 muted"),
+                None,
+                id="unread_present_starting_in_muted_topic",  # TODO See other 2 cases
+            ),
+            case(
+                {(3, "topic3 muted")},
+                None,
+                None,
+                id="unread_present_only_in_muted_topic_in_muted_stream",
+            ),
+            case(
+                {(2, "topic2"), (2, "topic2 muted"), (4, "topic4")},
+                (2, "topic2"),
+                (4, "topic4"),
+                id="unread_present_after_previous_topic_skipping_muted_topic",
+            ),
+            case(
+                {(2, "topic2"), (2, "muted topic2")},
+                (2, "muted topic2"),
+                (2, "topic2"),
+                id="unread_present_after_previous_topic_muted",
+            ),
         ],
     )
     def test_get_next_unread_topic(
@@ -3249,6 +3309,22 @@ def test_get_next_unread_topic(
         }
         model._last_unread_topic = last_unread_topic
 
+        # Minimal extra streams for muted stream testing (should not exist otherwise)
+        assert {3, 4} & set(model.stream_dict) == set()
+        model.stream_dict[3] = {"name": "Stream 3"}
+        model.stream_dict[4] = {"name": "Stream 4"}
+        model.muted_streams = {3}
+
+        # date data unimportant (if present)
+        model._muted_topics = {
+            stream_topic: None
+            for stream_topic in [
+                ("Stream 2", "muted topic2"),
+                ("Stream 2", "topic2 muted"),
+                ("Stream 3", "topic3 muted"),
+            ]
+        }
+
         unread_topic = model.get_next_unread_topic()
 
         assert unread_topic == next_unread_topic
diff --git a/zulipterminal/model.py b/zulipterminal/model.py
index f4da1f33c9..39f7b8b46a 100644
--- a/zulipterminal/model.py
+++ b/zulipterminal/model.py
@@ -801,16 +801,20 @@ def is_muted_topic(self, stream_id: int, topic: str) -> bool:
     def get_next_unread_topic(self) -> Optional[Tuple[int, str]]:
         topics = sorted(self.unread_counts["unread_topics"].keys())
         next_topic = False
-        for topic in topics:
-            if next_topic is True:
+        if self._last_unread_topic not in topics:
+            next_topic = True
+        # loop over topics list twice for the case that last_unread_topic was
+        # the last valid unread_topic in topics list.
+        for topic in topics * 2:
+            if (
+                not self.is_muted_topic(stream_id=topic[0], topic=topic[1])
+                and not self.is_muted_stream(stream_id=topic[0])
+                and next_topic
+            ):
                 self._last_unread_topic = topic
                 return topic
             if topic == self._last_unread_topic:
                 next_topic = True
-        if len(topics) > 0:
-            topic = topics[0]
-            self._last_unread_topic = topic
-            return topic
         return None
 
     def _fetch_initial_data(self) -> None: