Skip to content

Commit

Permalink
helper/model: Add footer notification for message sent outside narrow.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
zee-bit authored and neiljp committed Feb 18, 2021
1 parent 8c27df5 commit 3260e3a
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 7 deletions.
36 changes: 36 additions & 0 deletions tests/helper/test_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
hash_util_decode,
index_messages,
notify,
notify_if_message_sent_outside_narrow,
powerset,
)

Expand Down Expand Up @@ -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', '<strong>xss</strong>'),
Expand Down
32 changes: 28 additions & 4 deletions tests/model/test_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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!",
Expand All @@ -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),
Expand All @@ -530,23 +539,38 @@ 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)

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)
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,
Expand Down
27 changes: 26 additions & 1 deletion zulipterminal/helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
20 changes: 18 additions & 2 deletions zulipterminal/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.')

Expand All @@ -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 = {
Expand All @@ -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:
Expand Down

0 comments on commit 3260e3a

Please sign in to comment.