Bug 19021 - Inventory column sorting
Summary: Inventory column sorting
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Fridolin Somers
QA Contact: Nick Clemens
URL:
Keywords: Academy
Depends on:
Blocks:
 
Reported: 2017-08-02 08:34 UTC by Fridolin Somers
Modified: 2018-09-28 06:30 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 19021 - inventory column not sortable (1.59 KB, patch)
2017-08-02 08:38 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 19021 - inventory column sorting (2.30 KB, patch)
2017-08-02 09:45 UTC, Fridolin Somers
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 19021 - inventory column sorting (2.36 KB, patch)
2017-08-02 13:34 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 19021 - inventory column sorting (2.31 KB, patch)
2017-09-14 14:49 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 19021 - inventory column sorting (2.37 KB, patch)
2017-10-07 14:41 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 19021 - inventory column sorting (2.42 KB, patch)
2017-10-27 14:01 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin Somers 2017-08-02 08:34:32 UTC
In inventory result page, items are shown in a JS DataTable.
A column is not sortable for no reason.
Comment 1 Fridolin Somers 2017-08-02 08:38:40 UTC Comment hidden (obsolete)
Comment 2 Fridolin Somers 2017-08-02 09:45:17 UTC
Created attachment 65417 [details] [review]
Bug 19021 - inventory column sorting

In inventory result page, items are shown in a JS DataTable.
For an inventory with barcode file second column is not sortable for no reason.
For an inventory without barcode file first column is not sortable but since it is the default sort order the sorting icon is still displayed.

This patch corrects by setting default sort order on barcode column.

Test plan :
- perform inventory with barcode file
- look at result table
- table is sorted by barcode
=> Without patch second column is not sortable
=> With patch all columns are sortable
- perform inventory without barcode file
- look at result table
=> Without patch first column show sort icon and table is not sorted by barcode
=> With patch first column is not sortable and table is sorted by barcode
Comment 3 Fridolin Somers 2017-08-02 09:46:04 UTC
Oh I see, this JS code is usefull for inventory without barcode file.
I have correct patch and renamed bug a little
Comment 4 Owen Leonard 2017-08-02 13:34:52 UTC
Created attachment 65424 [details] [review]
[SIGNED-OFF] Bug 19021 - inventory column sorting

In inventory result page, items are shown in a JS DataTable.
For an inventory with barcode file second column is not sortable for no reason.
For an inventory without barcode file first column is not sortable but since it is the default sort order the sorting icon is still displayed.

This patch corrects by setting default sort order on barcode column.

Test plan :
- perform inventory with barcode file
- look at result table
- table is sorted by barcode
=> Without patch second column is not sortable
=> With patch all columns are sortable
- perform inventory without barcode file
- look at result table
=> Without patch first column show sort icon and table is not sorted by barcode
=> With patch first column is not sortable and table is sorted by barcode

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 5 Nick Clemens 2017-09-01 10:48:02 UTC
> => With patch first column is not sortable and table is sorted by barcode

When performing an inventory without a barcode file we are essentially generating a shelf list, shouldn't callnumber (or cn_sort ideally) be the default sort here?

Even when comparing to a barcode file I think we should probably be sorting by callnumber in our results
Comment 6 Fridolin Somers 2017-09-14 14:49:48 UTC
Created attachment 67138 [details] [review]
Bug 19021 - inventory column sorting

In inventory result page, items are shown in a JS DataTable.
For an inventory with barcode file second column is not sortable for no reason.
For an inventory without barcode file first column is not sortable but since it is the default sort order the sorting icon is still displayed.

This patch corrects by setting default sort order on callnumber column.

Test plan :
- perform inventory with barcode file
- look at result table
- table is sorted by callnumber
=> Without patch second column is not sortable
=> With patch all columns are sortable
- perform inventory without barcode file
- look at result table
=> Without patch first column show sort icon and table is not sorted by callnumber
=> With patch first column is not sortable and table is sorted by callnumber
Comment 7 Fridolin Somers 2017-09-14 14:51:53 UTC
(In reply to Nick Clemens from comment #5)
> > => With patch first column is not sortable and table is sorted by barcode
> 
> When performing an inventory without a barcode file we are essentially
> generating a shelf list, shouldn't callnumber (or cn_sort ideally) be the
> default sort here?
> 
> Even when comparing to a barcode file I think we should probably be sorting
> by callnumber in our results

Makes sens to me, I have pushed new patch.

Using cn_sort is I think possible but needs some dev so it would be in another bug report.

Regards ;)
Comment 8 Katrin Fischer 2017-10-07 14:41:50 UTC
Created attachment 67804 [details] [review]
Bug 19021 - inventory column sorting

