From 3260e3a1d004cf9462873f92fdbe2b19ae9cd6e1 Mon Sep 17 00:00:00 2001 From: zee-bit Date: Mon, 18 Jan 2021 00:44:45 +0530 Subject: [PATCH] helper/model: Add footer notification for message sent outside narrow. This commit generalizes the code for footer notification when a message is sent/edited outside the current narrow. This will give the user a pointer as to why the message disappeared from the screen. The code is hooked to the part where message request is sent to the server, hence preventing the extra check for user_id being same as message_sender_id. Tests amended. Fixes #781. --- tests/helper/test_helper.py | 36 ++++++++++++++++++++++++++++++++++++ tests/model/test_model.py | 32 ++++++++++++++++++++++++++++---- zulipterminal/helper.py | 27 ++++++++++++++++++++++++++- zulipterminal/model.py | 20 ++++++++++++++++++-- 4 files changed, 108 insertions(+), 7 deletions(-) diff --git a/tests/helper/test_helper.py b/tests/helper/test_helper.py index 3dc7d1fe1e..bf2f408b8c 100644 --- a/tests/helper/test_helper.py +++ b/tests/helper/test_helper.py @@ -9,6 +9,7 @@ hash_util_decode, index_messages, notify, + notify_if_message_sent_outside_narrow, powerset, ) @@ -309,6 +310,41 @@ def test_display_error_if_present(mocker, response, footer_updated): set_footer_text.assert_not_called() +@pytest.mark.parametrize(['req', 'narrow', 'footer_updated'], [ + ({'type': 'private', 'to': 'foo@gmail.com', 'content': 'bar'}, + [['is', 'private']], False), + ({'type': 'private', 'to': 'user@abc.com, user@chat.com', 'content': 'Hi'}, + [['pm_with', 'user@0abc.com']], True), + ({'type': 'private', 'to': 'bar-bar@foo.com', 'content': ':party_parrot:'}, + [['pm_with', 'user@abc.com, user@chat.com, bar-bar@foo.com']], True), + ({'type': 'stream', 'to': 'ZT', 'subject': '1', 'content': 'foo'}, + [['stream', 'ZT'], ['topic', '1']], False), + ({'type': 'stream', 'to': 'here', 'subject': 'pytest', 'content': 'py'}, + [['stream', 'test here']], True), + ({'type': 'stream', 'to': '|new_stream|', 'subject': '(no topic)', + 'content': 'Hi `|new_stream|`'}, [], False), + ({'type': 'stream', 'to': 'zulip-terminal', 'subject': 'issue#T781', + 'content': 'Added tests'}, [['is', 'starred']], True), + ({'type': 'private', 'to': '2@aBd%8@random.com', 'content': 'fist_bump'}, + [['is', 'mentioned']], True), + ({'type': 'stream', 'to': 'PTEST', 'subject': 'TEST', 'content': 'Test'}, + [['stream', 'PTEST'], ['search', 'FOO']], True) +]) +def test_notify_if_message_sent_outside_narrow(mocker, req, narrow, + footer_updated): + controller = mocker.Mock() + set_footer_text = controller.view.set_footer_text + controller.model.narrow = narrow + + notify_if_message_sent_outside_narrow(req, controller) + + if footer_updated: + set_footer_text.assert_called_once_with( + 'Message is sent outside of current narrow.', 3) + else: + set_footer_text.assert_not_called() + + @pytest.mark.parametrize('quoted_string, expected_unquoted_string', [ ('(no.20topic)', '(no topic)'), ('.3Cstrong.3Exss.3C.2Fstrong.3E', 'xss'), diff --git a/tests/model/test_model.py b/tests/model/test_model.py index 9606205bd5..70ae893203 100644 --- a/tests/model/test_model.py +++ b/tests/model/test_model.py @@ -23,6 +23,8 @@ def mock_external_classes(self, mocker: Any) -> None: mocker.patch('zulipterminal.model.Model._start_presence_updates') self.display_error_if_present = mocker.patch( 'zulipterminal.model.display_error_if_present') + self.notify_if_message_sent_outside_narrow = mocker.patch( + 'zulipterminal.model.notify_if_message_sent_outside_narrow') @pytest.fixture def model(self, mocker, initial_data, user_profile, @@ -481,6 +483,9 @@ def test_send_private_message(self, mocker, model, assert result == return_value self.display_error_if_present.assert_called_once_with(response, self.controller) + if result == 'success': + self.notify_if_message_sent_outside_narrow.assert_called_once_with( + req, self.controller) def test_send_private_message_with_no_recipients(self, model, content="hi!", @@ -507,6 +512,10 @@ def test_send_stream_message(self, mocker, model, self.display_error_if_present.assert_called_once_with(response, self.controller) + if result == 'success': + self.notify_if_message_sent_outside_narrow.assert_called_once_with( + req, self.controller) + @pytest.mark.parametrize('response, return_value', [ ({'result': 'success'}, True), ({'result': 'some_failure'}, False), @@ -530,16 +539,25 @@ def test_update_private_message(self, mocker, model, ({'result': 'success'}, True), ({'result': 'some_failure'}, False), ]) - @pytest.mark.parametrize('req', [ + @pytest.mark.parametrize('req, old_topic, footer_updated', [ + ({'message_id': 1, 'propagate_mode': 'change_one', + 'content': 'hi!', 'topic': 'Some topic'}, 'Some topic', False), ({'message_id': 1, 'propagate_mode': 'change_one', - 'content': 'hi!', 'topic': 'Some topic'}), + 'topic': 'Topic change'}, 'Old topic', True), + ({'message_id': 1, 'propagate_mode': 'change_all', + 'topic': 'Old topic'}, 'Old topic', False), + ({'message_id': 1, 'propagate_mode': 'change_later', + 'content': ':smile:', 'topic': 'terminal'}, 'terminal', False), ({'message_id': 1, 'propagate_mode': 'change_one', - 'topic': 'Topic change'}), + 'content': 'Hey!', 'topic': 'grett'}, 'greet', True), + ({'message_id': 1, 'propagate_mode': 'change_all', + 'content': 'Lets party!', 'topic': 'party'}, 'lets_party', True), ]) def test_update_stream_message(self, mocker, model, response, return_value, - req): + req, old_topic, footer_updated): self.client.update_message = mocker.Mock(return_value=response) + model.index['messages'][req['message_id']]['subject'] = old_topic result = model.update_stream_message(**req) @@ -547,6 +565,12 @@ def test_update_stream_message(self, mocker, model, assert result == return_value self.display_error_if_present.assert_called_once_with(response, self.controller) + set_footer_text = model.controller.view.set_footer_text + if result and footer_updated: + set_footer_text.assert_called_once_with( + "You changed a message's topic.", 3) + else: + set_footer_text.assert_not_called() # NOTE: This tests only getting next-unread, not a fixed anchor def test_success_get_messages(self, mocker, messages_successful_response, diff --git a/zulipterminal/helper.py b/zulipterminal/helper.py index 0ad98fcada..821cc63b3f 100644 --- a/zulipterminal/helper.py +++ b/zulipterminal/helper.py @@ -25,7 +25,7 @@ import lxml.html from typing_extensions import Literal, TypedDict -from zulipterminal.api_types import Message +from zulipterminal.api_types import Composition, Message MACOS = platform.system() == "Darwin" @@ -651,6 +651,31 @@ def display_error_if_present(response: Dict[str, Any], controller: Any controller.view.set_footer_text(response['msg'], 3) +def check_narrow_and_notify(outer_narrow: List[Any], + inner_narrow: List[Any], + controller: Any) -> None: + current_narrow = controller.model.narrow + + if (current_narrow != [] and current_narrow != outer_narrow + and current_narrow != inner_narrow): + controller.view.set_footer_text( + 'Message is sent outside of current narrow.', 3) + + +def notify_if_message_sent_outside_narrow(message: Composition, + controller: Any) -> None: + current_narrow = controller.model.narrow + + if message['type'] == 'stream': + stream_narrow = [['stream', message['to']]] + topic_narrow = stream_narrow + [['topic', message['subject']]] + check_narrow_and_notify(stream_narrow, topic_narrow, controller) + elif message['type'] == 'private': + pm_narrow = [['is', 'private']] + pm_with_narrow = [['pm_with', ",".join(message['to'])]] + check_narrow_and_notify(pm_narrow, pm_with_narrow, controller) + + def hash_util_decode(string: str) -> str: """ Returns a decoded string given a hash_util_encode() [present in diff --git a/zulipterminal/model.py b/zulipterminal/model.py index 55d93cf163..ce2a66b6e8 100644 --- a/zulipterminal/model.py +++ b/zulipterminal/model.py @@ -43,6 +43,7 @@ index_messages, initial_index, notify, + notify_if_message_sent_outside_narrow, set_count, ) from zulipterminal.ui_tools.utils import create_msg_box_list @@ -391,7 +392,12 @@ def send_private_message(self, recipients: List[str], ) response = self.client.send_message(composition) display_error_if_present(response, self.controller) - return response['result'] == 'success' + message_was_sent = response['result'] == 'success' + if message_was_sent: + notify_if_message_sent_outside_narrow( + composition, self.controller, + ) + return message_was_sent else: raise RuntimeError('Empty recipients list.') @@ -405,7 +411,10 @@ def send_stream_message(self, stream: str, topic: str, ) response = self.client.send_message(composition) display_error_if_present(response, self.controller) - return response['result'] == 'success' + message_was_sent = response['result'] == 'success' + if message_was_sent: + notify_if_message_sent_outside_narrow(composition, self.controller) + return message_was_sent def update_private_message(self, msg_id: int, content: str) -> bool: request = { @@ -429,6 +438,13 @@ def update_stream_message(self, topic: str, message_id: int, response = self.client.update_message(request) display_error_if_present(response, self.controller) + if response['result'] == 'success': + old_topic = self.index['messages'][message_id].get('subject', None) + new_topic = request['topic'] + view = self.controller.view + if old_topic != new_topic: + view.set_footer_text("You changed a message's topic.", 3) + return response['result'] == 'success' def fetch_custom_emojis(self) -> NamedEmojiData: