Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions slack_sdk/web/async_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
_to_v2_file_upload_item,
_update_call_participants,
_validate_for_legacy_client,
_warn_if_text_or_attachment_fallback_is_missing,
_warn_if_message_text_content_is_missing,
)


Expand Down Expand Up @@ -2695,6 +2695,7 @@ async def chat_postEphemeral(
link_names: Optional[bool] = None,
username: Optional[str] = None,
parse: Optional[str] = None,
markdown_text: Optional[str] = None,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added the markdown_text argument in the methods definitions, but we could omit this since developers are able to pass this value through keyword arguments, should we explicitly define this argument?

praise: Adding this I think is a nice improvement for the typeahead hints found in some editors with virtual environments!

thought: It's nice to know that missing arguments aren't blocking new features though 🤖 ✨

**kwargs,
) -> AsyncSlackResponse:
"""Sends an ephemeral message to a user in a channel.
Expand All @@ -2714,11 +2715,12 @@ async def chat_postEphemeral(
"link_names": link_names,
"username": username,
"parse": parse,
"markdown_text": markdown_text,
}
)
_parse_web_class_objects(kwargs)
kwargs = _remove_none_values(kwargs)
_warn_if_text_or_attachment_fallback_is_missing("chat.postEphemeral", kwargs)
_warn_if_message_text_content_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)

Expand All @@ -2742,6 +2744,7 @@ async def chat_postMessage(
username: Optional[str] = None,
parse: Optional[str] = None, # none, full
metadata: Optional[Union[Dict, Metadata]] = None,
markdown_text: Optional[str] = None,
**kwargs,
) -> AsyncSlackResponse:
"""Sends a message to a channel.
Expand All @@ -2766,11 +2769,12 @@ async def chat_postMessage(
"username": username,
"parse": parse,
"metadata": metadata,
"markdown_text": markdown_text,
}
)
_parse_web_class_objects(kwargs)
kwargs = _remove_none_values(kwargs)
_warn_if_text_or_attachment_fallback_is_missing("chat.postMessage", kwargs)
_warn_if_message_text_content_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)

Expand All @@ -2779,7 +2783,7 @@ async def chat_scheduleMessage(
*,
channel: str,
post_at: Union[str, int],
text: str,
text: Optional[str] = None,
as_user: Optional[bool] = None,
attachments: Optional[Union[str, Sequence[Union[Dict, Attachment]]]] = None,
blocks: Optional[Union[str, Sequence[Union[Dict, Block]]]] = None,
Expand All @@ -2790,6 +2794,7 @@ async def chat_scheduleMessage(
unfurl_media: Optional[bool] = None,
link_names: Optional[bool] = None,
metadata: Optional[Union[Dict, Metadata]] = None,
markdown_text: Optional[str] = None,
**kwargs,
) -> AsyncSlackResponse:
"""Schedules a message.
Expand All @@ -2810,11 +2815,12 @@ async def chat_scheduleMessage(
"unfurl_media": unfurl_media,
"link_names": link_names,
"metadata": metadata,
"markdown_text": markdown_text,
}
)
_parse_web_class_objects(kwargs)
kwargs = _remove_none_values(kwargs)
_warn_if_text_or_attachment_fallback_is_missing("chat.scheduleMessage", kwargs)
_warn_if_message_text_content_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)

Expand Down Expand Up @@ -2867,6 +2873,7 @@ async def chat_update(
parse: Optional[str] = None, # none, full
reply_broadcast: Optional[bool] = None,
metadata: Optional[Union[Dict, Metadata]] = None,
markdown_text: Optional[str] = None,
**kwargs,
) -> AsyncSlackResponse:
"""Updates a message in a channel.
Expand All @@ -2884,6 +2891,7 @@ async def chat_update(
"parse": parse,
"reply_broadcast": reply_broadcast,
"metadata": metadata,
"markdown_text": markdown_text,
}
)
if isinstance(file_ids, (list, tuple)):
Expand All @@ -2892,7 +2900,7 @@ async def chat_update(
kwargs.update({"file_ids": file_ids})
_parse_web_class_objects(kwargs)
kwargs = _remove_none_values(kwargs)
_warn_if_text_or_attachment_fallback_is_missing("chat.update", kwargs)
_warn_if_message_text_content_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)

Expand Down
20 changes: 14 additions & 6 deletions slack_sdk/web/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
_to_v2_file_upload_item,
_update_call_participants,
_validate_for_legacy_client,
_warn_if_text_or_attachment_fallback_is_missing,
_warn_if_message_text_content_is_missing,
)


