Skip to content

model: Notify user if a request fails. #714

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

model: Notify user if a request fails. #714

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
33c2be3
tests: model: Remove redundant patch in star status test.
kaustubh-nair Jul 14, 2020
3460187
tests: model: refactor: Use a variable for client response.
kaustubh-nair Jul 14, 2020
f1e98f5
model: Notify user if a request fails.
kaustubh-nair Jul 14, 2020
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
20 changes: 18 additions & 2 deletions tests/helper/test_helper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

import zulipterminal.helper
from zulipterminal.helper import (
canonicalize_color, classify_unread_counts, index_messages, notify,
powerset,
canonicalize_color, classify_unread_counts, display_error_if_present,
index_messages, notify, powerset,
)


Expand Down Expand Up @@ -280,3 +280,19 @@ def test_notify_quotes(monkeypatch, mocker,
assert len(params[0][0][0]) == cmd_length

# NOTE: If there is a quoting error, we may get a ValueError too


@pytest.mark.parametrize(['response', 'footer_updated'], [
({'result': 'error', 'msg': 'Request failed.'}, True),
({'result': 'success', 'msg': 'msg content'}, False),
])
def test_display_error_if_present(mocker, response, footer_updated):
controller = mocker.Mock()
set_footer_text = controller.view.set_footer_text

display_error_if_present(response, controller)

if footer_updated:
set_footer_text.assert_called_once_with(response['msg'], 3)
else:
set_footer_text.assert_not_called()
34 changes: 28 additions & 6 deletions tests/model/test_model.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ def mock_external_classes(self, mocker: Any) -> None:
'Controller.client')
self.client.base_url = 'chat.zulip.zulip'
mocker.patch('zulipterminal.model.Model._start_presence_updates')
self.display_error_if_present = mocker.patch(
'zulipterminal.model.display_error_if_present')

