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 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 zulip#781.
  • Loading branch information
zee-bit committed Feb 17, 2021
1 parent 306a394 commit 1d2152d
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 6 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
25 changes: 25 additions & 0 deletions zulipterminal/helper.py
Original file line number Diff line number Diff line change
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: Dict[str, Any],
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
18 changes: 16 additions & 2 deletions zulipterminal/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,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 @@ -385,7 +386,10 @@ def send_private_message(self, recipients: List[str],
}
response = self.client.send_message(request)
display_error_if_present(response, self.controller)
return response['result'] == 'success'
result = response['result'] == 'success'
if result is True:
notify_if_message_sent_outside_narrow(request, self.controller)
return result
else:
raise RuntimeError('Empty recipients list.')

Expand All @@ -399,7 +403,10 @@ def send_stream_message(self, stream: str, topic: str,
}
response = self.client.send_message(request)
display_error_if_present(response, self.controller)
return response['result'] == 'success'
result = response['result'] == 'success'
if result is True:
notify_if_message_sent_outside_narrow(request, self.controller)
return result

def update_private_message(self, msg_id: int, content: str) -> bool:
request = {
Expand All @@ -423,6 +430,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 1d2152d

Please sign in to comment.