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.

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

feat: add new conversations.externalInvitePermissions.set API #1517

Conversation

WilliamBergamin
Copy link
Contributor

Summary

This is a duplicate of #1516 and adds support for https://api.slack.com/methods/conversations.externalInvitePermissions.set

I did not get a chance to test out the integration tests locally, before noticing this was a duplicate

Category (place an x in each of the [ ])

  • slack_sdk.web.WebClient (sync/async) (Web API client)
  • slack_sdk.webhook.WebhookClient (sync/async) (Incoming Webhook, response_url sender)
  • slack_sdk.socket_mode (Socket Mode client)
  • slack_sdk.signature (Request Signature Verifier)
  • slack_sdk.oauth (OAuth Flow Utilities)
  • slack_sdk.models (UI component builders)
  • slack_sdk.scim (SCIM API client)
  • slack_sdk.audit_logs (Audit Logs API client)
  • slack_sdk.rtm_v2 (RTM client)
  • /docs-src (Documents, have you run ./scripts/docs.sh?)
  • /docs-src-v2 (Documents, have you run ./scripts/docs-v2.sh?)
  • /tutorial (PythOnBoardingBot tutorial)
  • tests/integration_tests (Automated tests for this library)

Requirements (place an x in each [ ])

  • I've read and understood the Contributing Guidelines and have done my best effort to follow them.
  • I've read and agree to the Code of Conduct.
  • I've run python3 -m venv .venv && source .venv/bin/activate && ./scripts/run_validation.sh after making the changes.

@filmaj
Copy link
Contributor

filmaj commented Jun 25, 2024

Yours has integration tests! Better than my PR. Let's use this PR instead.

@WilliamBergamin WilliamBergamin marked this pull request as ready for review June 25, 2024 17:02
Copy link

codecov bot commented Jun 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.90%. Comparing base (e64677b) to head (8548f56).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1517      +/-   ##
==========================================
+ Coverage   84.89%   84.90%   +0.01%     
==========================================
  Files         113      113              
  Lines       12480    12489       +9     
==========================================
+ Hits        10595    10604       +9     
  Misses       1885     1885              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@filmaj filmaj left a comment

Choose a reason for hiding this comment

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

LGTM, but FYI there are issues with this API when using a bot token, so we should hold off on merging/releasing until this is fixed on the backend.


sender_approval = sender.conversations_acceptSharedInvite(invite_id=invite["invite_id"], team_id=connect_team_id)
self.assertIsNone(sender_approval["error"])

Copy link
Member

Choose a reason for hiding this comment

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

@filmaj @WilliamBergamin To make this test function, perhaps you need to call conversations.approveSharedInvite here. Without that, the followng calls will result in channel_not_found error

slack_sdk/web/client.py Outdated Show resolved Hide resolved
"target_team": target_team,
}
)
return await self.api_call("conversations.externalInvitePermissions.set", http_verb="GET", params=kwargs)
Copy link
Member

Choose a reason for hiding this comment

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

Can you run the generator script to synchronize the changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done 🟢

@WilliamBergamin WilliamBergamin requested a review from seratch July 3, 2024 15:26
@seratch seratch merged commit 40f7a57 into slackapi:main Jul 4, 2024
12 checks passed
@WilliamBergamin WilliamBergamin deleted the conversations.externalInvitePermissions.set branch July 4, 2024 13:26
seratch added a commit to seratch/python-slack-sdk that referenced this pull request Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants