Skip to content

Commit 0080312

Browse files
Ezio-SarthakEzio-Sarthak
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 5fa8d1e commit 0080312

File tree

2 files changed

+84
-0
lines changed

2 files changed

+84
-0
lines changed

‎tests/ui_tools/test_buttons.py

Lines changed: 62 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,6 +750,7 @@ 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
),
@@ -749,6 +761,7 @@ def test__parse_narrow_link(
749761
True,
750762
None,
751763
None,
764+
None,
752765
"",
753766
id="valid_stream_data_with_stream_id",
754767
),
@@ -760,6 +773,7 @@ def test__parse_narrow_link(
760773
False,
761774
None,
762775
None,
776+
None,
763777
"The stream seems to be either unknown or unsubscribed",
764778
id="invalid_stream_data_with_stream_id",
765779
),
@@ -769,6 +783,7 @@ def test__parse_narrow_link(
769783
stream=DecodedStream(stream_id=None, stream_name="Stream 1"),
770784
), # ...
771785
None,
786+
None,
772787
True,
773788
None,
774789
"",
@@ -780,18 +795,62 @@ def test__parse_narrow_link(
780795
stream=DecodedStream(stream_id=None, stream_name="foo"),
781796
), # ...
782797
None,
798+
None,
783799
False,
784800
None,
785801
"The stream seems to be either unknown or unsubscribed",
786802
id="invalid_stream_data_with_stream_name",
787803
),
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+
),
788846
],
789847
)
790848
def test__validate_narrow_link(
791849
self,
792850
stream_dict: Dict[int, Any],
793851
parsed_link: ParsedNarrowLink,
794852
is_user_subscribed_to_stream: Optional[bool],
853+
is_valid_private_recipient: Optional[bool],
795854
is_valid_stream: Optional[bool],
796855
topics_in_stream: Optional[List[str]],
797856
expected_error: str,
@@ -800,6 +859,9 @@ def test__validate_narrow_link(
800859
self.controller.model.is_user_subscribed_to_stream.return_value = (
801860
is_user_subscribed_to_stream
802861
)
862+
self.controller.model.is_valid_private_recipient.return_value = (
863+
is_valid_private_recipient
864+
)
803865
self.controller.model.is_valid_stream.return_value = is_valid_stream
804866
self.controller.model.topics_in_stream.return_value = topics_in_stream
805867
mocked_button = self.message_link_button()

‎zulipterminal/ui_tools/buttons.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -505,6 +505,22 @@ 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+
519+
if not self.model.is_valid_private_recipient(recipient_id, email):
520+
return "The PM has one or more invalid recipient(s)"
521+
522+
return ""
523+
508524
def _validate_narrow_link(self, parsed_link: ParsedNarrowLink) -> str:
509525
"""
510526
Returns either an empty string for a successful validation or an
@@ -519,6 +535,12 @@ def _validate_narrow_link(self, parsed_link: ParsedNarrowLink) -> str:
519535
if error:
520536
return error
521537

538+
# Validate PM data.
539+
if "pm_with" in parsed_link:
540+
error = self._validate_pm_data(parsed_link)
541+
if error:
542+
return error
543+
522544
# Validate topic name.
523545
if "topic_name" in parsed_link:
524546
topic_name = parsed_link["topic_name"]

0 commit comments

Comments
 (0)