@pytest.fixture
def model(self, mocker, initial_data, user_profile):
Expand Down Expand Up @@ -322,6 +324,9 @@ def test_get_topics_in_streams(self, mocker, response, model, return_value,
self.client.get_stream_topics.assert_called_once_with(23)
assert model.index['topics'] == expected_index
assert result == return_value
if response['result'] != 'success':
(self.display_error_if_present.
assert_called_once_with(response, self.controller))

@pytest.mark.parametrize("user_key", ['user_id', 'id'])
@pytest.mark.parametrize("msg_id, existing_reactions, expected_method", [
Expand All @@ -331,7 +336,7 @@ def test_get_topics_in_streams(self, mocker, response, model, return_value,
(5, [dict(user='me', emoji_code='1f614')], 'POST'),
(5, [dict(user='not me', emoji_code='1f614')], 'POST'),
])
def test_react_to_message_with_thumbs_up(self, model,
def test_react_to_message_with_thumbs_up(self, mocker, model,
user_key,
msg_id,
existing_reactions,
Expand All @@ -349,6 +354,9 @@ def test_react_to_message_with_thumbs_up(self, model,
reaction_type='unicode_emoji',
emoji_code='1f44d',
message_id=str(msg_id))
response = mocker.Mock()
model.client.add_reaction.return_value = response
model.client.remove_reaction.return_value = response

model.react_to_message(message, 'thumbs_up')

Expand All @@ -358,6 +366,8 @@ def test_react_to_message_with_thumbs_up(self, model,
elif expected_method == 'DELETE':
model.client.remove_reaction.assert_called_once_with(reaction_spec)
model.client.add_reaction.assert_not_called()
self.display_error_if_present.assert_called_once_with(response,
self.controller)

def test_react_to_message_for_not_thumbs_up(self, model):
with pytest.raises(AssertionError):
Expand All @@ -379,6 +389,8 @@ def test_send_private_message(self, mocker, model,
self.client.send_message.assert_called_once_with(req)

assert result == return_value
self.display_error_if_present.assert_called_once_with(response,
self.controller)

@pytest.mark.parametrize('response, return_value', [
({'result': 'success'}, True),
Expand All @@ -396,6 +408,8 @@ def test_send_stream_message(self, mocker, model,
self.client.send_message.assert_called_once_with(req)

assert result == return_value
self.display_error_if_present.assert_called_once_with(response,
self.controller)

@pytest.mark.parametrize('response, return_value', [
({'result': 'success'}, True),
Expand All @@ -413,6 +427,8 @@ def test_update_private_message(self, mocker, model,
self.client.update_message.assert_called_once_with(req)

assert result == return_value
self.display_error_if_present.assert_called_once_with(response,
self.controller)

@pytest.mark.parametrize('response, return_value', [
({'result': 'success'}, True),
Expand All @@ -432,6 +448,8 @@ def test_update_stream_message(self, mocker, model,
self.client.update_message.assert_called_once_with(req)

assert result == return_value
self.display_error_if_present.assert_called_once_with(response,
self.controller)

# NOTE: This tests only getting next-unread, not a fixed anchor
def test_success_get_messages(self, mocker, messages_successful_response,
Expand Down Expand Up @@ -535,11 +553,10 @@ def test_fail_get_messages(self, mocker, error_response,
], ids=['muting_205', 'unmuting_205', 'first_muted_205',
'last_unmuted_205'])
def test_toggle_stream_muted_status(self, mocker, model,
initial_muted_streams, value):
initial_muted_streams, value,
response={'result': 'success'}):
Copy link
Member

Choose a reason for hiding this comment

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

Is this to overcome pytest's caching issue or just to reuse the dict?

Copy link
Member Author

Choose a reason for hiding this comment

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

Just for reuse

model.muted_streams = initial_muted_streams
model.client.update_subscription_settings.return_value = {
'result': "success"
}
model.client.update_subscription_settings.return_value = response
model.toggle_stream_muted_status(205)
request = [{
'stream_id': 205,
Expand All @@ -548,6 +565,8 @@ def test_toggle_stream_muted_status(self, mocker, model,
}]
(model.client.update_subscription_settings
.assert_called_once_with(request))
self.display_error_if_present.assert_called_once_with(response,
self.controller)

@pytest.mark.parametrize('flags_before, expected_operator', [
([], 'add'),
Expand All @@ -558,7 +577,8 @@ def test_toggle_stream_muted_status(self, mocker, model,
])
def test_toggle_message_star_status(self, mocker, model, flags_before,
expected_operator):
mocker.patch('zulip.Client')
response = mocker.Mock()
model.client.update_message_flags.return_value = response
message = {
'id': 99,
'flags': flags_before,
Expand All @@ -571,6 +591,8 @@ def test_toggle_message_star_status(self, mocker, model, flags_before,
'op': expected_operator
}
model.client.update_message_flags.assert_called_once_with(request)
self.display_error_if_present.assert_called_once_with(response,
self.controller)

def test_mark_message_ids_as_read(self, model, mocker: Any) -> None:
mock_api_query = mocker.patch('zulipterminal.core.Controller'
Expand Down
6 changes: 6 additions & 0 deletions zulipterminal/helper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -630,3 +630,9 @@ def notify(title: str, html_text: str) -> str:
# This likely means the notification command could not be found
return command_list[0]
return ""


def display_error_if_present(response: Dict[str, Any], controller: Any
) -> None:
if response['result'] == 'error':
controller.view.set_footer_text(response['msg'], 3)
14 changes: 12 additions & 2 deletions zulipterminal/model.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from zulipterminal.config.keys import keys_for_command
from zulipterminal.helper import (
Message, asynch, canonicalize_color, classify_unread_counts,
index_messages, initial_index, notify, set_count,
display_error_if_present, index_messages, initial_index, notify, set_count,
)
from zulipterminal.ui_tools.utils import create_msg_box_list

Expand Down Expand Up @@ -269,6 +269,7 @@ def react_to_message(self,
response = self.client.remove_reaction(reaction_to_toggle_spec)
else:
response = self.client.add_reaction(reaction_to_toggle_spec)
display_error_if_present(response, self.controller)

@asynch
def toggle_message_star_status(self, message: Message) -> None:
Expand All @@ -278,16 +279,18 @@ def toggle_message_star_status(self, message: Message) -> None:
else:
request = dict(base_request, op='add')
response = self.client.update_message_flags(request)
display_error_if_present(response, self.controller)

@asynch
def mark_message_ids_as_read(self, id_list: List[int]) -> None:
if not id_list:
return
self.client.update_message_flags({
response = self.client.update_message_flags({
'messages': id_list,
'flag': 'read',
'op': 'add',
})
display_error_if_present(response, self.controller)

def send_private_message(self, recipients: str,
content: str) -> bool:
Expand All @@ -297,6 +300,7 @@ def send_private_message(self, recipients: str,
'content': content,
}
response = self.client.send_message(request)
display_error_if_present(response, self.controller)
return response['result'] == 'success'

def send_stream_message(self, stream: str, topic: str,
Expand All @@ -308,6 +312,7 @@ def send_stream_message(self, stream: str, topic: str,
'content': content,
}
response = self.client.send_message(request)
display_error_if_present(response, self.controller)
return response['result'] == 'success'

def update_private_message(self, msg_id: int, content: str) -> bool:
Expand All @@ -316,6 +321,7 @@ def update_private_message(self, msg_id: int, content: str) -> bool:
"content": content,
}
response = self.client.update_message(request)
display_error_if_present(response, self.controller)
return response['result'] == 'success'

def update_stream_message(self, topic: str, msg_id: int,
Expand All @@ -328,6 +334,7 @@ def update_stream_message(self, topic: str, msg_id: int,
"subject": topic,
}
response = self.client.update_message(request)
display_error_if_present(response, self.controller)
return response['result'] == 'success'

def get_messages(self, *,
Expand Down Expand Up @@ -359,6 +366,7 @@ def get_messages(self, *,
query_range = num_after + num_before + 1
self.found_newest = len(response['messages']) < query_range
return ""
display_error_if_present(response, self.controller)
return response['msg']

def get_topics_in_stream(self, stream_list: Iterable[int]) -> str:
Expand All @@ -373,6 +381,7 @@ def get_topics_in_stream(self, stream_list: Iterable[int]) -> str:
self.index['topics'][stream_id] = [topic['name'] for
topic in response['topics']]
else:
display_error_if_present(response, self.controller)
return response['msg']
return ""

Expand Down Expand Up @@ -609,6 +618,7 @@ def toggle_stream_muted_status(self, stream_id: int) -> bool:
# True for muting and False for unmuting.
}]
response = self.client.update_subscription_settings(request)
display_error_if_present(response, self.controller)
return response['result'] == 'success'

def _handle_subscription_event(self, event: Event) -> None:
Expand Down