getkirby / kirby

Kirby's core application folder
https://getkirby.com
Other
1.31k stars 168 forks source link

Pagination doesn't redirect to error page, shows last page instead. #1887

Closed manuelmoreale closed 5 years ago

manuelmoreale commented 5 years ago

Describe the bug The current pagination, instead of returning an error page when trying to visit a page number that doesn't exist (for example visiting page:50 in a collection that has only 10 pages) it's returning the last page of the pagination.

To Reproduce Steps to reproduce the behavior:

  1. In the starterkit homepage template, add a paginate() in the foreach loop. For example foreach (page('photography')->children()->listed()->paginate(2)
  2. Try open a page with a stupidly high page number

Expected behavior You should be redirected to the error page.

Screenshots screenshot_2019-06-28_at_12 27 45

Kirby Version This is happening in the current 3.2.0 but @texnixe has tested this and told me it goes back to 3.1.0

lukasbestle commented 5 years ago

I'm not sure if it should redirect to the error page by default – depending on the use-case, the behavior should be configurable. Maybe the dev wants to show a custom error message instead or redirect to the URL of the last existing page.

I have implemented a new $pagination->hasCurrentPage() method that can be used in the controller to check if an overflow happened and to react accordingly.

manuelmoreale commented 5 years ago

@lukasbestle the reason why I flagged this at first it's because the old K2 behaviour was to redirect to the 404 page.

Which make sense in my opinion since you're trying to access a page that doesn't exist.

Returning the last page also breaks every infinite scroll implementation and that's how I noticed the error in the first place.

IMO, the default behaviour should be to return a 404 and then maybe add the possibility to overwrite that if someone wants to handle this specific situation in a different way.

bastianallgeier commented 5 years ago