Bugzilla – Attachment 28493 Details for
Bug 12197
Exceeding the maxreserves preference does not prevent librarian from placing the hold
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 12197: enforce the maxreserves preference when staff members place hold requests
Bug-12197-enforce-the-maxreserves-preference-when-.patch (text/plain), 9.26 KB, created by
Kyle M Hall (khall)
on 2014-05-27 14:22:15 UTC
(
hide
)
Description:
Bug 12197: enforce the maxreserves preference when staff members place hold requests
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2014-05-27 14:22:15 UTC
Size:
9.26 KB
patch
obsolete
>From e7b559145b5d783c5dad557acfd5d563d1878d9f Mon Sep 17 00:00:00 2001 >From: Kyle M Hall <kyle@bywatersolutions.com> >Date: Tue, 6 May 2014 11:32:02 -0400 >Subject: [PATCH] Bug 12197: enforce the maxreserves preference when staff members place hold requests > >This patch ensures that the global maxreserves preference is enforced >when staff members place hold requests. > >For example: > >Create 3 items to place holds on. Set the circulation rule to allow 50 >holds for all items. Set maxreserves to 2. Place a hold on 3 different >items. On the third item, it will give a warning, but you can still >place the hold. Despite what the circulation rule is set for (which is >only a specific case rule), maxreserves is a global rule and >should stop this from happening, not just give a warning. > >Test Plan: >1) Reproduce the bug by following the steps above >2) Verify the bug exists >3) Apply this patch >4) Verify the librarian cannot place the hold now >5) Enable AllowHoldPolicyOverride >6) Verify the librarian can forcefully place the hold > >Signed-off-by: Galen Charlton <gmc@esilibrary.com> > >Bug 12197: (follow-up) rename variable for greater clarity > >"maxreserves" was referring both to the system preference and to the >condition of having exceeded the number of hold requests allowed. > >This patch renames a variable to remove the ambguity. > >Test plan: > >* Same as the main patch. > >Signed-off-by: Galen Charlton <gmc@esilibrary.com> >--- > .../prog/en/modules/reserve/request.tt | 11 ++- > reserve/request.pl | 73 +++++++++++++------- > 2 files changed, 55 insertions(+), 29 deletions(-) > >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 cafc072..3c7da64 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt >@@ -237,14 +237,14 @@ function checkMultiHold() { > </form> > [% ELSE %] > >-[% IF ( maxreserves || alreadyreserved || none_available || alreadypossession ) %] >+[% IF ( exceeded_maxreserves || alreadyreserved || none_available || alreadypossession ) %] > <div class="dialog alert"> > > [% UNLESS ( multi_hold ) %] > <h3>Cannot place hold</h3> > <ul> >- [% IF ( maxreserves ) %] >- <li><strong>Too many holds: </strong> <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% borrowernumber %]">[% borrowerfirstname %] [% borrowersurname %] </a> has too many holds.</li> >+ [% IF ( exceeded_maxreserves ) %] >+ <li><strong>Too many holds: </strong> <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% borrowernumber %]">[% borrowerfirstname %] [% borrowersurname %] </a> can only place a maximum of [% maxreserves %] total holds.</li> > [% END %] > [% IF ( 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> >@@ -257,7 +257,10 @@ function checkMultiHold() { > [% END %] > </ul> > [% ELSE %] >- <h3>Cannot place hold on some items</h3> >+ <h3>Cannot place hold on some items</h3> >+ [% IF ( exceeded_maxreserves ) %] >+ <li><strong>Too many holds: </strong> <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% borrowernumber %]">[% borrowerfirstname %] [% borrowersurname %] </a> can place [% new_reserves_allowed %] of the requested [% new_reserves_count %] holds for a maximum of [% maxreserves %] total holds.</li> >+ [% END %] > [% END %] > > </div> >diff --git a/reserve/request.pl b/reserve/request.pl >index 05321b8..e31aaca 100755 >--- a/reserve/request.pl >+++ b/reserve/request.pl >@@ -77,9 +77,9 @@ $findborrower = '' unless defined $findborrower; > $findborrower =~ s|,| |g; > my $borrowernumber_hold = $input->param('borrowernumber') || ''; > my $messageborrower; >-my $maxreserves; > my $warnings; > my $messages; >+my $exceeded_maxreserves; > > my $date = C4::Dates->today('iso'); > my $action = $input->param('action'); >@@ -116,23 +116,46 @@ if ($findborrower) { > } > } > >+my @biblionumbers = (); >+my $biblionumbers = $input->param('biblionumbers'); >+if ($multihold) { >+ @biblionumbers = split '/', $biblionumbers; >+} else { >+ push @biblionumbers, $input->param('biblionumber'); >+} >+ >+ > # If we have the borrowernumber because we've performed an action, then we > # don't want to try to place another reserve. > if ($borrowernumber_hold && !$action) { > my $borrowerinfo = GetMember( borrowernumber => $borrowernumber_hold ); > my $diffbranch; > my @getreservloop; >- my $count_reserv = 0; > > # we check the reserves of the borrower, and if he can reserv a document > # FIXME At this time we have a simple count of reservs, but, later, we could improve the infos "title" ... > >- my $number_reserves = >+ my $reserves_count = > GetReserveCount( $borrowerinfo->{'borrowernumber'} ); > >- if ( C4::Context->preference('maxreserves') && ($number_reserves >= C4::Context->preference('maxreserves')) ) { >- $warnings = 1; >- $maxreserves = 1; >+ my $new_reserves_count = scalar( @biblionumbers ); >+ >+ my $maxreserves = C4::Context->preference('maxreserves'); >+ if ( $maxreserves >+ && ( $reserves_count + $new_reserves_count > $maxreserves ) ) >+ { >+ my $new_reserves_allowed = >+ $maxreserves - $reserves_count > 0 >+ ? $maxreserves - $reserves_count >+ : 0; >+ $warnings = 1; >+ $exceeded_maxreserves = 1; >+ $template->param( >+ new_reserves_allowed => $new_reserves_allowed, >+ new_reserves_count => $new_reserves_count, >+ reserves_count => $reserves_count, >+ maxreserves => $maxreserves, >+ ); > } > > # we check the date expiry of the borrower (only if there is an expiry date, otherwise, set to 1 (warn) >@@ -161,7 +184,6 @@ if ($borrowernumber_hold && !$action) { > borroweremail => $borrowerinfo->{'email'}, > borroweremailpro => $borrowerinfo->{'emailpro'}, > borrowercategory => $borrowerinfo->{'category'}, >- borrowerreservs => $count_reserv, > cardnumber => $borrowerinfo->{'cardnumber'}, > expiry => $expiry, > diffbranch => $diffbranch, >@@ -175,14 +197,6 @@ $template->param( messageborrower => $messageborrower ); > # FIXME launch another time GetMember perhaps until > my $borrowerinfo = GetMember( borrowernumber => $borrowernumber_hold ); > >-my @biblionumbers = (); >-my $biblionumbers = $input->param('biblionumbers'); >-if ($multihold) { >- @biblionumbers = split '/', $biblionumbers; >-} else { >- push @biblionumbers, $input->param('biblionumber'); >-} >- > my $itemdata_enumchron = 0; > my @biblioloop = (); > foreach my $biblionumber (@biblionumbers) { >@@ -192,7 +206,9 @@ foreach my $biblionumber (@biblionumbers) { > my $dat = GetBiblioData($biblionumber); > > unless ( CanBookBeReserved($borrowerinfo->{borrowernumber}, $biblionumber) ) { >- $maxreserves = 1; >+ $exceeded_maxreserves = 1; # note that exceeding the circ policy's hold >+ # limit is not the only reason CanBookBeReserved() >+ # can return false > } > > my $alreadypossession; >@@ -289,15 +305,21 @@ foreach my $biblionumber (@biblionumbers) { > my $num_override = 0; > my $hiddencount = 0; > >- $biblioitem->{description} = >- $itemtypes->{ $biblioitem->{itemtype} }{description}; >- if($biblioitem->{biblioitemnumber} ne $biblionumber){ >- $biblioitem->{hostitemsflag}=1; >- } >+ if ( $biblioitem->{biblioitemnumber} ne $biblionumber ) { >+ $biblioitem->{hostitemsflag} = 1; >+ } >+ > $biblioloopiter{description} = $biblioitem->{description}; >- $biblioloopiter{itypename} = $biblioitem->{description}; >- $biblioloopiter{imageurl} = >- getitemtypeimagelocation('intranet', $itemtypes->{$biblioitem->{itemtype}}{imageurl}); >+ $biblioloopiter{itypename} = $biblioitem->{description}; >+ if ( $biblioitem->{itemtype} ) { >+ >+ $biblioitem->{description} = >+ $itemtypes->{ $biblioitem->{itemtype} }{description}; >+ >+ $biblioloopiter{imageurl} = >+ getitemtypeimagelocation( 'intranet', >+ $itemtypes->{ $biblioitem->{itemtype} }{imageurl} ); >+ } > > foreach my $itemnumber ( @{ $itemnumbers_of_biblioitem{$biblioitemnumber} } ) { > my $item = $iteminfos_of->{$itemnumber}; >@@ -411,6 +433,7 @@ foreach my $biblionumber (@biblionumbers) { > > if ( > $policy_holdallowed >+ && !$exceeded_maxreserves > && !$item->{cantreserve} > && IsAvailableForItemLevelRequest($itemnumber) > && CanItemBeReserved( >@@ -575,7 +598,7 @@ foreach my $biblionumber (@biblionumbers) { > > $template->param( biblioloop => \@biblioloop ); > $template->param( biblionumbers => $biblionumbers ); >-$template->param( maxreserves => $maxreserves ); >+$template->param( exceeded_maxreserves => $exceeded_maxreserves ); > > if ($multihold) { > $template->param( multi_hold => 1 ); >-- >1.7.2.5
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=28493">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 12197
:
27984
|
27985
|
27986
|
28493
|
36952
|
42873
|
42936
|
43749