Bug 38356: Check patrons current checkouts separately

If syspref CheckPrevCheckout is enabled, Koha checks if patron
has previously checked out item within timeperiod set in
syspref CheckPrevCheckoutDelay. This however doesn't take into
account cases where item is currently checked out for patron and
they are trying to check out another item from same record (can
be done if syspref AllowMultipleIssuesOnABiblio is enabled). We
should always check if item is currently checked out for the
patron if CheckPrevCheckout is enabled.

To test:
1. Enable syspref CheckPrevCheckout and set CheckPrevCheckoutDelay
as e.g. 10. Also enable syspref AllowMultipleIssuesOnABiblio.
2. Find a record that has been checked out for patron over 10 days ago.
3. Attempt to check out item from same record for same patron.
=> Note that you have to confirm if you want to check out item for
the patron.
4. Find record that is currently checked out for a patron.
5. Attempt to check out item from same record for same patron.
=> Note that item is checked out for patron without confirm message.
6. Apply this patch and restart services if needed.
7. Repeat steps 2. and 3.
=> Confirm that you still have to confirm if you want to check out
item for the patron.
8. Repeat steps 4. and 5.
=> New confirm message "Patron has this title currently checked out:..."
should be displayed and you have to confirm if you want to check out
item for the patron.

Also prove t/db_dependent/Patron/Borrower_PrevCheckout.t

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Felicie <felicie.thiery@biblibre.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

C4/Circulation.pm

