Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add pagination to /admin/users/list API #4866

Merged
merged 2 commits into from May 22, 2017

Conversation

@JaredReisinger
Copy link
Contributor

commented May 12, 2017

Prior to this, only the first 100 active/new/etc. users were available via the /admin/users/list API. This change adds support for a page=# querystring parameter so that all of the users can be retrieved. Requests for pages past the last user result in an empty-list response; requests for negative pages (or zero) just return the first page.

Added tests to cover pagination.

Prior to this, only the first 100 active/new/etc. users were available
via the `/admin/users/list` API.  This change adds support for a
`page=#` querystring parameter so that *all* of the users can be
retrieved.  Requests for pages past the last user result in an
empty-list response; requests for negative pages (or zero) just return
the first page.

Added tests to cover pagination.
@discoursebot

This comment has been minimized.

Copy link

commented May 12, 2017

You've signed the CLA, JaredReisinger. Thank you! This pull request is ready for review.

JaredReisinger added a commit to JaredReisinger/discourse_api_docs that referenced this pull request May 12, 2017
Related to discourse/discourse#4866, document
the `page` parameter.

I also noted that several of the parameters were marked as in `path`,
but I believe these should be in `query`... if I'm wrong, and these
need to be `path` for some reason, just let me know and I'll roll them
back.  But, FWIW, when I try using the API myself, those values only
seem to work when in the querystring.
@JaredReisinger

This comment has been minimized.

Copy link
Contributor Author

commented May 17, 2017

FWIW, the test failure looks unrelated (and is a timing-based test to boot, which I know can sometimes be tricky to test reliably).

@JaredReisinger

This comment has been minimized.

Copy link
Contributor Author

commented May 22, 2017

@oblakeerickson : Hey, Blake, is there anything special I should do to get traction on this PR? I know you likely have other things on your plate, I just wanted to make sure this was seen. Thanks!

@oblakeerickson oblakeerickson merged commit 4e8beda into discourse:master May 22, 2017
1 check failed
1 check failed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
@oblakeerickson

This comment has been minimized.

Copy link
Member

commented May 22, 2017

Thanks!! Sorry for the delay.

@JaredReisinger

This comment has been minimized.

Copy link
Contributor Author

commented May 23, 2017

No worries! Thanks!

@JaredReisinger JaredReisinger deleted the JaredReisinger:admin-user-pagination branch May 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.