Skip to content

Add cursor parameter to Audit Logs API method#1147

Merged
seratch merged 2 commits intoslackapi:mainfrom
seratch:audit-logs-cursor
Dec 10, 2021
Merged

Add cursor parameter to Audit Logs API method#1147
seratch merged 2 commits intoslackapi:mainfrom
seratch:audit-logs-cursor

Conversation

@seratch
Copy link
Copy Markdown
Contributor

@seratch seratch commented Dec 9, 2021

Summary

The Audit Logs API's /logs endpoint supports pagination in requests. Although developers can use additional_query_params as workaround now, we should have the cursor parameter in the method arguments.

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 bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented Version: 3x audit-logs-client labels Dec 9, 2021
@seratch seratch added this to the 3.13.0 milestone Dec 9, 2021
@seratch seratch self-assigned this Dec 9, 2021
@codecov
Copy link
Copy Markdown

codecov bot commented Dec 9, 2021

Codecov Report

Merging #1147 (c20bcc9) into main (a58ab92) will decrease coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1147      +/-   ##
==========================================
- Coverage   86.62%   86.59%   -0.04%     
==========================================
  Files         110      110              
  Lines       10755    10755              
==========================================
- Hits         9317     9313       -4     
- Misses       1438     1442       +4
Impacted Files Coverage Δ
slack_sdk/audit_logs/v1/async_client.py 89.16% <ø> (ø)
slack_sdk/audit_logs/v1/client.py 90.62% <ø> (ø)
slack_sdk/audit_logs/v1/response.py 95.23% <ø> (ø)
slack_sdk/socket_mode/builtin/internals.py 70.22% <0.00%> (-1.78%) ⬇️

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 a58ab92...c20bcc9. Read the comment docs.

@srajiang srajiang self-requested a review December 9, 2021 22:37
srajiang
srajiang suggested changes Dec 9, 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.

Reading more about cursor-pagination enabled methods. Would adding limit field also be appropriate to include along with these changes?

@seratch
Copy link
Copy Markdown
Contributor Author

seratch commented Dec 9, 2021 •
edited
Loading

@srajiang "limit" argument is already supported! The reason why I missed "cursor" was that only "cursor" is not listed here: https://api.slack.com/admins/audit-logs#audit_logs_endpoints

@srajiang
Copy link
Copy Markdown
Contributor

srajiang commented Dec 10, 2021

@seratch Of course! I needed to expand the code editor-view to see the other field 🤦‍♀️ - silly me. This looks good to me!

@srajiang srajiang self-requested a review December 10, 2021 00:04
@seratch seratch merged commit bcaee74 into slackapi:main Dec 10, 2021
@seratch seratch deleted the audit-logs-cursor branch December 10, 2021 00:50
@andrewkroh andrewkroh mentioned this pull request Jul 14, 2023
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@srajiang srajiang srajiang approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@seratch seratch

Labels

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

Projects

None yet

Milestone

3.13.0

Development

Successfully merging this pull request may close these issues.

2 participants

@seratch @srajiang