In inventory result page, items are shown in a JS DataTable.
For an inventory with barcode file second column is not sortable for no reason.
For an inventory without barcode file first column is not sortable but since it is the default sort order the sorting icon is still displayed.

This patch corrects by setting default sort order on callnumber column.

Test plan :
- perform inventory with barcode file
- look at result table
- table is sorted by callnumber
=> Without patch second column is not sortable
=> With patch all columns are sortable
- perform inventory without barcode file
- look at result table
=> Without patch first column show sort icon and table is not sorted by callnumber
=> With patch first column is not sortable and table is sorted by callnumber

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 9 Kyle M Hall 2017-10-27 14:01:33 UTC
Created attachment 68715 [details] [review]
Bug 19021 - inventory column sorting

In inventory result page, items are shown in a JS DataTable.
For an inventory with barcode file second column is not sortable for no reason.
For an inventory without barcode file first column is not sortable but since it is the default sort order the sorting icon is still displayed.

This patch corrects by setting default sort order on callnumber column.

Test plan :
- perform inventory with barcode file
- look at result table
- table is sorted by callnumber
=> Without patch second column is not sortable
=> With patch all columns are sortable
- perform inventory without barcode file
- look at result table
=> Without patch first column show sort icon and table is not sorted by callnumber
=> With patch first column is not sortable and table is sorted by callnumber

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 10 Jonathan Druart 2017-10-27 19:11:02 UTC
Pushed to master for 17.11, thanks to everybody involved!
Comment 11 David Cook 2018-09-28 02:48:03 UTC
(In reply to Nick Clemens from comment #5)
> > => With patch first column is not sortable and table is sorted by barcode
> 
> When performing an inventory without a barcode file we are essentially
> generating a shelf list, shouldn't callnumber (or cn_sort ideally) be the
> default sort here?
> 
> Even when comparing to a barcode file I think we should probably be sorting
> by callnumber in our results

Just found this problem in the wild, so I'm glad you piped up about call number last year, Nick!
Comment 12 David Cook 2018-09-28 02:53:35 UTC
Oh the "Export to CSV file" doesn't sort by call number though. I'll look at master to see if that's still an issue...
Comment 13 David Cook 2018-09-28 03:29:15 UTC
(In reply to David Cook from comment #12)
> Oh the "Export to CSV file" doesn't sort by call number though. I'll look at
> master to see if that's still an issue...

OK it should be sorting using cn_sort as per commit 885c68e269419813dafde37fbeb32052e16974a1. 

I'll investigate more and stop spamming here...
Comment 14 David Cook 2018-09-28 03:41:47 UTC
I need to do more investigation, but here's the relevant SQL query:

SELECT items.itemnumber, barcode, itemcallnumber, items.cn_sort, title, author, biblio.biblionumber, biblio.frameworkcode, datelastseen, homebranch, location, notforloan, damaged, itemlost, withdrawn, stocknumber, ccode
FROM items
LEFT JOIN biblio ON items.biblionumber = biblio.biblionumber
LEFT JOIN biblioitems on items.biblionumber = biblioitems.biblionumber
WHERE ccode = ? AND items.homebranch = ? ORDER BY items.cn_sort, itemcallnumber, title

I have different items with the same itemcallnumber values but different cn_sort values, so they get sorted into different parts of the list, which are completely not intuitive to the user.

This patch's template fix remedies that on the user interface but not in the exported shelf list. 

I'm going to check into this more this afternoon...
Comment 15 David Cook 2018-09-28 05:36:42 UTC
I'm not very familiar with this code, but I think this might actually be a data issue rather than a code issue.

I had 2 items with near identical itemcallnumbers but very different cn_sort values:

AL AH 616.951 SEX 2013  -  616_951000000000000_SEX_2013
AL AH 616.951 SEX 2010  -  AL_AH_616_951000000000000_SEX_2010

I've just edited the first item in the staff client and it's changed 616_951000000000000_SEX_2013 to AL_AH_616_951000000000000_SEX_2013.

I'm trying to find the code responsible for generating the cn_sort value, but I'm guessing that the code must have changed at some point, and the database upgrade script didn't re-run the cn_sort re-generation en masse, which is fair enough since that would be a big task for some libraries. 

I'm going to run touch_all_items.pl and see if it solves my problem without applying the patch on this bug report...
Comment 16 David Cook 2018-09-28 06:05:55 UTC
Running touch_all_items.pl did the trick. Now everything is sorting correctly by cn_sort even without this patch. I'll open up a separate bugzilla issue regarding this...