Repository: zulip/zulip-terminal
Pull request #1166 (2022-03-15 01:30:58)

Recognize setting for hiding of PM content in notifications
<!-- Please see https://github.com/zulip/zulip-terminal#contributor-guidelines ! -->

**What does this PR do?**  <!-- Overall description goes here -->

This primarily fetches and dynamically updates a user setting which optionally hides private message content from appearing in notifications.

This requires fetching a new category of data (in older servers), as well as handling the update of that data.

This user setting is, along with the 24h/12h setting in a refactor, in the new user settings structure added recently.

**Tested?** <!-- Fine to leave some of these unchecked if this is a draft/work-in-progress -->
- [x] Manually
- [x] Existing tests (adapted, if necessary)
- [x] New tests added (for any new behavior)
- [x] Passed linting & tests (each commit)
<!-- Code must pass CI (GitHub Actions) before merging - look for the green tick! -->

<!-- See https://github.com/zulip/zulip-terminal#commit-style -->
**Commit flow** <!-- if more than one commit; add/delete/fill-in as appropriate -->
- Migrate 24h/12h setting into new structure (refactor)
- Do initial fetch of new setting
- Support updating of new setting
- Use new setting to control display of content

**Notes & Questions** <!-- if any; add/delete/fill-in as appropriate -->

Settings will eventually be initialized and updated differently as per #1108.


Patch:
diff --git a/zulipterminal/api_types.py b/zulipterminal/api_types.py
index 1642f31..db6b8d9 100644
--- a/zulipterminal/api_types.py
+++ b/zulipterminal/api_types.py
@@ -228,6 +228,16 @@ class UpdateUserSettingsEvent(TypedDict):
     value: Any
 
 
+# This is specifically only those supported by ZT
+SupportedGlobalNotificationSettings = Literal["pm_content_in_desktop_notifications"]
+
+
+class UpdateGlobalNotificationsEvent(TypedDict):
+    type: Literal["update_global_notifications"]
+    notification_name: SupportedGlobalNotificationSettings
+    setting: Any
+
+
 Event = Union[
     MessageEvent,
     UpdateMessageEvent,
@@ -238,4 +248,5 @@ Event = Union[
     UpdateDisplaySettings,
     UpdateRealmEmojiEvent,
     UpdateUserSettingsEvent,
+    UpdateGlobalNotificationsEvent,
 ]
diff --git a/zulipterminal/model.py b/zulipterminal/model.py
index d5f3f3d..8e040fe 100644
--- a/zulipterminal/model.py
+++ b/zulipterminal/model.py
@@ -78,6 +78,8 @@ def sort_streams(streams: List[StreamData]) -> None:
 
 class UserSettings(TypedDict):
     send_private_typing_notifications: bool
+    twenty_four_hour_time: bool
+    pm_content_in_desktop_notifications: bool
 
 
 class Model:
@@ -116,6 +118,7 @@ class Model:
             "muted_topics",
             "realm_user",  # Enables cross_realm_bots
             "realm_user_groups",
+            "update_global_notifications",
             "update_display_settings",
             "user_settings",
             "realm_emoji",
@@ -133,6 +136,10 @@ class Model:
                 ("subscription", self._handle_subscription_event),
                 ("typing", self._handle_typing_event),
                 ("update_message_flags", self._handle_update_message_flags_event),
+                (
+                    "update_global_notifications",
+                    self._handle_update_global_notifications_event,
+                ),
                 ("update_display_settings", self._handle_update_display_settings_event),
                 ("user_settings", self._handle_user_settings_event),
                 ("realm_emoji", self._handle_update_emoji_event),
@@ -196,16 +203,19 @@ class Model:
             self.initial_data["realm_emoji"]
         )
 
-        self.twenty_four_hr_format = self.initial_data["twenty_four_hour_time"]
-
         # "user_settings" only present in ZFl 89+ (v5.0)
         user_settings = self.initial_data.get("user_settings", None)
