Skip to content

Commit

Permalink
model: Skip muted streams and topics in getting next unread topic.
Browse files Browse the repository at this point in the history
Logic originally developed by Aman Agrawal (amanagr) in zulip#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>
  • Loading branch information
neiljp committed Aug 16, 2022
1 parent 3e793e9 commit 39f13a3
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 7 deletions.
78 changes: 77 additions & 1 deletion tests/model/test_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
{},
Expand All @@ -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(
Expand All @@ -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
Expand Down
16 changes: 10 additions & 6 deletions zulipterminal/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down

0 comments on commit 39f13a3

Please sign in to comment.