Repository: slackapi/python-slack-sdk
Pull request #1409 (2023-10-05 03:38:33)

Make next_cursor extraction logic even more robust (ref #1407)
## Summary

This pull request improves the internal logic to extract next_cursor from Slack Web API response. As of today, all the public APIs return empty string (meaning `""`) when the API does not have its next page. With that being said, the platform may add some inconsistent behavior in the future. Thus, just in case, we want to make this SDK code even more robust for such scenario. Refer to #1407 for its context.

### Category (place an `x` in each of the `[ ]`)

- [x] **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)
- [x] `tests`/`integration_tests` (Automated tests for this library)

## Requirements (place an `x` in each `[ ]`)

- [x] I've read and understood the [Contributing Guidelines](https://github.com/slackapi/python-slack-sdk/blob/main/.github/contributing.md) and have done my best effort to follow them.
- [x] I've read and agree to the [Code of Conduct](https://slackhq.github.io/code-of-conduct).
- [x] I've run `python3 -m venv .venv && source .venv/bin/activate && ./scripts/run_validation.sh` after making the changes.


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/slackapi/python-slack-sdk/pull/1409?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=slackapi) Report
> Merging [#1409](https://app.codecov.io/gh/slackapi/python-slack-sdk/pull/1409?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=slackapi) (4d58b5b) into [main](https://app.codecov.io/gh/slackapi/python-slack-sdk/commit/2caacbbfb2e2babb197c592c0710ca36e4e6b7e7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=slackapi) (2caacbb) will **decrease** coverage by `0.01%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##             main    #1409      +/-   ##
==========================================
- Coverage   85.47%   85.46%   -0.01%     
==========================================
  Files         111      111              
  Lines       11857    11857              
==========================================
- Hits        10135    10134       -1     
- Misses       1722     1723       +1     
```


| [Files](https://app.codecov.io/gh/slackapi/python-slack-sdk/pull/1409?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=slackapi) | Coverage Δ | |
|---|---|---|
| [slack\_sdk/web/internal\_utils.py](https://app.codecov.io/gh/slackapi/python-slack-sdk/pull/1409?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=slackapi#diff-c2xhY2tfc2RrL3dlYi9pbnRlcm5hbF91dGlscy5weQ==) | `82.72% <100.00%> (ø)` | |

... and [1 file with indirect coverage changes](https://app.codecov.io/gh/slackapi/python-slack-sdk/pull/1409/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=slackapi)

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=slackapi)


Patch:
diff --git a/slack_sdk/web/internal_utils.py b/slack_sdk/web/internal_utils.py
index 2c34e58..c7d1456 100644
--- a/slack_sdk/web/internal_utils.py
+++ b/slack_sdk/web/internal_utils.py
@@ -230,9 +230,10 @@ def _next_cursor_is_present(data) -> bool:
         A boolean value.
     """
     # Only admin.conversations.search returns next_cursor at the top level
-    present = ("next_cursor" in data and data["next_cursor"] != "") or (
+    present = ("next_cursor" in data and data["next_cursor"] is not None and data["next_cursor"] != "") or (
         "response_metadata" in data
         and "next_cursor" in data["response_metadata"]
+        and data["response_metadata"]["next_cursor"] is not None
         and data["response_metadata"]["next_cursor"] != ""
     )
     return present


Test patch:
diff --git a/tests/slack_sdk/web/test_internal_utils.py b/tests/slack_sdk/web/test_internal_utils.py
index dfac719..13da2c1 100644
--- a/tests/slack_sdk/web/test_internal_utils.py
+++ b/tests/slack_sdk/web/test_internal_utils.py
@@ -11,6 +11,7 @@ from slack_sdk.web.internal_utils import (
     _build_unexpected_body_error_message,
     _parse_web_class_objects,
     _to_v2_file_upload_item,
+    _next_cursor_is_present,
 )
 
 
@@ -98,3 +99,12 @@ class TestInternalUtils(unittest.TestCase):
         assert file_io_item.get("filename") == "Uploaded file"
         file_io_item = _to_v2_file_upload_item({"file": file_io, "filename": "foo.txt"})
         assert file_io_item.get("filename") == "foo.txt"
+
+    def test_next_cursor_is_present(self):
+        assert _next_cursor_is_present({"next_cursor": "next-page"}) is True
+        assert _next_cursor_is_present({"next_cursor": ""}) is False
+        assert _next_cursor_is_present({"next_cursor": None}) is False
+        assert _next_cursor_is_present({"response_metadata": {"next_cursor": "next-page"}}) is True
+        assert _next_cursor_is_present({"response_metadata": {"next_cursor": ""}}) is False
+        assert _next_cursor_is_present({"response_metadata": {"next_cursor": None}}) is False
+        assert _next_cursor_is_present({"something_else": {"next_cursor": "next-page"}}) is False