From a844a2211f58cb6f19efa88568bca300287dcd60 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Fri, 25 Jun 2021 12:44:19 +0200 Subject: [PATCH] Bug 28567: Fix 0 vs "" vs undef on the admin library form There are two things here: * Branches.pickup_location has a default = 1 in DB, we should not set to undef if 0 or it will be set to 1 when stored. * The other fields are all text (varchar, mediumtext or longtext) and can be NULL. They are correct set to NULL when a new library is created but set to an empty string when the library is modified. That's not consistent Test plan: 0. Don't apply the patch 1. Create a new library, set pickup location to "No" 2. Save => Pickup location is set to YES => In DB notice that the different values you didn't fill in are set to NULL 3. Edit the library 4. Save => In DB notice that the different values you didn't fill in are now set to an empty string 5. Apply the patch, restart_all 6. Run the updatedatabase script => In DB all the empty string values are set to NULL 7. Repeat 1 to 4 and confirm that everything is now working as expected Signed-off-by: Owen Leonard Signed-off-by: Marcel de Rooy Signed-off-by: Jonathan Druart Signed-off-by: Kyle M Hall --- admin/branches.pl | 20 ++++++++--- .../data/mysql/atomicupdate/bug_28567.perl | 34 +++++++++++++++++++ 2 files changed, 50 insertions(+), 4 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug_28567.perl diff --git a/admin/branches.pl b/admin/branches.pl index 9bcb379178..ffe12f3170 100755 --- a/admin/branches.pl +++ b/admin/branches.pl @@ -91,7 +91,12 @@ if ( $op eq 'add_form' ) { if ($is_a_modif) { my $library = Koha::Libraries->find($branchcode); for my $field (@fields) { - $library->$field( scalar $input->param($field) ); + if ( $field eq 'pickup_location' ) { + # Don't fallback to undef/NULL, default is 1 in DB + $library->$field( scalar $input->param($field) ); + } else { + $library->$field( scalar $input->param($field) || undef ); + } } try { @@ -119,12 +124,19 @@ if ( $op eq 'add_form' ) { } catch { push @messages, { type => 'alert', code => 'error_on_update' }; - } + }; } else { $branchcode =~ s|\s||g; my $library = Koha::Library->new( - { branchcode => $branchcode, - ( map { $_ => scalar $input->param($_) || undef } @fields ) + { + branchcode => $branchcode, + ( + map { + $_ eq 'pickup_location' # Don't fallback to undef/NULL, default is 1 in DB + ? ( $_ => scalar $input->param($_) ) + : ( $_ => scalar $input->param($_) || undef ) + } @fields + ) } ); diff --git a/installer/data/mysql/atomicupdate/bug_28567.perl b/installer/data/mysql/atomicupdate/bug_28567.perl new file mode 100644 index 0000000000..462b10c591 --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_28567.perl @@ -0,0 +1,34 @@ +$DBversion = 'XXX'; # will be replaced by the RM +if( CheckVersion( $DBversion ) ) { + + my @fields = qw( + branchname + branchaddress1 + branchaddress2 + branchaddress3 + branchzip + branchcity + branchstate + branchcountry + branchphone + branchfax + branchemail + branchillemail + branchreplyto + branchreturnpath + branchurl + branchip + branchnotes + opac_info + marcorgcode + ); + for my $f ( @fields ) { + $dbh->do(qq{ + UPDATE branches + SET $f = NULL + WHERE $f = "" + }); + } + + NewVersion( $DBversion, 28567, "Set to NULL empty branches fields"); +}