Alignment of Pagination Datatype - #698
Conversation
There was a problem hiding this comment.
See suggestion below ... instead of adding about 27 lines I suggest to delete 13 lines. We should avoid to repeat definitions from CAMARA_common.yaml verbatim going forward, and we should encourage API authors in the examples to use refs instead of local definitions.
| @@ -646,21 +646,48 @@ ResourceList: | |||
| $ref: "#/components/schemas/Pagination" | |||
There was a problem hiding this comment.
The fix below duplicates the complete definition from CAMARA_common.yaml which should be referenced by all APIs using pagination. In this sense CAMARA_common.yaml owns now the definition of Pagination + Page/PerPage/TotalCount/TotalPages. The PR would resolve the drift today correctly but it will happen again.
Also would the example here disagree with the API templates which does $ref: "../common/CAMARA_common.yaml#/components/schemas/Pagination" instead defining the schema locally.
In line 546 we have here in the same document already a precedent for using ../common/CAMARA_common.yaml#/... instead of using local definitions.
And finally: the example defines the envelope, not the Pagination itself "defines a paginated response schema".
Therefore I suggest to delete the whole local Pagination block and change the line 646 to:
| $ref: "#/components/schemas/Pagination" | |
| $ref: "../common/CAMARA_common.yaml#/components/schemas/Pagination" |
| Pagination: | ||
| description: Pagination details helping to navigate through paged results efficiently. | ||
| type: object | ||
| properties: | ||
| page: | ||
| type: integer | ||
| minimum: 1 | ||
| $ref: "#/components/schemas/Page" | ||
| perPage: | ||
| type: integer | ||
| minimum: 1 | ||
| maximum: 100 | ||
| $ref: "#/components/schemas/PerPage" | ||
| totalCount: | ||
| type: integer | ||
| minimum: 0 | ||
| $ref: "#/components/schemas/TotalCount" | ||
| totalPages: | ||
| type: integer | ||
| minimum: 0 | ||
| $ref: "#/components/schemas/TotalPages" | ||
|
|
||
| Page: | ||
| type: integer | ||
| format: int32 | ||
| minimum: 1 | ||
| maximum: 2147483647 | ||
| default: 1 | ||
| description: Current page number (1-indexed). | ||
| example: 1 | ||
| PerPage: | ||
| type: integer | ||
| format: int32 | ||
| minimum: 1 | ||
| maximum: 100 | ||
| default: 20 | ||
| description: Number of items per page. | ||
| example: 20 | ||
| TotalCount: | ||
| type: integer | ||
| format: int32 | ||
| minimum: 0 | ||
| maximum: 2147483647 | ||
| description: Total number of items matching the query, after filters applied. MAY be omitted where a full count query is prohibitively expensive. | ||
| example: 87 | ||
| TotalPages: | ||
| type: integer | ||
| format: int32 | ||
| minimum: 0 | ||
| maximum: 2147483647 | ||
| description: Total number of pages. Equals ceil(totalCount / perPage). MAY be omitted where totalCount is omitted. | ||
| example: 5 |
There was a problem hiding this comment.
Once pagination refs CAMARA_common.yaml above, this local copy of Pagination/Page/PerPage/TotalCount/TotalPages should be removed — nothing points to it.
6 participants
What type of PR is this?
What this PR does / why we need it:
The PR aligns the Pagination Datatype definition in the example to CAMARA_common.yaml.
Which issue(s) this PR fixes:
Fixes #696
Does this PR introduce a breaking change?
Special notes for reviewers:
Changelog input
Additional documentation
This section can be blank.