Bugzilla – Attachment 11876 Details for
Bug 6976
User hold limit can be avoided by users in OPAC
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 6976 Close loophole allowing borrowers extra holds via opac
Bug-6976-Close-loophole-allowing-borrowers-extra-h.patch (text/plain), 6.93 KB, created by
Chris Cormack
on 2012-08-29 00:03:35 UTC
(
hide
)
Description:
Bug 6976 Close loophole allowing borrowers extra holds via opac
Filename:
MIME Type:
Creator:
Chris Cormack
Created:
2012-08-29 00:03:35 UTC
Size:
6.93 KB
patch
obsolete
>From ef017ffa747a99a687ca94f8ec8feeacee601163 Mon Sep 17 00:00:00 2001 >From: Colin Campbell <colin.campbell@ptfs-europe.com> >Date: Tue, 28 Aug 2012 14:34:55 +0100 >Subject: [PATCH] Bug 6976 Close loophole allowing borrowers extra holds via > opac >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 > >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> >--- > opac/opac-reserve.pl | 84 +++++++++++++++++++++++++++++++------------------- > 1 file changed, 53 insertions(+), 31 deletions(-) > >diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl >index 1fb4df0..d94f422 100755 >--- a/opac/opac-reserve.pl >+++ b/opac/opac-reserve.pl >@@ -174,7 +174,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'); >@@ -205,55 +208,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"); >+ } >+ >+ 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. >+ # 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"); >@@ -274,14 +296,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, >-- >1.7.9.5
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 6976
:
6371
|
7832
|
11872
|
11874
| 11876