jolicode / slack-php-api

:hash: PHP Slack Client based on the official OpenAPI specification
https://jolicode.github.io/slack-php-api/
MIT License
222 stars 56 forks source link

Pagination #64

Closed atymic closed 3 years ago

atymic commented 4 years ago

Hey 👋

Your library is awesome, but one thing that seems to be missing is pagination. I'm working with pretty large slack teams (over 10k members), so I need to use the cursor based pagination pretty often.

I was wondering if you had plans on adding an abstraction around the pagination.

I'd be happy to have a go if you are keen :)

Thanks!

pyrech commented 4 years ago

Hello :slightly_smiling_face:

Thanks for your suggestion. This is not something we had yet on our roadmap yet but let's discuss here if we need it. Currently, using the pagination does not look too much complicated to me, as shown in our examples: https://github.com/jolicode/slack-php-api/blob/master/doc/examples/retrieve-users.php

Do you have an idea about a potential implementation that would be easy to use? And in a perfect world, that would be compatible with the different endpoints using cursor based pagination?

bickart commented 4 years ago

@pyrech this works fine for the retrieve-users but does not work for other items such as channels.list

pyrech commented 4 years ago

Could you elaborate on why it does not work with channels.list?

According to https://api.slack.com/docs/pagination, a few dozens of endpoint works with cursor based pagination. They also explains that exclude_members could break the pagination in the case of channels.list. Is it your case?

Anyway, I'm thinking about a potential implementation that should not be too bad. I should give a try soon 😉

bickart commented 4 years ago

@pyrech I submitted a pull request to add response_metadata to the schema for the results returned from channels.list

pyrech commented 4 years ago

Thanks :slightly_smiling_face:

In the mean time, I opened a PR to propose a solution (and an alternative) to this issue :wink:

JakeBooher commented 4 years ago

@pyrech any update on this? I need the next_cursor from channels.list to be exposed in some way, or at a bare minimum a way to get the raw response data from the models as right now there isn't a way for me to correctly paginate channels

pyrech commented 4 years ago

Sorry, no update at the moment. But I just got an idea to better complete the missing schema in the spec. I will try to look at it in the coming days. In the meantime, you can submit a PR to add the missing properties (see our doc about how to update the SDK) :wink:

pyrech commented 4 years ago

@JakeBooher Sorry I missread your comment. Actually @bickart already worked on this particular endpoint and I just merged its commit :slightly_smiling_face:

I plan to release a new minor version (2.5) in a few days containing this fix. You can wait for it or already use it with the dev-master version

pyrech commented 3 years ago

Closing as a solution for pagination have finally been introduced in #69 and merged a few days ago :)