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

Unexpected behavior on negative page number #757

Closed
bart-degreed opened this issue May 14, 2020 · 3 comments · Fixed by #792
Closed

Unexpected behavior on negative page number #757

bart-degreed opened this issue May 14, 2020 · 3 comments · Fixed by #792

Comments

@bart-degreed
Copy link
Contributor

It doesn't seem right that a negative page number reverses the sort order.

I believe a better syntax for reversing the sort order would be: ?sort=- (without any attribute). This can be combined with attributes: ?sort=lastname,-age,-

With that in place, we should change the meaning of a negative page number, to indicate (n) pages back from the last one. This requires to know the number of total pages, so it should fail with an error in case options.IncludeTotalRecordCount is set to false.

@maurei
Copy link
Member

maurei commented May 15, 2020

related: #643

@maurei
Copy link
Member

maurei commented May 15, 2020

From #643 :

Backward paging would start at the end. It would field

  • for the first backward page (page[number]=-1) the set { n } ... { n-9 }, and
  • for the third backward page (-3) the set { n-20 } ... { n-29 }

Agreed: this does not make sense. Should be

  • { n-9 } ... { n }
  • { n-29 } ... { n-20 }

@bart-degreed
Copy link
Contributor Author

On second thought, I believe that reversing the sort order of an unordered set is not a very useful feature to have. Users can always sort descending by ID if desired.

Making negative page numbers mean 'from end' has some drawbacks:

  • It could only ever work top-level
  • If -1 means "1 page from the end" then there is no way to access the last page using negative notation. On the other hand, if -1 means the last page, it may look counter-intuitive.
  • options.IncludeTotalRecordCount controls whether response metadata is added that shows the count. That we need to retrieve the could separately for that is only a side effect, so this option does not match the scenario. Instead we should an option AllowPagingFromEnd that, when enabled, retrieves the count and throws otherwise.

For now, I intend to remove support for negative page numbers entirely. When the need for page-from-end comes up, we can revisit how to best implement that.

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

Successfully merging a pull request may close this issue.

2 participants