From 44b757e72302052c41531a15c47d292d2efb29bd Mon Sep 17 00:00:00 2001 From: Colin Campbell 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