[bugfix] paging rel links (#2883)

* fix paging so it uses correct cursor query parameter name

* improved code comment

* whoops, flip the cursoring 🤦

* fix the broken test
This commit is contained in:
kim 2024-04-30 11:19:33 +01:00 committed by GitHub
parent bfc21e4850
commit 4f87ef246c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 49 additions and 26 deletions

View file

@ -162,7 +162,7 @@ func (suite *RepliesGetTestSuite) TestGetRepliesNext() {
"id": targetStatus.URI + "/replies?limit=20&only_other_accounts=false", "id": targetStatus.URI + "/replies?limit=20&only_other_accounts=false",
"partOf": targetStatus.URI + "/replies?only_other_accounts=false", "partOf": targetStatus.URI + "/replies?only_other_accounts=false",
"next": "http://localhost:8080/users/the_mighty_zork/statuses/01F8MHAMCHF6Y650WCRSCP4WMY/replies?limit=20&min_id=01FF25D5Q0DH7CHD57CTRS6WK0&only_other_accounts=false", "next": "http://localhost:8080/users/the_mighty_zork/statuses/01F8MHAMCHF6Y650WCRSCP4WMY/replies?limit=20&min_id=01FF25D5Q0DH7CHD57CTRS6WK0&only_other_accounts=false",
"prev": "http://localhost:8080/users/the_mighty_zork/statuses/01F8MHAMCHF6Y650WCRSCP4WMY/replies?limit=20&max_id=01FF25D5Q0DH7CHD57CTRS6WK0&only_other_accounts=false", "prev": "http://localhost:8080/users/the_mighty_zork/statuses/01F8MHAMCHF6Y650WCRSCP4WMY/replies?limit=20&min_id=01FF25D5Q0DH7CHD57CTRS6WK0&only_other_accounts=false",
"orderedItems": []string{"http://localhost:8080/users/admin/statuses/01FF25D5Q0DH7CHD57CTRS6WK0"}, "orderedItems": []string{"http://localhost:8080/users/admin/statuses/01FF25D5Q0DH7CHD57CTRS6WK0"},
"totalItems": 1, "totalItems": 1,
}) })

View file

@ -23,26 +23,36 @@ package paging
func EitherMinID(minID, sinceID string) Boundary { func EitherMinID(minID, sinceID string) Boundary {
/* /*
Paging with `since_id` vs `min_id`: Paging with `since_id` vs `min_id`:
limit = 4 limit = 4 limit = 4 limit = 4
+----------+ +----------+ +----------+ +----------+
max_id--> |xxxxxxxxxx| | | <-- max_id max_id--> |xxxxxxxxxx| | | <-- max_id
+----------+ +----------+ +----------+ +----------+
|xxxxxxxxxx| | | |xxxxxxxxxx| | |
+----------+ +----------+ +----------+ +----------+
|xxxxxxxxxx| | | |xxxxxxxxxx| | |
+----------+ +----------+ +----------+ +----------+
|xxxxxxxxxx| |xxxxxxxxxx| |xxxxxxxxxx| |xxxxxxxxxx|
+----------+ +----------+ +----------+ +----------+
| | |xxxxxxxxxx| | | |xxxxxxxxxx|
+----------+ +----------+ +----------+ +----------+
| | |xxxxxxxxxx| | | |xxxxxxxxxx|
+----------+ +----------+ +----------+ +----------+
since_id--> | | |xxxxxxxxxx| <-- min_id since_id--> | | |xxxxxxxxxx| <-- min_id
+----------+ +----------+ +----------+ +----------+
| | | | | | | |
+----------+ +----------+ +----------+ +----------+
To sum it up in words:
when paging with since_id, max_id is used as
the cursor value, and since_id provides a
limiting value to the results.
when paging with min_id, min_id is used as
the cursor value, and max_id provides a
limiting value to the results.
*/ */
switch { switch {

View file

@ -289,14 +289,27 @@ func (p *Page) ToLinkURL(proto, host, path string, queryParams url.Values) *url.
queryParams = cloneQuery(queryParams) queryParams = cloneQuery(queryParams)
} }
if p.Min.Value != "" { var cursor string
// A page-minimum query parameter is available.
queryParams.Set(p.Min.Name, p.Min.Value) // Depending on page ordering, the
// page will be cursored by either
// the min or max query parameter.
if p.order().Ascending() {
cursor = p.Min.Name
} else {
cursor = p.Max.Name
} }
if p.Max.Value != "" { if cursor != "" {
// A page-maximum query parameter is available. if p.Min.Value != "" {
queryParams.Set(p.Max.Name, p.Max.Value) // Set page-minimum cursor value.
queryParams.Set(cursor, p.Min.Value)
}
if p.Max.Value != "" {
// Set page-maximum cursor value.
queryParams.Set(cursor, p.Max.Value)
}
} }
if p.Limit > 0 { if p.Limit > 0 {