+        # TODO: Support multiple settings locations via settings migration #1108
         self._user_settings = UserSettings(
             send_private_typing_notifications=(
                 True
                 if user_settings is None
                 else user_settings["send_private_typing_notifications"]
             ),  # ZFL 105, Zulip 5.0
+            twenty_four_hour_time=self.initial_data["twenty_four_hour_time"],
+            pm_content_in_desktop_notifications=self.initial_data[
+                "pm_content_in_desktop_notifications"
+            ],
         )
 
         self.new_user_input = True
@@ -1257,6 +1267,7 @@ class Model:
             return ""
 
         recipient = ""
+        content = message["content"]
         if message["type"] == "private":
             recipient = "you"
             if len(message["display_recipient"]) > 2:
@@ -1266,6 +1277,8 @@ class Model:
                     if recip["id"] not in (self.user_id, message["sender_id"])
                 ]
                 recipient = ", ".join(extra_targets)
+            if not self.user_settings()["pm_content_in_desktop_notifications"]:
+                content = f"New private message from {message['sender_full_name']}"
         elif message["type"] == "stream":
             stream_id = message["stream_id"]
             if {"mentioned", "wildcard_mentioned"}.intersection(
@@ -1277,7 +1290,7 @@ class Model:
             return notify(
                 f"{self.server_name}:\n"
                 f"{message['sender_full_name']} (to {recipient})",
-                message["content"],
+                content,
             )
         return ""
 
@@ -1534,13 +1547,14 @@ class Model:
         self, timestamp: int, *, show_seconds: bool, show_year: bool = False
     ) -> str:
         local_time = datetime.fromtimestamp(timestamp)
+        use_24h_format = self.user_settings()["twenty_four_hour_time"]
         format_codes = (
             "%a %b %d "
             f"{'%Y ' if show_year else ''}"
-            f"{'%H:' if self.twenty_four_hr_format else '%I:'}"
+            f"{'%H:' if use_24h_format else '%I:'}"
             "%M"
             f"{':%S' if show_seconds else ''}"
-            f"{'' if self.twenty_four_hr_format else ' %p'}"
+            f"{'' if use_24h_format else ' %p'}"
         )
         return local_time.strftime(format_codes)
 
@@ -1624,6 +1638,12 @@ class Model:
                 setting = event["property"]
                 self._user_settings[setting] = event["value"]
 
