Repository: slackapi/python-slackclient
Pull request #521 (2019-09-28 16:34:26)

Fix Web Client custom iterator
###  Summary

For Web Client, since `SlackResponse` class overrides an initial response data when iterating through the response object, it only returns the last fetched response data when it's iterated twice.

```py
import slack

client = slack.WebClient(token='redacted')
response = client.channels_list(limit=50)

print('#### 1st iteration ####')

for i in response:
    print(i['channels'][0]['id'])

print('#### 2nd iteration ####')

for i in response:
    print(i['channels'][0]['id'])
```

### Actual result
```sh
#### 1st iteration ####
C9A81JQQ4
CB2LQDJTB
CB34D9SQ3
CB3BF4953
CB48SJH1V
#### 2nd iteration ####
CB48SJH1V
```

### Expected result
```sh
#### 1st iteration ####
C9A81JQQ4
CB2LQDJTB
CB34D9SQ3
CB3BF4953
CB48SJH1V
#### 2nd iteration ####
C9A81JQQ4
CB2LQDJTB
CB34D9SQ3
CB3BF4953
CB48SJH1V
```

### Requirements (place an `x` in each `[ ]`)

* [x] I've read and understood the [Contributing Guidelines](https://github.com/slackapi/python-slackclient/blob/master/.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).