@@ -1066,8 +1066,13 @@ sub CanBookBeIssued {
     #
     $patron = Koha::Patrons->find( $patron->borrowernumber )
         ;    # FIXME Refetch just in case, to avoid regressions. But must not be needed
-    if ( $patron->wants_check_for_previous_checkout && $patron->do_check_for_previous_checkout($item_unblessed) ) {
-        $needsconfirmation{PREVISSUE} = 1;
+    my $check_previous_checkout = $patron->do_check_for_previous_checkout($item_unblessed);
+    if ( $patron->wants_check_for_previous_checkout && $check_previous_checkout ) {
+        if ( $check_previous_checkout eq "currentlycheckedout" ) {
+            $needsconfirmation{CURRENTISSUE} = 1;
+        } else {
+            $needsconfirmation{PREVISSUE} = 1;
+        }
     }
 
     #

Koha/Patron.pm

@@ -865,6 +865,10 @@ sub do_check_for_previous_checkout {
         itemnumber     => \@item_nos,
     };
 
+    # Check current issues table
+    my $issues = Koha::Checkouts->search($criteria);
+    return "currentlycheckedout" if $issues->count;    # 0 || N
+
     my $delay = C4::Context->preference('CheckPrevCheckoutDelay') || 0;
     if ($delay) {
         my $dtf        = Koha::Database->new->schema->storage->datetime_parser;
@@ -872,13 +876,9 @@ sub do_check_for_previous_checkout {
         $criteria->{'returndate'} = { '>' => $dtf->format_datetime($newer_than), };
     }
 
-    # Check current issues table
-    my $issues = Koha::Checkouts->search($criteria);
-    return 1 if $issues->count;    # 0 || N
-
     # Check old issues table
     my $old_issues = Koha::Old::Checkouts->search($criteria);
-    return $old_issues->count;     # 0 || N
+    return $old_issues->count;                         # 0 || N
 }
 
 =head3 is_debarred

koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt

@@ -272,6 +272,10 @@
                     <li class="needsconfirm highholds">High demand item. Loan period shortened to [% HIGHHOLDS.duration | html %] days (due [% HIGHHOLDS.returndate | $KohaDates %]). Check out anyway?</li>
                 [% END %]
 
+                [% IF CURRENTISSUE %]
+                    <li class="needsconfirm currentissue">Patron has this title currently checked out: <strong>[% biblio.title | html %] [% IF biblio.author %]by [% biblio.author | html %][% END %]</strong>. Check out anyway?</li>
+                [% END %]
+
                 [% IF PREVISSUE %]
                     <li class="needsconfirm previssue">Patron has previously checked out this title: <strong>[% biblio.title | html %] [% IF biblio.author %]by [% biblio.author | html %][% END %]</strong>. Check out anyway?</li>
                 [% END %]

t/db_dependent/Patron/Borrower_PrevCheckout.t

@@ -8,7 +8,7 @@ use Koha::DateUtils qw( dt_from_string );
 use Koha::Patrons;
 
 use Test::NoWarnings;
-use Test::More tests => 62;
+use Test::More tests => 64;
 
 use_ok('Koha::Patron');
 
@@ -311,6 +311,52 @@ BAIL_OUT("Issue failed")
 
 # Then test:
 my $cpvPmappings = [
+    {
+        msg    => "Same item, same patron [1]",
+        item   => $item_1,
+        patron => $patron,
+        result => "currentlycheckedout",
+    },
+    {
+        msg    => "Diff item, same bib, same patron [1]",
+        item   => $item_2,
+        patron => $patron,
+        result => "currentlycheckedout",
+    },
+    {
+        msg    => "Diff item, diff bib, same patron [0]",
+        item   => $item_d,
+        patron => $patron,
+        result => 0,
+    },
+    {
+        msg    => "Same item, diff patron [0]",
+        item   => $item_1,
+        patron => $patron_d,
+        result => 0,
+    },
+    {
+        msg    => "Diff item, same bib, diff patron [0]",
+        item   => $item_2,
+        patron => $patron_d,
+        result => 0,
+    },
+    {
+        msg    => "Diff item, diff bib, diff patron [0]",
+        item   => $item_d,
+        patron => $patron_d,
+        result => 0,
+    },
+];
+
+test_it( $cpvPmappings, "PostIssue" );
+
+# Return item_1 from patron:
+BAIL_OUT("Return Failed") unless AddReturn( $item_1->{barcode}, $patron->{branchcode} );
+
+#Since currently checked in item now return status "currentlycheckedout" we need use
+#same test scenarions for returned item as above but without "currentlycheckedout"
+$cpvPmappings = [
     {
         msg    => "Same item, same patron [1]",
         item   => $item_1,
@@ -349,11 +395,6 @@ my $cpvPmappings = [
     },
 ];
 
-test_it( $cpvPmappings, "PostIssue" );
-
-# Return item_1 from patron:
-BAIL_OUT("Return Failed") unless AddReturn( $item_1->{barcode}, $patron->{branchcode} );
-
 # Then:
 test_it( $cpvPmappings, "PostReturn" );
 
@@ -364,7 +405,18 @@ test_it( $cpvPmappings, "PostReturn" );
 # whetherthe different combinational outcomes of the above return values in
 # CanBookBeIssued result in the appropriate $needsconfirmation.
 
-# We want to test:
+# We want to test when item is currently issued to the patron:
+# - DESCRIPTION [RETURNVALUE (0/1)]
+# - patron, !wants_check_for_previous_checkout, !do_check_for_previous_checkout
+#   [!$issuingimpossible,!$needsconfirmation->{CURRENTISSUE}]
+# - patron, wants_check_for_previous_checkout, !do_check_for_previous_checkout
+#   [!$issuingimpossible,!$needsconfirmation->{CURRENTISSUE}]
+# - patron, !wants_check_for_previous_checkout, do_check_for_previous_checkout
+#   [!$issuingimpossible,!$needsconfirmation->{CURRENTISSUE}]
+# - patron, wants_check_for_previous_checkout, do_check_for_previous_checkout
+#   [!$issuingimpossible,$needsconfirmation->{CURRENTISSUE}]
+
+# And we also need to test when item has been previously issued to the patron:
 # - DESCRIPTION [RETURNVALUE (0/1)]
 # - patron, !wants_check_for_previous_checkout, !do_check_for_previous_checkout
 #   [!$issuingimpossible,!$needsconfirmation->{PREVISSUE}]
@@ -424,6 +476,32 @@ my $CBBI_mappings = [
     },
 ];
 
+# Tests
+map {
+    t::lib::Mocks::mock_preference( 'checkprevcheckout', $_->{syspref} );
+    my ( $issuingimpossible, $needsconfirmation ) = C4::Circulation::CanBookBeIssued( $patron, $_->{item}->{barcode} );
+    is( $needsconfirmation->{CURRENTISSUE}, $_->{result}, $_->{msg} );
+} @{$CBBI_mappings};
+
+# Return $prev_item from patron:
+BAIL_OUT("Return Failed") unless AddReturn( $prev_item->{barcode}, $patron->{branchcode} );
+
+# Mappings
+$CBBI_mappings = [
+    {
+        syspref => 'hardno',
+        item    => $prev_item,
+        result  => undef,
+        msg     => "patron, !wants_check_for_previous_checkout, do_check_for_previous_checkout"
+    },
+    {
+        syspref => 'hardyes',
+        item    => $prev_item,
+        result  => 1,
+        msg     => "patron, wants_check_for_previous_checkout, do_check_for_previous_checkout"
+    },
+];
+
 # Tests
 map {
     t::lib::Mocks::mock_preference( 'checkprevcheckout', $_->{syspref} );
@@ -456,7 +534,7 @@ subtest 'Check previous checkouts for serial' => sub {
     AddIssue( $patron, $item1->barcode );
 
     is(
-        $patron->do_check_for_previous_checkout( $item1->unblessed ), 1,
+        $patron->do_check_for_previous_checkout( $item1->unblessed ), "currentlycheckedout",
         'Check only one item if bibliographic record is serial'
     );
     is(