+    def _handle_update_global_notifications_event(self, event: Event) -> None:
+        assert event["type"] == "update_global_notifications"
+        to_update = event["notification_name"]
+        if to_update == "pm_content_in_desktop_notifications":
+            self._user_settings[to_update] = event["setting"]
+
     def _handle_update_display_settings_event(self, event: Event) -> None:
         """
         Handle change to user display setting (Eg: Time format)
@@ -1631,7 +1651,7 @@ class Model:
         assert event["type"] == "update_display_settings"
         view = self.controller.view
         if event["setting_name"] == "twenty_four_hour_time":
-            self.twenty_four_hr_format = event["setting"]
+            self._user_settings["twenty_four_hour_time"] = event["setting"]
             for msg_w in view.message_view.log:
                 msg_box = msg_w.original_widget
                 msg_id = msg_box.message["id"]


Test patch:
diff --git a/tests/conftest.py b/tests/conftest.py
index 01bf428..e16b101 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -458,6 +458,11 @@ def group_pm_template() -> Message:
     return msg_template_factory(537288, "private", 1520918737, recipients=recipients)
 
 
+@pytest.fixture(params=["pm_template", "group_pm_template"])
+def private_message_fixture(request: Any) -> Message:
+    return request.getfixturevalue(request.param)
+
+
 @pytest.fixture(
     params=["stream_msg_template", "pm_template", "group_pm_template"],
     ids=["stream_message", "pm_message", "group_pm_message"],
@@ -752,6 +757,7 @@ def initial_data(
             },
         },
         "twenty_four_hour_time": True,
+        "pm_content_in_desktop_notifications": True,
         "realm_emoji": realm_emojis,
         "realm_message_retention_days": 74,
         "last_event_id": -1,
diff --git a/tests/model/test_model.py b/tests/model/test_model.py
index 410ffd3..509a6a1 100644
--- a/tests/model/test_model.py
+++ b/tests/model/test_model.py
@@ -108,7 +108,6 @@ class TestModel:
         assert model.active_emoji_data["joker"]["type"] == "realm_emoji"
         # zulip_extra_emoji replaces all other emoji types for 'zulip' emoji.
         assert model.active_emoji_data["zulip"]["type"] == "zulip_extra_emoji"
-        assert model.twenty_four_hr_format == initial_data["twenty_four_hour_time"]
 
     @pytest.mark.parametrize(
         "sptn, expected_sptn_value",
@@ -126,12 +125,18 @@ class TestModel:
         model = Model(self.controller)
 
         assert model.user_settings() == UserSettings(
-            send_private_typing_notifications=expected_sptn_value
+            send_private_typing_notifications=expected_sptn_value,
+            twenty_four_hour_time=initial_data["twenty_four_hour_time"],
+            pm_content_in_desktop_notifications=initial_data[
+                "pm_content_in_desktop_notifications"
+            ],
         )
 
     def test_user_settings_expected_contents(self, model):
         expected_keys = {
             "send_private_typing_notifications",
+            "twenty_four_hour_time",
+            "pm_content_in_desktop_notifications",
         }
         settings = model.user_settings()
         assert set(settings) == expected_keys
@@ -226,6 +231,7 @@ class TestModel:
             "subscription",
             "typing",
             "update_message_flags",
+            "update_global_notifications",
             "update_display_settings",
             "user_settings",
             "realm_emoji",
@@ -240,6 +246,7 @@ class TestModel:
             "muted_topics",
             "realm_user",
             "realm_user_groups",
+            "update_global_notifications",
             "update_display_settings",
             "user_settings",
             "realm_emoji",
@@ -1666,6 +1673,27 @@ class TestModel:
         model.notify_user(message_fixture)
         assert notify.called == is_notify_called
 
+    @pytest.mark.parametrize(
+        "hide_content, expected_content",
+        [(True, "New private message from Foo Foo"), (False, "private content here.")],
+    )
+    def test_notify_users_hides_PM_content_based_on_user_setting(
+        self, mocker, model, private_message_fixture, hide_content, expected_content
+    ):
+        notify = mocker.patch(MODULE + ".notify")
+        model._user_settings["pm_content_in_desktop_notifications"] = not hide_content
+
+        message = private_message_fixture
+        message["user_id"] = 5179
+        message["flags"] = []
+
+        model.notify_user(message)
+
+        others = ", Boo Boo, Bar Bar" if len(message["display_recipient"]) > 2 else ""
+        notify.assert_called_once_with(
+            f"Test Organization Name:\nFoo Foo (to you{others})", expected_content
+        )
+
     @pytest.mark.parametrize(
         "event, expected_times_messages_rerendered, expected_index, topic_view_enabled",
         [
@@ -2977,6 +3005,20 @@ class TestModel:
         model._handle_user_settings_event(event)
         assert model.user_settings()[setting] == value
 
+    @pytest.mark.parametrize("setting", [True, False])
+    def test_update_pm_content_in_desktop_notifications(self, mocker, model, setting):
+        setting_name = "pm_content_in_desktop_notifications"
+        event = {
+            "type": "update_global_notifications",
+            "notification_name": setting_name,
+            "setting": setting,
+        }
+        model._user_settings[setting_name] = not setting
+
+        model._handle_update_global_notifications_event(event)
+
+        assert model.user_settings()[setting_name] == setting
+
     @pytest.mark.parametrize("setting", [True, False])
     def test_update_twenty_four_hour_format(self, mocker, model, setting):
         event = {
@@ -2990,11 +3032,11 @@ class TestModel:
         second_msg_w.original_widget.message = {"id": 2}
         self.controller.view.message_view = mocker.Mock(log=[first_msg_w, second_msg_w])
         create_msg_box_list = mocker.patch(MODULE + ".create_msg_box_list")
-        model.twenty_four_hr_format = None  # initial value is not True/False
+        model._user_settings["twenty_four_hour_format"] = not setting
 
         model._handle_update_display_settings_event(event)
 
-        assert model.twenty_four_hr_format == event["setting"]
+        assert model.user_settings()["twenty_four_hour_time"] == event["setting"]
         assert create_msg_box_list.call_count == len(
             self.controller.view.message_view.log
         )