Expand Down Expand Up @@ -2685,6 +2685,7 @@ def chat_postEphemeral(
link_names: Optional[bool] = None,
username: Optional[str] = None,
parse: Optional[str] = None,
markdown_text: Optional[str] = None,
**kwargs,
) -> SlackResponse:
"""Sends an ephemeral message to a user in a channel.
Expand All @@ -2704,11 +2705,12 @@ def chat_postEphemeral(
"link_names": link_names,
"username": username,
"parse": parse,
"markdown_text": markdown_text,
}
)
_parse_web_class_objects(kwargs)
kwargs = _remove_none_values(kwargs)
_warn_if_text_or_attachment_fallback_is_missing("chat.postEphemeral", kwargs)
_warn_if_message_text_content_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)

Expand All @@ -2732,6 +2734,7 @@ def chat_postMessage(
username: Optional[str] = None,
parse: Optional[str] = None, # none, full
metadata: Optional[Union[Dict, Metadata]] = None,
markdown_text: Optional[str] = None,
**kwargs,
) -> SlackResponse:
"""Sends a message to a channel.
Expand All @@ -2756,11 +2759,12 @@ def chat_postMessage(
"username": username,
"parse": parse,
"metadata": metadata,
"markdown_text": markdown_text,
}
)
_parse_web_class_objects(kwargs)
kwargs = _remove_none_values(kwargs)
_warn_if_text_or_attachment_fallback_is_missing("chat.postMessage", kwargs)
_warn_if_message_text_content_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)

Expand All @@ -2769,7 +2773,7 @@ def chat_scheduleMessage(
*,
channel: str,
post_at: Union[str, int],
text: str,
text: Optional[str] = None,
as_user: Optional[bool] = None,
attachments: Optional[Union[str, Sequence[Union[Dict, Attachment]]]] = None,
blocks: Optional[Union[str, Sequence[Union[Dict, Block]]]] = None,
Expand All @@ -2780,6 +2784,7 @@ def chat_scheduleMessage(
unfurl_media: Optional[bool] = None,
link_names: Optional[bool] = None,
metadata: Optional[Union[Dict, Metadata]] = None,
markdown_text: Optional[str] = None,
**kwargs,
) -> SlackResponse:
"""Schedules a message.
Expand All @@ -2800,11 +2805,12 @@ def chat_scheduleMessage(
"unfurl_media": unfurl_media,
"link_names": link_names,
"metadata": metadata,
"markdown_text": markdown_text,
}
)
_parse_web_class_objects(kwargs)
kwargs = _remove_none_values(kwargs)
_warn_if_text_or_attachment_fallback_is_missing("chat.scheduleMessage", kwargs)
_warn_if_message_text_content_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)

Expand Down Expand Up @@ -2857,6 +2863,7 @@ def chat_update(
parse: Optional[str] = None, # none, full
reply_broadcast: Optional[bool] = None,
metadata: Optional[Union[Dict, Metadata]] = None,
markdown_text: Optional[str] = None,
**kwargs,
) -> SlackResponse:
"""Updates a message in a channel.
Expand All @@ -2874,6 +2881,7 @@ def chat_update(
"parse": parse,
"reply_broadcast": reply_broadcast,
"metadata": metadata,
"markdown_text": markdown_text,
}
)
if isinstance(file_ids, (list, tuple)):
Expand All @@ -2882,7 +2890,7 @@ def chat_update(
kwargs.update({"file_ids": file_ids})
_parse_web_class_objects(kwargs)
kwargs = _remove_none_values(kwargs)
_warn_if_text_or_attachment_fallback_is_missing("chat.update", kwargs)
_warn_if_message_text_content_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)

Expand Down
7 changes: 6 additions & 1 deletion slack_sdk/web/internal_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -247,12 +247,17 @@ def _to_0_or_1_if_bool(v: Any) -> Union[Any, str]:
return v


def _warn_if_text_or_attachment_fallback_is_missing(endpoint: str, kwargs: Dict[str, Any]) -> None:
def _warn_if_message_text_content_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

markdown_text = kwargs.get("markdown_text")
if markdown_text and len(markdown_text.strip()) > 0:
# If a top-level markdown_text arg is provided, we are good. It should not be used in conjunction with text.
return

# for unit tests etc.
skip_deprecation = os.environ.get("SKIP_SLACK_SDK_WARNING")
if skip_deprecation:
Expand Down
20 changes: 14 additions & 6 deletions slack_sdk/web/legacy_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
_to_v2_file_upload_item,
_update_call_participants,
_validate_for_legacy_client,
_warn_if_text_or_attachment_fallback_is_missing,
_warn_if_message_text_content_is_missing,
)


Expand Down Expand Up @@ -2697,6 +2697,7 @@ def chat_postEphemeral(
link_names: Optional[bool] = None,
username: Optional[str] = None,
parse: Optional[str] = None,
markdown_text: Optional[str] = None,
**kwargs,
) -> Union[Future, SlackResponse]:
"""Sends an ephemeral message to a user in a channel.
Expand All @@ -2716,11 +2717,12 @@ def chat_postEphemeral(
"link_names": link_names,
"username": username,
"parse": parse,
"markdown_text": markdown_text,
}
)
_parse_web_class_objects(kwargs)
kwargs = _remove_none_values(kwargs)
_warn_if_text_or_attachment_fallback_is_missing("chat.postEphemeral", kwargs)
_warn_if_message_text_content_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)

Expand All @@ -2744,6 +2746,7 @@ def chat_postMessage(
username: Optional[str] = None,
parse: Optional[str] = None, # none, full
metadata: Optional[Union[Dict, Metadata]] = None,
markdown_text: Optional[str] = None,
**kwargs,
) -> Union[Future, SlackResponse]:
"""Sends a message to a channel.
Expand All @@ -2768,11 +2771,12 @@ def chat_postMessage(
"username": username,
"parse": parse,
"metadata": metadata,
"markdown_text": markdown_text,
}
)
_parse_web_class_objects(kwargs)
kwargs = _remove_none_values(kwargs)
_warn_if_text_or_attachment_fallback_is_missing("chat.postMessage", kwargs)
_warn_if_message_text_content_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)

