Tweaking warnings related to missing accessibility fields text and …

…`fallback` when posting messages. (#1208)

slack_sdk/web/async_client.py

@@ -19,7 +19,7 @@
 from .internal_utils import (
     _parse_web_class_objects,
     _update_call_participants,
-    _warn_if_text_is_missing,
+    _warn_if_text_or_attachment_fallback_is_missing,
     _remove_none_values,
 )
 from ..models.attachments import Attachment
@@ -2075,7 +2075,7 @@ async def chat_postEphemeral(
         )
         _parse_web_class_objects(kwargs)
         kwargs = _remove_none_values(kwargs)
-        _warn_if_text_is_missing("chat.postEphemeral", kwargs)
+        _warn_if_text_or_attachment_fallback_is_missing("chat.postEphemeral", kwargs)
         # NOTE: intentionally using json over params for the API methods using blocks/attachments
         return await self.api_call("chat.postEphemeral", json=kwargs)
 
@@ -2129,7 +2129,7 @@ async def chat_postMessage(
         )
         _parse_web_class_objects(kwargs)
         kwargs = _remove_none_values(kwargs)
-        _warn_if_text_is_missing("chat.postMessage", kwargs)
+        _warn_if_text_or_attachment_fallback_is_missing("chat.postMessage", kwargs)
         # NOTE: intentionally using json over params for the API methods using blocks/attachments
         return await self.api_call("chat.postMessage", json=kwargs)
 
@@ -2173,7 +2173,7 @@ async def chat_scheduleMessage(
         )
         _parse_web_class_objects(kwargs)
         kwargs = _remove_none_values(kwargs)
-        _warn_if_text_is_missing("chat.scheduleMessage", kwargs)
+        _warn_if_text_or_attachment_fallback_is_missing("chat.scheduleMessage", kwargs)
         # NOTE: intentionally using json over params for the API methods using blocks/attachments
         return await self.api_call("chat.scheduleMessage", json=kwargs)
 
@@ -2246,7 +2246,7 @@ async def chat_update(
             kwargs.update({"file_ids": file_ids})
         _parse_web_class_objects(kwargs)
         kwargs = _remove_none_values(kwargs)
-        _warn_if_text_is_missing("chat.update", kwargs)
+        _warn_if_text_or_attachment_fallback_is_missing("chat.update", kwargs)
         # NOTE: intentionally using json over params for API methods using blocks/attachments
         return await self.api_call("chat.update", json=kwargs)
 

slack_sdk/web/client.py

@@ -10,7 +10,7 @@
 from .internal_utils import (
     _parse_web_class_objects,
     _update_call_participants,
-    _warn_if_text_is_missing,
+    _warn_if_text_or_attachment_fallback_is_missing,
     _remove_none_values,
 )
 from ..models.attachments import Attachment
@@ -2024,7 +2024,7 @@ def chat_postEphemeral(
         )
         _parse_web_class_objects(kwargs)
         kwargs = _remove_none_values(kwargs)
-        _warn_if_text_is_missing("chat.postEphemeral", kwargs)
+        _warn_if_text_or_attachment_fallback_is_missing("chat.postEphemeral", kwargs)
         # NOTE: intentionally using json over params for the API methods using blocks/attachments
         return self.api_call("chat.postEphemeral", json=kwargs)
 
@@ -2078,7 +2078,7 @@ def chat_postMessage(
         )
         _parse_web_class_objects(kwargs)
         kwargs = _remove_none_values(kwargs)
-        _warn_if_text_is_missing("chat.postMessage", kwargs)
+        _warn_if_text_or_attachment_fallback_is_missing("chat.postMessage", kwargs)
         # NOTE: intentionally using json over params for the API methods using blocks/attachments
         return self.api_call("chat.postMessage", json=kwargs)
 
@@ -2122,7 +2122,7 @@ def chat_scheduleMessage(
         )
         _parse_web_class_objects(kwargs)
         kwargs = _remove_none_values(kwargs)
-        _warn_if_text_is_missing("chat.scheduleMessage", kwargs)
+        _warn_if_text_or_attachment_fallback_is_missing("chat.scheduleMessage", kwargs)
         # NOTE: intentionally using json over params for the API methods using blocks/attachments
         return self.api_call("chat.scheduleMessage", json=kwargs)
 
@@ -2195,7 +2195,7 @@ def chat_update(
             kwargs.update({"file_ids": file_ids})
         _parse_web_class_objects(kwargs)
         kwargs = _remove_none_values(kwargs)
-        _warn_if_text_is_missing("chat.update", kwargs)
+        _warn_if_text_or_attachment_fallback_is_missing("chat.update", kwargs)
         # NOTE: intentionally using json over params for API methods using blocks/attachments
         return self.api_call("chat.update", json=kwargs)
 

slack_sdk/web/internal_utils.py

@@ -241,41 +241,55 @@ def _to_0_or_1_if_bool(v: Any) -> Union[Any, str]:
     return v
 
 
-def _warn_if_text_is_missing(endpoint: str, kwargs: Dict[str, Any]) -> None:
-    missing = "text"
+def _warn_if_text_or_attachment_fallback_is_missing(
+    endpoint: str, kwargs: Dict[str, Any]
+) -> None:
+    text = kwargs.get("text")
+    if text and len(text.strip()) > 0:
+        # If a top-level text arg is provided, we are good. This is the recommended accessibility field to always provide.
+        return
+
+    # for unit tests etc.
+    skip_deprecation = os.environ.get("SKIP_SLACK_SDK_WARNING")
+    if skip_deprecation:
+        return
+
+    # At this point, at a minimum, text argument is missing. Warn the user about this.
+    message = (
+        f"The top-level `text` argument is missing in the request payload for a {endpoint} call - "
+        f"It's a best practice to always provide a `text` argument when posting a message. "
+        f"The `text` argument is used in places where content cannot be rendered such as: "
+        "system push notifications, assistive technology such as screen readers, etc."
+    )
+    warnings.warn(message, UserWarning)
+
+    # Additionally, specifically for attachments, there is a legacy field available at the attachment level called `fallback`
+    # Even with a missing text, one can provide a `fallback` per attachment.
+    # More details here: https://api.slack.com/reference/messaging/attachments#legacy_fields
     attachments = kwargs.get("attachments")
     # Note that this method does not verify attachments
     # if the value is already serialized as a single str value.
-    if attachments is not None and isinstance(attachments, list):
-        # https://api.slack.com/reference/messaging/attachments
-        # Check if the fallback field exists for all the attachments
-        if all(
+    if (
+        attachments is not None
+        and isinstance(attachments, list)
+        and not all(
             [
                 isinstance(attachment, dict)
                 and len(attachment.get("fallback", "").strip()) > 0
                 for attachment in attachments
             ]
-        ):
-            # The attachments are all good
-            return
-        missing = "fallback"
-    else:
-        text = kwargs.get("text")
-        if text and len(text.strip()) > 0:
-            # Note that this is applicable only for blocks.
-            return
-
-    message = (
-        f"The `{missing}` argument is missing in the request payload for a {endpoint} call - "
-        f"It's a best practice to always provide a `{missing}` argument when posting a message. "
-        f"The `{missing}` argument is used in places where content cannot be rendered such as: "
-        "system push notifications, assistive technology such as screen readers, etc."
-    )
-    # for unit tests etc.
-    skip_deprecation = os.environ.get("SKIP_SLACK_SDK_WARNING")
-    if skip_deprecation:
-        return
-    warnings.warn(message, UserWarning)
+        )
+    ):
+        # https://api.slack.com/reference/messaging/attachments
+        # Check if the fallback field exists for all the attachments
+        # Not all attachments have a fallback property; warn about this too!
+        message = (
+            f"Additionally, the attachment-level `fallback` argument is missing in the request payload for a {endpoint} call"
+            f" - To avoid this warning, it is recommended to always provide a top-level `text` argument when posting a"
+            f" message. Alternatively you can provide an attachment-level `fallback` argument, though this is now considered"
+            f" a legacy field (see https://api.slack.com/reference/messaging/attachments#legacy_fields for more details)."
+        )
+        warnings.warn(message, UserWarning)
 
 
 def _build_unexpected_body_error_message(body: str) -> str:

slack_sdk/web/legacy_client.py

@@ -21,7 +21,7 @@
 from .internal_utils import (
     _parse_web_class_objects,
     _update_call_participants,
-    _warn_if_text_is_missing,
+    _warn_if_text_or_attachment_fallback_is_missing,
     _remove_none_values,
 )
 from ..models.attachments import Attachment
@@ -2035,7 +2035,7 @@ def chat_postEphemeral(
         )
         _parse_web_class_objects(kwargs)
         kwargs = _remove_none_values(kwargs)
-        _warn_if_text_is_missing("chat.postEphemeral", kwargs)
+        _warn_if_text_or_attachment_fallback_is_missing("chat.postEphemeral", kwargs)
         # NOTE: intentionally using json over params for the API methods using blocks/attachments
         return self.api_call("chat.postEphemeral", json=kwargs)
 
@@ -2089,7 +2089,7 @@ def chat_postMessage(
         )
         _parse_web_class_objects(kwargs)
         kwargs = _remove_none_values(kwargs)
-        _warn_if_text_is_missing("chat.postMessage", kwargs)
+        _warn_if_text_or_attachment_fallback_is_missing("chat.postMessage", kwargs)
         # NOTE: intentionally using json over params for the API methods using blocks/attachments
         return self.api_call("chat.postMessage", json=kwargs)
 
@@ -2133,7 +2133,7 @@ def chat_scheduleMessage(
         )
         _parse_web_class_objects(kwargs)
         kwargs = _remove_none_values(kwargs)
-        _warn_if_text_is_missing("chat.scheduleMessage", kwargs)
+        _warn_if_text_or_attachment_fallback_is_missing("chat.scheduleMessage", kwargs)
         # NOTE: intentionally using json over params for the API methods using blocks/attachments
         return self.api_call("chat.scheduleMessage", json=kwargs)
 
@@ -2206,7 +2206,7 @@ def chat_update(
             kwargs.update({"file_ids": file_ids})
         _parse_web_class_objects(kwargs)
         kwargs = _remove_none_values(kwargs)
-        _warn_if_text_is_missing("chat.update", kwargs)
+        _warn_if_text_or_attachment_fallback_is_missing("chat.update", kwargs)
         # NOTE: intentionally using json over params for API methods using blocks/attachments
         return self.api_call("chat.update", json=kwargs)
 

tests/slack_sdk/web/test_web_client_issue_891.py

@@ -14,24 +14,54 @@ def setUp(self):
     def tearDown(self):
         cleanup_mock_web_api_server(self)
 
-    def test_missing_text_warnings_chat_postMessage(self):
+    def test_missing_text_warning_chat_postMessage(self):
         client = WebClient(base_url="http://localhost:8888", token="xoxb-api_test")
-        resp = client.chat_postMessage(channel="C111", blocks=[])
+        with self.assertWarnsRegex(UserWarning, "`text` argument is missing"):
+            resp = client.chat_postMessage(channel="C111", blocks=[])
         self.assertIsNone(resp["error"])
 
-    def test_missing_text_warnings_chat_postEphemeral(self):
+    def test_missing_text_warning_chat_postEphemeral(self):
         client = WebClient(base_url="http://localhost:8888", token="xoxb-api_test")
-        resp = client.chat_postEphemeral(channel="C111", user="U111", blocks=[])
+        with self.assertWarnsRegex(UserWarning, "`text` argument is missing"):
+            resp = client.chat_postEphemeral(channel="C111", user="U111", blocks=[])
         self.assertIsNone(resp["error"])
 
-    def test_missing_text_warnings_chat_scheduleMessage(self):
+    def test_missing_text_warning_chat_scheduleMessage(self):
         client = WebClient(base_url="http://localhost:8888", token="xoxb-api_test")
-        resp = client.chat_scheduleMessage(
-            channel="C111", post_at="299876400", text="", blocks=[]
-        )
+        with self.assertWarnsRegex(UserWarning, "`text` argument is missing"):
+            resp = client.chat_scheduleMessage(
+                channel="C111", post_at="299876400", text="", blocks=[]
+            )
         self.assertIsNone(resp["error"])
 
-    def test_missing_text_warnings_chat_update(self):
+    def test_missing_text_warning_chat_update(self):
         client = WebClient(base_url="http://localhost:8888", token="xoxb-api_test")
-        resp = client.chat_update(channel="C111", ts="111.222", blocks=[])
+        with self.assertWarnsRegex(UserWarning, "`text` argument is missing"):
+            resp = client.chat_update(channel="C111", ts="111.222", blocks=[])
+        self.assertIsNone(resp["error"])
+
+    def test_missing_fallback_warning_chat_postMessage(self):
+        client = WebClient(base_url="http://localhost:8888", token="xoxb-api_test")
+        with self.assertWarnsRegex(UserWarning, "`fallback` argument is missing"):
+            resp = client.chat_postMessage(channel="C111", blocks=[], attachments=[{"text": "hi"}])
+        self.assertIsNone(resp["error"])
+
+    def test_missing_fallback_warning_chat_postEphemeral(self):
+        client = WebClient(base_url="http://localhost:8888", token="xoxb-api_test")
+        with self.assertWarnsRegex(UserWarning, "`fallback` argument is missing"):
+            resp = client.chat_postEphemeral(channel="C111", user="U111", blocks=[], attachments=[{"text": "hi"}])
+        self.assertIsNone(resp["error"])
+
+    def test_missing_fallback_warning_chat_scheduleMessage(self):
+        client = WebClient(base_url="http://localhost:8888", token="xoxb-api_test")
+        with self.assertWarnsRegex(UserWarning, "`fallback` argument is missing"):
+            resp = client.chat_scheduleMessage(
+                channel="C111", post_at="299876400", text="", blocks=[], attachments=[{"text": "hi"}]
+            )
+        self.assertIsNone(resp["error"])
+
+    def test_missing_fallback_warning_chat_update(self):
+        client = WebClient(base_url="http://localhost:8888", token="xoxb-api_test")
+        with self.assertWarnsRegex(UserWarning, "`fallback` argument is missing"):
+            resp = client.chat_update(channel="C111", ts="111.222", blocks=[], attachments=[{"text": "hi"}])
         self.assertIsNone(resp["error"])

tests/slack_sdk/web/test_web_client_issue_971.py

@@ -33,6 +33,7 @@ def test_blocks_without_text_arg(self):
         client = WebClient(
             base_url="http://localhost:8888", token="xoxb-api_test", team_id="T111"
         )
+        # this generates a warning because "text" is missing
         with self.assertWarns(UserWarning):
             resp = client.chat_postMessage(channel="C111", blocks=[])
         self.assertTrue(resp["ok"])
@@ -41,6 +42,7 @@ def test_attachments_with_fallback(self):
         client = WebClient(
             base_url="http://localhost:8888", token="xoxb-api_test", team_id="T111"
         )
+        # this generates a warning because "text" is missing
         resp = client.chat_postMessage(
             channel="C111", attachments=[{"fallback": "test"}]
         )
@@ -50,6 +52,7 @@ def test_attachments_with_empty_fallback(self):
         client = WebClient(
             base_url="http://localhost:8888", token="xoxb-api_test", team_id="T111"
         )
+        # this generates two warnings: "text" is missing, and also one attachment with no fallback
         with self.assertWarns(UserWarning):
             resp = client.chat_postMessage(
                 channel="C111", attachments=[{"fallback": ""}]
@@ -60,25 +63,16 @@ def test_attachments_without_fallback(self):
         client = WebClient(
             base_url="http://localhost:8888", token="xoxb-api_test", team_id="T111"
         )
+        # this generates two warnings: "text" is missing, and also one attachment with no fallback
         with self.assertWarns(UserWarning):
             resp = client.chat_postMessage(channel="C111", attachments=[{}])
         self.assertTrue(resp["ok"])
 
-    def test_attachments_without_fallback_with_text_arg(self):
-        client = WebClient(
-            base_url="http://localhost:8888", token="xoxb-api_test", team_id="T111"
-        )
-        # this warns because each attachment should have its own fallback, even with "text"
-        with self.assertWarns(UserWarning):
-            resp = client.chat_postMessage(
-                channel="C111", text="test", attachments=[{}]
-            )
-        self.assertTrue(resp["ok"])
-
     def test_multiple_attachments_one_without_fallback(self):
         client = WebClient(
             base_url="http://localhost:8888", token="xoxb-api_test", team_id="T111"
         )
+        # this generates two warnings: "text" is missing, and also one attachment with no fallback
         with self.assertWarns(UserWarning):
             resp = client.chat_postMessage(
                 channel="C111", attachments=[{"fallback": "test"}, {}]