Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip muted_streams/topics in getting next unread topic. #442

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 32 additions & 2 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import pytest

from zulipterminal.model import Model
from zulipterminal.ui_tools.boxes import MessageBox
from zulipterminal.ui_tools.buttons import (
StreamButton,
Expand Down Expand Up @@ -32,15 +33,17 @@ def no_asynch(mocker):


@pytest.fixture
def stream_button(mocker):
def stream_button(mocker, model_fixture):
"""
Mocked stream button.
"""
view_mock = mocker.Mock()
view_mock.palette = [(None, 'black', 'white')]
controller = mocker.patch('zulipterminal.core.Controller')
controller.model = model_fixture
button = StreamButton(
properties=['PTEST', 205, '#bfd56f', False],
controller=mocker.patch('zulipterminal.core.Controller'),
controller=controller,
width=40,
view=view_mock,
count=30
Expand Down Expand Up @@ -79,6 +82,33 @@ def msg_box(mocker, messages_successful_response):

# --------------- Model Fixtures ----------------------------------------------

@pytest.fixture
def model_fixture(mocker, initial_data, user_profile):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems great at first glance as another standardized fixture, but while you use it in the following commit, you could illustrate its broader use and appeal in this commit by refactoring some other tests using this new fixture.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

mocker.patch('urllib.parse.urlparse')
controller = mocker.patch('zulipterminal.core.'
'Controller',
return_value=None)
client = mocker.patch('zulipterminal.core.'
'Controller.client')
client.base_url = 'chat.zulip.zulip'
mocker.patch('zulipterminal.model.Model._start_presence_updates')
mocker.patch('zulipterminal.model.Model.get_messages')
client.register.return_value = initial_data
mocker.patch('zulipterminal.model.Model.get_all_users',
return_value=[])
mocker.patch('zulipterminal.model.Model.'
'_stream_info_from_subscriptions',
return_value=({}, set(), [], []))
classify_unread_counts = mocker.patch(
'zulipterminal.model.classify_unread_counts',
return_value=[])
client.get_profile.return_value = user_profile
model = Model(controller)
model.muted_streams = []
model.muted_topics = []
return model


@pytest.fixture
def users_fixture(logged_on_user):
users = [logged_on_user]
Expand Down
25 changes: 25 additions & 0 deletions tests/model/test_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -1188,3 +1188,28 @@ def test_update_subscription(self, model, mocker, event, stream_button,
else:
mark_muted.assert_called_once_with()
model.controller.update_screen.assert_called_once_with()

@pytest.mark.parametrize('stream_id, is_muted', [
(1, True), # muted stream
(2, False), # unmuted stream
])
def test_is_muted_stream(self, stream_id, is_muted, stream_dict, model):
model.stream_dict = stream_dict
model.muted_streams = [1]
assert model.is_muted_stream(stream_id) == is_muted

@pytest.mark.parametrize('topic, is_muted', [
((1, 'stream muted & unmuted topic'), True),
((2, 'muted topic'), True),
((1, 'muted stream muted topic'), True),
((2, 'unmuted topic'), False),
])
def test_is_muted_topic(self, topic, is_muted, stream_dict, model):
model.stream_dict = stream_dict
model.muted_streams = [1]
model.muted_topics = [
('Stream 2', 'muted topic'),
('Stream 1', 'muted stream muted topic'),
]
assert model.is_muted_topic(stream_id=topic[0],
topic=topic[1]) == is_muted
79 changes: 54 additions & 25 deletions tests/ui/test_ui_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -593,30 +593,58 @@ def test_init(self, mid_col_view):
self.super.assert_called_once_with("MSG_LIST", header=self.search_box,
footer=self.write_box)

def test_get_next_unread_topic(self, mid_col_view):
mid_col_view.model.unread_counts = {
'unread_topics': {1: 1, 2: 1}
}
return_value = mid_col_view.get_next_unread_topic()
assert return_value == 1
assert mid_col_view.last_unread_topic == 1

def test_get_next_unread_topic_again(self, mid_col_view):
mid_col_view.model.unread_counts = {
'unread_topics': {1: 1, 2: 1}
}
mid_col_view.last_unread_topic = 1
return_value = mid_col_view.get_next_unread_topic()
assert return_value == 2
assert mid_col_view.last_unread_topic == 2

def test_get_next_unread_topic_no_unread(self, mid_col_view):
mid_col_view.model.unread_counts = {
'unread_topics': {}
@pytest.mark.parametrize('last_unread_topic, next_unread_topic', [
((1, 'stream muted & unmuted topic'), (2, 'unmuted topic 1')),
((2, 'unmuted topic 1'), (2, 'unmuted topic 2')),
((2, 'unmuted topic 2'), (2, 'unmuted topic 3')),
((2, 'unmuted topic 3'), (2, 'unmuted topic 1')),
])
def test_get_next_unread_topic(self, mid_col_view, last_unread_topic,
next_unread_topic, stream_dict,
model_fixture):
mid_col_view.model = model_fixture
mid_col_view.model.unread_counts = dict()
unread_topics = OrderedDict()
unread_topics[(1, 'stream muted & unmuted topic')] = 1,
unread_topics[(1, 'muted stream muted topic')] = 1,
unread_topics[(2, 'muted topic')] = 1,
unread_topics[(2, 'unmuted topic 1')] = 1,
unread_topics[(2, 'unmuted topic 2')] = 1,
unread_topics[(2, 'unmuted topic 3')] = 1,
mid_col_view.model.unread_counts['unread_topics'] = unread_topics
mid_col_view.model.stream_dict = stream_dict
mid_col_view.model.muted_streams = [1]
mid_col_view.model.muted_topics = [
('Stream 2', 'muted topic'),
('Stream 1', 'muted stream muted topic'),
]
mid_col_view.last_unread_topic = last_unread_topic
assert mid_col_view.get_next_unread_topic() == next_unread_topic

@pytest.mark.parametrize('last_unread_topic, next_unread_topic', [
((1, 'stream muted & unmuted topic'), None),
((2, 'muted topic'), None),
(None, None),
])
def test_get_next_unread_topic_no_unread(self, mid_col_view,
stream_dict,
last_unread_topic,
next_unread_topic,
model_fixture):
mid_col_view.model = model_fixture
mid_col_view.model.unread_counts = dict()
mid_col_view.model.unread_counts['unread_topics'] = {
(1, 'stream muted & unmuted topic'): 1,
(1, 'muted stream muted topic'): 1,
(2, 'muted topic'): 1,
}
return_value = mid_col_view.get_next_unread_topic()
assert return_value is None
assert mid_col_view.last_unread_topic is None
mid_col_view.model.stream_dict = stream_dict
mid_col_view.model.muted_streams = [1]
mid_col_view.model.muted_topics = [
('Stream 2', 'muted topic'),
('Stream 1', 'muted stream muted topic'),
]
assert mid_col_view.get_next_unread_topic() == next_unread_topic

def test_get_next_unread_pm(self, mid_col_view):
mid_col_view.model.unread_counts = {
Expand Down Expand Up @@ -1742,11 +1770,12 @@ class TestStreamButton:
(25, 199, 'caption'),
(25, 1999, 'caption'),
])
def test_text_content(self, mocker,
def test_text_content(self, mocker, model_fixture,
is_private, expected_prefix,
width, count, short_text, caption='caption'):
mocker.patch(STREAMBUTTON + ".mark_muted")
controller = mocker.Mock()
controller.model = model_fixture
controller.model.muted_streams = {}
properties = [caption, 5, '#ffffff', is_private]
view_mock = mocker.Mock()
Expand Down Expand Up @@ -1778,7 +1807,7 @@ def test_text_content(self, mocker,
])
def test_mark_stream_muted(self, mocker, stream_button, is_action_muting,
stream_id, muted_streams, called_value,
updated_all_msgs) -> None:
updated_all_msgs, model_fixture) -> None:
stream_button.stream_id = stream_id
update_count = mocker.patch(TOPBUTTON + ".update_count")
stream_button.controller.model.unread_counts = {
Expand Down
9 changes: 5 additions & 4 deletions tests/ui/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,9 @@
False
),
])
def test_is_muted(mocker, msg, narrow, muted_streams, muted_topics, muted):
model = mocker.Mock()
def test_is_muted(mocker, msg, narrow, muted_streams, muted_topics, muted,
model_fixture):
model = model_fixture
model.narrow = narrow
model.muted_streams = muted_streams
model.muted_topics = muted_topics
Expand Down Expand Up @@ -136,8 +137,8 @@ def test_is_muted(mocker, msg, narrow, muted_streams, muted_topics, muted):

])
def test_create_msg_box_list(mocker, narrow, messages, focus_msg_id,
muted, unsubscribed, len_w_list):
model = mocker.Mock()
muted, unsubscribed, len_w_list, model_fixture):
model = model_fixture
model.narrow = narrow
model.index = {
'all_msg_ids': {1, 2},
Expand Down
4 changes: 2 additions & 2 deletions zulipterminal/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,8 @@ def search_messages(self, text: str) -> None:
self.model.msg_list.set_focus(focus_position)

def stream_muting_confirmation_popup(self, button: Any) -> None:
type_of_action = "unmuting" if button.stream_id in \
self.model.muted_streams else "muting"
type_of_action = "unmuting" if self.model.is_muted_stream(
button.stream_id) else "muting"
question = urwid.Text(("bold", "Confirm " + type_of_action +
" of stream '" + button.stream_name+"' ?"),
"center")
Expand Down
2 changes: 1 addition & 1 deletion zulipterminal/helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ def set_count(id_list: List[int], controller: Any, new_count: int) -> None:
if msg_type == 'stream':
stream_id = messages[id]['stream_id']
msg_topic = messages[id]['subject']
if stream_id in controller.model.muted_streams:
if controller.model.is_muted_stream(stream_id):
add_to_counts = False # if muted, don't add to eg. all_msg
else:
for stream_button in stream_buttons_log:
Expand Down
15 changes: 14 additions & 1 deletion zulipterminal/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,19 @@ def fetch_all_topics(self, workers: int) -> None:
if not result]
raise ServerConnectionFailure(", ".join(failures))

