fix(jsonapi): add support for pagination array parameter name #6319
Conversation
src/State/Pagination/Pagination.php
Outdated
| * page[page] => $contextFilters['page'] with default configuration page_parameter_name: page | ||
| * page[number] => $contextFilters['_page'][number] with configuration page_parameter_name: page[number]. | ||
| */ | ||
| private function extractParameter(array $contextFilters, string $parameterName) |
There was a problem hiding this comment.
i've added mixed, but not very top
| /** | ||
| * Extract pagination parameter | ||
| * page[page] => $contextFilters['page'] with default configuration page_parameter_name: page | ||
| * page[number] => $contextFilters['_page'][number] with configuration page_parameter_name: page[number]. |
There was a problem hiding this comment.
you mean that you configure page[number] on the page_parameter_name, then you use page[number]=1?
There was a problem hiding this comment.
To add more context to this, JSON:API consider query parameter such as two parameters named page[offset] and page[limit] really are two separate parameters, instead of a single page parameter in an array form.
There was a problem hiding this comment.
Yeah or page[number] & page[size] in my implementation if you work with page numbers instead of offset/limit cf https://jsonapi.org/format/#fetching-pagination
There was a problem hiding this comment.
can you add a function test and a unit test?
| /** | ||
| * Extract pagination parameter | ||
| * page[page] => $contextFilters['page'] with default configuration page_parameter_name: page | ||
| * page[number] => $contextFilters['_page'][number] with configuration page_parameter_name: page[number]. |
There was a problem hiding this comment.
can you add a function test and a unit test?
| @@ -219,13 +219,38 @@ private function getGraphQlEnabled(?Operation $operation): bool | |||
| return $operation?->getPaginationEnabled() ?? $enabled; | |||
| } | |||
|
|
|||
| /** | |||
| * Extract pagination parameter | |||
| */ | ||
| private function extractParameter(array $contextFilters, string $parameterName): mixed | ||
| { | ||
| preg_match_all("/[\w-]+/", $parameterName, $matches); |
There was a problem hiding this comment.
I don't understand the need of this regex here, we know the possible parameter names no? I'd love a test to understand this better.
This bugfix is related to the jsonapi spec & the handling of pagination parameters in the pagination service.
Following the JSON:API specification around pagination => https://jsonapi.org/format/#fetching-pagination
When i setup api_platform.yaml with
And use ApiPlatform\State\Pagination\Pagination in a custom provider, we can't retrieve the
page[number]&page[size]in$context['filters'].Thanks to @GregoireHebert for the help.
In addition this fix that let us specify array in pagination parameter names will fix the generated documentation.