Bugzilla – Attachment 21891 Details for
Bug 10993
Transfer limits should be checked when placing a hold/reservation in OPAC.
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 10993 [ENH] - Transfer limits should be checked when placing a hold/reservation in OPAC.
Bug-10993-ENH---Transfer-limits-should-be-checked-.patch (text/plain), 19.67 KB, created by
Olli-Antti Kivilahti
on 2013-10-09 10:15:09 UTC
(
hide
)
Description:
Bug 10993 [ENH] - Transfer limits should be checked when placing a hold/reservation in OPAC.
Filename:
MIME Type:
Creator:
Olli-Antti Kivilahti
Created:
2013-10-09 10:15:09 UTC
Size:
19.67 KB
patch
obsolete
>From df69201ff248c30af1b8ba2f3f0aa418a683fc5b Mon Sep 17 00:00:00 2001 >From: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> >Date: Wed, 9 Oct 2013 13:13:49 +0300 >Subject: [PATCH] Bug 10993 [ENH] - Transfer limits should be checked when > placing a hold/reservation in OPAC. > >--- > C4/Reserves.pm | 22 ++++-- > .../opac-tmpl/prog/en/modules/opac-reserve.tt | 79 +++++++++++++------ > opac/opac-reserve.pl | 91 ++++++++++++++-------- > 3 files changed, 132 insertions(+), 60 deletions(-) > >diff --git a/C4/Reserves.pm b/C4/Reserves.pm >index bba4933..519053c 100644 >--- a/C4/Reserves.pm >+++ b/C4/Reserves.pm >@@ -409,22 +409,32 @@ sub GetReservesFromBorrowernumber { > #------------------------------------------------------------------------------------- > =head2 CanBookBeReserved > >- $error = &CanBookBeReserved($borrowernumber, $biblionumber) >+ $error = &CanBookBeReserved($borrowernumber, $biblionumber, $pickupLibrary) > >+C<$pickupLibrary> OPTIONAL, undef OR the branchcode of the item transfer destination. >+ > =cut > > sub CanBookBeReserved{ >- my ($borrowernumber, $biblionumber) = @_; >+ my ($borrowernumber, $biblionumber, $pickupLibrary) = @_; > > my $items = GetItemnumbersForBiblio($biblionumber); > #get items linked via host records > my @hostitems = get_hostitemnumbers_of($biblionumber); > if (@hostitems){ >- push (@$items,@hostitems); >+ push (@$items,@hostitems); > } > > foreach my $item (@$items){ >- return 1 if CanItemBeReserved($borrowernumber, $item); >+ >+ if (defined $pickupLibrary) { >+ return 1 if ( CanItemBeReserved($borrowernumber, $item) and >+ CheckBranchTransferLimits($pickupLibrary, $item->{holdingbranch}, $item, undef) >+ ); >+ } >+ else { >+ return 1 if ( CanItemBeReserved($borrowernumber, $item) ); >+ } > } > return 0; > } >@@ -432,7 +442,7 @@ sub CanBookBeReserved{ > =head2 CanItemBeReserved > > $error = &CanItemBeReserved($borrowernumber, $itemnumber) >- >+ > This function return 1 if an item can be issued by this borrower. > > =cut >@@ -527,7 +537,7 @@ sub CanItemBeReserved{ > return 0; > } > } >- >+ > return 1; > } > #-------------------------------------------------------------------------------- >diff --git a/koha-tmpl/opac-tmpl/prog/en/modules/opac-reserve.tt b/koha-tmpl/opac-tmpl/prog/en/modules/opac-reserve.tt >index bf334fa..040bc25 100644 >--- a/koha-tmpl/opac-tmpl/prog/en/modules/opac-reserve.tt >+++ b/koha-tmpl/opac-tmpl/prog/en/modules/opac-reserve.tt >@@ -5,6 +5,7 @@ > <script type="text/javascript"> > // <![CDATA[ > var MSG_NO_COPY_SELECTED = _("Expecting a specific copy selection."); >+ var MSG_NO_TRANSFERRABLE_ITEMS = _("No item can be transferred to your pickup location. Please try another pickup location for "); > var ForceHoldNotesReasons=new Array( > _("This title consists of multiple volumes/parts. Please indicate which part you need. Clicking on specific copy information may be helpful."), > "*** Add a new reason above this line ***" ); // NOTE: Do not renumber reasons; this will affect use of existing ones. >@@ -20,6 +21,21 @@ > > $(document).ready(function() { > $('#hold-request-form').preventDoubleFormSubmit(); >+ >+ $("#hold-request-form").find("select[name='branch']").change( function() { >+ var newSelection = $(this).val(); >+ >+ //Make sure that when changing the pickup location by resubmitting the form, >+ // user doesn't accidentally confirm his reservations. >+ var place_reserveInput = $("#hold-request-form").find('[name="place_reserve"]'); >+ place_reserveInput.attr("value", 0); >+ >+ //Make sure the pickup location is changed to the first select as well. >+ // The branch selection uses only the first given pickuplocations branch value. >+ $("#hold-request-form").find("select[name='branch']").val(newSelection); >+ >+ $("#hold-request-form").submit(); >+ }); > > var copiesRowId = null; > var wasSpecific = false; >@@ -137,6 +153,21 @@ > alert(MSG_NO_RECORD_SELECTED); > return false; > } >+ >+ //Check for each biblio if there are no available Items that can be transferred to the pickup location, >+ // fail the hold request and alert the user. >+ var anotherPickupLocationSuggesters = $('#hold-request-form').find('div[id^="suggestAnotherPickupLocation_"]'); >+ if ( $(anotherPickupLocationSuggesters).size() > 0 ) { >+ anotherPickupLocationSuggesters.each( function() { >+ //Extract the biblionumber from the id and fetch the Biblio title >+ var idString = $(this).attr('id'); >+ var bibnumber = idString.replace('suggestAnotherPickupLocation_', ''); >+ var title = $('#title_'+bibnumber).text(); >+ >+ alert(MSG_NO_TRANSFERRABLE_ITEMS + title); >+ }); >+ return false; >+ } > > // Find the items with the 'Hold' box checked > var badBib = null; >@@ -189,7 +220,6 @@ > > > >- > [% FOREACH bibitemloo IN bibitemloop %] > [% IF ( bibitemloo.holdable ) %] > // http://jqueryui.com/demos/datepicker/#date-range >@@ -210,7 +240,7 @@ > [% END %] > [% END %] > >- }); >+ }); > // ]]> > </script> > <style type="text/css">td ul { padding : 0; } td li { white-space: nowrap; font-size: 90%; list-style-type:none; padding : .3em 0; }</style> >@@ -270,28 +300,29 @@ > <div id="bad_data" class="dialog alert">ERROR: Internal error: incomplete hold request.</div> > [% END %] > [% ELSE %] >- [% IF ( none_available ) %] >+ [% IF ( none_available ) %] > <div id="none_available" class="dialog alert"><strong>Sorry</strong>, none of these items can be placed on hold. > </div> >- [% END %] >+ [% END %] > [% END %]<!-- NAME="message" --> > >- [% UNLESS ( message ) %][% UNLESS ( none_available ) %]<h3>Confirm holds for: >- [% FOREACH USER_INF IN USER_INFO %] >- [% USER_INF.firstname %] [% USER_INF.surname %] ([% USER_INF.cardnumber %]) >- [% END %] >- </h3>[% END %] >- [% IF (RESERVE_CHARGE) %] >- <div class="dialog alert" id="reserve_fee"> >- There is a charge of [% RESERVE_CHARGE %] for placing this hold >- </div> >- [% END %] >+ [% UNLESS ( message ) %][% IF ( 1 ) %] >+ <h3>Confirm holds for: >+ [% FOREACH USER_INF IN USER_INFO %] >+ [% USER_INF.firstname %] [% USER_INF.surname %] ([% USER_INF.cardnumber %]) >+ [% END %] >+ </h3>[% END %] >+ [% IF (RESERVE_CHARGE) %] >+ <div class="dialog alert" id="reserve_fee"> >+ There is a charge of [% RESERVE_CHARGE %] for placing this hold >+ </div> >+ [% END %] > > <form action="/cgi-bin/koha/opac-reserve.pl" method="post" id="hold-request-form"> > <input type="hidden" name="place_reserve" value="1"/> > >- <!-- These values are set dynamically by js --> >- <input type="hidden" name="biblionumbers" id="biblionumbers"/> >+ <!-- These values are set dynamically by js. Biblionumbers is needed for the page reload when changing pickup branch --> >+ <input type="hidden" name="biblionumbers" id="biblionumbers" value="[% biblionumbers %]"/> > <input type="hidden" name="selecteditems" id="selections"/> > <div id="bigloop"> > >@@ -314,7 +345,7 @@ > [% ELSE %] > [% END %] > >- <a class="title" href="/cgi-bin/koha/opac-detail.pl?biblionumber=[% bibitemloo.biblionumber %]">[% bibitemloo.title |html %][% IF ( bibitemloo.subtitle ) %] [% FOREACH subtitl IN bibitemloo.subtitle %][% subtitl.subfield %][% END %][% END %]</a> >+ <a class="title" id="title_[% bibitemloo.biblionumber %]" href="/cgi-bin/koha/opac-detail.pl?biblionumber=[% bibitemloo.biblionumber %]">[% bibitemloo.title |html %][% IF ( bibitemloo.subtitle ) %] [% FOREACH subtitl IN bibitemloo.subtitle %][% subtitl.subfield %][% END %][% END %]</a> > [% IF ( bibitemloo.author ) %], by [% bibitemloo.author %][% END %]</p> > > >@@ -359,12 +390,16 @@ > > > [% UNLESS ( singleBranchMode ) %] >+ [% IF ( bibitemloo.suggestAnotherPickupLocation ) %] >+ <div id="suggestAnotherPickupLocation_[% bibitemloo.biblionumber %]" class="dialog alert">None of the available items can be transferred to your pickup location. You could try another pickup location. >+ </div> >+ [% END %] > [% IF ( bibitemloo.holdable ) %] > [% IF ( choose_branch ) %] > <li class="branch"> > <label for="branch_[% bibitemloo.biblionumber %]">Pick up location:</label> > [% UNLESS ( bibitemloo.holdable ) %] >- <select name="branch" id="branch_[% bibitemloo.biblionumber %]" disabled="disabled"> >+ <select onClick="alert('digg')" name="branch" id="branch_[% bibitemloo.biblionumber %]" disabled="disabled"> > [% FOREACH branchloo IN bibitemloo.branchloop %] > [% IF ( branchloo.selected ) %] > <option value="[% branchloo.branchcode %]" selected="selected">[% branchloo.branchname %]</option> >@@ -398,8 +433,8 @@ > <span class="date-format from" data-biblionumber="[% bibitemloo.biblionumber %]">[% INCLUDE 'date-format.inc' %]</span> > </li> > [% END %] >- >- [% END %] >+ [% END %][%#End of IF ( bibitemloo.holdable ) %] >+ > [% IF ( bibitemloo.holdable ) %] > <li> > <label for="to[% bibitemloo.biblionumber %]">Hold not needed after:</label> >@@ -440,8 +475,8 @@ > checked="checked" > /> > [% END %] >- > <label for="reqany_[% bibitemloo.biblionumber %]">Next available copy</label> >+ > [% UNLESS ( bibitemloo.holdable ) %] > <input type="radio" name="reqtype_[% bibitemloo.biblionumber %]" > id="reqspecific_[% bibitemloo.biblionumber %]" >@@ -551,10 +586,8 @@ > </div><!-- bigloop --> > > [% UNLESS ( message ) %] >- [% UNLESS ( none_available ) %] > <input type="submit" value="Place hold" class="placehold" /> > [% END %] >- [% END %] > > </form> > >diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl >index 4bba9a6..d53b2db 100755 >--- a/opac/opac-reserve.pl >+++ b/opac/opac-reserve.pl >@@ -91,7 +91,8 @@ if ((! $biblionumbers) && (! $query->param('place_reserve'))) { > } > > # Pass the numbers to the page so they can be fed back >-# when the hold is confirmed. TODO: Not necessary? >+# when the hold is confirmed. Biblionumbers are also needed if the >+# pickup branch changes and the page needs to be reloaded. > $template->param( biblionumbers => $biblionumbers ); > > # Each biblio number is suffixed with '/', e.g. "1/2/3/" >@@ -103,12 +104,12 @@ if (($#biblionumbers < 0) && (! $query->param('place_reserve'))) { > } > > # pass the pickup branch along.... >-my $branch = $query->param('branch') || $borr->{'branchcode'} || C4::Context->userenv->{branch} || '' ; >-($branches->{$branch}) or $branch = ""; # Confirm branch is real >-$template->param( branch => $branch ); >+my $pickupBranch = $query->param('branch') || $borr->{'branchcode'} || C4::Context->userenv->{branch} || '' ; >+($branches->{$pickupBranch}) or $pickupBranch = ""; # Confirm branch is real >+$template->param( branch => $pickupBranch ); > > # make branch selection options... >-my $branchloop = GetBranchesLoop($branch); >+my $branchloop = GetBranchesLoop($pickupBranch); > > # Is the person allowed to choose their branch > my $OPACChooseBranch = (C4::Context->preference("OPACAllowUserToChooseBranch")) ? 1 : 0; >@@ -202,19 +203,19 @@ 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 $biblioNum = shift(@selectedItems); >+ my $itemNum = shift(@selectedItems); >+ my $pickupLocation = 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 >- $branch = $borr->{'branchcode'}; >+ $pickupLocation = $borr->{'branchcode'}; > } > >-#item may belong to a host biblio, if yes change biblioNum to hosts bilbionumber >+ #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 ) { >@@ -235,20 +236,26 @@ if ( $query->param('place_reserve') ) { > > 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 '' ) { >+ my $item = GetItem($itemNum); > $canreserve = 1 if CanItemBeReserved( $borrowernumber, $itemNum ); > $rank = '0' unless C4::Context->preference('ReservesNeedReturns'); >- my $item = GetItem($itemNum); >- if ( $item->{'holdingbranch'} eq $branch ) { >+ if ( $item->{'holdingbranch'} eq $pickupLocation ) { > $found = 'W' > unless C4::Context->preference('ReservesNeedReturns'); > } >+ >+ # UseBranchTransferLimits checking. >+ my ($transferOk, $message) = CheckBranchTransferAllowed( $pickupLocation, $item->{'holdingbranch'}, $item, undef ); >+ if (! $transferOk) { >+ $canreserve = 0; >+ } > } > else { >- $canreserve = 1 if CanBookBeReserved( $borrowernumber, $biblioNum ); >+ $canreserve = 1 if CanBookBeReserved( $borrowernumber, $biblioNum, $pickupLocation ); > > # Inserts a null into the 'itemnumber' field of 'reserves' table. > $itemNum = undef; >@@ -264,7 +271,7 @@ if ( $query->param('place_reserve') ) { > # Here we actually do the reserveration. Stage 3. > if ($canreserve) { > AddReserve( >- $branch, $borrowernumber, >+ $pickupLocation, $borrowernumber, > $biblioNum, 'a', > [$biblioNum], $rank, > $startdate, $expiration_date, >@@ -398,6 +405,10 @@ foreach my $biblioNum (@biblionumbers) { > } > } > >+ #Collect the amout of items that pass the CheckBranchTransferAllowed-check. This is needed to tell >+ # the Borrower if some or all Items cannot be transferred to the pickup location. >+ my $branchTransferableItemsCount = 0; >+ > $biblioLoopIter{itemLoop} = []; > my $numCopiesAvailable = 0; > foreach my $itemInfo (@{$biblioData->{itemInfos}}) { >@@ -434,10 +445,10 @@ foreach my $biblioNum (@biblionumbers) { > my ($reservedate,$reservedfor,$expectedAt) = GetReservesFromItemnumber($itemNum); > my $ItemBorrowerReserveInfo = GetMemberDetails( $reservedfor, 0); > >- # the item could be reserved for this borrower vi a host record, flag this >- if ($reservedfor eq $borrowernumber){ >- $itemLoopIter->{already_reserved} = 1; >- } >+ # the item could be reserved for this borrower vi a host record, flag this >+ if ($reservedfor eq $borrowernumber){ >+ $itemLoopIter->{already_reserved} = 1; >+ } > > if ( defined $reservedate ) { > $itemLoopIter->{backgroundcolor} = 'reserved'; >@@ -480,12 +491,12 @@ foreach my $biblioNum (@biblionumbers) { > $itemLoopIter->{nocancel} = 1; > } > >- # if the items belongs to a host record, show link to host record >- if ($itemInfo->{biblionumber} ne $biblioNum){ >- $biblioLoopIter{hostitemsflag} = 1; >- $itemLoopIter->{hostbiblionumber} = $itemInfo->{biblionumber}; >- $itemLoopIter->{hosttitle} = GetBiblioData($itemInfo->{biblionumber})->{title}; >- } >+ # if the items belongs to a host record, show link to host record >+ if ($itemInfo->{biblionumber} ne $biblioNum){ >+ $biblioLoopIter{hostitemsflag} = 1; >+ $itemLoopIter->{hostbiblionumber} = $itemInfo->{biblionumber}; >+ $itemLoopIter->{hosttitle} = GetBiblioData($itemInfo->{biblionumber})->{title}; >+ } > > # If there is no loan, return and transfer, we show a checkbox. > $itemLoopIter->{notforloan} = $itemLoopIter->{notforloan} || 0; >@@ -501,20 +512,32 @@ foreach my $biblioNum (@biblionumbers) { > } > > if (IsAvailableForItemLevelRequest($itemNum) and $policy_holdallowed and CanItemBeReserved($borrowernumber,$itemNum) and ($itemLoopIter->{already_reserved} ne 1)) { >- $itemLoopIter->{available} = 1; >+ >+ $itemLoopIter->{available} = 1; > $numCopiesAvailable++; >+ >+ #Check for UseBranchTransferLimit. $numCopiesAvailable is incremented because this Item >+ # could still be available from another pickup location >+ my ($transferOk, $errorMsg) = CheckBranchTransferAllowed( $pickupBranch, undef, GetItem($itemNum), undef ); >+ if (! $transferOk) { >+ $itemLoopIter->{available} = 0; >+ $itemLoopIter->{branchTransferBlocked} = 1; >+ } >+ else { >+ $branchTransferableItemsCount++; >+ } > } > >- # FIXME: move this to a pm >+ # FIXME: move this to a pm > my $dbh = C4::Context->dbh; > my $sth2 = $dbh->prepare("SELECT * FROM reserves WHERE borrowernumber=? AND itemnumber=? AND found='W'"); > $sth2->execute($itemLoopIter->{ReservedForBorrowernumber}, $itemNum); > while (my $wait_hashref = $sth2->fetchrow_hashref) { > $itemLoopIter->{waitingdate} = format_date($wait_hashref->{waitingdate}); > } >- $itemLoopIter->{imageurl} = getitemtypeimagelocation( 'opac', $itemTypes->{ $itemInfo->{itype} }{imageurl} ); >+ $itemLoopIter->{imageurl} = getitemtypeimagelocation( 'opac', $itemTypes->{ $itemInfo->{itype} }{imageurl} ); > >- # Show serial enumeration when needed >+ # Show serial enumeration when needed > if ($itemLoopIter->{enumchron}) { > $itemdata_enumchron = 1; > } >@@ -523,6 +546,7 @@ foreach my $biblioNum (@biblionumbers) { > } > $template->param( itemdata_enumchron => $itemdata_enumchron ); > >+ ## Set the behaviour flags for the template > if ($numCopiesAvailable > 0) { > $numBibsAvailable++; > $biblioLoopIter{bib_available} = 1; >@@ -538,13 +562,18 @@ foreach my $biblioNum (@biblionumbers) { > $biblioLoopIter{holdable} = undef; > $biblioLoopIter{already_patron_possession} = 1; > } >+ if ($branchTransferableItemsCount == 0) { >+ #We can tell our Borrowers that they can try another pickup location if they don't find what they need. >+ $biblioLoopIter{suggestAnotherPickupLocation} = 1 ; >+ } >+ > > if( $biblioLoopIter{holdable} ){ $anyholdable++; } > > push @$biblioLoop, \%biblioLoopIter; > } > >-if ( $numBibsAvailable == 0 || $anyholdable == 0) { >+if ( $numBibsAvailable == 0 || $anyholdable == 0 ) { > $template->param( none_available => 1 ); > } > >-- >1.8.1.2
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=21891">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 10993
:
21891
|
21892
|
22000
|
22001
|
22322
|
23163