From 00ffab60ba0166a2df58e783198d2a96b7c08fcd Mon Sep 17 00:00:00 2001
From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Date: Mon, 29 Jan 2024 16:32:09 +0000
Subject: [PATCH] Bug 35248: Unit tests for CanBookBeIssued

Whilst writing the test, I found a minor flaw in the logic and fixed
that in CanBookBeIssued at the same time.
---
 C4/Circulation.pm            | 30 +++++++++-------
 t/db_dependent/Circulation.t | 66 +++++++++++++++++++++++++++++++++++-
 2 files changed, 83 insertions(+), 13 deletions(-)

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 109809d4b99..b30d51e9515 100644
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -1251,31 +1251,37 @@ sub CanBookBeIssued {
                 patron_id     => $patron->borrowernumber
             }
         )
-        )
+      )
     {
+        my $booking_start = dt_from_string( $booking->start_date );
+
         # Booked to this patron :)
         if ( $booking->patron_id == $patron->borrowernumber ) {
-            if ( $now < dt_from_string( $booking->start_date ) ) {
+            if ( $now < $booking_start ) {
                 $needsconfirmation{'BOOKED_EARLY'} = $booking;
-            } else {
+            }
+            else {
                 $alerts{'BOOKED'} = $booking;
             }
         }
 
-        # Booking starts before due date, reduce loan?
-        elsif ( $duedate > dt_from_string( $booking->start_date ) ) {
-            $needsconfirmation{'BOOKED_TO_ANOTHER'} = $booking;
-        }
-
-        # Loan falls inside booking
         else {
-            $issuingimpossible{'BOOKED_TO_ANOTHER'} = $booking;
+            # Loan falls inside booking
+            if ( $now > $booking_start ) {
+                $issuingimpossible{'BOOKED_TO_ANOTHER'} = $booking;
+            }
+
+            # Booking starts before due date, reduce loan?
+            else {
+                $needsconfirmation{'BOOKED_TO_ANOTHER'} = $booking;
+            }
         }
     }
 
     ## CHECK AGE RESTRICTION
-    my $agerestriction  = $biblioitem->agerestriction;
-    my ($restriction_age, $daysToAgeRestriction) = GetAgeRestriction( $agerestriction, $patron->unblessed );
+    my $agerestriction = $biblioitem->agerestriction;
+    my ( $restriction_age, $daysToAgeRestriction ) =
+      GetAgeRestriction( $agerestriction, $patron->unblessed );
     if ( $daysToAgeRestriction && $daysToAgeRestriction > 0 ) {
         if ( C4::Context->preference('AgeRestrictionOverride') ) {
             $needsconfirmation{AGE_RESTRICTION} = "$agerestriction";
diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t
index 5095afde735..c837c77d416 100755
--- a/t/db_dependent/Circulation.t
+++ b/t/db_dependent/Circulation.t
@@ -18,7 +18,7 @@
 use Modern::Perl;
 use utf8;
 
-use Test::More tests => 68;
+use Test::More tests => 69;
 use Test::Exception;
 use Test::MockModule;
 use Test::Deep qw( cmp_deeply );
@@ -42,6 +42,7 @@ use C4::Overdues qw( CalcFine UpdateFine get_chargeable_units );
 use C4::Members::Messaging qw( SetMessagingPreference );
 use Koha::DateUtils qw( dt_from_string output_pref );
 use Koha::Database;
+use Koha::Bookings;
 use Koha::Items;
 use Koha::Item::Transfers;
 use Koha::Checkouts;
@@ -4584,6 +4585,69 @@ subtest 'CanBookBeIssued | recalls' => sub {
     $recall->set_cancelled;
 };
 
+subtest 'CanBookBeIssued | bookings' => sub {
+    plan tests => 4;
+
+    my $schema = Koha::Database->schema;
+    $schema->storage->txn_begin;
+
+    my $patron1 = $builder->build_object( { class => 'Koha::Patrons' } );
+    my $patron2 = $builder->build_object( { class => 'Koha::Patrons' } );
+    my $item    = $builder->build_sample_item( { bookable => 1 } );
+
+    # item-level booking
+    my $booking = Koha::Booking->new(
+        {
+            patron_id  => $patron1->borrowernumber,
+            item_id    => $item->itemnumber,
+            biblio_id  => $item->biblio->biblionumber,
+            start_date => dt_from_string()->subtract( days => 1),
+            end_date   => dt_from_string()->add( days => 6 ),
+        }
+    )->store();
+
+    # Booking encompasses proposed checkout
+    my ( $issuingimpossible, $needsconfirmation, $alerts, $messages ) =
+      CanBookBeIssued( $patron2, $item->barcode,
+        dt_from_string()->add( days => 5 ),
+        undef, undef, undef );
+    is(
+        $issuingimpossible->{BOOKED_TO_ANOTHER}->booking_id,
+        $booking->booking_id,
+        "Another patron has booked this item with a start date before the proposed due date"
+    );
+
+    ( $issuingimpossible, $needsconfirmation, $alerts, $messages ) =
+      CanBookBeIssued( $patron1, $item->barcode,
+        dt_from_string()->add( days => 5 ),
+        undef, undef, undef );
+    is( $alerts->{BOOKED}->booking_id,
+        $booking->booking_id, "Booked to this user" );
+
+    # Booking start will clash before issue due
+    $booking->start_date( dt_from_string()->add( days => 3 ) )->store();
+
+    ( $issuingimpossible, $needsconfirmation, $alerts, $messages ) =
+      CanBookBeIssued( $patron2, $item->barcode,
+        dt_from_string()->add( days => 5 ),
+        undef, undef, undef );
+    is(
+        $needsconfirmation->{BOOKED_TO_ANOTHER}->booking_id,
+        $booking->booking_id,
+"Another patron has booked this item for a period starting before the proposed due date"
+    );
+
+    ( $issuingimpossible, $needsconfirmation, $alerts, $messages ) =
+      CanBookBeIssued( $patron1, $item->barcode,
+        dt_from_string()->add( days => 5 ),
+        undef, undef, undef );
+    is( $needsconfirmation->{BOOKED_EARLY}->booking_id,
+        $booking->booking_id,
+        "Booked to this user, but they're collecting early" );
+
+    $schema->storage->txn_rollback;
+};
+
 subtest 'AddReturn should clear items.onloan for unissued items' => sub {
     plan tests => 1;
 
-- 
2.43.0