From 065b0ba0ce689d5f15d0dab74089b7ea355ee9b8 Mon Sep 17 00:00:00 2001
From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Date: Tue, 4 Jun 2024 11:58:54 +0100
Subject: [PATCH] Bug 37000: [Bug 36120 follow-up] Improve reliability of
 database update

This patch adds a series of fallthroughs to ensure pickup_library_id is
always set prior to adding the NOT NULL constraint.

We initially only looked at items.homebranch but as that's a nullable
field itself, we now look at items.holdingbranch before finally
defaulting to the first available branch in the branches table in the
worst case.
---
 installer/data/mysql/db_revs/231200044.pl | 40 +++++++++++++++++++----
 1 file changed, 33 insertions(+), 7 deletions(-)

diff --git a/installer/data/mysql/db_revs/231200044.pl b/installer/data/mysql/db_revs/231200044.pl
index 9ed97b8ec44..af6b69966d7 100755
--- a/installer/data/mysql/db_revs/231200044.pl
+++ b/installer/data/mysql/db_revs/231200044.pl
@@ -1,4 +1,5 @@
 use Modern::Perl;
+use Koha::Installer::Output qw(say_warning say_failure say_success say_info);
 
 return {
     bug_number  => "36120",
@@ -14,24 +15,49 @@ return {
               ADD COLUMN `pickup_library_id` varchar(10) DEFAULT NULL COMMENT 'Identifier for booking pickup library' AFTER `item_id`,
               ADD CONSTRAINT `bookings_ibfk_4` FOREIGN KEY (`pickup_library_id`) REFERENCES `branches` (`branchcode`) ON DELETE CASCADE ON UPDATE CASCADE
           }
+                ) == 1
+                && say_success( $out, "Added column 'bookings.pickup_library_id'" );
+
+            my $updated = $dbh->do(
+                q{UPDATE bookings JOIN items ON bookings.item_id = items.itemnumber SET bookings.pickup_library_id = items.homebranch }
             );
 
-            say $out "Added column 'bookings.pickup_library_id'";
+            if ( $updated != '0E0' ) {
+                say_success( $out, "Set $updated existing bookings pickup location to item homebranch" );
+            } else {
+                say_info( $out, "No bookings found that need updating to include a pickup library" );
+            }
 
-            $dbh->do(
-                q{UPDATE bookings JOIN items ON bookings.item_id = items.itemnumber SET bookings.pickup_library_id = items.homebranch }
+            $updated = $dbh->do(
+                q{UPDATE bookings JOIN items ON bookings.item_id = items.itemnumber SET pickup_library_id = items.holdingbranch WHERE pickup_library_id IS NULL}
             );
 
-            say $out "Set existing bookings pickup location to item homebranch";
+            if ( $updated != '0E0' ) {
+                say_success(
+                    $out,
+                    "Set $updated existing bookings pickup location to item holdingbranch where items.homebranch was null"
+                );
+            }
+
+            my ($firstBranch) = $dbh->selectrow_array(q{SELECT branchcode FROM branches LIMIT 1});
+            $updated = $dbh->do(
+                q{UPDATE bookings SET pickup_library_id = ? WHERE pickup_library_id IS NULL}, undef,
+                $firstBranch
+            );
+
+            if ( $updated != '0E0' ) {
+                say_warning(
+                    $out,
+                    "Some $updated bookings still had a null pickup location value so we have set them to $firstBranch"
+                );
+            }
 
             $dbh->do(
                 q{
               ALTER TABLE bookings
               MODIFY pickup_library_id varchar(10) NOT NULL COMMENT 'Identifier for booking pickup library'
           }
-            );
-
-            say $out "Set pickup_library_id to NOT NULL";
+            ) == 1 && say_success( $out, "Updated column 'bookings.pickup_library_id' to NOT NULL" );
         }
     },
 };
-- 
2.45.1