-
-
Notifications
You must be signed in to change notification settings - Fork 949
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: do not omit last page on identity list #3169
Conversation
@@ -180,7 +180,7 @@ func (h *Handler) list(w http.ResponseWriter, r *http.Request, _ httprouter.Para | |||
isam[i] = WithCredentialsMetadataAndAdminMetadataInJSON(identity) | |||
} | |||
|
|||
migrationpagination.PaginationHeader(w, urlx.AppendPaths(h.r.Config().SelfAdminURL(r.Context()), RouteCollection), total, page, itemsPerPage) | |||
migrationpagination.PaginationHeader(w, urlx.AppendPaths(h.r.Config().SelfAdminURL(r.Context()), RouteCollection), total+int64(itemsPerPage), page, itemsPerPage) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a super ugly hack, please suggest a better way if you know any. page-1
does not work because then we get 1
as the next page for 1
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this necessary? Doesn't that value come from the DB?
Ah, nvm.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This breaks the x-total-count unfortunately.
Codecov Report
@@ Coverage Diff @@
## master #3169 +/- ##
==========================================
- Coverage 77.71% 77.62% -0.09%
==========================================
Files 317 317
Lines 20036 20036
==========================================
- Hits 15571 15553 -18
- Misses 3277 3291 +14
- Partials 1188 1192 +4
... and 4 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
The problem is that the persister uses
1
as the fist page, while the pagination header helper assumes0
is the first page. Therefore, until now, the second-last page was considered the last and nonext
rel was set in theLink
header.This is the only way I could think of to make this not a breaking change, but it is super ugly.