Repository: zulip/zulip-terminal
Pull request #1173 (2022-03-20 19:51:13)

Hide spoiler content in notifications
<!-- Please see https://github.com/zulip/zulip-terminal#contributor-guidelines ! -->

**What does this PR do?**  <!-- Overall description goes here -->

This is complementary to #1061, which is aimed at the message content and accessing the hidden content. #688 doesn't discuss the specifics, but hiding in notifications is one part of this. 

This PR migrates the html processing from the platform code into the model, where beautiful soup is used to translate and extract content accordingly.

Prior to this PR, spoiler text in a message gives rise to extra lines of text and doesn't hide the spoiler content. The updated output is intended to match the style from the web app, which translates a spoiler with a `title` and `content` into `title (...)` in a notification.

**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)

**Notes & Questions** <!-- if any; add/delete/fill-in as appropriate -->

This provides good infrastructure for a follow-up set of work to explore how different markdown is rendered in notifications and improve it if necessary.


Patch:
diff --git a/zulipterminal/model.py b/zulipterminal/model.py
index 8e040fe..d04cdf1 100644
--- a/zulipterminal/model.py
+++ b/zulipterminal/model.py
@@ -21,6 +21,7 @@ from typing import (
 from urllib.parse import urlparse
 
 import zulip
+from bs4 import BeautifulSoup
 from typing_extensions import Literal, TypedDict
 
 from zulipterminal import unicode_emojis
@@ -1268,6 +1269,7 @@ class Model:
 
         recipient = ""
         content = message["content"]
+        hidden_content = False
         if message["type"] == "private":
             recipient = "you"
             if len(message["display_recipient"]) > 2:
@@ -1279,6 +1281,7 @@ class Model:
                 recipient = ", ".join(extra_targets)
             if not self.user_settings()["pm_content_in_desktop_notifications"]:
                 content = f"New private message from {message['sender_full_name']}"
+                hidden_content = True
         elif message["type"] == "stream":
             stream_id = message["stream_id"]
             if {"mentioned", "wildcard_mentioned"}.intersection(
@@ -1287,10 +1290,30 @@ class Model:
                 recipient = "{display_recipient} -> {subject}".format(**message)
 
         if recipient:
+            if hidden_content:
+                text = content
+            else:
+                soup = BeautifulSoup(content, "lxml")
+                for spoiler_tag in soup.find_all(
+                    "div", attrs={"class": "spoiler-block"}
+                ):
+                    header = spoiler_tag.find("div", attrs={"class": "spoiler-header"})
+                    header.contents = [ele for ele in header.contents if ele != "\n"]
+                    empty_header = len(header.contents) == 0
+                    header.unwrap()
+
+                    to_hide = spoiler_tag.find(
+                        "div", attrs={"class": "spoiler-content"}
+                    )
+                    to_hide.string = "(...)" if empty_header else " (...)"
+
+                    spoiler_tag.unwrap()
+                text = soup.text
+
             return notify(
                 f"{self.server_name}:\n"
                 f"{message['sender_full_name']} (to {recipient})",
-                content,
+                text,
             )
         return ""
 
diff --git a/zulipterminal/platform_code.py b/zulipterminal/platform_code.py
index a30c8d7..1919bc5 100644
--- a/zulipterminal/platform_code.py
+++ b/zulipterminal/platform_code.py
@@ -1,7 +1,6 @@
 import platform
 import subprocess
 
-import lxml.html
 from typing_extensions import Literal
 
 
@@ -25,10 +24,7 @@ else:
 MOUSE_SELECTION_KEY = "Fn + Alt" if PLATFORM == "MacOS" else "Shift"
 
 
-def notify(title: str, html_text: str) -> str:
-    document = lxml.html.document_fromstring(html_text)
-    text = document.text_content()
-
+def notify(title: str, text: str) -> str:
     command_list = None
     if PLATFORM == "MacOS":
         command_list = [


Test patch:
diff --git a/tests/model/test_model.py b/tests/model/test_model.py
index 509a6a1..a500f3a 100644
--- a/tests/model/test_model.py
+++ b/tests/model/test_model.py
@@ -1656,6 +1656,48 @@ class TestModel:
         else:
             notify.assert_not_called
 
+    @pytest.mark.parametrize(
+        "content, expected_notification_text",
+        [
+            case("<p>hi</p>", "hi", id="simple_text"),
+            case(
+                "\n".join(
+                    [
+                        '<div class="spoiler-block"><div class="spoiler-header">'
+                        "<p>title of spoiler</p>"
+                        '</div><div class="spoiler-content" aria-hidden="true">'
+                        "<p>content of spoiler</p>"
+                        "</div></div>"
+                    ]
+                ),
+                "title of spoiler (...)",
+                id="spoiler_with_title",
+            ),
+            case(
+                "\n".join(
+                    [
+                        '<div class="spoiler-block"><div class="spoiler-header">'
+                        '</div><div class="spoiler-content" aria-hidden="true">'
+                        "<p>content of spoiler</p>"
+                        "</div></div>"
+                    ]
+                ),
+                "(...)",
+                id="spoiler_no_title",
+            ),
+        ],
+    )
+    def test_notify_user_transformed_content(
+        self, mocker, model, message_fixture, content, expected_notification_text
+    ):
+        mocker.patch(MODEL + ".is_visual_notifications_enabled", lambda s, id: True)
+        notify = mocker.patch(MODULE + ".notify")
+        message_fixture["content"] = content
+
+        model.notify_user(message_fixture)
+
+        notify.assert_called_once_with(mocker.ANY, expected_notification_text)
+
     @pytest.mark.parametrize(
         "notify_enabled, is_notify_called",
         [