From 6c0808293050b2ab402f354e1c1df06c196ed730 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Fri, 17 Oct 2025 10:04:09 +0200 Subject: [PATCH] Bug 41042: Make 'default sort order' available to new installations If a row exists in DB for a given table, default_sort_order will be NULL in DB which will replace the flag from the YAML file: the feature won't be available. Test plan: 0. checkout main (we don't want bug 30633) 1. Go to http://localhost:8081/cgi-bin/koha/admin/columns_settings.pl#biblio-detail%7Choldingst => Notice that "Default sort order" is not available for 'holdingst' (it's added by bug 30633) 2. Save 3. Verify what you have in DB: select * from tables_settings where module="opac" and tablename="holdingst"; 4. Apply 30633, restart_all 5. Go to http://localhost:8081/cgi-bin/koha/admin/columns_settings.pl#biblio-detail%7Choldingst => Notice that "Default sort order" is not available for 'holdingst' (not ok) 6. Save 7. Verify what you have in DB: select * from tables_settings where module="opac" and tablename="holdingst"; 8. Apply this patch set 9. Go to http://localhost:8081/cgi-bin/koha/admin/columns_settings.pl#biblio-detail%7Choldingst => Notice that "Default sort order" is available for 'holdingst'! 10. Save, verify what you have in DB. --- C4/Utils/DataTables/TablesSettings.pm | 30 ++-- t/db_dependent/TablesSettings.t | 239 +++++++++++++++++++++++++- 2 files changed, 255 insertions(+), 14 deletions(-) diff --git a/C4/Utils/DataTables/TablesSettings.pm b/C4/Utils/DataTables/TablesSettings.pm index 1c3ab24ba11..e67520295c5 100644 --- a/C4/Utils/DataTables/TablesSettings.pm +++ b/C4/Utils/DataTables/TablesSettings.pm @@ -113,20 +113,24 @@ sub get_table_settings { } )->next; - my $table_settings = $list->{modules}->{$module}->{$page}->{$tablename}; + my $table_settings = $list->{modules}->{$module}->{$page}->{$tablename}; + my $default_display_length = $rs && $rs->default_display_length; + $default_display_length //= $table_settings->{default_display_length}; + + my $default_sort_order = $rs && $rs->default_sort_order; + $default_sort_order //= $table_settings->{default_sort_order}; + + my $default_save_state = $rs && $rs->default_save_state; + $default_save_state //= $table_settings->{default_save_state} // 0; + + my $default_save_state_search = $rs && $rs->default_save_state_search; + $default_save_state_search //= $table_settings->{default_save_state_search} // 0; + return { - default_display_length => $rs - ? $rs->default_display_length - : $table_settings->{default_display_length}, - default_sort_order => $rs - ? $rs->default_sort_order - : $table_settings->{default_sort_order}, - default_save_state => $rs ? $rs->default_save_state - : exists $table_settings->{default_save_state} ? $table_settings->{default_save_state} - : 1, - default_save_state_search => $rs ? $rs->default_save_state_search - : exists $table_settings->{default_save_state_search} ? $table_settings->{default_save_state_search} - : 0, + default_display_length => $default_display_length, + default_sort_order => $default_sort_order, + default_save_state => $default_save_state, + default_save_state_search => $default_save_state_search, }; } diff --git a/t/db_dependent/TablesSettings.t b/t/db_dependent/TablesSettings.t index fc846aaa807..15d605686b7 100755 --- a/t/db_dependent/TablesSettings.t +++ b/t/db_dependent/TablesSettings.t @@ -2,7 +2,7 @@ use Modern::Perl; use Test::NoWarnings; -use Test::More tests => 2; +use Test::More tests => 3; use Test::MockModule; use C4::Context; @@ -237,3 +237,240 @@ subtest 'update_columns, get_modules, get_columns, get_table_settings' => sub { $new_table_settings, ); }; + +subtest 'test upgrade' => sub { + + plan tests => 2; + + subtest 'has default_* in yaml' => sub { + + plan tests => 6; + + $schema->storage->txn_begin; + my $dbh = C4::Context->dbh; + + $dbh->do(q|DELETE FROM columns_settings|); + $dbh->do(q|DELETE FROM tables_settings|); + + $yaml = { + modules => { + admin => { + currency => { + 'currencies-table' => { + default_display_length => 20, + default_sort_order => 1, + default_save_state => 1, + default_save_state_search => 0, + columns => [ + { + columnname => 'currency', + cannot_be_toggled => '1', + cannot_be_modified => '1' + }, + ] + } + } + }, + } + }; + + my $modules_expected = { + 'admin' => { + 'currency' => { + 'currencies-table' => { + + # undef in DB we want the values from yaml + default_display_length => 20, + default_sort_order => 1, + default_save_state => 1, + default_save_state_search => 0, + columns => [ + { + columnname => 'currency', + cannot_be_toggled => 1, + cannot_be_modified => 1, + + # TODO + # hidden => 0 is missing because we don't add it in get_modules for all columns + # It should not cause any problems for now + }, + ] + } + } + } + }; + + my $modules = C4::Utils::DataTables::TablesSettings::get_modules(); + + is_deeply( $modules, $modules_expected, 'get_modules returns all values' ); + + my $columns = C4::Utils::DataTables::TablesSettings::get_columns( 'admin', 'currency', 'currencies-table' ); + is_deeply( $modules->{admin}, $modules_expected->{admin} ); + + my $table_settings = + C4::Utils::DataTables::TablesSettings::get_table_settings( 'admin', 'currency', 'currencies-table' ); + is_deeply( + $table_settings, + { + default_display_length => 20, + default_save_state => 1, + default_save_state_search => 0, + default_sort_order => 1 + } + ); + + C4::Utils::DataTables::TablesSettings::update_columns( + { + columns => [ + { + module => 'admin', + page => 'currency', + tablename => 'currencies-table', + columnname => 'currency', + cannot_be_toggled => 1, + cannot_be_modified => 1, + }, + ] + } + ); + + $table_settings = + C4::Utils::DataTables::TablesSettings::get_table_settings( 'admin', 'currency', 'currencies-table' ); + is_deeply( + $table_settings, + { + default_display_length => 20, + default_save_state => 1, + default_save_state_search => 0, + default_sort_order => 1 + } + ); + + C4::Utils::DataTables::TablesSettings::update_table_settings( + { + module => 'admin', + page => 'currency', + tablename => 'currencies-table', + default_display_length => undef, + default_sort_order => undef, + default_save_state => undef, + default_save_state_search => undef, + } + ); + + $table_settings = + C4::Utils::DataTables::TablesSettings::get_table_settings( 'admin', 'currency', 'currencies-table' ); + is_deeply( + $table_settings, + { + default_display_length => 20, + default_sort_order => 1, + default_save_state => 1, + default_save_state_search => 0, + } + ); + + C4::Utils::DataTables::TablesSettings::update_table_settings( + { + module => 'admin', + page => 'currency', + tablename => 'currencies-table', + default_display_length => 42, + default_sort_order => 2, + default_save_state => 0, + default_save_state_search => 0, + } + ); + + $table_settings = + C4::Utils::DataTables::TablesSettings::get_table_settings( 'admin', 'currency', 'currencies-table' ); + is_deeply( + $table_settings, + { + default_display_length => 42, + default_sort_order => 2, + default_save_state => 0, + default_save_state_search => 0, + } + ); + + }; + + subtest 'does not have default_ in yaml' => sub { + + plan tests => 3; + + $schema->storage->txn_begin; + my $dbh = C4::Context->dbh; + + $dbh->do(q|DELETE FROM columns_settings|); + $dbh->do(q|DELETE FROM tables_settings|); + + $yaml = { + modules => { + admin => { + currency => { + 'currencies-table' => { + columns => [ + { + columnname => 'currency', + cannot_be_toggled => '1', + cannot_be_modified => '1' + }, + ] + } + } + }, + } + }; + + my $modules_expected = { + 'admin' => { + 'currency' => { + 'currencies-table' => { + + # FIXME we don't include table settings if they are not present in the yaml + #default_display_length => undef, + #default_sort_order => undef, + #default_save_state => undef, + #default_save_state_search => undef, + columns => [ + { + columnname => 'currency', + cannot_be_toggled => 1, + cannot_be_modified => 1, + + # TODO + # hidden => 0 is missing because we don't add it in get_modules for all columns + # It should not cause any problems for now + }, + ] + } + } + } + }; + + my $modules = C4::Utils::DataTables::TablesSettings::get_modules(); + + is_deeply( $modules, $modules_expected, 'get_modules returns all values' ); + + my $columns = C4::Utils::DataTables::TablesSettings::get_columns( 'admin', 'currency', 'currencies-table' ); + is_deeply( $modules->{admin}, $modules_expected->{admin} ); + + my $table_settings = + C4::Utils::DataTables::TablesSettings::get_table_settings( 'admin', 'currency', 'currencies-table' ); + is_deeply( + $table_settings, + { + # default_display_length and default_sort_order default to undef + # => the feature is not available + default_display_length => undef, + default_sort_order => undef, + + # default_save_state is disabled by default but can be enabled. + default_save_state => 0, + default_save_state_search => 0, + } + ); + }; +}; -- 2.34.1