Bug 22806: (QA follow-up)

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

C4/Reserves.pm

@@ -321,7 +321,7 @@ sub CanBookBeReserved{
     # Check that patron have not checked out this biblio (if AllowHoldsOnPatronsPossessions set)
     if ( !C4::Context->preference('AllowHoldsOnPatronsPossessions')
         && C4::Circulation::CheckIfIssuedToPatron( $borrowernumber, $biblionumber ) ) {
-        return { status =>'itemAlreadyOnLoan' };
+        return { status =>'alreadypossession' };
     }
 
     my @itemnumbers = Koha::Items->search({ biblionumber => $biblionumber})->get_column("itemnumber");
@@ -395,7 +395,7 @@ sub CanItemBeReserved {
     # Check that patron have not checked out this biblio (if AllowHoldsOnPatronsPossessions set)
     if ( !C4::Context->preference('AllowHoldsOnPatronsPossessions')
         && C4::Circulation::CheckIfIssuedToPatron( $patron->borrowernumber, $biblio->biblionumber ) ) {
-        return { status =>'itemAlreadyOnLoan' };
+        return { status =>'alreadypossession' };
     }
 
     my $controlbranch = C4::Context->preference('ReservesControlBranch');

koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt

@@ -200,15 +200,15 @@
                                     [% IF ( bibitemloo.already_reserved ) %]
                                         <div class="alert alert-warning">You have already requested this title.</div>
                                     [% ELSE %]
-                                        [% UNLESS ( bibitemloo.bib_available ) %]
-                                            <div class="alert alert-warning">There are no items that can be placed on hold.</div>
+                                        [% IF ( bibitemloo.already_patron_possession ) %]
+                                            <div class="alert alert-warning">This title cannot be requested because it's already in your possession.</div>
                                         [% ELSE %]
-                                            [% IF ( bibitemloo.already_patron_possession ) %]
-                                                <div class="alert alert-warning">This title cannot be requested because it's already in your possession.</div>
+                                            [% UNLESS ( bibitemloo.bib_available ) %]
+                                                <div class="alert">There are no items that can be placed on hold.</div>
                                             [% ELSE %]
-                                                <div class="alert alert-warning">This title cannot be requested.</div>
+                                                <div class="alert">This title cannot be requested.</div>
                                             [% END %]
-                                        [% END # / UNLESS bibitemloo.bib_available %]
+                                        [% END # / UNLESS bibitemloo.already_patron_possession %]
                                     [% END # / IF bibitemloo.already_reserved %]
                                 [% END # / UNLESS bibitemloo.holdable %]
 

opac/opac-reserve.pl

@@ -601,11 +601,6 @@ foreach my $biblioNum (@biblionumbers) {
         $biblioLoopIter{holdable} = undef;
         $biblioLoopIter{itemholdable} = undef;
     }
-    if(not C4::Context->preference('AllowHoldsOnPatronsPossessions') and CheckIfIssuedToPatron($borrowernumber,$biblioNum)) {
-        $biblioLoopIter{holdable} = undef;
-        $biblioLoopIter{already_patron_possession} = 1;
-    }
-
     if ( $biblioLoopIter{holdable} ) {
         @not_available_at = uniq @not_available_at;
         $biblioLoopIter{not_available_at} = \@not_available_at ;
@@ -620,7 +615,9 @@ foreach my $biblioNum (@biblionumbers) {
         }
     }
 
-    $biblioLoopIter{holdable} &&= CanBookBeReserved( $borrowernumber, $biblioNum )->{status} eq 'OK';
+    my $status = CanBookBeReserved( $borrowernumber, $biblioNum )->{status};
+    $biblioLoopIter{holdable} &&= $status eq 'OK';
+    $biblioLoopIter{already_patron_possession} = $status eq 'alreadypossession';
 
     # For multiple holds per record, if a patron has previously placed a hold,
     # the patron can only place more holds of the same type. That is, if the

reserve/request.pl

@@ -314,6 +314,10 @@ foreach my $biblionumber (@biblionumbers) {
                 $template->param( $canReserve->{status} => 1 );
                 $biblioloopiter{ $canReserve->{status} } = 1;
             }
+            elsif ( $canReserve->{status} eq 'alreadypossession' ) {
+                $template->param( $canReserve->{status} => 1);
+                $biblioloopiter{ $canReserve->{status} } = 1;
+            }
             else {
                 $biblioloopiter{ $canReserve->{status} } = 1;
             }
@@ -342,16 +346,6 @@ foreach my $biblionumber (@biblionumbers) {
         $biblioloopiter{remaining_holds_for_record} = $max_holds_for_record;
         $template->param( max_holds_for_record => $max_holds_for_record );
         $template->param( remaining_holds_for_record => $remaining_holds_for_record );
-
-        { # alreadypossession
-            # Check to see if patron is allowed to place holds on records where the
-            # patron already has an item from that record checked out
-            if ( !C4::Context->preference('AllowHoldsOnPatronsPossessions')
-                && CheckIfIssuedToPatron( $patron->borrowernumber, $biblionumber ) )
-            {
-                $template->param( alreadypossession => 1, );
-            }
-        }
     }
 
 

t/db_dependent/ILSDI_Services.t

@@ -424,7 +424,7 @@ subtest 'Holds test' => sub {
     $query->param( 'pickup_location', $origin_branch->{branchcode});
     $reply = C4::ILSDI::Services::HoldItem( $query );
 
-    is( $reply->{code}, 'itemAlreadyOnLoan', "Patron has issued same book" );
+    is( $reply->{code}, 'alreadypossession', "Patron has issued same book" );
     is( $reply->{pickup_location}, undef, "No reserve placed");
 
     $schema->storage->txn_rollback;

t/db_dependent/Reserves.t

@@ -17,7 +17,7 @@
 
 use Modern::Perl;
 
-use Test::More tests => 64;
+use Test::More tests => 65;
 use Test::MockModule;
 use Test::Warn;
 
@@ -1169,12 +1169,12 @@ subtest 'AllowHoldOnPatronPossession test' => sub {
 
     is(C4::Reserves::CanBookBeReserved($patron->borrowernumber,
                                        $item->biblionumber)->{status},
-       'itemAlreadyOnLoan',
+       'alreadypossession',
        'Patron cannot place hold on a book loaned to itself');
 
     is(C4::Reserves::CanItemBeReserved($patron->borrowernumber,
                                        $item->itemnumber)->{status},
-       'itemAlreadyOnLoan',
+       'alreadypossession',
        'Patron cannot place hold on an item loaned to itself');
 
     t::lib::Mocks::mock_preference('AllowHoldsOnPatronsPossessions', 1);