[Patches] [PATCH] Bug 6976 Close loophole allowing borrowers extra holds

[koha-patchbot at kohaaloha.com]

From: Colin Campbell <colin.campbell at ptfs-europe.com>
Date: Thu, 13 Oct 2011 16:28:59 +0100
Subject: [PATCH] Bug 6976 Close loophole allowing borrowers extra holds
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The check that the max number of reserves has
not been reached needs to take account of the fact
that we may have added enouch reserves to
reach the limit while this script is running.
Add a check against the incrementing count

The variable canreserve was only used when looping
through multiple holds but was not being
reinitialized on every iteration
make variable local to loop it is used in and
initialize to zero

Some numeric comparisons were using a string operator
fix them before strange bugs manifest themselves

The loophole manifests thus:
borrowers are only allowed Y holds.  If holds are done individually, it is
obeying that law i.e. X has held Y separate books – when I tried to do a 6th
book it told me that I could not place a hold.  However, if a borrower checks
the catalogue, gets the intial list up and clicks items on the “results” box
and then clicks “place hold” they can hold more than Y items
---
 opac/opac-reserve.pl |   36 ++++++++++++++++++++++++++++++------
 1 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl
index 2713cf3..49bf1ab 100755
--- a/opac/opac-reserve.pl
+++ b/opac/opac-reserve.pl
@@ -164,7 +164,14 @@ foreach my $biblioNumber (@biblionumbers) {
 #
 if ( $query->param('place_reserve') ) {
     my $notes = $query->param('notes');
-	my $canreserve=0;
+    my $reserve_cnt;
+    if ( $MAXIMUM_NUMBER_OF_RESERVES ) {
+        my @res = GetReservesFromBorrowernumber( $borrowernumber );
+        $reserve_cnt = @res;
+    }
+    else {
+        $reserve_cnt = 0;
+    }
 
     # List is composed of alternating biblio/item/branch
     my $selectedItems = $query->param('selecteditems');
@@ -197,6 +204,8 @@ if ( $query->param('place_reserve') ) {
         my $itemNum   = shift(@selectedItems);
         my $branch    = shift(@selectedItems); # i.e., branch code, not name
 
+        my $canreserve = 0;
+
         my $singleBranchMode = $template->param('singleBranchMode');
         if ($singleBranchMode || ! $OPACChooseBranch) { # single branch mode or disabled user choosing
             $branch = $borr->{'branchcode'};
@@ -241,9 +250,24 @@ if ( $query->param('place_reserve') ) {
             $itemNum = undef;
         }
 
+        if (   $MAXIMUM_NUMBER_OF_RESERVES
+            && $reserve_cnt >= $MAXIMUM_NUMBER_OF_RESERVES )
+        {
+            $canreserve = 0;
+        }
+
         # Here we actually do the reserveration. Stage 3.
-        AddReserve($branch, $borrowernumber, $biblioNum, 'a', [$biblioNum], $rank, $startdate, $expiration_date, $notes,
-                   $biblioData->{'title'}, $itemNum, $found) if ($canreserve);
+        if ($canreserve) {
+            AddReserve(
+                $branch,      $borrowernumber,
+                $biblioNum,   'a',
+                [$biblioNum], $rank,
+                $startdate,   $expiration_date,
+                $notes,       $biblioData->{title},
+                $itemNum,     $found
+            );
+            ++$reserve_cnt;
+        }
     }
 
     print $query->redirect("/cgi-bin/koha/opac-user.pl#opac-user-holds");
@@ -264,21 +288,21 @@ if ( $borr->{'amountoutstanding'} && ($borr->{'amountoutstanding'} > $maxoutstan
     $noreserves = 1;
     $template->param( too_much_oweing => $amount );
 }
-if ( $borr->{gonenoaddress} && ($borr->{gonenoaddress} eq 1) ) {
+if ( $borr->{gonenoaddress} && ($borr->{gonenoaddress} == 1) ) {
     $noreserves = 1;
     $template->param(
                      message => 1,
                      GNA     => 1
                     );
 }
-if ( $borr->{lost} && ($borr->{lost} eq 1) ) {
+if ( $borr->{lost} && ($borr->{lost} == 1) ) {
     $noreserves = 1;
     $template->param(
                      message => 1,
                      lost    => 1
                     );
 }
-if ( $borr->{debarred} && ($borr->{debarred} eq 1) ) {
+if ( $borr->{debarred} && ($borr->{debarred} == 1) ) {
     $noreserves = 1;
     $template->param(
                      message  => 1,
-- 
1.7.5.1