Skip to content

Commit b8fb00f

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 IDs are valid or not. Tests added and amended.
1 parent 7473a42 commit b8fb00f

File tree

2 files changed

+128
-0
lines changed

2 files changed

+128
-0
lines changed

tests/ui_tools/test_buttons.py

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -612,6 +612,7 @@ def test__parse_narrow_link(
612612
[
613613
"parsed_link",
614614
"is_user_subscribed_to_stream",
615+
"is_valid_private_recipient",
615616
"is_valid_stream",
616617
"topics_in_stream",
617618
"expected_error",
@@ -624,6 +625,7 @@ def test__parse_narrow_link(
624625
True,
625626
None,
626627
None,
628+
None,
627629
"",
628630
id="valid_modern_stream_narrow_parsed_link",
629631
),
@@ -635,6 +637,7 @@ def test__parse_narrow_link(
635637
False,
636638
None,
637639
None,
640+
None,
638641
"The stream seems to be either unknown or unsubscribed",
639642
id="invalid_modern_stream_narrow_parsed_link",
640643
),
@@ -644,6 +647,7 @@ def test__parse_narrow_link(
644647
stream=DecodedStream(stream_id=None, stream_name="Stream 1"),
645648
),
646649
None,
650+
None,
647651
True,
648652
None,
649653
"",
@@ -655,6 +659,7 @@ def test__parse_narrow_link(
655659
stream=DecodedStream(stream_id=None, stream_name="foo"),
656660
),
657661
None,
662+
None,
658663
False,
659664
None,
660665
"The stream seems to be either unknown or unsubscribed",
@@ -668,6 +673,7 @@ def test__parse_narrow_link(
668673
),
669674
True,
670675
None,
676+
None,
671677
["Valid"],
672678
"",
673679
id="valid_topic_narrow_parsed_link",
@@ -680,6 +686,7 @@ def test__parse_narrow_link(
680686
),
681687
True,
682688
None,
689+
None,
683690
[],
684691
"Invalid topic name",
685692
id="invalid_topic_narrow_parsed_link",
@@ -693,6 +700,7 @@ def test__parse_narrow_link(
693700
True,
694701
None,
695702
None,
703+
None,
696704
"",
697705
id="valid_stream_near_narrow_parsed_link",
698706
),
@@ -705,6 +713,7 @@ def test__parse_narrow_link(
705713
True,
706714
None,
707715
None,
716+
None,
708717
"Invalid message ID",
709718
id="invalid_stream_near_narrow_parsed_link",
710719
),
@@ -717,6 +726,7 @@ def test__parse_narrow_link(
717726
),
718727
True,
719728
None,
729+
None,
720730
["Valid"],
721731
"",
722732
id="valid_topic_near_narrow_parsed_link",
@@ -730,6 +740,7 @@ def test__parse_narrow_link(
730740
),
731741
True,
732742
None,
743+
None,
733744
["Valid"],
734745
"Invalid message ID",
735746
id="invalid_topic_near_narrow_parsed_link",
@@ -739,16 +750,107 @@ def test__parse_narrow_link(
739750
None,
740751
None,
741752
None,
753+
None,
742754
"The narrow link seems to be either broken or unsupported",
743755
id="invalid_narrow_link",
744756
),
757+
case(
758+
ParsedNarrowLink(
759+
narrow="stream", stream=DecodedStream(stream_id=1, stream_name=None)
760+
), # ...
761+
True,
762+
None,
763+
None,
764+
None,
765+
"",
766+
id="valid_stream_data_with_stream_id",
767+
),
768+
case(
769+
ParsedNarrowLink(
770+
narrow="stream",
771+
stream=DecodedStream(stream_id=462, stream_name=None),
772+
), # ...
773+
False,
774+
None,
775+
None,
776+
None,
777+
"The stream seems to be either unknown or unsubscribed",
778+
id="invalid_stream_data_with_stream_id",
779+
),
780+
case(
781+
ParsedNarrowLink(
782+
narrow="stream",
783+
stream=DecodedStream(stream_id=None, stream_name="Stream 1"),
784+
), # ...
785+
None,
786+
None,
787+
True,
788+
None,
789+
"",
790+
id="valid_stream_data_with_stream_name",
791+
),
792+
case(
793+
ParsedNarrowLink(
794+
narrow="stream",
795+
stream=DecodedStream(stream_id=None, stream_name="foo"),
796+
), # ...
797+
None,
798+
None,
799+
False,
800+
None,
801+
"The stream seems to be either unknown or unsubscribed",
802+
id="invalid_stream_data_with_stream_name",
803+
),
804+
case(
805+
ParsedNarrowLink(
806+
narrow="pm_with",
807+
pm_with=DecodedPM(
808+
type=None, recipient_ids=[1001, 11], recipient_emails=None
809+
),
810+
), # ...
811+
None,
812+
True,
813+
None,
814+
None,
815+
"",
816+
id="valid_pm_data",
817+
),
818+
case(
819+
ParsedNarrowLink(
820+
narrow="pm_with",
821+
pm_with=DecodedPM(
822+
type=None, recipient_ids=[1001, 11, 12], recipient_emails=None
823+
),
824+
), # ...
825+
None,
826+
True,
827+
None,
828+
None,
829+
"",
830+
id="valid_group_pm_data",
831+
),
832+
case(
833+
ParsedNarrowLink(
834+
narrow="pm_with",
835+
pm_with=DecodedPM(
836+
type=None, recipient_ids=[1001, -1], recipient_emails=None
837+
),
838+
), # ...
839+
None,
840+
False,
841+
None,
842+
None,
843+
"The PM has one or more invalid recipient(s)",
844+
id="invalid_recipient",
845+
),
745846
],
746847
)
747848
def test__validate_narrow_link(
748849
self,
749850
stream_dict: Dict[int, Any],
750851
parsed_link: ParsedNarrowLink,
751852
is_user_subscribed_to_stream: Optional[bool],
853+
is_valid_private_recipient: Optional[bool],
752854
is_valid_stream: Optional[bool],
753855
topics_in_stream: Optional[List[str]],
754856
expected_error: str,
@@ -757,6 +859,9 @@ def test__validate_narrow_link(
757859
self.controller.model.is_user_subscribed_to_stream.return_value = (
758860
is_user_subscribed_to_stream
759861
)
862+
self.controller.model.is_valid_private_recipient.return_value = (
863+
is_valid_private_recipient
864+
)
760865
self.controller.model.is_valid_stream.return_value = is_valid_stream
761866
self.controller.model.topics_in_stream.return_value = topics_in_stream
762867
mocked_button = self.message_link_button()

zulipterminal/ui_tools/buttons.py

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

506506
return ""
507507

508+
def _validate_pm_data(self, parsed_link: ParsedNarrowLink) -> str:
509+
"""
510+
Validates PM data and returns either an empty string for a successful
511+
validation or an appropriate validation error.
512+
"""
513+
recipient_ids = parsed_link["pm_with"]["recipient_ids"]
514+
515+
for recipient_id in recipient_ids:
516+
user = self.model._all_users_by_id.get(recipient_id, None)
517+
email = user["email"] if user else ""
518+
name = user["full_name"] if user else ""
519+
520+
if not self.model.is_valid_private_recipient(email, name):
521+
return "The PM has one or more invalid recipient(s)"
522+
523+
return ""
524+
508525
def _validate_narrow_link(self, parsed_link: ParsedNarrowLink) -> str:
509526
"""
510527
Returns either an empty string for a successful validation or an
@@ -519,6 +536,12 @@ def _validate_narrow_link(self, parsed_link: ParsedNarrowLink) -> str:
519536
if error:
520537
return error
521538

539+
# Validate PM data.
540+
if "pm_with" in parsed_link:
541+
error = self._validate_pm_data(parsed_link)
542+
if error:
543+
return error
544+
522545
# Validate topic name.
523546
if "topic_name" in parsed_link:
524547
topic_name = parsed_link["topic_name"]

0 commit comments

Comments
 (0)