gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Bug 6976 Close loophole allowing borrowers extra holds via opac 

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

Have used perltidy to correct the indention of
the while loop as it was misleading to the reader

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 Please enter the commit message for your changes. Lines starting

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
This commit is contained in:
Colin Campbell 2012-08-28 14:34:55 +01:00 committed by Chris Cormack
parent 92da15bec3
commit 1f3fcc3abe
1 changed files with 53 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

84
opac/opac-reserve.pl
View File

@ -169,7 +169,10 @@ foreach my $biblioNumber (@biblionumbers) {
#
if ( $query->param('place_reserve') ) {
my $notes = $query->param('notes');
my $canreserve=0;
my $reserve_cnt = 0;
if ($MAXIMUM_NUMBER_OF_RESERVES) {
$reserve_cnt = GetReservesFromBorrowernumber( $borrowernumber );
}
# List is composed of alternating biblio/item/branch
my $selectedItems = $query->param('selecteditems');
@ -200,55 +203,74 @@ if ( $query->param('place_reserve') ) {
while (@selectedItems) {
my $biblioNum = shift(@selectedItems);
my $itemNum = shift(@selectedItems);
my $branch = shift(@selectedItems); # i.e., branch code, not name
my $branch = shift(@selectedItems); # i.e., branch code, not name
my $canreserve = 0;
my $singleBranchMode = C4::Context->preference("singleBranchMode");
if ($singleBranchMode || ! $OPACChooseBranch) { # single branch mode or disabled user choosing
if ( $singleBranchMode || !$OPACChooseBranch )
{ # single branch mode or disabled user choosing
$branch = $borr->{'branchcode'};
}
#item may belong to a host biblio, if yes change biblioNum to hosts bilbionumber
if ($itemNum ne '') {
my $hostbiblioNum = GetBiblionumberFromItemnumber($itemNum);
if ($hostbiblioNum ne $biblioNum) {
$biblioNum = $hostbiblioNum;
}
}
#item may belong to a host biblio, if yes change biblioNum to hosts bilbionumber
if ( $itemNum ne '' ) {
my $hostbiblioNum = GetBiblionumberFromItemnumber($itemNum);
if ( $hostbiblioNum ne $biblioNum ) {
$biblioNum = $hostbiblioNum;
}
}
my $biblioData = $biblioDataHash{$biblioNum};
my $found;
# Check for user supplied reserve date
my $startdate;
if (
C4::Context->preference( 'AllowHoldDateInFuture' ) &&
C4::Context->preference( 'OPACAllowHoldDateInFuture' )
) {
$startdate = $query->param("reserve_date_$biblioNum");
}
my $expiration_date = $query->param("expiration_date_$biblioNum");
# Check for user supplied reserve date
my $startdate;
if ( C4::Context->preference('AllowHoldDateInFuture')
&& C4::Context->preference('OPACAllowHoldDateInFuture') )
{
$startdate = $query->param("reserve_date_$biblioNum");
}
# If a specific item was selected and the pickup branch is the same as the
# holdingbranch, force the value $rank and $found.
my $expiration_date = $query->param("expiration_date_$biblioNum");
# If a specific item was selected and the pickup branch is the same as the
# holdingbranch, force the value $rank and $found.
my $rank = $biblioData->{rank};
if ($itemNum ne ''){
$canreserve = 1 if CanItemBeReserved($borrowernumber,$itemNum);
if ( $itemNum ne '' ) {
$canreserve = 1 if CanItemBeReserved( $borrowernumber, $itemNum );
$rank = '0' unless C4::Context->preference('ReservesNeedReturns');
my $item = GetItem($itemNum);
if ( $item->{'holdingbranch'} eq $branch ){
$found = 'W' unless C4::Context->preference('ReservesNeedReturns');
if ( $item->{'holdingbranch'} eq $branch ) {
$found = 'W'
unless C4::Context->preference('ReservesNeedReturns');
}
}
else {
$canreserve = 1 if CanBookBeReserved($borrowernumber,$biblioNum);
$canreserve = 1 if CanBookBeReserved( $borrowernumber, $biblioNum );
# Inserts a null into the 'itemnumber' field of 'reserves' table.
$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");
@ -269,14 +291,14 @@ 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,