Expand All @@ -2781,7 +2785,7 @@ def chat_scheduleMessage(
*,
channel: str,
post_at: Union[str, int],
text: str,
text: Optional[str] = None,
as_user: Optional[bool] = None,
attachments: Optional[Union[str, Sequence[Union[Dict, Attachment]]]] = None,
blocks: Optional[Union[str, Sequence[Union[Dict, Block]]]] = None,
Expand All @@ -2792,6 +2796,7 @@ def chat_scheduleMessage(
unfurl_media: Optional[bool] = None,
link_names: Optional[bool] = None,
metadata: Optional[Union[Dict, Metadata]] = None,
markdown_text: Optional[str] = None,
**kwargs,
) -> Union[Future, SlackResponse]:
"""Schedules a message.
Expand All @@ -2812,11 +2817,12 @@ def chat_scheduleMessage(
"unfurl_media": unfurl_media,
"link_names": link_names,
"metadata": metadata,
"markdown_text": markdown_text,
}
)
_parse_web_class_objects(kwargs)
kwargs = _remove_none_values(kwargs)
_warn_if_text_or_attachment_fallback_is_missing("chat.scheduleMessage", kwargs)
_warn_if_message_text_content_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)

Expand Down Expand Up @@ -2869,6 +2875,7 @@ def chat_update(
parse: Optional[str] = None, # none, full
reply_broadcast: Optional[bool] = None,
metadata: Optional[Union[Dict, Metadata]] = None,
markdown_text: Optional[str] = None,
**kwargs,
) -> Union[Future, SlackResponse]:
"""Updates a message in a channel.
Expand All @@ -2886,6 +2893,7 @@ def chat_update(
"parse": parse,
"reply_broadcast": reply_broadcast,
"metadata": metadata,
"markdown_text": markdown_text,
}
)
if isinstance(file_ids, (list, tuple)):
Expand All @@ -2894,7 +2902,7 @@ def chat_update(
kwargs.update({"file_ids": file_ids})
_parse_web_class_objects(kwargs)
kwargs = _remove_none_values(kwargs)
_warn_if_text_or_attachment_fallback_is_missing("chat.update", kwargs)
_warn_if_message_text_content_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)

Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import unittest
import warnings

from slack_sdk import WebClient
from tests.web.mock_web_api_handler import MockHandler
from tests.mock_web_api_server import setup_mock_web_api_server, cleanup_mock_web_api_server


class TestWebClient_Issue_891(unittest.TestCase):
class TestWebClientMessageTextContentWarnings(unittest.TestCase):
def setUp(self):
setup_mock_web_api_server(self, MockHandler)

Expand Down Expand Up @@ -65,3 +66,39 @@ def test_missing_fallback_warning_chat_update(self):
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"])

def test_no_warning_when_markdown_text_is_provided_chat_postMessage(self):
client = WebClient(base_url="http://localhost:8888", token="xoxb-api_test")
with warnings.catch_warnings(record=True) as warning_list:
warnings.simplefilter("always")
resp = client.chat_postMessage(channel="C111", markdown_text="# hello")

self.assertEqual(warning_list, [])
self.assertIsNone(resp["error"])

def test_no_warning_when_markdown_text_is_provided_chat_postEphemeral(self):
client = WebClient(base_url="http://localhost:8888", token="xoxb-api_test")
with warnings.catch_warnings(record=True) as warning_list:
warnings.simplefilter("always")
resp = client.chat_postEphemeral(channel="C111", user="U111", markdown_text="# hello")

self.assertEqual(warning_list, [])
self.assertIsNone(resp["error"])

def test_no_warning_when_markdown_text_is_provided_chat_scheduleMessage(self):
client = WebClient(base_url="http://localhost:8888", token="xoxb-api_test")
with warnings.catch_warnings(record=True) as warning_list:
warnings.simplefilter("always")
resp = client.chat_scheduleMessage(channel="C111", post_at="299876400", markdown_text="# hello")

self.assertEqual(warning_list, [])
self.assertIsNone(resp["error"])

def test_no_warning_when_markdown_text_is_provided_chat_update(self):
client = WebClient(base_url="http://localhost:8888", token="xoxb-api_test")
with warnings.catch_warnings(record=True) as warning_list:
warnings.simplefilter("always")
resp = client.chat_update(channel="C111", ts="111.222", markdown_text="# hello")

self.assertEqual(warning_list, [])
self.assertIsNone(resp["error"])
Loading
Loading