Now that the sorting of a table can be defined in administration > table settings, we should move OPACHoldingsDefaultSortField there and remove the pref.
Does 23.05 include the ability to sort the holdingst table? There is no sort capability on that table in 22.11 so the system preference is still needed.
I believe making this happen would solve bug 35319 - Default sorting of the holdings table with OPACHoldingsDefaultSortField is broken?
Feature introduced by bug 16552, accidentally removed by bug 16759.
I can work on this if we get one tester and one QAer in advance for quick inclusion.
*** Bug 35319 has been marked as a duplicate of this bug. ***
We don't have server access but assuming this is testable in a sandbox, I'll try for the first signoff.
Created attachment 187856 [details] [review] Bug 30633: Prepare the ground Make the OPAC holdings table ready to use visibility based on column's names.
Created attachment 187857 [details] [review] Bug 30633: Hide columns when needed We can use the force_visibility flag to hide column when they are empty or should not be displayed because of syspref.
Created attachment 187858 [details] [review] Bug 30633: Add default_sort_order to table settings So we can pick a column from the table settings http://localhost:8081/cgi-bin/koha/admin/columns_settings.pl#biblio-detail%7Choldingst
Created attachment 187859 [details] [review] Bug 30633: Remove OPACHoldingsDefaultSortField It's no longer needed
I am suggesting to remove OPACHoldingsDefaultSortField and have the default sort order option available for this table in the table settings. With OpacLocationBranchToDisplay (introduced by bug 35006) it might improve the situation and answer the different needs.
Created attachment 187886 [details] [review] Bug 30633: Prepare the ground Make the OPAC holdings table ready to use visibility based on column's names. Signed-off-by: Michaela <michaela.sieber@kit.edu>
Created attachment 187887 [details] [review] Bug 30633: Hide columns when needed We can use the force_visibility flag to hide column when they are empty or should not be displayed because of syspref. Signed-off-by: Michaela <michaela.sieber@kit.edu>
Created attachment 187888 [details] [review] Bug 30633: Add default_sort_order to table settings So we can pick a column from the table settings http://localhost:8081/cgi-bin/koha/admin/columns_settings.pl#biblio-detail%7Choldingst Signed-off-by: Michaela <michaela.sieber@kit.edu>
Created attachment 187889 [details] [review] Bug 30633: Remove OPACHoldingsDefaultSortField It's no longer needed Signed-off-by: Michaela <michaela.sieber@kit.edu>
Tested in Sandbox and additionally with ktd. Testplan: - search for a biblio with multiple items, e.g. biblionumber=211 - change the items so that they have different home libraries, current libraries, itemtypes, collections, and so on ... - try different variations of sysprefs OPACHoldingsDefaultSortField and check that the sort order in the opac is always by item type - apply patch - Go to system preferences and verify that OPACHoldingsDefaultSortField does not exist any more - Go to table settings and check the new drop down for default sort order - try different variations of the default sort order settings and verify that the holdings table in the opac is sorted correctly
I'm happy to QA this, but we need someone to test and signoff on bug 41010 first to unblock this.
Michaela found a bug when testing this, I opened bug 41042.
Created attachment 188067 [details] [review] Bug 30633: Default to 'Item type'
Created attachment 188068 [details] [review] Bug 30633: Default to 'Current library' if item_level_itypes is off We cannot set the default value in the yaml file dynamically. This is not ideal but we are going to modify the default "Item type" if item-level_itypes is off.
Created attachment 188069 [details] [review] Bug 30633: Prepare the ground Make the OPAC holdings table ready to use visibility based on column's names. Signed-off-by: Michaela <michaela.sieber@kit.edu> Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Created attachment 188070 [details] [review] Bug 30633: Hide columns when needed We can use the force_visibility flag to hide column when they are empty or should not be displayed because of syspref. Signed-off-by: Michaela <michaela.sieber@kit.edu> Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Created attachment 188071 [details] [review] Bug 30633: Add default_sort_order to table settings So we can pick a column from the table settings http://localhost:8081/cgi-bin/koha/admin/columns_settings.pl#biblio-detail%7Choldingst Signed-off-by: Michaela <michaela.sieber@kit.edu> Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Created attachment 188072 [details] [review] Bug 30633: Remove OPACHoldingsDefaultSortField It's no longer needed Signed-off-by: Michaela <michaela.sieber@kit.edu> Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Created attachment 188073 [details] [review] Bug 30633: Default to 'Item type' Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Created attachment 188074 [details] [review] Bug 30633: Default to 'Current library' if item_level_itypes is off We cannot set the default value in the yaml file dynamically. This is not ideal but we are going to modify the default "Item type" if item-level_itypes is off. Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Hmm, 1) OPAC datatables.js lacks force_visibility - The OPAC version doesn't have _dt_force_visibility() function like staff interface, so are we sure this is working for the OPAC? 2) Could we perhaps use the following for ordering instead of relying on indexes that may well change with future work: // Find column by name instead of index let current_location_col = holdingst_table_settings.columns.findIndex( c => c.columnname == 'item_current_location' ); holdingst_table_settings.default_sort_order = current_location_col; 3) We need to make it clear in the release notes that the preference content is not migrated into the dt configs
Created attachment 188192 [details] [review] Bug 30633: Remove force_visibility flag It's not available at the OPAC, and we actually don't need it: we don't save/restore DT's state.
(In reply to Martin Renvoize (ashimema) from comment #27) > Hmm, > > 1) OPAC datatables.js lacks force_visibility - The OPAC version doesn't have > _dt_force_visibility() function like staff interface, so are we sure this is > working for the OPAC? Indeed, it's not needed at the OPAC. force_visibility has been introduced to force visiblity to restore DT's state. And it is not available for OPAC tables. > 2) Could we perhaps use the following for ordering instead of relying on > indexes that may well change with future work: > > // Find column by name instead of index > let current_location_col = holdingst_table_settings.columns.findIndex( > c => c.columnname == 'item_current_location' > ); > holdingst_table_settings.default_sort_order = current_location_col; Maybe, I don't know. We are storing/manipulating the index in the yaml and the DB for default_sort_order. We could try and see if it's possible to remove the index completely and use the column's names everywhere, but that's for another day :) > 3) We need to make it clear in the release notes that the preference content > is not migrated into the dt configs Added something to the release notes.
(In reply to Jonathan Druart from comment #29) > (In reply to Martin Renvoize (ashimema) from comment #27) > > 2) Could we perhaps use the following for ordering instead of relying on > > indexes that may well change with future work: > > > > // Find column by name instead of index > > let current_location_col = holdingst_table_settings.columns.findIndex( > > c => c.columnname == 'item_current_location' > > ); > > holdingst_table_settings.default_sort_order = current_location_col; > > Maybe, I don't know. We are storing/manipulating the index in the yaml and > the DB for default_sort_order. > We could try and see if it's possible to remove the index completely and use > the column's names everywhere, but that's for another day :) > Hey Martin and Jonathan, we would very much appreciate it if the column's names would be used. If you want to put this topic on the roadmap for 26.05, I'm happy to take care of testing it.
Created attachment 188286 [details] [review] Bug 30633: Prepare the ground Make the OPAC holdings table ready to use visibility based on column's names. Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Created attachment 188287 [details] [review] Bug 30633: Hide columns when needed We can use the force_visibility flag to hide column when they are empty or should not be displayed because of syspref. Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Created attachment 188288 [details] [review] Bug 30633: Add default_sort_order to table settings So we can pick a column from the table settings http://localhost:8081/cgi-bin/koha/admin/columns_settings.pl#biblio-detail%7Choldingst Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Created attachment 188289 [details] [review] Bug 30633: Remove OPACHoldingsDefaultSortField It's no longer needed Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Created attachment 188290 [details] [review] Bug 30633: Default to 'Item type' Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Created attachment 188291 [details] [review] Bug 30633: Default to 'Current library' if item_level_itypes is off We cannot set the default value in the yaml file dynamically. This is not ideal but we are going to modify the default "Item type" if item-level_itypes is off. Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Created attachment 188292 [details] [review] Bug 30633: Remove force_visibility flag It's not available at the OPAC, and we actually don't need it: we don't save/restore DT's state. Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Thanks everyone, Passing QA
Nice work everyone! Pushed to main for 25.11
Nice work everyone! Pushed to 25.05.x