Attachment 123247 Details for Bug 28567 – Bug 28567: Fix 0 vs "" vs undef on the admin library form

[patch] Bug 28567: Fix 0 vs "" vs undef on the admin library form

Bug-28567-Fix-0-vs--vs-undef-on-the-admin-library-.patch (text/plain), 3.93 KB, created by Marcel de Rooy on 2021-07-28 12:01:26 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Marcel de Rooy

Created: 2021-07-28 12:01:26 UTC

Size: 3.93 KB

patch

obsolete

>From b26291d918e577ca236059c2181e7d2b9065a78f 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
>Content-Type: text/plain; charset=utf-8
>
>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 <oleonard@myacpl.org>
>
>Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>---
> 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 b821f2008a..66b69868f0 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($_) )
>+                          : ( $_ => 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");
>+}
>-- 
>2.20.1

Actions: View | Diff | Splinter Review

Attachments on bug 28567: 122424 | 122461 | 122603 | 122772 | 122799 | 123247