[bugfix] paging rel links (superseriousbusiness#2883)

* fix paging so it uses correct cursor query parameter name * improved code comment * whoops, flip the cursoring 🤦 * fix the broken test
nyarla · Jun 19, 2024 · 40c903c · 40c903c
1 parent 3bdab72
commit 40c903c
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 27 deletions.
diff --git a/internal/api/activitypub/users/repliesget_test.go b/internal/api/activitypub/users/repliesget_test.go
@@ -162,7 +162,7 @@ func (suite *RepliesGetTestSuite) TestGetRepliesNext() {
 		"id":           targetStatus.URI + "/replies?limit=20&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",
-		"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"},
 		"totalItems":   1,
 	})

diff --git a/internal/paging/boundary.go b/internal/paging/boundary.go
@@ -23,26 +23,36 @@ package paging
 func EitherMinID(minID, sinceID string) Boundary {
 	/*
 
-	           Paging with `since_id` vs `min_id`:
-
-	                limit = 4       limit = 4
-	               +----------+    +----------+
-	     max_id--> |xxxxxxxxxx|    |          | <-- max_id
-	               +----------+    +----------+
-	               |xxxxxxxxxx|    |          |
-	               +----------+    +----------+
-	               |xxxxxxxxxx|    |          |
-	               +----------+    +----------+
-	               |xxxxxxxxxx|    |xxxxxxxxxx|
-	               +----------+    +----------+
-	               |          |    |xxxxxxxxxx|
-	               +----------+    +----------+
-	               |          |    |xxxxxxxxxx|
-	               +----------+    +----------+
-	   since_id--> |          |    |xxxxxxxxxx| <-- min_id
-	               +----------+    +----------+
-	               |          |    |          |
-	               +----------+    +----------+
+				Paging with `since_id` vs `min_id`:
+
+					limit = 4       limit = 4
+					+----------+    +----------+
+		  max_id--> |xxxxxxxxxx|    |          | <-- max_id
+					+----------+    +----------+
+					|xxxxxxxxxx|    |          |
+					+----------+    +----------+
+					|xxxxxxxxxx|    |          |
+					+----------+    +----------+
+					|xxxxxxxxxx|    |xxxxxxxxxx|
+					+----------+    +----------+
+					|          |    |xxxxxxxxxx|
+					+----------+    +----------+
+					|          |    |xxxxxxxxxx|
+					+----------+    +----------+
+		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 {

diff --git a/internal/paging/page.go b/internal/paging/page.go
@@ -289,14 +289,27 @@ func (p *Page) ToLinkURL(proto, host, path string, queryParams url.Values) *url.
 		queryParams = cloneQuery(queryParams)
 	}
 
-	if p.Min.Value != "" {
-		// A page-minimum query parameter is available.
-		queryParams.Set(p.Min.Name, p.Min.Value)
+	var cursor string
+
+	// 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 != "" {
-		// A page-maximum query parameter is available.
-		queryParams.Set(p.Max.Name, p.Max.Value)
+	if cursor != "" {
+		if p.Min.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 {