Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make WebhookClient (sync/async) #send method accept link unfurl params #1047

Merged
merged 18 commits into from Jun 28, 2021
Merged

Make WebhookClient (sync/async) #send method accept link unfurl params #1047

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
e65f9b6
Adds unfurl optional fields to webhook clients
srajiang Jun 23, 2021
da827cd
Add reference comments
srajiang Jun 23, 2021
dae98dd
Merge branch 'main' into sj_update_webhook_clients
srajiang Jun 23, 2021
75efbee
Integer 0|1 instead of string
srajiang Jun 24, 2021
4782359
Revert import for async module
srajiang Jun 24, 2021
5ad43b9
Removes test script + updates fxn return type
srajiang Jun 24, 2021
80f61c1
Add example integration test for comment
srajiang Jun 24, 2021
12f07c6
Remove repetetive logic
srajiang Jun 24, 2021
52c44f8
Remove boolean conversion and revert web util
srajiang Jun 24, 2021
326ca99
Revert type
srajiang Jun 24, 2021
9d32daa
Cleanup comments
srajiang Jun 28, 2021
ac4f8a4
Add setup
srajiang Jun 28, 2021
d8389f4
Add async webhook integration test
srajiang Jun 28, 2021
da13780
Use async test case class
srajiang Jun 28, 2021
693b27b
Add unit tests
srajiang Jun 28, 2021
784bee9
Remove dep
srajiang Jun 28, 2021
4d5d95b
Merge branch 'slackapi:main' into sj_update_webhook_clients
srajiang Jun 28, 2021
d410e8d
Remove async class
srajiang Jun 28, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion slack_sdk/web/internal_utils.py
View file
Open in desktop
Expand Up @@ -229,7 +229,7 @@ def _next_cursor_is_present(data) -> bool:

def _to_0_or_1_if_bool(v: Any) -> Union[Any, str]:
srajiang marked this conversation as resolved.
Show resolved Hide resolved
if isinstance(v, bool):
return "1" if v else "0"
return 1 if v else 0
Copy link
Member

Choose a reason for hiding this comment

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

Probably, this change is fine for WebClient etc. but I will check the behavior later.

Copy link
Member

Choose a reason for hiding this comment

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

As WebookClient sends content-type: application/json data, we don't necessarily convert boolean values to 1/0. Can you modify slack_sdk/webhook/internal_utils.py this way instead?

diff --git a/slack_sdk/webhook/internal_utils.py b/slack_sdk/webhook/internal_utils.py
index 37def28..091c06c 100644
--- a/slack_sdk/webhook/internal_utils.py
+++ b/slack_sdk/webhook/internal_utils.py
@@ -12,7 +12,6 @@ from .webhook_response import WebhookResponse
 def _build_body(original_body: Optional[Dict[str, Any]]) -> Optional[Dict[str, Any]]:
     if original_body:
         body = {k: v for k, v in original_body.items() if v is not None}
-        body = convert_bool_to_0_or_1(body)
         _parse_web_class_objects(body)
         return body
     return None

Copy link
Member Author

@srajiang srajiang Jun 24, 2021 •
edited

Choose a reason for hiding this comment

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

@seratch. Okay, I see that changing it this way reduces the number of possible affected surfaces. Thanks! I have also reverted the above line to return strings instead of integers. Feel free to resolve this, if these changes are what you had intended!

return v


Expand Down
2 changes: 1 addition & 1 deletion slack_sdk/webhook/__init__.py
View file
Open in desktop
@@ -1,6 +1,6 @@
"""You can use slack_sdk.webhook.WebhookClient for Incoming Webhooks
and message responses using response_url in payloads.
"""
# from .async_client import AsyncWebhookClient # noqa
from .async_client import AsyncWebhookClient # noqa
srajiang marked this conversation as resolved.
Show resolved Hide resolved
from .client import WebhookClient # noqa
from .webhook_response import WebhookResponse # noqa
6 changes: 6 additions & 0 deletions slack_sdk/webhook/async_client.py
View file
Open in desktop
Expand Up @@ -87,6 +87,8 @@ async def send(
response_type: Optional[str] = None,
replace_original: Optional[bool] = None,
delete_original: Optional[bool] = None,
unfurl_links: Optional[bool] = None,
unfurl_media: Optional[bool] = None,
headers: Optional[Dict[str, str]] = None,
) -> WebhookResponse:
"""Performs a Slack API request and returns the result.
Expand All @@ -98,6 +100,8 @@ async def send(
response_type: The type of message (either 'in_channel' or 'ephemeral')
replace_original: True if you use this option for response_url requests
delete_original: True if you use this option for response_url requests
unfurl_links: Option to indicate whether text url should unfurl
unfurl_media: Option to indicate whether media url should unfurl
headers: Request headers to append only for this request

Returns:
Expand All @@ -113,6 +117,8 @@ async def send(
"response_type": response_type,
"replace_original": replace_original,
"delete_original": delete_original,
"unfurl_links": unfurl_links,
"unfurl_media": unfurl_media,
},
headers=headers,
)
Expand Down
6 changes: 6 additions & 0 deletions slack_sdk/webhook/client.py
View file
Open in desktop
Expand Up @@ -77,6 +77,8 @@ def send(
response_type: Optional[str] = None,
replace_original: Optional[bool] = None,
delete_original: Optional[bool] = None,
unfurl_links: Optional[bool] = None,
unfurl_media: Optional[bool] = None,
headers: Optional[Dict[str, str]] = None,
) -> WebhookResponse:
"""Performs a Slack API request and returns the result.
Expand All @@ -89,6 +91,8 @@ def send(
response_type: The type of message (either 'in_channel' or 'ephemeral')
replace_original: True if you use this option for response_url requests
delete_original: True if you use this option for response_url requests
unfurl_links: Option to indicate whether text url should unfurl
unfurl_media: Option to indicate whether media url should unfurl
headers: Request headers to append only for this request

Returns:
Expand All @@ -104,6 +108,8 @@ def send(
"response_type": response_type,
"replace_original": replace_original,
"delete_original": delete_original,
"unfurl_links": unfurl_links,
"unfurl_media": unfurl_media,
},
headers=headers,
)
Expand Down