CLAassistant: [![CLA assistant check](https://cla-assistant.io/pull/badge/not_signed)](https://cla-assistant.io/slackapi/python-slackclient?pullRequest=521) <br/>Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our [Contributor License Agreement](https://cla-assistant.io/slackapi/python-slackclient?pullRequest=521) before we can accept your contribution.<br/><hr/>**smaeda** seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please [add the email address used for this commit to your account](https://help.github.com/articles/why-are-my-commits-linked-to-the-wrong-user/#commits-are-not-linked-to-any-user).<br/><sub>You have signed the CLA already but the status is still pending? Let us [recheck](https://cla-assistant.io/check/slackapi/python-slackclient?pullRequest=521) it.</sub>
codecov[bot]: # [Codecov](https://codecov.io/gh/slackapi/python-slackclient/pull/521?src=pr&el=h1) Report
> Merging [#521](https://codecov.io/gh/slackapi/python-slackclient/pull/521?src=pr&el=desc) into [master](https://codecov.io/gh/slackapi/python-slackclient/commit/128305a6fca496a5933a18f0b4666052ea48c83a?src=pr&el=desc) will **increase** coverage by `0.03%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/slackapi/python-slackclient/pull/521/graphs/tree.svg?width=650&token=F0O7meYmtD&height=150&src=pr)](https://codecov.io/gh/slackapi/python-slackclient/pull/521?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #521      +/-   ##
==========================================
+ Coverage   68.89%   68.92%   +0.03%     
==========================================
  Files          15       15              
  Lines        1707     1709       +2     
  Branches       96       96              
==========================================
+ Hits         1176     1178       +2     
  Misses        508      508              
  Partials       23       23
```


| [Impacted Files](https://codecov.io/gh/slackapi/python-slackclient/pull/521?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [slack/web/slack\_response.py](https://codecov.io/gh/slackapi/python-slackclient/pull/521/diff?src=pr&el=tree#diff-c2xhY2svd2ViL3NsYWNrX3Jlc3BvbnNlLnB5) | `97.77% <100%> (+0.1%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/slackapi/python-slackclient/pull/521?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/slackapi/python-slackclient/pull/521?src=pr&el=footer). Last update [128305a...990f64d](https://codecov.io/gh/slackapi/python-slackclient/pull/521?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

seratch: I have tried to reproduce your situation but I can't.
smaeda-ks: @seratch Thanks, how did you test it? FYI, I tested it on my private workspace where there are over 200+ channels (and that's why I set `limit=50` sorry if that was not obvious for you).
smaeda-ks: Let me know if you need more clarification here. I'm pretty sure it's reproducible as long as you set the `limit` parameter value less than the total number of channels you're testing on.
seratch: >set the limit parameter value less than the total number of channels

Thanks, I managed to reproduce it.
seratch: I confirmed that your fix addresses the issue by running it towards actual Slack APIs.

I wrote a unit test for this PR. Would you consider merging https://github.com/smaeda-ks/python-slackclient/pull/1 (or feel free to modify it as you like)? Then, I'll give 👍 to this PR.
smaeda-ks: @seratch Thanks for your help! Merged the PR to my forked branch so feel free to merge this PR accordingly. 

Patch:
diff --git a/README.md b/README.md
index f6adf1d..af4dba4 100755
--- a/README.md
+++ b/README.md
@@ -81,16 +81,16 @@ Slack provide a Web API that gives you the ability to build applications that in
 One of the most common use-cases is sending a message to Slack. If you want to send a message as your app, or as a user, this method can do both. In our examples, we specify the channel name, however it is recommended to use the `channel_id` where possible.
 
 ```python
-    import os
-    import slack
+import os
+import slack
 
-    client = slack.WebClient(token=os.environ['SLACK_API_TOKEN'])
+client = slack.WebClient(token=os.environ['SLACK_API_TOKEN'])
 
-    response = client.chat_postMessage(
-        channel='#random',
-        text="Hello world!")
-    assert response["ok"]
-    assert response["message"]["text"] == "Hello world!"
+response = client.chat_postMessage(
+    channel='#random',
+    text="Hello world!")
+assert response["ok"]
+assert response["message"]["text"] == "Hello world!"
 ```
 
 Here we also ensure that the response back from Slack is a successful one and that the message is the one we sent by using the `assert` statement.
@@ -100,15 +100,15 @@ Here we also ensure that the response back from Slack is a successful one and th
 We've changed the process for uploading files to Slack to be much easier and straight forward. You can now just include a path to the file directly in the API call and upload it that way. You can find the details on this api call [here][files.upload]
 
 ```python
-    import os
-    import slack
+import os
+import slack
 
-    client = slack.WebClient(token=os.environ['SLACK_API_TOKEN'])
+client = slack.WebClient(token=os.environ['SLACK_API_TOKEN'])
 
-    response = client.files_upload(
-        channels='#random',
-        file="my_file.pdf")
-    assert response["ok"]
+response = client.files_upload(
+    channels='#random',
+    file="my_file.pdf")
+assert response["ok"]
 ```
 
 ### Basic Usage of the RTM Client
@@ -129,28 +129,28 @@ information it receives. We also give you the ability to call our web client fro
 In our example below, we watch for a [message event][message-event] that contains "Hello" and if its received, we call the `say_hello()` function. We then issue a call to the web client to post back to the channel saying "Hi" to the user.
 
 ```python
-    import os
-    import slack
-
-    @slack.RTMClient.run_on(event='message')
-    def say_hello(**payload):
-        data = payload['data']
-        web_client = payload['web_client']
-        rtm_client = payload['rtm_client']
-        if 'Hello' in data.get('text', []):
-            channel_id = data['channel']
-            thread_ts = data['ts']
-            user = data['user']
-
-            web_client.chat_postMessage(
-                channel=channel_id,
-                text=f"Hi <@{user}>!",
-                thread_ts=thread_ts
-            )
+import os
+import slack
+
+@slack.RTMClient.run_on(event='message')
+def say_hello(**payload):
+    data = payload['data']
+    web_client = payload['web_client']
+    rtm_client = payload['rtm_client']
+    if 'Hello' in data.get('text', []):
+        channel_id = data['channel']
+        thread_ts = data['ts']
+        user = data['user']
+
+        web_client.chat_postMessage(
+            channel=channel_id,
+            text=f"Hi <@{user}>!",
+            thread_ts=thread_ts
+        )
 
-    slack_token = os.environ["SLACK_API_TOKEN"]
-    rtm_client = slack.RTMClient(token=slack_token)
-    rtm_client.start()
+slack_token = os.environ["SLACK_API_TOKEN"]
+rtm_client = slack.RTMClient(token=slack_token)
+rtm_client.start()
 ```
 
 ### Async usage
diff --git a/slack/web/slack_response.py b/slack/web/slack_response.py
index df8371f..d25e116 100644
--- a/slack/web/slack_response.py
+++ b/slack/web/slack_response.py
@@ -70,6 +70,7 @@ class SlackResponse(object):
         self.data = data
         self.headers = headers
         self.status_code = status_code
+        self._initial_data = data
         self._client = client
         self._logger = logging.getLogger(__name__)
 
@@ -101,6 +102,7 @@ class SlackResponse(object):
             (SlackResponse) self
         """
         self._iteration = 0
+        self.data = self._initial_data
         return self
 
     def __next__(self):


Test patch:
diff --git a/tests/web/test_web_client.py b/tests/web/test_web_client.py
index ee22f2e..b5b4297 100644
--- a/tests/web/test_web_client.py
+++ b/tests/web/test_web_client.py
@@ -86,6 +86,47 @@ class TestWebClient(unittest.TestCase):
             users = users + page["members"]
         self.assertTrue(len(users) == 4)
 
+    def test_response_can_be_paginated_multiple_times(self, mock_request):
+        # This test suite verifies the changes in #521 work as expected
+        page1 = {
+            "data": {
+                "ok": True,
+                "channels": [{"id": "C1"}],
+                "response_metadata": {"next_cursor": "has_page2"},
+            },
+            "status_code": 200,
+            "headers": {},
+        }
+        page2 = {
+            "data": {
+                "ok": True,
+                "channels": [{"id": "C2"}],
+                "response_metadata": {"next_cursor": "has_page3"},
+            },
+            "status_code": 200,
+            "headers": {},
+        }
+        page3 = {
+            "data": {"ok": True, "channels": [{"id": "C3"}]},
+            "status_code": 200,
+            "headers": {},
+        }
+        # The initial pagination
+        mock_request.response.side_effect = [page1, page2, page3]
+        response = self.client.channels_list(limit=1)
+        ids = []
+        for page in response:
+            ids.append(page["channels"][0]["id"])
+        self.assertEqual(ids, ["C1", "C2", "C3"])
+
+        # The second iteration starting with page 2
+        # (page1 is already cached in `response`)
+        mock_request.response.side_effect = [page2, page3]
+        ids = []
+        for page in response:
+            ids.append(page["channels"][0]["id"])
+        self.assertEqual(ids, ["C1", "C2", "C3"])
+
     def test_request_pagination_stops_when_next_cursor_is_missing(self, mock_request):
         mock_request.response.side_effect = [
             {