Skip to content

WIP: Update Send-SlackAPI to support cursor-based pagination#91

Open
rkeithhill wants to merge 1 commit into
RamblingCookieMonster:masterfrom
rkeithhill:enable-cursor-based-pagination
Open

WIP: Update Send-SlackAPI to support cursor-based pagination#91
rkeithhill wants to merge 1 commit into
RamblingCookieMonster:masterfrom
rkeithhill:enable-cursor-based-pagination

Conversation

@rkeithhill
Copy link
Copy Markdown

Fix #88

This is a work in progress because while Get-SlackUser now retrieves all users. I'm not sure what we want to do about the other commands that implement paging themselves. There are also two forms of paging on Slack - an older approach called class paging and a new approach called cursor-based pagination. I've implemented the latter in Send-SlackAPI. Also, with Get-SlackUser I'm not exposing any pagination parameters yet. I think most of the time, folks just want to get all users. That said, we could expose parameters similar to the other commands.

@rkeithhill rkeithhill force-pushed the enable-cursor-based-pagination branch from 68bc48f to 635ff39 Compare June 24, 2019 19:51
@rkeithhill
Copy link
Copy Markdown
Author

rkeithhill commented Jul 5, 2019

@RamblingCookieMonster Any thoughts on this PR? I think I need to switch the parameter name from EnablePagination to EnablePaging. I could update those other commands to use EnablePaging with an alias of Paging to make this PR not be a breaking change. I'd do something similar with MaxNumberPages with an alias of MaxQueries. So yeah, it would probably be better to update those other commands to remove their paging logic and have them use this new paging logic in Send-SlackAPI.

@RamblingCookieMonster
Copy link
Copy Markdown
Owner

Agreed on all counts! Alias to avoid breaking change, remove ad hoc paging if you can offload it : D thank you so much for putting the time into this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Get-Slackuser only getting first 1000 users. Results are paged, can we get all pages please?

3 participants