Bugzilla – Attachment 44145 Details for
Bug 14695
Add ability to place multiple item holds on a given record per patron
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 14695 - - Add ability to place multiple item holds on a given record per patron
Bug-14695-----Add-ability-to-place-multiple-item-h.patch (text/plain), 29.13 KB, created by
Kyle M Hall (khall)
on 2015-10-29 11:07:51 UTC
(
hide
)
Description:
Bug 14695 - - Add ability to place multiple item holds on a given record per patron
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2015-10-29 11:07:51 UTC
Size:
29.13 KB
patch
obsolete
>From 168280f912288f54eb141afe03d0e12f50e48dd3 Mon Sep 17 00:00:00 2001 >From: Kyle M Hall <kyle@bywatersolutions.com> >Date: Mon, 5 Oct 2015 09:47:26 -0400 >Subject: [PATCH] Bug 14695 - - Add ability to place multiple item holds on a given record per patron > >Test Plan: >1) Apply this patch set >2) Run updatedatabase.pl >3) Edit your circulation rules to allow 3 holds per record >4) In the staff client, test the ability to place multiple holds per > patron per record. You should be able to place any combination of 3 > holds per patron per record, with the exception that you cannot > place multiple item level holds for the same item for the same > patron. >5) Repeat step 4 for the OPAC. >--- > C4/Reserves.pm | 120 ++++++++++++-------- > circ/returns.pl | 34 +++--- > .../intranet-tmpl/prog/en/modules/circ/returns.tt | 33 +++--- > .../prog/en/modules/reserve/request.tt | 2 +- > .../opac-tmpl/bootstrap/en/modules/opac-reserve.tt | 2 +- > opac/opac-reserve.pl | 13 +-- > reserve/request.pl | 6 +- > t/db_dependent/Holds.t | 47 +++++++-- > t/db_dependent/Reserves.t | 2 +- > 9 files changed, 158 insertions(+), 101 deletions(-) > >diff --git a/C4/Reserves.pm b/C4/Reserves.pm >index 4027ab3..e60d864 100644 >--- a/C4/Reserves.pm >+++ b/C4/Reserves.pm >@@ -148,23 +148,30 @@ BEGIN { > > =head2 AddReserve > >- AddReserve($branch,$borrowernumber,$biblionumber,$bibitems,$priority,$resdate,$expdate,$notes,$title,$checkitem,$found) >+ AddReserve( >+ $branch, $borrowernumber, $biblionumber, $bibitems, $priority, $resdate, >+ $expdate, $notes, $title, $itemnumber, $found >+ ) > > =cut > > sub AddReserve { > my ( >- $branch, $borrowernumber, $biblionumber, >- $bibitems, $priority, $resdate, $expdate, $notes, >- $title, $checkitem, $found >+ $branch, $borrowernumber, $biblionumber, $bibitems, $priority, $resdate, >+ $expdate, $notes, $title, $itemnumber, $found > ) = @_; > >- if ( Koha::Holds->search( { borrowernumber => $borrowernumber, biblionumber => $biblionumber } )->count() > 0 ) { >- carp("AddReserve: borrower $borrowernumber already has a hold for biblionumber $biblionumber"); >+ # Ensure the holds should be allowed >+ my $can_reserve = >+ $itemnumber >+ ? CanItemBeReserved( $borrowernumber, $itemnumber ) >+ : CanBookBeReserved( $borrowernumber, $biblionumber ); >+ if ( $can_reserve eq 'maxHoldsForRecord' ) { >+ carp("AddReserve: borrower $borrowernumber has already reached the max holds for biblionumber $biblionumber"); > return; > } > >- my $dbh = C4::Context->dbh; >+ my $dbh = C4::Context->dbh; > > $resdate = format_date_in_iso( $resdate ) if ( $resdate ); > $resdate = C4::Dates->today( 'iso' ) unless ( $resdate ); >@@ -197,7 +204,7 @@ sub AddReserve { > branchcode => $branch, > priority => $priority, > reservenotes => $notes, >- itemnumber => $checkitem, >+ itemnumber => $itemnumber, > found => $found, > waitingdate => $waitingdate, > expirationdate => $expdate >@@ -223,7 +230,7 @@ sub AddReserve { > 'branches' => $branch_details, > 'borrowers' => $borrower, > 'biblio' => $biblionumber, >- 'items' => $checkitem, >+ 'items' => $itemnumber, > }, > ) ) { > >@@ -465,27 +472,12 @@ sub CanItemBeReserved { > > my $controlbranch = C4::Context->preference('ReservesControlBranch'); > >- # we retrieve user rights on this itemtype and branchcode >- my $sth = $dbh->prepare( >- "SELECT categorycode, itemtype, branchcode, reservesallowed >- FROM issuingrules >- WHERE (categorycode in (?,'*') ) >- AND (itemtype IN (?,'*')) >- AND (branchcode IN (?,'*')) >- ORDER BY >- categorycode DESC, >- itemtype DESC, >- branchcode DESC;" >- ); >- >- my $querycount = "SELECT >- count(*) as count >- FROM reserves >- LEFT JOIN items USING (itemnumber) >- LEFT JOIN biblioitems ON (reserves.biblionumber=biblioitems.biblionumber) >- LEFT JOIN borrowers USING (borrowernumber) >- WHERE borrowernumber = ? >- "; >+ my $querycount = "SELECT COUNT(*) as count >+ FROM reserves >+ LEFT JOIN items USING (itemnumber) >+ LEFT JOIN biblioitems ON (reserves.biblionumber=biblioitems.biblionumber) >+ LEFT JOIN borrowers USING (borrowernumber) >+ WHERE borrowernumber = ?"; > > my $branchcode = ""; > my $branchfield = "reserves.branchcode"; >@@ -499,11 +491,13 @@ sub CanItemBeReserved { > $branchcode = $borrower->{branchcode}; > } > >+ # we retrieve user rights on this itemtype and branchcode >+ my $rule = _get_rule( $borrower->{categorycode}, $item->{itype}, $branchcode ); >+ > # we retrieve rights >- $sth->execute( $borrower->{'categorycode'}, $item->{'itype'}, $branchcode ); >- if ( my $rights = $sth->fetchrow_hashref() ) { >- $ruleitemtype = $rights->{itemtype}; >- $allowedreserves = $rights->{reservesallowed}; >+ if ( $rule ) { >+ $ruleitemtype = $rule->get_column('itemtype'); >+ $allowedreserves = $rule->get_column('reservesallowed'); > } > else { > $ruleitemtype = '*'; >@@ -564,6 +558,24 @@ sub CanItemBeReserved { > } > } > >+ my $holds_on_record = Koha::Holds->search( >+ { >+ borrowernumber => $borrowernumber, >+ biblionumber => $item->{biblionumber} >+ } >+ )->count(); >+ return 'maxHoldsForRecord' if ( $holds_on_record >= $rule->holds_per_record ); >+ >+ if ($itemnumber) { >+ my $hold_on_item = Koha::Holds->search( >+ { >+ borrowernumber => $borrowernumber, >+ itemnumber => $itemnumber >+ } >+ )->count(); >+ return 'itemAlreadyOnHold' if ($hold_on_item); >+ } >+ > return 'OK'; > } > >@@ -1273,7 +1285,7 @@ sub ModReserveStatus { > > =head2 ModReserveAffect > >- &ModReserveAffect($itemnumber,$borrowernumber,$diffBranchSend); >+&ModReserveAffect( $itemnumber, $borrowernumber, $diffBranchSend, $reserve_id ); > > This function affect an item and a status for a given reserve > The itemnumber parameter is used to find the biblionumber. >@@ -1287,7 +1299,7 @@ take care of the waiting status > =cut > > sub ModReserveAffect { >- my ( $itemnumber, $borrowernumber,$transferToDo ) = @_; >+ my ( $itemnumber, $borrowernumber, $transferToDo, $reserve_id ) = @_; > my $dbh = C4::Context->dbh; > > # we want to attach $itemnumber to $borrowernumber, find the biblionumber >@@ -1298,10 +1310,12 @@ sub ModReserveAffect { > > # get request - need to find out if item is already > # waiting in order to not send duplicate hold filled notifications >- my $reserve_id = GetReserveId({ >- borrowernumber => $borrowernumber, >- biblionumber => $biblionumber, >- }); >+ $reserve_id ||= GetReserveId( >+ { >+ borrowernumber => $borrowernumber, >+ biblionumber => $biblionumber, >+ } >+ ); > return unless defined $reserve_id; > my $request = GetReserveInfo($reserve_id); > my $already_on_shelf = ($request && $request->{found} eq 'W') ? 1 : 0; >@@ -1314,8 +1328,7 @@ sub ModReserveAffect { > SET priority = 0, > itemnumber = ?, > found = 'T' >- WHERE borrowernumber = ? >- AND biblionumber = ? >+ WHERE reserve_id = ? > "; > } > else { >@@ -1326,12 +1339,11 @@ sub ModReserveAffect { > found = 'W', > waitingdate = NOW(), > itemnumber = ? >- WHERE borrowernumber = ? >- AND biblionumber = ? >+ WHERE reserve_id = ? > "; > } > $sth = $dbh->prepare($query); >- $sth->execute( $itemnumber, $borrowernumber,$biblionumber); >+ $sth->execute( $itemnumber, $reserve_id ); > _koha_notify_reserve( $itemnumber, $borrowernumber, $biblionumber ) if ( !$transferToDo && !$already_on_shelf ); > _FixPriority( { biblionumber => $biblionumber } ); > if ( C4::Context->preference("ReturnToShelvingCart") ) { >@@ -2435,6 +2447,24 @@ sub IsItemOnHoldAndFound { > return $found; > } > >+sub _get_rule { >+ my ( $categorycode, $itemtype, $branchcode ) = @_; >+ >+ my $schema = Koha::Database->new()->schema(); >+ >+ my $rule = $schema->resultset('Issuingrule')->search( >+ { >+ categorycode => { -in => $categorycode ? [ $categorycode, '*' ] : ['*'] }, >+ itemtype => { -in => $itemtype ? [ $itemtype, '*' ] : ['*'] }, >+ branchcode => { -in => $branchcode ? [ $branchcode, '*' ] : ['*'] }, >+ >+ }, >+ { order_by => { -desc => [ 'categorycode', 'itemtype', 'branchcode' ] } } >+ )->next(); >+ >+ return $rule; >+} >+ > =head1 AUTHOR > > Koha Development Team <http://koha-community.org/> >diff --git a/circ/returns.pl b/circ/returns.pl >index 637f4b2..f4297df 100755 >--- a/circ/returns.pl >+++ b/circ/returns.pl >@@ -142,23 +142,24 @@ if ($query->param('WT-itemNumber')){ > updateWrongTransfer ($query->param('WT-itemNumber'),$query->param('WT-waitingAt'),$query->param('WT-From')); > } > >-if ( $query->param('resbarcode') ) { >- my $item = $query->param('itemnumber'); >- my $borrowernumber = $query->param('borrowernumber'); >- my $resbarcode = $query->param('resbarcode'); >+if ( $query->param('reserve_id') ) { >+ my $reserve_id = $query->param('reserve_id'); >+ my $item = $query->param('itemnumber'); >+ my $borrowernumber = $query->param('borrowernumber'); > my $diffBranchReturned = $query->param('diffBranch'); >+ my $cancel_reserve = $query->param('cancel_reserve'); >+ > my $iteminfo = GetBiblioFromItemNumber($item); >- my $cancel_reserve = $query->param('cancel_reserve'); > # fix up item type for display > $iteminfo->{'itemtype'} = C4::Context->preference('item-level_itypes') ? $iteminfo->{'itype'} : $iteminfo->{'itemtype'}; > > if ( $cancel_reserve ) { >- CancelReserve({ borrowernumber => $borrowernumber, itemnumber => $item, charge_cancel_fee => !$forgivemanualholdsexpire }); >+ CancelReserve({ reserve_id => $reserve_id, charge_cancel_fee => !$forgivemanualholdsexpire }); > } else { > my $diffBranchSend = ($userenv_branch ne $diffBranchReturned) ? $diffBranchReturned : undef; > # diffBranchSend tells ModReserveAffect whether document is expected in this library or not, > # i.e., whether to apply waiting status >- ModReserveAffect( $item, $borrowernumber, $diffBranchSend); >+ ModReserveAffect( $item, $borrowernumber, $diffBranchSend, $reserve_id ); > } > # check if we have other reserves for this document, if we have a return send the message of transfer > my ( $messages, $nextreservinfo ) = GetOtherReserves($item); >@@ -426,13 +427,12 @@ if ( $messages->{'ResFound'}) { > > if ( $reserve->{'ResFound'} eq "Waiting" or $reserve->{'ResFound'} eq "Reserved" ) { > if ( $reserve->{'ResFound'} eq "Waiting" ) { >+ $template->param( waiting => $userenv_branch eq $reserve->{'branchcode'}, ); >+ } >+ elsif ( $reserve->{'ResFound'} eq "Reserved" ) { > $template->param( >- waiting => ($userenv_branch eq $reserve->{'branchcode'} ? 1 : 0 ), >- ); >- } elsif ( $reserve->{'ResFound'} eq "Reserved" ) { >- $template->param( >- intransit => ($userenv_branch eq $reserve->{'branchcode'} ? 0 : 1 ), >- transfertodo => ($userenv_branch eq $reserve->{'branchcode'} ? 0 : 1 ), >+ intransit => $userenv_branch eq $reserve->{'branchcode'}, >+ transfertodo => $userenv_branch eq $reserve->{'branchcode'}, > resbarcode => $barcode, > reserved => 1, > ); >@@ -440,9 +440,10 @@ if ( $messages->{'ResFound'}) { > > # same params for Waiting or Reserved > $template->param( >+ reserve => $reserve, >+ patron => $borr, > found => 1, >- currentbranch => $branches->{$userenv_branch}->{'branchname'}, >- destbranchname => $branches->{ $reserve->{'branchcode'} }->{'branchname'}, >+ barcode => $barcode, > name => $borr->{'surname'} . ", " . $borr->{'title'} . " " . $borr->{'firstname'}, > borfirstname => $borr->{'firstname'}, > borsurname => $borr->{'surname'}, >@@ -457,11 +458,8 @@ if ( $messages->{'ResFound'}) { > borcnum => $borr->{'cardnumber'}, > debarred => $borr->{'debarred'}, > gonenoaddress => $borr->{'gonenoaddress'}, >- barcode => $barcode, >- destbranch => $reserve->{'branchcode'}, > borrowernumber => $reserve->{'borrowernumber'}, > itemnumber => $reserve->{'itemnumber'}, >- reservenotes => $reserve->{'reservenotes'}, > ); > } # else { ; } # error? > } >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt >index aa43b75..0432db4 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt >@@ -193,7 +193,7 @@ $(document).ready(function () { > [% END %]</div> > [% END %] > >-[% IF ( found ) %] >+[% IF ( reserve ) %] > <!-- found --> > <!-- case of a reservation found, and display info --> > [% IF ( waiting ) %] >@@ -205,7 +205,7 @@ $(document).ready(function () { > > <div id="hold-found1" class="dialog message"> > <h3>Hold found (item is already waiting): <a href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=[% itembiblionumber %]">[% title |html %]</a></h3> >- [% IF ( reservenotes ) %]<h4>Notes: [% reservenotes %]</h4>[% END %] >+ [% IF ( reserve.reservenotes ) %]<h4>Notes: [% reserve.reservenotes %]</h4>[% END %] > <h4>Hold for:</h4> > <ul> > <li><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% borrowernumber %]">[% borsurname %], [% borfirstname %]</a> ([% borcnum %])</li> >@@ -217,9 +217,9 @@ $(document).ready(function () { > [% IF ( debarred ) %]<li class="error">Patron is RESTRICTED</li>[% END %] > [% IF ( gonenoaddress ) %]<li class="error">Patron's address is in doubt</li>[% END %]</ul> > [% IF ( transfertodo ) %] >- <h4><strong>Transfer to:</strong> [% destbranchname %]</h4> >+ <h4><strong>Transfer to:</strong> [% Branches.GetName( reserve.branchcode ) %]</h4> > [% ELSE %] >- <h4><strong>Hold at</strong> [% destbranchname %]</h4> >+ <h4><strong>Hold at</strong> [% Branches.GetName( reserve.branchcode ) %]</h4> > [% END %] > <form method="post" action="returns.pl" class="confirm"> > <input type="hidden" name="cancel_reserve" value="0" /> >@@ -238,7 +238,7 @@ $(document).ready(function () { > <input type="hidden" name="borrowernumber" value="[% borrowernumber %]" /> > <input type="hidden" name="biblionumber" value="[% itembiblionumber %]" /> > <input type="hidden" name="resbarcode" value="[% barcode %]" /> >- <input type="hidden" name="diffBranch" value="[% destbranch %]" /> >+ <input type="hidden" name="diffBranch" value="[% reserves.branchcode %]" /> > <input type="hidden" name="exemptfine" value="[% exemptfine %]" /> > <input type="hidden" name="dropboxmode" value="[% dropboxmode %]" /> > <input type="hidden" name="forgivemanualholdsexpire" value="[% forgivemanualholdsexpire %]" /> >@@ -268,9 +268,9 @@ $(document).ready(function () { > [% IF ( gonenoaddress ) %]<li class="error">Patron's address is in doubt</li>[% END %] > </ul> > [% IF ( transfertodo ) %] >- <h4><strong>Transfer to:</strong> [% destbranchname %]</h4> >+ <h4><strong>Transfer to:</strong> [% Branches.GetName( reserve.branchcode ) %]</h4> > [% ELSE %] >- <h4><strong>Hold at</strong> [% destbranchname %]</h4> >+ <h4><strong>Hold at</strong> [% Branches.GetName( reserve.branchcode ) %]</h4> > [% END %] > > <form method="post" action="returns.pl" class="confirm"> >@@ -284,7 +284,7 @@ $(document).ready(function () { > <input type="hidden" name="dd-[% inputloo.counter %]" value="[% inputloo.duedate %]" /> > <input type="hidden" name="bn-[% inputloo.counter %]" value="[% inputloo.borrowernumber %]" /> > [% END %] >- <input type="hidden" name="diffBranch" value="[% destbranch %]" /> >+ <input type="hidden" name="diffBranch" value="[% reserves.branchcode %]" /> > <input type="hidden" name="exemptfine" value="[% exemptfine %]" /> > <input type="hidden" name="dropboxmode" value="[% dropboxmode %]" /> > <input type="hidden" name="forgivemanualholdsexpire" value="[% forgivemanualholdsexpire %]" /> >@@ -375,7 +375,7 @@ $(document).ready(function () { > > <div id="hold-found2" class="dialog message"> > <h3>Hold found: <a href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=[% itembiblionumber %]">[% title |html %]</a></h3> >- [% IF ( reservenotes ) %]<h4>Notes: [% reservenotes %]</h4>[% END %] >+ [% IF ( reserve.reservenotes ) %]<h4>Notes: [% reserve.reservenotes %]</h4>[% END %] > <h5>Hold for:</h5> > <ul> > <li><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% borrowernumber %]"> >@@ -390,9 +390,9 @@ $(document).ready(function () { > [% IF ( gonenoaddress ) %]<li class="error">Patron's address is in doubt</li>[% END %] > </ul> > [% IF ( transfertodo ) %] >- <h4><strong>Transfer to:</strong> [% destbranchname %]</h4> >+ <h4><strong>Transfer to:</strong> [% Branches.GetName( reserve.branchcode ) %]</h4> > [% ELSE %] >- <h4><strong>Hold at</strong> [% destbranchname %]</h4> >+ <h4><strong>Hold at</strong> [% Branches.GetName( reserve.branchcode ) %]</h4> > [% END %] > <form method="post" action="returns.pl" class="confirm"> > <input type="hidden" name="print_slip" value="0" /> >@@ -405,14 +405,15 @@ $(document).ready(function () { > [% END %] > <input type="submit" class="deny" value="Ignore" onclick="$('.dialog:visible').hide('slow'); $('#barcode').focus(); return false;" /> > [% FOREACH inputloo IN inputloop %] >- <input type="hidden" name="ri-[% inputloo.counter %]" value="[% inputloo.barcode %]" /> >- <input type="hidden" name="dd-[% inputloo.counter %]" value="[% inputloo.duedate %]" /> >- <input type="hidden" name="bn-[% inputloo.counter %]" value="[% inputloo.borrowernumber %]" />[% END %] >+ <input type="hidden" name="ri-[% inputloo.counter %]" value="[% inputloo.barcode %]" /> >+ <input type="hidden" name="dd-[% inputloo.counter %]" value="[% inputloo.duedate %]" /> >+ <input type="hidden" name="bn-[% inputloo.counter %]" value="[% inputloo.borrowernumber %]" /> >+ [% END %] > <input type="hidden" name="itemnumber" value="[% itemnumber %]" /> > <input type="hidden" name="borrowernumber" value="[% borrowernumber %]" /> > <input type="hidden" name="biblionumber" value="[% itembiblionumber %]" /> >- <input type="hidden" name="resbarcode" value="[% barcode %]" /> >- <input type="hidden" name="diffBranch" value="[% destbranch %]" /> >+ <input type="hidden" name="reserve_id" value="[% reserve.reserve_id %]" /> >+ <input type="hidden" name="diffBranch" value="[% reserves.branchcode %]" /> > <input type="hidden" name="exemptfine" value="[% exemptfine %]" /> > <input type="hidden" name="dropboxmode" value="[% dropboxmode %]" /> > <input type="hidden" name="forgivemanualholdsexpire" value="[% forgivemanualholdsexpire %]" /> >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt >index bf85bf6..f248d5e 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt >@@ -309,7 +309,7 @@ function checkMultiHold() { > [% ELSIF ( alreadypossession ) %] > <li> <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% borrowernumber %]">[% borrowerfirstname %] [% borrowersurname %]</a> <strong>is already in possession</strong> of one item</li> > [% ELSIF ( alreadyreserved ) %] >- <li><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% borrowernumber %]">[% borrowerfirstname %] [% borrowersurname %]</a> <strong>already has a hold</strong> on this item </li> >+ <li><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% borrowernumber %]">[% borrowerfirstname %] [% borrowersurname %]</a> <strong>has already placed the maximum number of holds allowed</strong> on this record </li> > [% ELSIF ( ageRestricted ) %] > <li><strong>Age restricted</strong></li> > [% ELSIF ( none_available ) %] >diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt >index a864dde..fc46f21 100644 >--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt >+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt >@@ -147,7 +147,7 @@ > <div class="alert">Sorry, you are too young to reserve this material.</div> > [% END %] > [% IF ( bibitemloo.already_reserved ) %] >- <div class="alert">You have already requested this title.</div> >+ <div class="alert">You have already placed the maximum number of requests for this title.</div> > [% ELSE %] > [% UNLESS ( bibitemloo.bib_available ) %] > <div class="alert">No available items.</div> >diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl >index f0843a2..0cacbf6 100755 >--- a/opac/opac-reserve.pl >+++ b/opac/opac-reserve.pl >@@ -338,7 +338,7 @@ if ( IsDebarred($borrowernumber) ) { > } > > my @reserves = GetReservesFromBorrowernumber( $borrowernumber ); >-my $reserves_count = scalar(@reserves); >+my $reserves_count = scalar @reserves; > $template->param( RESERVES => \@reserves ); > if ( $maxreserves && ( $reserves_count >= $maxreserves ) ) { > $template->param( message => 1 ); >@@ -353,14 +353,9 @@ unless ( $noreserves ) { > } > } > >-foreach my $res (@reserves) { >- foreach my $biblionumber (@biblionumbers) { >- if ( $res->{'biblionumber'} == $biblionumber && $res->{'borrowernumber'} == $borrowernumber) { >-# $template->param( message => 1 ); >-# $noreserves = 1; >-# $template->param( already_reserved => 1 ); >- $biblioDataHash{$biblionumber}->{already_reserved} = 1; >- } >+foreach my $biblionumber (@biblionumbers) { >+ if ( CanBookBeReserved( $borrowernumber, $biblionumber ) eq 'maxHoldsForRecord' ) { >+ $biblioDataHash{$biblionumber}->{already_reserved} = 1; > } > } > >diff --git a/reserve/request.pl b/reserve/request.pl >index 80cf0d6..8f343a1 100755 >--- a/reserve/request.pl >+++ b/reserve/request.pl >@@ -226,6 +226,10 @@ foreach my $biblionumber (@biblionumbers) { > elsif ( $canReserve eq 'tooManyReserves' ) { > $exceeded_maxreserves = 1; > } >+ elsif ( $canReserve eq 'maxHoldsForRecord' ) { >+ $template->param( alreadyreserved => 1 ); >+ $biblioloopiter{warn} = 1; >+ } > elsif ( $canReserve eq 'ageRestricted' ) { > $template->param( $canReserve => 1 ); > $biblioloopiter{$canReserve} = 1; >@@ -257,13 +261,11 @@ foreach my $biblionumber (@biblionumbers) { > } > > if ( $holds_count ) { >- $alreadyreserved = 1; > $biblioloopiter{warn} = 1; > $biblioloopiter{alreadyres} = 1; > } > > $template->param( >- alreadyreserved => $alreadyreserved, > alreadypossession => $alreadypossession, > ); > >diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t >index 9a44084..9cf1a7b 100755 >--- a/t/db_dependent/Holds.t >+++ b/t/db_dependent/Holds.t >@@ -8,7 +8,7 @@ use t::lib::TestBuilder; > use C4::Context; > use C4::Branch; > >-use Test::More tests => 56; >+use Test::More tests => 59; > use MARC::Record; > use C4::Biblio; > use C4::Items; >@@ -217,16 +217,16 @@ my ($foreign_item_bibnum, $foreign_item_bibitemnum, $foreign_itemnumber) > = AddItem({ homebranch => $branch_2, holdingbranch => $branch_2 } , $foreign_bibnum); > $dbh->do('DELETE FROM issuingrules'); > $dbh->do( >- q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed) >- VALUES (?, ?, ?, ?)}, >+ q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_record) >+ VALUES (?, ?, ?, ?, ?)}, > {}, >- '*', '*', '*', 25 >+ '*', '*', '*', 25, 5 > ); > $dbh->do( >- q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed) >- VALUES (?, ?, ?, ?)}, >+ q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_record) >+ VALUES (?, ?, ?, ?, ?)}, > {}, >- '*', '*', 'CANNOT', 0 >+ '*', '*', 'CANNOT', 0, 5 > ); > > # make sure some basic sysprefs are set >@@ -317,7 +317,7 @@ is( $reserve3->{priority}, 1, "After ModReserve, the 3rd reserve becomes the fir > > ModItem({ damaged => 1 }, $item_bibnum, $itemnumber); > C4::Context->set_preference( 'AllowHoldsOnDamagedItems', 1 ); >-ok( CanItemBeReserved( $borrowernumbers[0], $itemnumber) eq 'OK', "Patron can reserve damaged item with AllowHoldsOnDamagedItems enabled" ); >+is( CanItemBeReserved( $borrowernumbers[0], $itemnumber), 'OK', "Patron can reserve damaged item with AllowHoldsOnDamagedItems enabled" ); > ok( defined( ( CheckReserves($itemnumber) )[1] ), "Hold can be trapped for damaged item with AllowHoldsOnDamagedItems enabled" ); > C4::Context->set_preference( 'AllowHoldsOnDamagedItems', 0 ); > ok( CanItemBeReserved( $borrowernumbers[0], $itemnumber) eq 'damaged', "Patron cannot reserve damaged item with AllowHoldsOnDamagedItems disabled" ); >@@ -453,6 +453,37 @@ my $res_id = AddReserve( $branch_1, $borrowernumbers[0], $bibnum, '', 1, ); > is( CanItemBeReserved( $borrowernumbers[0], $itemnumber ), > 'tooManyReserves', 'Patron cannot reserve item with hold limit of 1, 1 bib level hold placed' ); > >+# Test multipel holds per bib >+$dbh->do('DELETE FROM issuingrules'); >+ >+( $bibnum, $title, $bibitemnum ) = create_helper_biblio('DUMMY'); >+my ( $biblionumber_1, $biblioitemnumber_1, $itemnumber1 ) = >+ AddItem( { homebranch => $branch_1, holdingbranch => $branch_1 }, $bibnum ); >+my ( $biblionumber_2, $biblioitemnumber_2, $itemnumber2 ) = >+ AddItem( { homebranch => $branch_1, holdingbranch => $branch_1 }, $bibnum ); >+my ( $biblionumber_3, $biblioitemnumber_3, $itemnumber3 ) = >+ AddItem( { homebranch => $branch_1, holdingbranch => $branch_1 }, $bibnum ); >+ >+$dbh->do( >+ q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_record) >+ VALUES (?, ?, ?, ?, ?)}, >+ {}, >+ '*', '*', '*', 99, 3 >+); >+ >+AddReserve( $branch_1, $borrowernumbers[0], $bibnum, undef, undef, undef, undef, undef, undef, $itemnumber1 ); >+ >+is( CanItemBeReserved( $borrowernumbers[0], $itemnumber1 ), >+ 'itemAlreadyOnHold', 'Patron cannot request item the patron has already requested' ); >+ >+AddReserve( $branch_1, $borrowernumbers[0], $bibnum, undef, undef, undef, undef, undef, undef, $itemnumber2 ); >+ >+is( CanItemBeReserved( $borrowernumbers[0], $itemnumber3 ), 'OK', 'Patron can still request third item' ); >+ >+AddReserve( $branch_1, $borrowernumbers[0], $bibnum, undef, undef, undef, undef, undef, undef, undef ); >+ >+is( CanItemBeReserved( $borrowernumbers[0], $itemnumber3 ), >+ 'maxHoldsForRecord', 'Patron cannot place any more requests for this record' ); > > # Helper method to set up a Biblio. > sub create_helper_biblio { >diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t >index 7301757..7d67dca 100755 >--- a/t/db_dependent/Reserves.t >+++ b/t/db_dependent/Reserves.t >@@ -449,7 +449,7 @@ warning_is { > $reserve_id = AddReserve('CPL', $requesters{'CPL3'}, $bibnum, > $bibitems, $p, output_pref($resdate), $expdate, $notes, > $title, $checkitem, $found) >-} "AddReserve: borrower $requesters{CPL3} already has a hold for biblionumber $bibnum"; >+} "AddReserve: borrower $requesters{CPL3} has already reached the max holds for biblionumber $bibnum"; > is( $reserve_id, undef, 'Attempt to add a second reserve on a given record for the same patron fails.' ); > > # Tests for cancel reserves by users from OPAC. >-- >1.7.2.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 14695
:
44143
|
44144
|
44145
|
45058
|
45059
|
45060
|
45061
|
45063
|
45064
|
46548
|
46549
|
46550
|
46551
|
46552
|
46553
|
46554
|
46555
|
46556
|
46557
|
46914
|
46915
|
46916
|
46917
|
46918
|
46919
|
46920
|
47183
|
47184
|
47185
|
47186
|
47187
|
47188
|
47189
|
47190
|
47191
|
47196
|
47227
|
47228
|
47229
|
47230
|
47231
|
47232
|
47233
|
47234
|
47355
|
47372
|
47606
|
47607
|
47608
|
47609
|
47610
|
47611
|
47612
|
47613
|
47614
|
47615
|
48792
|
48795
|
48796
|
48797
|
48798
|
48799
|
48800
|
48801
|
48802
|
48803
|
50056
|
50057
|
50058
|
50059
|
50060
|
50061
|
50062
|
50063
|
50064
|
50065
|
50327
|
50391
|
51006
|
51007
|
51008
|
51009
|
51010
|
51011
|
51012
|
51013
|
51014
|
51015
|
51016
|
51017
|
51027
|
51030
|
51031
|
51032
|
51033
|
51034
|
51035
|
51036
|
51037
|
51038
|
51039
|
51040
|
51041
|
51042
|
51277
|
51278
|
51279
|
51280
|
51281
|
51282
|
51283
|
51284
|
51285
|
51286
|
51287
|
51288
|
51289
|
52457
|
54546
|
54547
|
54548
|
54549
|
54550
|
54551
|
54552
|
54553
|
54554
|
54555
|
54556
|
54557
|
54558
|
54559
|
55114
|
55115
|
55116
|
55117
|
55118
|
55119
|
55120
|
55121
|
55122
|
55123
|
55124
|
55125
|
55126
|
55127
|
55128
|
55148
|
55149
|
55150
|
55151
|
55152
|
55153
|
55154
|
55155
|
55156
|
55157
|
55158
|
55159
|
55160
|
55161
|
55162
|
55163
|
55299
|
55332
|
55730
|
55731