Skip to content

Commit

Permalink
boxes: Disable cycling using arrow keys if recipient is invalid.
Browse files Browse the repository at this point in the history 
This commit diables cycling from the recipient box using arrow keys
if recipient is invalid, similar to Tab. Cycling through the
MessageBox now looks for arrow key events(right and down mainly)
and checks for the recipient name's validity as well. This should
prevent sending of messages to incorrect recipients.
Currently, only RIGHT and DOWN keypress is monitored since these
are the only keys that can cycle focus from recipients box to
some other box, according to our current MessageBox layout.

Tests amended.
Fixes #779.
  • Loading branch information
@zee-bit
zee-bit committed Jan 25, 2021
1 parent fb3c5b5 commit dd23c0d
Show file tree
Hide file tree
Showing 2 changed files with 174 additions and 15 deletions.
117 changes: 117 additions & 0 deletions tests/ui_tools/test_boxes.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,123 @@ def test_keypress_typeahead_mode_autocomplete_key(self, mocker, write_box,
else:
self.view.set_footer_text.assert_not_called()

@pytest.mark.parametrize(["initial_focus_name",
"initial_focus_col_name",
"box_type",
"msg_body_edit_enabled",
"message_being_edited",
"expected_focus_name",
"expected_focus_col_name"], [
('CONTAINER_HEADER', 'HEADER_BOX_STREAM', "stream", True, False,
'CONTAINER_HEADER', 'HEADER_BOX_TOPIC'),
('CONTAINER_HEADER', 'HEADER_BOX_TOPIC', "stream", False, True,
'CONTAINER_HEADER', 'HEADER_BOX_EDIT'),
('CONTAINER_HEADER', 'HEADER_BOX_EDIT', "stream", False, True,
'CONTAINER_HEADER', 'HEADER_BOX_EDIT'),
('CONTAINER_HEADER', 'HEADER_BOX_STREAM', "stream", True, True,
'CONTAINER_HEADER', 'HEADER_BOX_TOPIC'),
('CONTAINER_HEADER', 'HEADER_BOX_TOPIC', "stream", True, True,
'CONTAINER_HEADER', 'HEADER_BOX_EDIT'),
('CONTAINER_HEADER', 'HEADER_BOX_RECIPIENT', "private", True, False,
'CONTAINER_HEADER', 'HEADER_BOX_RECIPIENT'),
('CONTAINER_MESSAGE', 'MESSAGE_BOX_BODY', "private", True, False,
'CONTAINER_MESSAGE', 'MESSAGE_BOX_BODY'),
], ids=[
'stream_name_to_topic_box',
'topic_edit_only-topic_to_edit_mode_box',
'topic_edit_only-edit_mode_to_edit_box',
'edit_box-stream_name_to_topic_box',
'edit_box-topic_to_edit_mode_box',
'recipient_to_recipient_box',
'recipient_to_recipient_box',
])
@pytest.mark.parametrize("key", ['right'])
def test_keypress_GO_RIGHT(self, write_box, key, initial_focus_name,
expected_focus_name, initial_focus_col_name,
expected_focus_col_name, box_type,
msg_body_edit_enabled, message_being_edited,
widget_size, mocker, stream_id=10):
mocker.patch(BOXES + '.WriteBox._set_stream_write_box_style')
if box_type == "stream":
(write_box.stream_box_edit_view(stream_id) if message_being_edited
else write_box.stream_box_view(stream_id))
else:
write_box.private_box_view()
size = widget_size(write_box)

def focus_val(x: str) -> int:
return getattr(write_box, 'FOCUS_' + x)
write_box.focus_position = focus_val(initial_focus_name)
write_box.header_write_box.focus_col = focus_val(
initial_focus_col_name)
write_box.msg_body_edit_enabled = msg_body_edit_enabled
write_box.keypress(size, key)

assert write_box.focus_position == focus_val(expected_focus_name)
# FIXME: Needs refactoring?
if write_box.focus_position == write_box.FOCUS_CONTAINER_HEADER:
assert (write_box.header_write_box.focus_col
== focus_val(expected_focus_col_name))
else:
assert (write_box.FOCUS_MESSAGE_BOX_BODY
== focus_val(expected_focus_col_name))

@pytest.mark.parametrize(["initial_focus_name",
"initial_focus_col_name",
"box_type",
"msg_body_edit_enabled",
"message_being_edited",
"expected_focus_name",
"expected_focus_col_name"], [
('CONTAINER_HEADER', 'HEADER_BOX_STREAM', "stream", True, True,
'CONTAINER_MESSAGE', 'MESSAGE_BOX_BODY'),
('CONTAINER_HEADER', 'HEADER_BOX_TOPIC', "stream", False, True,
'CONTAINER_HEADER', 'HEADER_BOX_TOPIC'),
('CONTAINER_HEADER', 'HEADER_BOX_EDIT', "stream", False, True,
'CONTAINER_HEADER', 'HEADER_BOX_EDIT'),
('CONTAINER_HEADER', 'HEADER_BOX_RECIPIENT', "private", True, False,
'CONTAINER_MESSAGE', 'MESSAGE_BOX_BODY'),
('CONTAINER_MESSAGE', 'MESSAGE_BOX_BODY', "private", True, True,
'CONTAINER_MESSAGE', 'MESSAGE_BOX_BODY'),
], ids=[
'stream_name_to_message_box',
'topic_to_topic_box',
'topic_edit_only-topic_to_edit_mode_box',
'recipient_to_message_box',
'message_to_message_box',
])
@pytest.mark.parametrize("key", ['down'])
def test_keypress_GO_DOWN(self, write_box, key, initial_focus_name,
expected_focus_name, initial_focus_col_name,
expected_focus_col_name, box_type,
msg_body_edit_enabled, message_being_edited,
widget_size, mocker, stream_id=10):
mocker.patch(BOXES + '.WriteBox._set_stream_write_box_style')
if box_type == "stream":
(write_box.stream_box_edit_view(stream_id) if message_being_edited
else write_box.stream_box_view(stream_id))
else:
write_box.private_box_view()
size = (48, )

def focus_val(x: str) -> int:
return getattr(write_box, 'FOCUS_' + x)
write_box.focus_position = focus_val(initial_focus_name)
write_box.header_write_box.focus_col = focus_val(
initial_focus_col_name)
write_box.msg_body_edit_enabled = msg_body_edit_enabled
write_box.model.get_invalid_recipient_emails.return_value = []
write_box.keypress(size, key)

assert write_box.focus_position == focus_val(expected_focus_name)
# FIXME: Needs refactoring?
if write_box.focus_position == write_box.FOCUS_CONTAINER_HEADER:
assert (write_box.header_write_box.focus_col
== focus_val(expected_focus_col_name))
else:
assert (write_box.FOCUS_MESSAGE_BOX_BODY
== focus_val(expected_focus_col_name))

@pytest.mark.parametrize(["initial_focus_name",
"initial_focus_col_name",
"box_type",
Expand Down
72 changes: 57 additions & 15 deletions zulipterminal/ui_tools/boxes.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,19 @@ def autocomplete_emojis(self, text: str, prefix_string: str

return emoji_typeahead, emojis

def footer_notify_invalid_stream(self) -> None:
invalid_stream_error = (
'Invalid stream name. Use {} or {} to autocomplete.'.format(
primary_key_for_command('AUTOCOMPLETE'),
primary_key_for_command('AUTOCOMPLETE_REVERSE'))
)
self.view.set_footer_text(invalid_stream_error, 3)

def footer_notify_invalid_emails(self, invalid_emails: List[str]) -> None:
invalid_emails_error = ('Invalid recipient(s) - '
+ ', '.join(invalid_emails))
self.view.set_footer_text(invalid_emails_error, 3)

def keypress(self, size: urwid_Size, key: str) -> Optional[str]:
if self.is_in_typeahead_mode:
if not (is_command_key('AUTOCOMPLETE', key)
Expand Down Expand Up @@ -491,6 +504,48 @@ def keypress(self, size: urwid_Size, key: str) -> Optional[str]:
self.model.save_draft(message)
elif message != saved_draft:
self.view.controller.save_draft_confirmation_popup(message)

# Since, there are two keys mapped to GO_RIGHT
# i.e. ['right', 'l'], we dont need 'l' key to satisfy
# the condition: is_command_key("GO_RIGHT", key).
# FIXME: Refactor this condition?
elif key == 'right':
header = self.header_write_box
if self.to_write_box is None:
if (self.focus_position == self.FOCUS_CONTAINER_HEADER
and header.focus_col == self.FOCUS_HEADER_BOX_STREAM):
cursor_pos = header[self.FOCUS_HEADER_BOX_STREAM].edit_pos
stream_name = (header[self.FOCUS_HEADER_BOX_STREAM]
.edit_text)
stream_name_len = len(stream_name)
if stream_name_len == cursor_pos:
if not self.model.is_valid_stream(stream_name):
self.footer_notify_invalid_stream()
return key

# FIXME: Refactor this condition.
elif key == 'down':
if self.focus_position == self.FOCUS_CONTAINER_HEADER:
header = self.header_write_box
if self.to_write_box is None:
if header.focus_col == self.FOCUS_HEADER_BOX_STREAM:
stream_name = (header[self.FOCUS_HEADER_BOX_STREAM]
.edit_text)
if not self.model.is_valid_stream(stream_name):
self.footer_notify_invalid_stream()
return key
else:
recipient_box = header[self.FOCUS_HEADER_BOX_RECIPIENT]
recipient_emails = [email.strip() for email in
recipient_box.edit_text.split(',')]
invalid_emails = self.model.get_invalid_recipient_emails(
recipient_emails)
if invalid_emails:
self.footer_notify_invalid_emails(invalid_emails)
return key
if not self.msg_body_edit_enabled:
return key

elif is_command_key('CYCLE_COMPOSE_FOCUS', key):
if len(self.contents) == 0:
return key
Expand All @@ -502,18 +557,7 @@ def keypress(self, size: urwid_Size, key: str) -> Optional[str]:
stream_name = (header[self.FOCUS_HEADER_BOX_STREAM]
.edit_text)
if not self.model.is_valid_stream(stream_name):
invalid_stream_error = (
'Invalid stream name.'
' Use {} or {} to autocomplete.'
.format(primary_key_for_command(
'AUTOCOMPLETE'
),
primary_key_for_command(
'AUTOCOMPLETE'
'_REVERSE'
))
)
self.view.set_footer_text(invalid_stream_error, 3)
self.footer_notify_invalid_stream()
return key
user_ids = self.model.get_other_subscribers_in_stream(
stream_name=stream_name)
Expand Down Expand Up @@ -543,9 +587,7 @@ def keypress(self, size: urwid_Size, key: str) -> Optional[str]:
invalid_emails = self.model.get_invalid_recipient_emails(
recipient_emails)
if invalid_emails:
invalid_emails_error = ('Invalid recipient(s) - '
+ ', '.join(invalid_emails))
self.view.set_footer_text(invalid_emails_error, 3)
self.footer_notify_invalid_emails(invalid_emails)
return key
users = self.model.user_dict
self.recipient_user_ids = [users[email]['user_id']
Expand Down

0 comments on commit dd23c0d

Please sign in to comment.