Summary: | Sorting not correct in pagination on OPAC lists | ||
---|---|---|---|
Product: | Koha | Reporter: | Nick Clemens (kidclamp) <nick> |
Component: | OPAC | Assignee: | Nick Clemens (kidclamp) <nick> |
Status: | CLOSED FIXED | QA Contact: | Marcel de Rooy <m.de.rooy> |
Severity: | normal | ||
Priority: | P5 - low | CC: | fridolin.somers, kyle, m.de.rooy, victor |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | Small patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: |
21.11.00,21.05.03,20.11.09
|
|
Circulation function: | |||
Attachments: |
Bug 28764: Parse direction parameter in public list view
Bug 28764: Parse direction parameter in public list view Bug 28764: Alter sort dropdown to use direction parameter Bug 28764: Parse direction parameter in public list view Bug 28764: Alter sort dropdown to use direction parameter Bug 28764: (QA follow-up) Add comment, remove two defined's |
Description
Nick Clemens (kidclamp)
2021-07-27 11:23:49 UTC
Created attachment 123205 [details] [review] Bug 28764: Parse direction parameter in public list view This patch adds a check for a 'direction' paramter in list sorting. It maintains the existing syntax for sorting 'sortfield=field:direction' but adds support for the form used in paginations and more commonly thtoughout Koha' sortfield=field&direction=direction' To test: 1 - Create a public list 2 - Add enough items to generate two pages (40 worked for me) 3 - View the list and enforce sorting by date added descending: sortfield=dateadded:desc 4 - Note the titles shown 5 - Click to second page using the pagination bar 6 - Note the same titles are shown, in a reversed order 7 - Apply patch 8 - repeat 305 9 - Second page is sorted correctly! Created attachment 123207 [details] [review] Bug 28764: Parse direction parameter in public list view This patch adds a check for a 'direction' paramter in list sorting. It maintains the existing syntax for sorting 'sortfield=field:direction' but adds support for the form used in paginations and more commonly thtoughout Koha' sortfield=field&direction=direction' To test: 1 - Create a public list 2 - Add enough items to generate two pages (40 worked for me) 3 - View the list and enforce sorting by date added descending: sortfield=dateadded:desc 4 - Note the titles shown 5 - Click to second page using the pagination bar 6 - Note the same titles are shown, in a reversed order 7 - Apply patch 8 - repeat 305 9 - Second page is sorted correctly! Signed-off-by: Owen Leonard <oleonard@myacpl.org> This is a bit confusing. We overrule the sortfield parameter which is still used in Koha by direction. When I am in Your Lists, I click Sort and get a URL with sortfield=title:desc If direction is the way to go, please be more thorough. Created attachment 123297 [details] [review] Bug 28764: Alter sort dropdown to use direction parameter This code is odd -we build a select in the template, then turn it into a list of links using javascript This patch adds a direction data element and uses it while building the links (In reply to Marcel de Rooy from comment #3) > This is a bit confusing. We overrule the sortfield parameter which is still > used in Koha by direction. > > When I am in Your Lists, I click Sort and get a URL with sortfield=title:desc > > If direction is the way to go, please be more thorough. I think the follow up is something like you wanted? We could remove the old style all together, but I felt libraries could have existing links that we should not break. Rewriting the code here to build the dropdown sorting list more straightforward is beyond scope I think. Created attachment 123650 [details] [review] Bug 28764: Parse direction parameter in public list view This patch adds a check for a 'direction' paramter in list sorting. It maintains the existing syntax for sorting 'sortfield=field:direction' but adds support for the form used in paginations and more commonly thtoughout Koha' sortfield=field&direction=direction' To test: 1 - Create a public list 2 - Add enough items to generate two pages (40 worked for me) 3 - View the list and enforce sorting by date added descending: sortfield=dateadded:desc 4 - Note the titles shown 5 - Click to second page using the pagination bar 6 - Note the same titles are shown, in a reversed order 7 - Apply patch 8 - repeat 305 9 - Second page is sorted correctly! Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 123651 [details] [review] Bug 28764: Alter sort dropdown to use direction parameter This code is odd -we build a select in the template, then turn it into a list of links using javascript This patch adds a direction data element and uses it while building the links Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 123652 [details] [review] Bug 28764: (QA follow-up) Add comment, remove two defined's If we no longer offer the sortfield with :asc actively, we should probably write it somewhere too. Replaced a few defined tests by empty test. No need to split empty string or zero etc. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Pushed to master for 21.11, thanks to everybody involved! Pushed to 21.05.x for 21.05.03 Pushed to 20.11.x for 20.11.09 Not backported to oldoldstable (20.05.x). Feel free to ask if it's needed. |