-
-
Notifications
You must be signed in to change notification settings - Fork 203
Fix pagination limit by using correct page[size] parameter #2441 #2442
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
Fix pagination limit by using correct page[size] parameter #2441 #2442
Conversation
|
@josephistired thanks for the PR, can you fix the lint errors that cropped up? Also looks like the default is set by the config which we'd want to use instead of hard coding the max limit. Fell free to publish the package config and update the default there. If you want to go a step further you can make this configurable by referencing an environment variable. Just change: // config/json-api-paginate.php
/*
* The maximum number of results that will be returned
* when using the JSON API paginator.
*/
'max_results' => 30,to... // config/json-api-paginate.php
/*
* The maximum number of results that will be returned
* when using the JSON API paginator.
*/
'max_results' => env('API_MAX_RESULTS', 30), |
alexjustesen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment above for changes ☝️
|
@alexjustesen Thanks for the feedback. I added the config file that uses max_results => env('API_MAX_RESULTS', 500), so the limit defaults to 500 unless the user sets their own environment value. I also made sure the default page size is 25 when page[size] isn’t provided in the query. The validation now checks against either the user’s configured limit or the default 500. Also, linted the code.. sorry. If you're good with these changes, let me know. I’ll also open a request to update the docs to include the new ENV setting and the updated query parameter. |
|
@josephistired go ahead and open a PR for the docs too. You don't need to update the API just the env variable list. Edit: also linting failed again, make sure to add those EOF lines and if you're using an agent add that to your guidelines as a best practice. |
|
@alexjustesen Just opened a PR for the docs. Confused about the linting, as GitHub states the test passed? Will add those EOF lines right now. Edit: Added EOF line to config file. All files I changed have EOF lines. |
Description
Fixes #2441 - Updates the results API controller to properly handle pagination with the
spatie/laravel-json-api-paginatepackage.Changes
per_pagetopage.sizeto match JSON:API standardjsonPaginate()call to explicitly set max (500) and default (25) valuespage[size]query parameter up to 500 resultsTesting
?page[size]=500now returns 500 results (previously capped at 30)?page[size]=25returns 25 results?page[size]=xreturns whatever x is, as long as under 500.