From d5fbd5b3591d7984bc1bc57a2c83f32fc2a8ce5d Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
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
---
 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 282c547afe5..55f6e66a806 100755
--- a/admin/branches.pl
+++ b/admin/branches.pl
@@ -90,7 +90,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 {
@@ -118,12 +123,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($_) || undef )
+                          : ( $_ => scalar $input->param($_) )
+                    } @fields
+                )
             }
         );
 
diff --git a/installer/data/mysql/atomicupdate/bug_28567.perl b/installer/data/mysql/atomicupdate/bug_28567.perl
new file mode 100644
index 00000000000..462b10c5915
--- /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");
+}
-- 
2.20.1