testable filter/sorting/pagination #182
Conversation
| @@ -63,46 +60,86 @@ func (m *MapMapper) Map(value string) (string, bool) { | |||
| return val, isValid | |||
| } | |||
|
|
|||
| // PaginationFromRequest extracts pagination query parameter and returns a function that adds the pagination to a query | |||
| func PaginationFromRequest(r *http.Request) (QueryOption, error) { | |||
| // UrlQueryParameters contains all information that are needed for pagination, sorting and filtering. | |||
There was a problem hiding this comment.
either pluralize information or use "that is needed", or maybe my english is not good enough :D
There was a problem hiding this comment.
I looked it up: There is no plural of "information" :)
| // even if any errors occur. The returned error combines all errors of pagination, filter and sorting. | ||
| func ReadURLQueryParameters(r *http.Request, mapper ColumnMapper, sanitizer ValueSanitizer) (*UrlQueryParameters, error) { | ||
| result := &UrlQueryParameters{} | ||
| errPagination := result.setPagination(r) |
There was a problem hiding this comment.
minor issue, setXXX implies that result does something to the pointer (method argument) it receives. This might be confusing when looking at these methods in the future.
Mabye readXXX or extractXXX?
|
|
||
| } | ||
|
|
||
| func (u *UrlQueryParameters) setPagination(r *http.Request) error { | ||
| pageStr := r.URL.Query().Get("page[number]") | ||
| sizeStr := r.URL.Query().Get("page[size]") | ||
| if pageStr == "" || sizeStr == "" { |
There was a problem hiding this comment.
Can't we have a default page size? Does the jsonapi spec require both parameter to be set?
I would suggest that we allow pagination with "only" the page number.
There was a problem hiding this comment.
e.g., we could use cfg.MinPageSize if none is set?
SchlottiP
force-pushed
the
testable_filter
branch
from
17f5f85
to
b39394c
Compare
No description provided.