Skip to content

Commit 83dc879

Browse files
filmajfilmaj
authored
Tweaking warnings related to missing accessibility fields text and fallback when posting messages. (#1208)
1 parent 8a4f44b commit 83dc879

File tree

8 files changed

+185
-83
lines changed

8 files changed

+185
-83
lines changed

‎slack_sdk/web/async_client.py‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
from .internal_utils import (
2020
_parse_web_class_objects,
2121
_update_call_participants,
22-
_warn_if_text_is_missing,
22+
_warn_if_text_or_attachment_fallback_is_missing,
2323
_remove_none_values,
2424
)
2525
from ..models.attachments import Attachment
@@ -2075,7 +2075,7 @@ async def chat_postEphemeral(
20752075
)
20762076
_parse_web_class_objects(kwargs)
20772077
kwargs = _remove_none_values(kwargs)
2078-
_warn_if_text_is_missing("chat.postEphemeral", kwargs)
2078+
_warn_if_text_or_attachment_fallback_is_missing("chat.postEphemeral", kwargs)
20792079
# NOTE: intentionally using json over params for the API methods using blocks/attachments
20802080
return await self.api_call("chat.postEphemeral", json=kwargs)
20812081

@@ -2129,7 +2129,7 @@ async def chat_postMessage(
21292129
)
21302130
_parse_web_class_objects(kwargs)
21312131
kwargs = _remove_none_values(kwargs)
2132-
_warn_if_text_is_missing("chat.postMessage", kwargs)
2132+
_warn_if_text_or_attachment_fallback_is_missing("chat.postMessage", kwargs)
21332133
# NOTE: intentionally using json over params for the API methods using blocks/attachments
21342134
return await self.api_call("chat.postMessage", json=kwargs)
21352135

@@ -2173,7 +2173,7 @@ async def chat_scheduleMessage(
21732173
)
21742174
_parse_web_class_objects(kwargs)
21752175
kwargs = _remove_none_values(kwargs)
2176-
_warn_if_text_is_missing("chat.scheduleMessage", kwargs)
2176+
_warn_if_text_or_attachment_fallback_is_missing("chat.scheduleMessage", kwargs)
21772177
# NOTE: intentionally using json over params for the API methods using blocks/attachments
21782178
return await self.api_call("chat.scheduleMessage", json=kwargs)
21792179

@@ -2246,7 +2246,7 @@ async def chat_update(
22462246
kwargs.update({"file_ids": file_ids})
22472247
_parse_web_class_objects(kwargs)
22482248
kwargs = _remove_none_values(kwargs)
2249-
_warn_if_text_is_missing("chat.update", kwargs)
2249+
_warn_if_text_or_attachment_fallback_is_missing("chat.update", kwargs)
22502250
# NOTE: intentionally using json over params for API methods using blocks/attachments
22512251
return await self.api_call("chat.update", json=kwargs)
22522252

‎slack_sdk/web/client.py‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
from .internal_utils import (
1111
_parse_web_class_objects,
1212
_update_call_participants,
13-
_warn_if_text_is_missing,
13+
_warn_if_text_or_attachment_fallback_is_missing,
1414
_remove_none_values,
1515
)
1616
from ..models.attachments import Attachment
@@ -2024,7 +2024,7 @@ def chat_postEphemeral(
20242024
)
20252025
_parse_web_class_objects(kwargs)
20262026
kwargs = _remove_none_values(kwargs)
2027-
_warn_if_text_is_missing("chat.postEphemeral", kwargs)
2027+
_warn_if_text_or_attachment_fallback_is_missing("chat.postEphemeral", kwargs)
20282028
# NOTE: intentionally using json over params for the API methods using blocks/attachments
20292029
return self.api_call("chat.postEphemeral", json=kwargs)
20302030

@@ -2078,7 +2078,7 @@ def chat_postMessage(
20782078
)
20792079
_parse_web_class_objects(kwargs)
20802080
kwargs = _remove_none_values(kwargs)
2081-
_warn_if_text_is_missing("chat.postMessage", kwargs)
2081+
_warn_if_text_or_attachment_fallback_is_missing("chat.postMessage", kwargs)
20822082
# NOTE: intentionally using json over params for the API methods using blocks/attachments
20832083
return self.api_call("chat.postMessage", json=kwargs)
20842084

@@ -2122,7 +2122,7 @@ def chat_scheduleMessage(
21222122
)
21232123
_parse_web_class_objects(kwargs)
21242124
kwargs = _remove_none_values(kwargs)
2125-
_warn_if_text_is_missing("chat.scheduleMessage", kwargs)
2125+
_warn_if_text_or_attachment_fallback_is_missing("chat.scheduleMessage", kwargs)
21262126
# NOTE: intentionally using json over params for the API methods using blocks/attachments
21272127
return self.api_call("chat.scheduleMessage", json=kwargs)
21282128

@@ -2195,7 +2195,7 @@ def chat_update(
21952195
kwargs.update({"file_ids": file_ids})
21962196
_parse_web_class_objects(kwargs)
21972197
kwargs = _remove_none_values(kwargs)
2198-
_warn_if_text_is_missing("chat.update", kwargs)
2198+
_warn_if_text_or_attachment_fallback_is_missing("chat.update", kwargs)
21992199
# NOTE: intentionally using json over params for API methods using blocks/attachments
22002200
return self.api_call("chat.update", json=kwargs)
22012201

‎slack_sdk/web/internal_utils.py‎

Lines changed: 41 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -241,41 +241,55 @@ def _to_0_or_1_if_bool(v: Any) -> Union[Any, str]:
241241
return v
242242

243243

244-
def _warn_if_text_is_missing(endpoint: str, kwargs: Dict[str, Any]) -> None:
245-
missing = "text"
244+
def _warn_if_text_or_attachment_fallback_is_missing(
245+
endpoint: str, kwargs: Dict[str, Any]
246+
) -> None:
247+
text = kwargs.get("text")
248+
if text and len(text.strip()) > 0:
249+
# If a top-level text arg is provided, we are good. This is the recommended accessibility field to always provide.
250+
return
251+
252+
# for unit tests etc.
253+
skip_deprecation = os.environ.get("SKIP_SLACK_SDK_WARNING")
254+
if skip_deprecation:
255+
return
256+
257+
# At this point, at a minimum, text argument is missing. Warn the user about this.
258+
message = (
259+
f"The top-level `text` argument is missing in the request payload for a {endpoint} call - "
260+
f"It's a best practice to always provide a `text` argument when posting a message. "
261+
f"The `text` argument is used in places where content cannot be rendered such as: "
262+
"system push notifications, assistive technology such as screen readers, etc."
263+
)
264+
warnings.warn(message, UserWarning)
265+
266+
# Additionally, specifically for attachments, there is a legacy field available at the attachment level called `fallback`
267+
# Even with a missing text, one can provide a `fallback` per attachment.
268+
# More details here: https://api.slack.com/reference/messaging/attachments#legacy_fields
246269
attachments = kwargs.get("attachments")
247270
# Note that this method does not verify attachments
248271
# if the value is already serialized as a single str value.
249-
if attachments is not None and isinstance(attachments, list):
250-
# https://api.slack.com/reference/messaging/attachments
251-
# Check if the fallback field exists for all the attachments
252-
if all(
272+
if (
273+
attachments is not None
274+
and isinstance(attachments, list)
275+
and not all(
253276
[
254277
isinstance(attachment, dict)
255278
and len(attachment.get("fallback", "").strip()) > 0
256279
for attachment in attachments
257280
]
258-
):
259-
# The attachments are all good
260-
return
261-
missing = "fallback"
262-
else:
263-
text = kwargs.get("text")
264-
if text and len(text.strip()) > 0:
265-
# Note that this is applicable only for blocks.
266-
return
267-
268-
message = (
269-
f"The `{missing}` argument is missing in the request payload for a {endpoint} call - "
270-
f"It's a best practice to always provide a `{missing}` argument when posting a message. "
271-
f"The `{missing}` argument is used in places where content cannot be rendered such as: "
272-
"system push notifications, assistive technology such as screen readers, etc."
273-
)
274-
# for unit tests etc.
275-
skip_deprecation = os.environ.get("SKIP_SLACK_SDK_WARNING")
276-
if skip_deprecation:
277-
return
278-
warnings.warn(message, UserWarning)
281+
)
282+
):
283+
# https://api.slack.com/reference/messaging/attachments
284+
# Check if the fallback field exists for all the attachments
285+
# Not all attachments have a fallback property; warn about this too!
286+
message = (
287+
f"Additionally, the attachment-level `fallback` argument is missing in the request payload for a {endpoint} call"
288+
f" - To avoid this warning, it is recommended to always provide a top-level `text` argument when posting a"
289+
f" message. Alternatively you can provide an attachment-level `fallback` argument, though this is now considered"
290+
f" a legacy field (see https://api.slack.com/reference/messaging/attachments#legacy_fields for more details)."
291+
)
292+
warnings.warn(message, UserWarning)
279293

280294

281295
def _build_unexpected_body_error_message(body: str) -> str:

‎slack_sdk/web/legacy_client.py‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
from .internal_utils import (
2222
_parse_web_class_objects,
2323
_update_call_participants,
24-
_warn_if_text_is_missing,
24+
_warn_if_text_or_attachment_fallback_is_missing,
2525
_remove_none_values,
2626
)
2727
from ..models.attachments import Attachment
@@ -2035,7 +2035,7 @@ def chat_postEphemeral(
20352035
)
20362036
_parse_web_class_objects(kwargs)
20372037
kwargs = _remove_none_values(kwargs)
2038-
_warn_if_text_is_missing("chat.postEphemeral", kwargs)
2038+
_warn_if_text_or_attachment_fallback_is_missing("chat.postEphemeral", kwargs)
20392039
# NOTE: intentionally using json over params for the API methods using blocks/attachments
20402040
return self.api_call("chat.postEphemeral", json=kwargs)
20412041

@@ -2089,7 +2089,7 @@ def chat_postMessage(
20892089
)
20902090
_parse_web_class_objects(kwargs)
20912091
kwargs = _remove_none_values(kwargs)
2092-
_warn_if_text_is_missing("chat.postMessage", kwargs)
2092+
_warn_if_text_or_attachment_fallback_is_missing("chat.postMessage", kwargs)
20932093
# NOTE: intentionally using json over params for the API methods using blocks/attachments
20942094
return self.api_call("chat.postMessage", json=kwargs)
20952095

@@ -2133,7 +2133,7 @@ def chat_scheduleMessage(
21332133
)
21342134
_parse_web_class_objects(kwargs)
21352135
kwargs = _remove_none_values(kwargs)
2136-
_warn_if_text_is_missing("chat.scheduleMessage", kwargs)
2136+
_warn_if_text_or_attachment_fallback_is_missing("chat.scheduleMessage", kwargs)
21372137
# NOTE: intentionally using json over params for the API methods using blocks/attachments
21382138
return self.api_call("chat.scheduleMessage", json=kwargs)
21392139

@@ -2206,7 +2206,7 @@ def chat_update(
22062206
kwargs.update({"file_ids": file_ids})
22072207
_parse_web_class_objects(kwargs)
22082208
kwargs = _remove_none_values(kwargs)
2209-
_warn_if_text_is_missing("chat.update", kwargs)
2209+
_warn_if_text_or_attachment_fallback_is_missing("chat.update", kwargs)
22102210
# NOTE: intentionally using json over params for API methods using blocks/attachments
22112211
return self.api_call("chat.update", json=kwargs)
22122212

‎tests/slack_sdk/web/test_web_client_issue_891.py‎

Lines changed: 40 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,24 +14,54 @@ def setUp(self):
1414
def tearDown(self):
1515
cleanup_mock_web_api_server(self)
1616

17-
def test_missing_text_warnings_chat_postMessage(self):
17+
def test_missing_text_warning_chat_postMessage(self):
1818
client = WebClient(base_url="http://localhost:8888", token="xoxb-api_test")
19-
resp = client.chat_postMessage(channel="C111", blocks=[])
19+
with self.assertWarnsRegex(UserWarning, "`text` argument is missing"):
20+
resp = client.chat_postMessage(channel="C111", blocks=[])
2021
self.assertIsNone(resp["error"])
2122

22-
def test_missing_text_warnings_chat_postEphemeral(self):
23+
def test_missing_text_warning_chat_postEphemeral(self):
2324
client = WebClient(base_url="http://localhost:8888", token="xoxb-api_test")
24-
resp = client.chat_postEphemeral(channel="C111", user="U111", blocks=[])
25+
with self.assertWarnsRegex(UserWarning, "`text` argument is missing"):
26+
resp = client.chat_postEphemeral(channel="C111", user="U111", blocks=[])
2527
self.assertIsNone(resp["error"])
2628

27-
def test_missing_text_warnings_chat_scheduleMessage(self):
29+
def test_missing_text_warning_chat_scheduleMessage(self):
2830
client = WebClient(base_url="http://localhost:8888", token="xoxb-api_test")
29-
resp = client.chat_scheduleMessage(
30-
channel="C111", post_at="299876400", text="", blocks=[]
31-
)
31+
with self.assertWarnsRegex(UserWarning, "`text` argument is missing"):
32+
resp = client.chat_scheduleMessage(
33+
channel="C111", post_at="299876400", text="", blocks=[]
34+
)
3235
self.assertIsNone(resp["error"])
3336

34-
def test_missing_text_warnings_chat_update(self):
37+
def test_missing_text_warning_chat_update(self):
3538
client = WebClient(base_url="http://localhost:8888", token="xoxb-api_test")
36-
resp = client.chat_update(channel="C111", ts="111.222", blocks=[])
39+
with self.assertWarnsRegex(UserWarning, "`text` argument is missing"):
40+
resp = client.chat_update(channel="C111", ts="111.222", blocks=[])
41+
self.assertIsNone(resp["error"])
42+
43+
def test_missing_fallback_warning_chat_postMessage(self):
44+
client = WebClient(base_url="http://localhost:8888", token="xoxb-api_test")
45+
with self.assertWarnsRegex(UserWarning, "`fallback` argument is missing"):
46+
resp = client.chat_postMessage(channel="C111", blocks=[], attachments=[{"text": "hi"}])
47+
self.assertIsNone(resp["error"])
48+
49+
def test_missing_fallback_warning_chat_postEphemeral(self):
50+
client = WebClient(base_url="http://localhost:8888", token="xoxb-api_test")
51+
with self.assertWarnsRegex(UserWarning, "`fallback` argument is missing"):
52+
resp = client.chat_postEphemeral(channel="C111", user="U111", blocks=[], attachments=[{"text": "hi"}])
53+
self.assertIsNone(resp["error"])
54+
55+
def test_missing_fallback_warning_chat_scheduleMessage(self):
56+
client = WebClient(base_url="http://localhost:8888", token="xoxb-api_test")
57+
with self.assertWarnsRegex(UserWarning, "`fallback` argument is missing"):
58+
resp = client.chat_scheduleMessage(
59+
channel="C111", post_at="299876400", text="", blocks=[], attachments=[{"text": "hi"}]
60+
)
61+
self.assertIsNone(resp["error"])
62+
63+
def test_missing_fallback_warning_chat_update(self):
64+
client = WebClient(base_url="http://localhost:8888", token="xoxb-api_test")
65+
with self.assertWarnsRegex(UserWarning, "`fallback` argument is missing"):
66+
resp = client.chat_update(channel="C111", ts="111.222", blocks=[], attachments=[{"text": "hi"}])
3767
self.assertIsNone(resp["error"])

‎tests/slack_sdk/web/test_web_client_issue_971.py‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ def test_blocks_without_text_arg(self):
3333
client = WebClient(
3434
base_url="http://localhost:8888", token="xoxb-api_test", team_id="T111"
3535
)
36+
# this generates a warning because "text" is missing
3637
with self.assertWarns(UserWarning):
3738
resp = client.chat_postMessage(channel="C111", blocks=[])
3839
self.assertTrue(resp["ok"])
@@ -41,6 +42,7 @@ def test_attachments_with_fallback(self):
4142
client = WebClient(
4243
base_url="http://localhost:8888", token="xoxb-api_test", team_id="T111"
4344
)
45+
# this generates a warning because "text" is missing
4446
resp = client.chat_postMessage(
4547
channel="C111", attachments=[{"fallback": "test"}]
4648
)
@@ -50,6 +52,7 @@ def test_attachments_with_empty_fallback(self):
5052
client = WebClient(
5153
base_url="http://localhost:8888", token="xoxb-api_test", team_id="T111"
5254
)
55+
# this generates two warnings: "text" is missing, and also one attachment with no fallback
5356
with self.assertWarns(UserWarning):
5457
resp = client.chat_postMessage(
5558
channel="C111", attachments=[{"fallback": ""}]
@@ -60,25 +63,16 @@ def test_attachments_without_fallback(self):
6063
client = WebClient(
6164
base_url="http://localhost:8888", token="xoxb-api_test", team_id="T111"
6265
)
66+
# this generates two warnings: "text" is missing, and also one attachment with no fallback
6367
with self.assertWarns(UserWarning):
6468
resp = client.chat_postMessage(channel="C111", attachments=[{}])
6569
self.assertTrue(resp["ok"])
6670

67-
def test_attachments_without_fallback_with_text_arg(self):
68-
client = WebClient(
69-
base_url="http://localhost:8888", token="xoxb-api_test", team_id="T111"
70-
)
71-
# this warns because each attachment should have its own fallback, even with "text"
72-
with self.assertWarns(UserWarning):
73-
resp = client.chat_postMessage(
74-
channel="C111", text="test", attachments=[{}]
75-
)
76-
self.assertTrue(resp["ok"])
77-
7871
def test_multiple_attachments_one_without_fallback(self):
7972
client = WebClient(
8073
base_url="http://localhost:8888", token="xoxb-api_test", team_id="T111"
8174
)
75+
# this generates two warnings: "text" is missing, and also one attachment with no fallback
8276
with self.assertWarns(UserWarning):
8377
resp = client.chat_postMessage(
8478
channel="C111", attachments=[{"fallback": "test"}, {}]

0 commit comments

Comments
 (0)