Skip to content

Add team.* & admin.users.session.resetBulk APIs#1132

Merged
seratch merged 3 commits intoslackapi:mainfrom
seratch:web-api-support
Nov 4, 2021
Merged

Add team.* & admin.users.session.resetBulk APIs#1132
seratch merged 3 commits intoslackapi:mainfrom
seratch:web-api-support

Conversation

@seratch
Copy link
Copy Markdown
Contributor

@seratch seratch commented Nov 3, 2021

Summary

This pull request adds the following three APIs to WebClient.

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.

@seratch seratch added enhancement M-T: A feature request for new functionality web-client Version: 3x labels Nov 3, 2021
@seratch seratch added this to the 3.12.0 milestone Nov 3, 2021
@seratch seratch self-assigned this Nov 3, 2021
@codecov
Copy link
Copy Markdown

codecov bot commented Nov 3, 2021 •
edited
Loading

Codecov Report

Merging #1132 (d4c27d1) into main (93fd69a) will decrease coverage by 1.34%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1132      +/-   ##
==========================================
- Coverage   87.96%   86.62%   -1.35%     
==========================================
  Files         110      110              
  Lines       10719    10749      +30     
==========================================
- Hits         9429     9311     -118     
- Misses       1290     1438     +148
Impacted Files Coverage Δ
slack_sdk/web/async_client.py 84.86% <100.00%> (-5.12%) ⬇️
slack_sdk/web/client.py 86.28% <100.00%> (-5.14%) ⬇️
slack_sdk/web/legacy_client.py 85.99% <100.00%> (-5.03%) ⬇️
slack_sdk/socket_mode/builtin/internals.py 72.00% <0.00%> (+1.77%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 93fd69a...d4c27d1. Read the comment docs.

filmaj
filmaj approved these changes Nov 3, 2021
View reviewed changes
Copy link
Copy Markdown
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!

My only comment is that for the bulk reset method, it looks like the python SDK accepts both a list and a string when specifying the user IDs, and the string-based argument is not covered by tests. Do you think it is worth adding that? I only bring it up as it looks like coverage drops 1.5% from this PR.

srajiang
srajiang approved these changes Nov 3, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@srajiang srajiang left a comment

Choose a reason for hiding this comment

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

Thanks for adding these!
+1 on adding an additional check for the case where a single user id is provide

@seratch
Copy link
Copy Markdown
Contributor Author

seratch commented Nov 3, 2021

@filmaj @srajiang Thanks. I can add the test later on. FYI, the single string argument is for a comma-separated value like "U111,U222". The WebClient consistently supports this pattern for array value args as our API document says that you can pass such value as those arguments.

@seratch seratch merged commit cf22599 into slackapi:main Nov 4, 2021
@seratch seratch deleted the web-api-support branch November 4, 2021 07:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@stevengill stevengill Awaiting requested review from stevengill

@misscoded misscoded Awaiting requested review from misscoded

2 more reviewers

@filmaj filmaj filmaj approved these changes

@srajiang srajiang srajiang approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@seratch seratch

Labels

enhancement M-T: A feature request for new functionality Version: 3x web-client

Projects

None yet

Milestone

3.12.0

Development

Successfully merging this pull request may close these issues.

3 participants

@seratch @filmaj @srajiang