Attachment 161900 Details for Bug 35248 – Bug 35248: Unit tests for CanBookBeIssued

[patch] Bug 35248: Unit tests for CanBookBeIssued

Bug-35248-Unit-tests-for-CanBookBeIssued.patch (text/plain), 6.14 KB, created by Nick Clemens (kidclamp) on 2024-02-08 13:10:29 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Nick Clemens (kidclamp)

Created: 2024-02-08 13:10:29 UTC

Size: 6.14 KB

patch

obsolete

>From f8cba3fbbcad9ece8e28a21acbe32ac5de899e44 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.
>
>Signed-off-by: David Nind <david@davidnind.com>
>Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
>---
> 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 109809d4b99..5f348af7826 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -1245,37 +1245,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 5095afde735..c5d0b2a60b2 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,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.30.2

Actions: View | Diff | Splinter Review

Attachments on bug 35248: 161061 | 161062 | 161608 | 161611 | 161620 | 161621 | 161632 | 161636 | 161651 | 161652 | 161653 | 161654 | 161655 | 161656 | 161657 | 161658 | 161659 | 161660 | 161821 | 161822 | 161823 | 161824 | 161825 | 161826 | 161827 | 161828 | 161829 | 161830 | 161831 | 161861 | 161862 | 161863 | 161864 | 161865 | 161866 | 161867 | 161868 | 161869 | 161870 | 161871 | 161890 | 161891 | 161893 | 161894 | 161895 | 161896 | 161897 | 161898 | 161899 | 161900 | 161901 | 161902 | 161903 | 161904 | 161905 | 161906 | 161907 | 161908 | 161909 | 161910 | 161911 | 161912