Reset "page" param on "per_page" change #528
Conversation
When the "per_page" param is changed, reset the "page" param to 1, to avoid paging outside the actual number of documents. Also disable the link for selecting the "per_page" value that is currently active.
|
Build failure seems to be caused by #527 |
|
The used to be the case, I think. I'd like to see some tests to ensure it remains true. |
|
Sure. I'll try to come up with a test or two. It seems easiest to expand on search.feature and search.steps. Maybe extract into search_paging.feature/search_paging.steps. Would that be ok? |
|
Sounds reasonable to me. I also wonder if we could/should push the business logic into params_for_search and just unit-test it? |
|
When I first started, I tried (although not very hard) to find somewhere else to put some logic that would determine if "per_page" had changed and then reset "page", but what happens in the before_filters in Blacklight::Catalog#search_session and Blacklight::Catalog#delete_or_assign_search_session_params kind stopped me on that track. If you have a rough sketch of an implementation, I'll be happy to have a go on it. I'll work a bit on the tests. If I do it right, they should be valid independent of the implementation. |
|
+1 |
|
Hang on while I fix something. I broke the layout in the sort dropdown. |
|
I cherry-picked your tests (thanks!) and pushed the actual logic into the #params_for_search method itself. |
|
Cool. Thanks. Something similar is happening when you remove a query constraint. When removing facet constraints, the page param is correctly reset/removed, but not when removing a query constraint. I'm going to try and have a look at it today. Steffen On Friday, February 22, 2013 at 00:13 , Chris Beer wrote:
|
Example:
Go to: http://demo.projectblacklight.org/?f%5Bformat%5D%5B%5D=Unknown&per_page=10
Change to page 2: http://demo.projectblacklight.org/?f%5Bformat%5D%5B%5D=Unknown&page=2&per_page=10
Change "per_page" to 100: http://demo.projectblacklight.org/?f%5Bformat%5D%5B%5D=Unknown&page=2&per_page=100
Now you're stuck on results "101 - 100 of 88" with a blank page and no next/prev links.
Our solution: When the "per_page" param is changed, reset the "page" param to 1,
to avoid paging outside the actual number of documents. Also disable
the link for selecting the "per_page" value that is currently active.