Skip to content

Fix #1049 by correcting request body format and next_cursor extraction#1061

Merged
seratch merged 1 commit intoslackapi:mainfrom
seratch:issue-1049-admin-convo-search-pagination
Jul 14, 2021
Merged

Fix #1049 by correcting request body format and next_cursor extraction#1061
seratch merged 1 commit intoslackapi:mainfrom
seratch:issue-1049-admin-convo-search-pagination

Conversation

@seratch
Copy link
Copy Markdown
Contributor

@seratch seratch commented Jul 14, 2021

Summary

This pull request resolves #1049. While checking this issue, I found that the __next__ iteration does not work if an API method use the JSON request body format. Thus, I've updated all other API methods that support pagination to use www-url-form-encoded request body.

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 ./docs.sh?)
  • /docs-src-v2 (Documents, have you run ./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 bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented web-client Version: 3x labels Jul 14, 2021
@seratch seratch added this to the 3.8.0 milestone Jul 14, 2021
async def admin_conversations_search(self, **kwargs) -> AsyncSlackResponse:
"""Search for public or private channels in an Enterprise organization."""
return await self.api_call("admin.conversations.search", json=kwargs)
return await self.api_call("admin.conversations.search", params=kwargs)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As I mentioned in the description, the api_call with the JSON request body is not compatible with the iterator by SlackResponse.

@codecov
Copy link
Copy Markdown

codecov bot commented Jul 14, 2021

Codecov Report

Merging #1061 (52efac9) into main (44e5554) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1061   +/-   ##
=======================================
  Coverage   84.33%   84.33%           
=======================================
  Files          95       95           
  Lines        8937     8938    +1     
=======================================
+ Hits         7537     7538    +1     
  Misses       1400     1400           
Impacted Files Coverage Δ
slack_sdk/web/async_client.py 93.93% <100.00%> (ø)
slack_sdk/web/client.py 95.38% <100.00%> (ø)
slack_sdk/web/internal_utils.py 94.16% <100.00%> (ø)
slack_sdk/web/legacy_client.py 95.25% <100.00%> (ø)
slack_sdk/web/slack_response.py 89.83% <100.00%> (+0.17%) ⬆️

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 44e5554...52efac9. Read the comment docs.

@seratch seratch merged commit df9d71b into slackapi:main Jul 14, 2021
@seratch seratch deleted the issue-1049-admin-convo-search-pagination branch July 14, 2021 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented Version: 3x web-client

Projects

None yet

Development

Successfully merging this pull request may close these issues.

WebClient's paginated response iterator does not work for admin.conversations.search API

1 participant