Skip to content

Commit 031f42e

Browse files
committed
refactor: buttons: Validate PM data prior to narrowing.
This commit adds validation helper for PM data, in an attempt to avoid and handle any potential exceptions before starting to switch to the PM narrow. The validation checks if all the recipients are valid private recpients or not, and whether the PM type is valid. Tests added.
1 parent d19365b commit 031f42e

File tree

2 files changed

+121
-0
lines changed

2 files changed

+121
-0
lines changed

tests/ui_tools/test_buttons.py

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -984,6 +984,96 @@ def test__patch_stream_data(
984984

985985
assert parsed_link == expected_parsed_link
986986

987+
@pytest.mark.parametrize(
988+
[
989+
"parsed_link",
990+
"is_valid_private_recipient",
991+
"expected_error",
992+
],
993+
[
994+
case(
995+
ParsedNarrowLink(
996+
pm_with=DecodedPM(
997+
type=Literal["pm"],
998+
recipient_ids=[1001, 11],
999+
recipient_emails=["[email protected]", "[email protected]"],
1000+
),
1001+
), # ...
1002+
True,
1003+
"",
1004+
id="valid_pm_data",
1005+
),
1006+
case(
1007+
ParsedNarrowLink(
1008+
pm_with=DecodedPM(
1009+
type=Literal["pm"],
1010+
recipient_ids=[1001, 11, 12],
1011+
recipient_emails=[
1012+
1013+
1014+
1015+
],
1016+
),
1017+
), # ...
1018+
True,
1019+
"Invalid PM type",
1020+
id="invalid_pm_data",
1021+
),
1022+
case(
1023+
ParsedNarrowLink(
1024+
pm_with=DecodedPM(
1025+
type=Literal["group"],
1026+
recipient_ids=[1001, 11, 12],
1027+
recipient_emails=[
1028+
1029+
1030+
1031+
],
1032+
),
1033+
), # ...
1034+
True,
1035+
"",
1036+
id="valid_group_pm_data",
1037+
),
1038+
case(
1039+
ParsedNarrowLink(
1040+
pm_with=DecodedPM(
1041+
type=Literal["group"],
1042+
recipient_ids=[1001, 11],
1043+
recipient_emails=["[email protected]", "[email protected]"],
1044+
),
1045+
), # ...
1046+
True,
1047+
"Invalid PM type",
1048+
id="invalid_group_pm_data",
1049+
),
1050+
case(
1051+
ParsedNarrowLink(
1052+
pm_with=DecodedPM(
1053+
type=Literal["pm"],
1054+
recipient_ids=[1001, -1],
1055+
recipient_emails=["[email protected]", ""],
1056+
),
1057+
), # ...
1058+
False,
1059+
"The PM has one or more invalid recipient(s)",
1060+
id="invalid_recipient",
1061+
),
1062+
],
1063+
)
1064+
def test__validate_pm_data(
1065+
self,
1066+
parsed_link: ParsedNarrowLink,
1067+
is_valid_private_recipient: bool,
1068+
expected_error: str,
1069+
) -> None:
1070+
self.controller.model.is_valid_private_recipient.return_value = (
1071+
is_valid_private_recipient
1072+
)
1073+
mocked_button = self.message_link_button()
1074+
1075+
assert mocked_button._validate_pm_data(parsed_link) == expected_error
1076+
9871077
@pytest.mark.parametrize(
9881078
[
9891079
"parsed_link",

zulipterminal/ui_tools/buttons.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -543,6 +543,31 @@ def _validate_stream_data(self, parsed_link: ParsedNarrowLink) -> str:
543543

544544
return ""
545545

546+
def _validate_pm_data(self, parsed_link: ParsedNarrowLink) -> str:
547+
"""
548+
Validates PM data and returns either an empty string for a successful
549+
validation or an appropriate validation error.
550+
"""
551+
pm_type = parsed_link["pm_with"]["type"]
552+
recipient_ids = parsed_link["pm_with"]["recipient_ids"]
553+
recipient_emails = parsed_link["pm_with"]["recipient_emails"]
554+
555+
# If a group PM is specified as normal PM or vice versa.
556+
if (len(recipient_ids) >= 3 and pm_type == Literal["pm"]) or (
557+
len(recipient_ids) <= 2 and pm_type == Literal["group"]
558+
):
559+
return "Invalid PM type"
560+
561+
# Validate all PM recipients
562+
for i in range(len(recipient_ids)):
563+
id = recipient_ids[i]
564+
email = recipient_emails[i]
565+
566+
if not self.model.is_valid_private_recipient(id, email):
567+
return "The PM has one or more invalid recipient(s)"
568+
569+
return ""
570+
546571
def _validate_narrow_link(self, parsed_link: ParsedNarrowLink) -> str:
547572
"""
548573
Returns either an empty string for a successful validation or an
@@ -559,6 +584,12 @@ def _validate_narrow_link(self, parsed_link: ParsedNarrowLink) -> str:
559584
# On successful validation, patch the optional stream data
560585
self._patch_stream_data(parsed_link)
561586

587+
# Validate PM data.
588+
if "pm-with" in parsed_link:
589+
error = self._validate_pm_data(parsed_link)
590+
if error:
591+
return error
592+
562593
# Validate topic name.
563594
if "topic_name" in parsed_link:
564595
topic_name = parsed_link["topic_name"]

0 commit comments

Comments
 (0)