Bug 38079

Summary: Add ability to choose sorting for holdings table
Product: Koha Reporter: Nick Clemens (kidclamp) <nick>
Component: Staff interfaceAssignee: Nick Clemens (kidclamp) <nick>
Status: Failed QA --- QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: catrina, chlee, david, emily.lamancusa, fridolin.somers, gmcharlt, jonathan.druart, kebliss, lisette, maxeinergl, mspinney, victor
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38107
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Bug Depends on:    
Bug Blocks: 38080    
Attachments: Bug 38079: Add ability to choose sorting for holdings tables on details page

Description Nick Clemens (kidclamp) 2024-10-03 17:15:31 UTC
We should add the option for sorting the holdings tables on details display
Comment 1 Nick Clemens (kidclamp) 2024-10-03 17:23:23 UTC
Created attachment 172365 [details] [review]
Bug 38079: Add ability to choose sorting for holdings tables on details page

This patch adds a new option 'default' to the sorting options in tables settings

We need this so that we can add the option to define sorting, without setting a new
default as the holdings table currently uses 'search_ordered' for a specific sorting
when no sorting is provided

To test:
 1 - Add many items to a record, with various values
 2 - View the holdings tab on details, note the order
 3 - Apply patch, restart_all, reload
 4 - Confirm order not changed
 5 - Click 'Configure'
 6 - Scroll to holdings table
 7 - Change page size and save (noting sort is set to default)
 8 - Reload and confirm the order has not changed
 9 - Now change the sorting in the table settings
10 - Reload and confirm the sorting is saved
Comment 2 Nick Clemens (kidclamp) 2024-10-03 17:24:08 UTC
There is a flaw in the design here that I cannot overcome. If a user has saved settings for a table, then sorting is NULL in the db - and if it is NULL you cannot set the sorting. 

We cannot override the DB from the yaml so I don't know how to correct if a user has already saved the page size for this table (or any in the group) since sort and page are in the same row.

We can't set NULL to 'default' because some tables shouldn't have the option and now they would. I am submitting my patch anyways :-)
Comment 3 Jonathan Druart 2024-10-08 13:54:11 UTC
Does it mean we could add this "default" to all tables actually? Instead of one bug per table :)
Comment 4 Fridolin Somers 2024-10-09 15:01:11 UTC
Looks great.

But I'm confused with default sort order = default.
Default of default ?

Maybe we could use 'inherit' like in CSS ?
Or 'none'
Comment 5 Catrina Berka 2024-10-11 16:29:42 UTC

*** This bug has been marked as a duplicate of bug 34638 ***
Comment 6 Nick Clemens (kidclamp) 2024-10-23 14:16:18 UTC
*** Bug 34638 has been marked as a duplicate of this bug. ***
Comment 7 Nick Clemens (kidclamp) 2024-10-23 14:18:22 UTC
(In reply to Fridolin Somers from comment #4)
> Looks great.
> 
> But I'm confused with default sort order = default.
> Default of default ?
> 
> Maybe we could use 'inherit' like in CSS ?
> Or 'none'

Some tables have an order built in to the api - items/holdings for example - so we don't want to automatically set this to overwrite.

I think 'default' make sense, none might be a lie.
Comment 8 Nick Clemens (kidclamp) 2024-10-23 14:19:00 UTC
(In reply to Jonathan Druart from comment #3)
> Does it mean we could add this "default" to all tables actually? Instead of
> one bug per table :)

That was the idea, add a general way to add the setting without affecting existing settings
Comment 9 Fridolin Somers 2024-10-25 08:04:22 UTC
(In reply to Nick Clemens (kidclamp) from comment #7)
> (In reply to Fridolin Somers from comment #4)
> > Looks great.
> > 
> > But I'm confused with default sort order = default.
> > Default of default ?
> > 
> > Maybe we could use 'inherit' like in CSS ?
> > Or 'none'
> 
> Some tables have an order built in to the api - items/holdings for example -
> so we don't want to automatically set this to overwrite.
> 
> I think 'default' make sense, none might be a lie.

OK all good :)
Comment 10 David Nind 2024-10-28 03:28:03 UTC
It doesn't seem to be working correctly for me:

1. I added several more items, and changed the data for items with different library branches, and item types, and checked some items out

2. If I set the default order in the table settings, it seems to choose another column to sort by, for example:
  . holdings_lastseen ==> sorts by Renewals
  . holdings_holdingbranch ==> Home library
  . holdings_itype ==> Current library
  . holdings_barcode ==> Copy number (had to change the columns to display)
Comment 11 Emily Lamancusa (emlam) 2024-10-29 19:45:23 UTC
(In reply to David Nind from comment #10)
> It doesn't seem to be working correctly for me:
> 
> 1. I added several more items, and changed the data for items with different
> library branches, and item types, and checked some items out
> 
> 2. If I set the default order in the table settings, it seems to choose
> another column to sort by, for example:
>   . holdings_lastseen ==> sorts by Renewals
>   . holdings_holdingbranch ==> Home library
>   . holdings_itype ==> Current library
>   . holdings_barcode ==> Copy number (had to change the columns to display)

Hmmm, this sounds similar to https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33568#c66:
> 3. If StaffDetailItemSelection and/or LocalCoverImages is turned off, the
> alignment of the drop-down filters for the other columns is offset by 1 for
> each of the two sysprefs that is disabled
> Example: StaffDetailItemSelection is enabled; LocalCoverImages is disabled.
> The first 5 columns are the selection column, Item Type, Current Library, Home
> Library, Collection.
> Item Type has a text filter instead of a drop-down.
> Current Library has a drop-down, but it contains Item Type values (and is thus
> useless).
> Home Library has a drop-down containing branch names (but this is presumably
> still shifted from Current Library)
> Collection has a drop-down containing branch names (again, useless)
> ** If the cover images column is hidden, but not disabled by syspref, the
> offset does not occur.

Which was fixed by commit a767e15c74bd63f4eed51beee982fc5ddd468a02: Fix columns shift when pref are off

Is the same thing happening here?
Comment 12 Jonathan Druart 2024-10-30 09:05:01 UTC
I would recommend to build anything DataTables related on top of bug 36640.
Comment 13 Victor Grousset/tuxayo 2024-11-23 21:41:20 UTC
Doesn't apply anymore, likely due to Bug 36640 being merged.

No sure that it will help with Comment 10