[Patches] [PATCH] [Signed off] bug_2830: Remove reserve when checking out if the borrower is not the first one in the reserve queue

koha-patchbot at kohaaloha.com koha-patchbot at kohaaloha.com
Fri Nov 18 06:00:03 NZDT 2011


From: Srdjan Jankovic <srdjan at catalyst.net.nz>
Date: Thu, 13 Oct 2011 16:57:41 +1300
Subject: [PATCH] [Signed off] bug_2830: Remove reserve when checking out if the borrower is not the first one in the reserve queue
Content-Type: text/plain; charset="utf-8"

To test:
Create 4 holds on a bib, for patrons A, B, C, and D,

Check in the item to mark hold as waiting for patron A
Check out the item to patron B -> reserve for patron B should be removed
Check in the item to mark hold as waiting for patron A
Check out the item to Patron A, hold should complete normally
Check in the item to mark hold as waiting for patron C
Check out the item to patron D -> reserve for patron D should be removed.
Check in the item to mark hold as waiting for patron C
Check out the item to patron C, hold should complete normally
Check in the item -> there should be no more reserves.

We also tested:
Created 4 holds on a bib with two items, for patrons A, B, C, and D

All worked as expected.

Signed-off-by: Liz Rea <wizzyrea at gmail.com>
---
 C4/Circulation.pm                       |   42 ++-----------------
 C4/Items.pm                             |    2 +-
 C4/Reserves.pm                          |   67 ++++++++++++++++++++++++++-----
 C4/Search.pm                            |    4 +-
 C4/XSLT.pm                              |    2 +-
 circ/circulation.pl                     |    2 +-
 opac/opac-user.pl                       |    2 +-
 t/db_dependent/Reserves.t               |    6 +-
 t/db_dependent/lib/KohaTest/Reserves.pm |    1 +
 9 files changed, 72 insertions(+), 56 deletions(-)

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 9f81773..12f0898 100644
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -319,7 +319,7 @@ sub transferbook {
 
     # find reserves.....
     # That'll save a database query.
-    my ( $resfound, $resrec ) =
+    my ( $resfound, $resrec, undef ) =
       CheckReserves( $itemnumber );
     if ( $resfound and not $ignoreRs ) {
         $resrec->{'ResFound'} = $resfound;
@@ -869,7 +869,7 @@ sub CanBookBeIssued {
     }
 
     # See if the item is on reserve.
-    my ( $restype, $res ) = C4::Reserves::CheckReserves( $item->{'itemnumber'} );
+    my ( $restype, $res, undef ) = C4::Reserves::CheckReserves( $item->{'itemnumber'} );
     if ($restype) {
 		my $resbor = $res->{'borrowernumber'};
 		my ( $resborrower ) = C4::Members::GetMember( borrowernumber => $resbor );
@@ -983,39 +983,7 @@ sub AddIssue {
 				);
 			}
 
-			# See if the item is on reserve.
-			my ( $restype, $res ) =
-			  C4::Reserves::CheckReserves( $item->{'itemnumber'} );
-			if ($restype) {
-				my $resbor = $res->{'borrowernumber'};
-				if ( $resbor eq $borrower->{'borrowernumber'} ) {
-					# The item is reserved by the current patron
-					ModReserveFill($res);
-				}
-				elsif ( $restype eq "Waiting" ) {
-					# warn "Waiting";
-					# The item is on reserve and waiting, but has been
-					# reserved by some other patron.
-				}
-				elsif ( $restype eq "Reserved" ) {
-					# warn "Reserved";
-					# The item is reserved by someone else.
-					if ($cancelreserve) { # cancel reserves on this item
-						CancelReserve(0, $res->{'itemnumber'}, $res->{'borrowernumber'});
-					}
-				}
-				if ($cancelreserve) {
-					CancelReserve($res->{'biblionumber'}, 0, $res->{'borrowernumber'});
-				}
-				else {
-					# set waiting reserve to first in reserve queue as book isn't waiting now
-					ModReserve(1,
-						$res->{'biblionumber'},
-						$res->{'borrowernumber'},
-						$res->{'branchcode'}
-					);
-				}
-			}
+            MoveReserve( $item->{'itemnumber'}, $borrower->{'borrowernumber'}, $cancelreserve );
 
 			# Starting process for transfer job (checking transfert and validate it if we have one)
             my ($datesent) = GetTransfers($item->{'itemnumber'});
@@ -1636,7 +1604,7 @@ sub AddReturn {
 
     # find reserves.....
     # if we don't have a reserve with the status W, we launch the Checkreserves routine
-    my ($resfound, $resrec) = C4::Reserves::CheckReserves( $item->{'itemnumber'} );
+    my ($resfound, $resrec, undef) = C4::Reserves::CheckReserves( $item->{'itemnumber'} );
     if ($resfound) {
           $resrec->{'ResFound'} = $resfound;
         $messages->{'ResFound'} = $resrec;
@@ -2199,7 +2167,7 @@ sub CanBookBeRenewed {
 			$error="too_many";
 		}
 		
-        my ( $resfound, $resrec ) = C4::Reserves::CheckReserves($itemnumber);
+        my ( $resfound, $resrec, undef ) = C4::Reserves::CheckReserves($itemnumber);
         if ($resfound) {
             $renewokay = 0;
 			$error="on_reserve"
diff --git a/C4/Items.pm b/C4/Items.pm
index 0b91b65..6429251 100644
--- a/C4/Items.pm
+++ b/C4/Items.pm
@@ -1228,7 +1228,7 @@ sub GetItemsInfo {
 			$serial = 1;
         }
 		if ( $datedue eq '' ) {
-            my ( $restype, $reserves ) =
+            my ( $restype, $reserves, undef ) =
               C4::Reserves::CheckReserves( $data->{'itemnumber'} );
 # Previous conditional check with if ($restype) is not needed because a true
 # result for one item will result in subsequent items defaulting to this true
diff --git a/C4/Reserves.pm b/C4/Reserves.pm
index 4af8a85..3259a68 100644
--- a/C4/Reserves.pm
+++ b/C4/Reserves.pm
@@ -109,6 +109,7 @@ BEGIN {
         &ModReserveStatus
         &ModReserveCancelAll
         &ModReserveMinusPriority
+        &MoveReserve
         
         &CheckReserves
         &CanBookBeReserved
@@ -523,7 +524,7 @@ sub GetOtherReserves {
     my ($itemnumber) = @_;
     my $messages;
     my $nextreservinfo;
-    my ( $restype, $checkreserves ) = CheckReserves($itemnumber);
+    my ( undef, $checkreserves, undef ) = CheckReserves($itemnumber);
     if ($checkreserves) {
         my $iteminfo = GetItem($itemnumber);
         if ( $iteminfo->{'holdingbranch'} ne $checkreserves->{'branchcode'} ) {
@@ -738,8 +739,8 @@ sub GetReserveStatus {
 
 =head2 CheckReserves
 
-  ($status, $reserve) = &CheckReserves($itemnumber);
-  ($status, $reserve) = &CheckReserves(undef, $barcode);
+  ($status, $reserve, $all_reserves) = &CheckReserves($itemnumber);
+  ($status, $reserve, $all_reserves) = &CheckReserves(undef, $barcode);
 
 Find a book in the reserves.
 
@@ -804,11 +805,11 @@ sub CheckReserves {
     # note: we get the itemnumber because we might have started w/ just the barcode.  Now we know for sure we have it.
     my ( $biblio, $bibitem, $notforloan_per_itemtype, $notforloan_per_item, $itemnumber ) = $sth->fetchrow_array;
 
-    return ( 0, 0 ) unless $itemnumber; # bail if we got nothing.
+    return ( '' ) unless $itemnumber; # bail if we got nothing.
 
     # if item is not for loan it cannot be reserved either.....
     #    execpt where items.notforloan < 0 :  This indicates the item is holdable. 
-    return ( 0, 0 ) if  ( $notforloan_per_item > 0 ) or $notforloan_per_itemtype;
+    return ( '' ) if  ( $notforloan_per_item > 0 ) or $notforloan_per_itemtype;
 
     # Find this item in the reserves
     my @reserves = _Findgroupreserve( $bibitem, $biblio, $itemnumber );
@@ -822,7 +823,7 @@ sub CheckReserves {
         my $priority = 10000000;
         foreach my $res (@reserves) {
             if ( $res->{'itemnumber'} == $itemnumber && $res->{'priority'} == 0) {
-                return ( "Waiting", $res ); # Found it
+                return ( "Waiting", $res, \@reserves ); # Found it
             } else {
                 # See if this item is more important than what we've got so far
                 if ( $res->{'priority'} && $res->{'priority'} < $priority ) {
@@ -843,11 +844,10 @@ sub CheckReserves {
     # We return the most important (i.e. next) reservation.
     if ($highest) {
         $highest->{'itemnumber'} = $item;
-        return ( "Reserved", $highest );
-    }
-    else {
-        return ( 0, 0 );
+        return ( "Reserved", $highest, \@reserves );
     }
+
+    return ( '' );
 }
 
 =head2 CancelExpiredReserves
@@ -1816,6 +1816,53 @@ sub _ShiftPriorityByDateAndPriority {
     return $new_priority;  # so the caller knows what priority they wind up receiving
 }
 
+=head2 MoveReserve
+
+  MoveReserve( $itemnumber, $borrowernumber, $cancelreserve )
+
+Use when checking out an item to handle reserves
+If $cancelreserve boolean is set to true, it will remove existing reserve
+
+=cut
+
+sub MoveReserve {
+    my ( $itemnumber, $borrowernumber, $cancelreserve ) = @_;
+
+    my ( $restype, $res, $all_reserves ) = CheckReserves( $itemnumber );
+    return unless $res;
+
+    my $biblionumber     =  $res->{biblionumber};
+    my $biblioitemnumber = $res->{biblioitemnumber};
+
+    if ($res->{borrowernumber} == $borrowernumber) {
+        ModReserveFill($res);
+    }
+    else {
+        # warn "Reserved";
+        # The item is reserved by someone else.
+        # Find this item in the reserves
+
+        my $borr_res;
+        foreach (@$all_reserves) {
+            $_->{'borrowernumber'} == $borrowernumber or next;
+            $_->{'biblionumber'}   == $biblionumber   or next;
+
+            $borr_res = $_;
+            last;
+        }
+        
+        if ( $borr_res ) {
+            # The item is reserved by the current patron
+            ModReserveFill($borr_res);
+        }
+
+        if ($cancelreserve) { # cancel reserves on this item
+            CancelReserve(0, $res->{'itemnumber'}, $res->{'borrowernumber'});
+            CancelReserve($res->{'biblionumber'}, 0, $res->{'borrowernumber'});
+        }
+    }
+}
+
 =head2 MergeHolds
 
   MergeHolds($dbh,$to_biblio, $from_biblio);
diff --git a/C4/Search.pm b/C4/Search.pm
index 98db887..428cb26 100644
--- a/C4/Search.pm
+++ b/C4/Search.pm
@@ -1673,7 +1673,7 @@ sub searchResults {
                 my ($transfertfrom, $transfertto);
 
                 # is item on the reserve shelf?
-		my $reservestatus = 0;
+		my $reservestatus = '';
 		my $reserveitem;
 
                 unless ($item->{wthdrawn}
@@ -1695,7 +1695,7 @@ sub searchResults {
                     #        should map transit status to record indexed in Zebra.
                     #
                     ($transfertwhen, $transfertfrom, $transfertto) = C4::Circulation::GetTransfers($item->{itemnumber});
-		    ($reservestatus, $reserveitem) = C4::Reserves::CheckReserves($item->{itemnumber});
+		    ($reservestatus, $reserveitem, undef) = C4::Reserves::CheckReserves($item->{itemnumber});
                 }
 
                 # item is withdrawn, lost, damaged, not for loan, reserved or in transit
diff --git a/C4/XSLT.pm b/C4/XSLT.pm
index 8bc4000..39071d5 100755
--- a/C4/XSLT.pm
+++ b/C4/XSLT.pm
@@ -190,7 +190,7 @@ sub buildKohaItemsNamespace {
 
         my ( $transfertwhen, $transfertfrom, $transfertto ) = C4::Circulation::GetTransfers($item->{itemnumber});
 
-	my ( $reservestatus, $reserveitem ) = C4::Reserves::CheckReserves($item->{itemnumber});
+	my ( $reservestatus, $reserveitem, undef ) = C4::Reserves::CheckReserves($item->{itemnumber});
 
         if ( $itemtypes->{ $item->{itype} }->{notforloan} || $item->{notforloan} || $item->{onloan} || $item->{wthdrawn} || $item->{itemlost} || $item->{damaged} || 
              (defined $transfertwhen && $transfertwhen ne '') || $item->{itemnotforloan} || (defined $reservestatus && $reservestatus eq "Waiting") ){ 
diff --git a/circ/circulation.pl b/circ/circulation.pl
index efb87da..5c239ce 100755
--- a/circ/circulation.pl
+++ b/circ/circulation.pl
@@ -430,7 +430,7 @@ sub build_issue_data {
             $it->{'borrowernumber'},$it->{'itemnumber'}
         );
         $it->{"renew_error_${can_renew_error}"} = 1 if defined $can_renew_error;
-        my ( $restype, $reserves ) = CheckReserves( $it->{'itemnumber'} );
+        my ( $restype, $reserves, undef ) = CheckReserves( $it->{'itemnumber'} );
         $it->{'can_renew'} = $can_renew;
         $it->{'can_confirm'} = !$can_renew && !$restype;
         $it->{'renew_error'} = $restype;
diff --git a/opac/opac-user.pl b/opac/opac-user.pl
index 03c9e81..6952a85 100755
--- a/opac/opac-user.pl
+++ b/opac/opac-user.pl
@@ -139,7 +139,7 @@ my $canrenew = 0;
 if ($issues){
 	foreach my $issue ( sort { $b->{'date_due'} cmp $a->{'date_due'} } @$issues ) {
 		# check for reserves
-		my ( $restype, $res ) = CheckReserves( $issue->{'itemnumber'} );
+		my ( $restype, $res, undef ) = CheckReserves( $issue->{'itemnumber'} );
 		if ( $restype ) {
 			$issue->{'reserved'} = 1;
 		}
diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t
index 177febb..ca4d42e 100755
--- a/t/db_dependent/Reserves.t
+++ b/t/db_dependent/Reserves.t
@@ -51,12 +51,12 @@ AddReserve($branch,    $borrowernumber, $biblionumber,
         $constraint, $bibitems,  $priority,       $notes,
         $title,      $checkitem, $found);
         
-my ($status, $reserve) = CheckReserves($itemnumber, $barcode);
+my ($status, $reserve, $all_reserves) = CheckReserves($itemnumber, $barcode);
 ok($status eq "Reserved", "CheckReserves Test 1");
 
-($status, $reserve) = CheckReserves($itemnumber);
+($status, $reserve, $all_reserves) = CheckReserves($itemnumber);
 ok($status eq "Reserved", "CheckReserves Test 2");
 
-($status, $reserve) = CheckReserves(undef, $barcode);
+($status, $reserve, $all_reserves) = CheckReserves(undef, $barcode);
 ok($status eq "Reserved", "CheckReserves Test 3");
 
diff --git a/t/db_dependent/lib/KohaTest/Reserves.pm b/t/db_dependent/lib/KohaTest/Reserves.pm
index 5317029..8b05dd0 100644
--- a/t/db_dependent/lib/KohaTest/Reserves.pm
+++ b/t/db_dependent/lib/KohaTest/Reserves.pm
@@ -29,6 +29,7 @@ sub methods : Test( 1 ) {
                        ModReserveAffect 
                        ModReserveCancelAll 
                        ModReserveMinusPriority 
+                       MoveReserve
                        GetReserveInfo 
                        _FixPriority 
                        _Findgroupreserve 
-- 
1.7.2.5


More information about the Patches mailing list