Skip to content
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

Include total pages in header #68

Open
dan-klasson opened this issue Oct 4, 2016 · 7 comments
Open

Include total pages in header #68

dan-klasson opened this issue Oct 4, 2016 · 7 comments

Comments

@dan-klasson
Copy link

The header has Per-Page and Total. But no Total Pages.

@davidcelis
Copy link
Owner

@dan-klasson You should be able to get this from the last rel in the Link header. For example, if you got the following Link header for requesting a page:

Link: <http://example.org/resource?page=1>; rel="first", <http://example.org/resource?page=4>; rel="prev", <http://example.org/resource?page=6>; rel="next", <http://example.org/resource?page=10>; rel="last"

You can grab that "last" link and parse the page number out of it.

@dan-klasson
Copy link
Author

@davidcelis Getting the page number from parsing a string isn't very convenient. Why not just add the total pages number to the header as well?

@davidcelis
Copy link
Owner

I'm hesitant to add another header, as it's starting to feel heavy-handed; we've already got Link, Total, and Per-Page. I was even hesitant to add Per-Page, but that ended up making sense because when there's a default value, it can end up being difficult to calculate depending on the number of pages returned. The total number of pages, on the other hand, is always calculable/parseable as far as I can tell. I can leave this issue open to let others voice their opinions, but this doesn't seem to be a compelling add for me right now. Sorry!

@davidcelis davidcelis reopened this Oct 5, 2016
@dan-klasson
Copy link
Author

dan-klasson commented Oct 5, 2016

@davidcelis Your gem is called api-pagination. How can you be hesitant to add headers that are intrinsic to your business domain? And how is one more header heavy handed? In my case I am not using the link headers at all. And parsing the total pages from a string, or calculating it myself as I ended up doing, that's heavy handed IMHO.

@davidcelis
Copy link
Owner

davidcelis commented Oct 5, 2016

Your gem is called api-pagination.

I'm aware. Also, opening with this made me want to immediately re-close this issue with very little further comment. I hope you'll take a step back from how you're starting to address me and think about how it would feel in my shoes.

How can you be hesitant to add headers that are intrinsic to your business domain? And how is one more header heavy handed?

Adding a header that doesn't provide new information isn't intrinsic to my "business domain". I prefer the software I write to do the minimum amount of work required of most use cases. So far, in the years that I've maintained this gem, you're the only person who has requested this header. Finally, in the web world, people really care about the size of HTTP requests. I know an addition header like Total-Pages: 1038 isn't much, but I'd rather keep this gem's footprint as small as possible.

And parsing the total pages from a string, or calculating it myself as I ended up doing, that's heavy handed IMHO.

You're always free to re-open any of this gem's classes in your own app to add the header! But I'm not a huge fan of adding functionality for a single person, especially if it's something that I'm not sure is particularly necessary. As I said before, I'll leave this open so that it's more discoverable for others who might also want this, but until they show up… I'd recommend having your own patch.

@dan-klasson
Copy link
Author

I'm aware. Also, opening with this made me want to immediately re-close this issue with very little further comment. I hope you'll take a step back from how you're starting to address me and think about how it would feel in my shoes.

I didn't mean to offend you. I merely tried to point out that your gem is awesome but it's missing something simple as a few extra bytes in the header.

But I'm not a huge fan of adding functionality for a single person, especially if it's something that I'm not sure is particularly necessary.

Just because someone hasn't had the need for it before, doesn't make it not necessary. Total rows and total pages are the most important details in pagination. Much more important than link headers IMHO.

@ebosantos
Copy link

ebosantos commented Nov 28, 2016

Having a Total Pages header parameter makes sense to me, since links doesn't at all.

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants