From df39d76f305deec6cf7c938299cd1b05f8b32591 Mon Sep 17 00:00:00 2001 From: Martin Renvoize 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. Signed-off-by: David Nind Signed-off-by: Nick Clemens Signed-off-by: Katrin Fischer --- C4/Circulation.pm | 33 ++++++++-------- t/db_dependent/Circulation.t | 74 +++++++++++++++++++++++++++++++++++- 2 files changed, 90 insertions(+), 17 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 522ac9d768..13d1758f49 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1235,37 +1235,38 @@ sub CanBookBeIssued { ## CHECK FOR BOOKINGS if ( my $booking = $item_object->find_booking( - { - checkout_date => $now, - due_date => $duedate, - patron_id => $patron->borrowernumber - } + { checkout_date => $now, due_date => $duedate, 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 { $alerts{'BOOKED'} = $booking; } - } - # Booking starts before due date, reduce loan? - elsif ( $duedate > dt_from_string( $booking->start_date ) ) { - $needsconfirmation{'BOOKED_TO_ANOTHER'} = $booking; - } + } else { - # 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 eb0aa2eeb1..8dd246578f 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; @@ -4648,6 +4649,77 @@ 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.39.2