def is_muted_stream(self, stream_id: int) -> bool:
if stream_id in self.muted_streams:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be just return stream_id in self.muted_streams.

return True
return False

def is_muted_topic(self, stream_id: int, topic: str) -> bool:
if stream_id in self.muted_streams:
return True
stream_name = self.stream_dict[stream_id]['name']
if (stream_name, topic) in self.muted_topics:
return True
return False

def _update_initial_data(self) -> None:
# Thread Processes to reduce start time.
# NOTE: Exceptions do not work well with threads
Expand Down Expand Up @@ -571,7 +584,7 @@ def toggle_stream_muted_status(self, stream_id: int) -> bool:
request = [{
'stream_id': stream_id,
'property': 'is_muted',
'value': stream_id not in self.muted_streams
'value': not self.is_muted_stream(stream_id)
# True for muting and False for unmuting.
}]
response = self.client.update_subscription_settings(request)
Expand Down
2 changes: 1 addition & 1 deletion zulipterminal/ui_tools/buttons.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ def __init__(self, properties: List[Any],
count=count)

# Mark muted streams 'M' during button creation.
if self.stream_id in self.model.muted_streams:
if self.model.is_muted_stream(self.stream_id):
self.mark_muted()

def mark_muted(self) -> None:
Expand Down
2 changes: 1 addition & 1 deletion zulipterminal/ui_tools/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def is_muted(msg: Dict[Any, Any], model: Any) -> bool:
# In a topic narrow
elif len(model.narrow) == 2:
return False
elif msg['stream_id'] in model.muted_streams:
elif model.is_muted_stream(msg['stream_id']):
return True
elif [msg['display_recipient'], msg['subject']] in model.muted_topics:
return True
Expand Down
14 changes: 8 additions & 6 deletions zulipterminal/ui_tools/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -399,16 +399,18 @@ def __init__(self, view: Any, model: Any,
def get_next_unread_topic(self) -> Optional[Tuple[int, str]]:
topics = list(self.model.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
amanagr marked this conversation as resolved.
Show resolved Hide resolved
# 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:
amanagr marked this conversation as resolved.
Show resolved Hide resolved
if not self.model.is_muted_topic(stream_id=topic[0],
topic=topic[1]) 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 get_next_unread_pm(self) -> Optional[int]:
Expand Down