Bugzilla – Attachment 45292 Details for
Bug 15243
Place a hold on... Fix display issue and improve translatability
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 15243 - Place a hold on... Fix display issue and improve translatability
Bug-15243---Place-a-hold-on-Fix-display-issue-and-.patch (text/plain), 13.21 KB, created by
Marc Véron
on 2015-12-01 21:11:43 UTC
(
hide
)
Description:
Bug 15243 - Place a hold on... Fix display issue and improve translatability
Filename:
MIME Type:
Creator:
Marc Véron
Created:
2015-12-01 21:11:43 UTC
Size:
13.21 KB
patch
obsolete
>From 3d43a452b32e86d94ecda01a996e847b2dce0ab6 Mon Sep 17 00:00:00 2001 >From: =?UTF-8?q?Marc=20V=C3=A9ron?= <veron@veron.ch> >Date: Sat, 21 Nov 2015 09:17:56 +0100 >Subject: [PATCH] Bug 15243 - Place a hold on... Fix display issue and improve > translatability > >This patch fixes strings in request.tt that are untranslatable because of >splitting by html tags inside the strings. > >Additionally, the value of maxreserves was used in one string, but not >filled in every case. > >That's why the patch introduces a new function GetAllowedReserves in >C4/Reserves.pm to retrieve the maximum count of allowed reserves (depending >on syspref maxreserves and on issuingrules). > >To test: >- If not yet done, apply patch from Bug 15244 >- Run t/db_dependent/Reserves.t, verify that it passes OK >- Apply this patch >- Run t/db_dependent/Reserves.t again, should pass OK >- Set syspref 'maxreserves' to 1 >- Try to place a hold for a patron who already has holds >- Verify that the warning message displays the value above >- Set the syspref to a high value >- Try to place a hold for a patron who has reached the maximum > allowed holds defined by issuing rules >- Verify that the warning message with the maximum number of > holds allowed by issuing rules >- Try to trigger other warning messages and/or carefully review > code in request.tt line 307 and following. > Additonal messages to trigger are: > - Already in possession > - Hold already placed > - Age restricted > - No items available > - Too many holds >--- > C4/Reserves.pm | 45 +++++++++++++------- > .../prog/en/modules/reserve/request.tt | 24 +++++------ > reserve/request.pl | 10 ++++- > t/db_dependent/Reserves.t | 7 ++- > 4 files changed, 57 insertions(+), 29 deletions(-) > >diff --git a/C4/Reserves.pm b/C4/Reserves.pm >index 6ed1543..9030f13 100644 >--- a/C4/Reserves.pm >+++ b/C4/Reserves.pm >@@ -107,6 +107,7 @@ BEGIN { > &GetReserveCount > &GetReserveInfo > &GetReserveStatus >+ &GetAllowedReserves > > &GetOtherReserves > >@@ -458,18 +459,6 @@ 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 >@@ -492,10 +481,11 @@ sub CanItemBeReserved{ > } > > # we retrieve rights >- $sth->execute($borrower->{'categorycode'}, $item->{'itype'}, $branchcode); >- if(my $rights = $sth->fetchrow_hashref()){ >+ my $rights = GetAllowedReserves($borrower->{'categorycode'}, $item->{'itype'}, $branchcode); >+ >+ if( $rights ){ > $ruleitemtype = $rights->{itemtype}; >- $allowedreserves = $rights->{reservesallowed}; >+ $allowedreserves = $rights->{reservesallowed}; > }else{ > $ruleitemtype = '*'; > } >@@ -581,6 +571,31 @@ sub CanReserveBeCanceledFromOpac { > > } > >+=head2 GetAllowedReserves >+ >+ $number = &GetAllowedReserves($borrowernumber, $itemtype, $branchcode); >+ >+this function returns the rights for a patron on itemtype and branchcode. >+=cut >+ >+sub GetAllowedReserves { >+ my ($categorycode, $itemtype, $branchcode) = @_; >+ my $dbh = C4::Context->dbh; >+ 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;" >+ ); >+ $sth->execute($categorycode, $itemtype, $branchcode); >+ return $sth->fetchrow_hashref(); >+} >+ >+ > =head2 GetReserveCount > > $number = &GetReserveCount($borrowernumber); >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..9187c0a 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt >@@ -233,7 +233,7 @@ function checkMultiHold() { > [% IF ( messagetransfert ) %] > <div class="dialog message"> > <h2>Hold found for ([% nextreservtitle %]), please transfer</h2> >- <p>Hold placed by : <strong> [% nextreservsurname %] [% nextreservfirstname %]</strong> at : <strong> [% branchname %] </strong>, Please transfer this item. >+ <p>Hold placed by: <strong> [% nextreservsurname %] [% nextreservfirstname %]</strong> at : <strong> [% branchname %] </strong>. Please transfer this item. > </p> > <form name="cancelReservewithtransfert" action="branchreserves.pl" method="post"> > <input type="submit" class="button" /> >@@ -305,23 +305,23 @@ function checkMultiHold() { > <h3>Cannot place hold</h3> > <ul> > [% 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> >+ <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 total [% maxreserves %] holds.</li> > [% 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> >+ <li><strong>Already in possession: </strong><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% borrowernumber %]">[% borrowerfirstname %] [% borrowersurname %]</a> has already one item checked out.</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><strong>Hold already placed: </strong><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% borrowernumber %]">[% borrowerfirstname %] [% borrowersurname %]</a> already has a hold on this item.</li> > [% ELSIF ( ageRestricted ) %] >- <li><strong>Age restricted</strong></li> >+ <li><strong>Age restricted: </strong><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% borrowernumber %]">[% borrowerfirstname %] [% borrowersurname %]</a> is not allowed to place this hold due to an age restriction.</li> > [% ELSIF ( none_available ) %] >- <li> <strong>No items are available</strong> to be placed on hold</li> >+ <li><strong>No items available: </strong>There are no items to be placed on hold.</li> > [% ELSIF ( 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> >+ <li><strong>Too many holds: </strong><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% borrowernumber %]">[% borrowerfirstname %] [% borrowersurname %] </a> has already too many holds.</li> > [% END %] > </ul> > [% ELSE %] > <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> >+ <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 %] > >@@ -331,19 +331,19 @@ function checkMultiHold() { > [% IF ( expiry || diffbranch ) %] > <div class="dialog message"><ul> > [% IF ( expiry ) %] >- <li><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% borrowernumber %]">[% borrowerfirstname %] [% borrowersurname %]</a>'s <strong>account has expired</strong></li> >+ <li><strong>Account expired: </strong><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% borrowernumber %]">[% borrowerfirstname %] [% borrowersurname %]</a>'s account has expired.</li> > [% END %] > > [% IF restricted %] >- <li><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% borrowernumber %]#reldebarments">[% borrowerfirstname %] [% borrowersurname %]</a> <strong>has restrictions</strong></li> >+ <li><strong>Restricted: </strong><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% borrowernumber %]#reldebarments">[% borrowerfirstname %] [% borrowersurname %]</a> has restrictions.</li> > [% END %] > > [% IF amount_outstanding && Koha.Preference('maxoutstanding') && amount_outstanding > Koha.Preference('maxoutstanding') %] >- <li><a href="/cgi-bin/koha/members/pay.pl?borrowernumber=[% borrowernumber %]">[% borrowerfirstname %] [% borrowersurname %]</a> <strong>owes [% amount_outstanding | format('%.2f') %]</strong></li> >+ <li><strong>Outstanding fines: </strong><a href="/cgi-bin/koha/members/pay.pl?borrowernumber=[% borrowernumber %]">[% borrowerfirstname %] [% borrowersurname %]</a> owes [% amount_outstanding | format('%.2f') %].</li> > [% END %] > > [% IF ( diffbranch ) %] >- <li> <strong>Pickup library is different</strong> than <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% borrowernumber %]">[% borrowerfirstname %] [% borrowersurname %]</a>'s home library ([% borrower_branchname %] / [% borrower_branchcode %] )</li> >+ <li><strong>Pickup library different: </strong><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% borrowernumber %]">[% borrowerfirstname %] [% borrowersurname %]</a>'s home library ([% borrower_branchname %] / [% borrower_branchcode %] ) does not match the pickup library.</li> > [% END %] > > </ul></div> >diff --git a/reserve/request.pl b/reserve/request.pl >index 80cf0d6..2596cb9 100755 >--- a/reserve/request.pl >+++ b/reserve/request.pl >@@ -82,6 +82,7 @@ my $messageborrower; > my $warnings; > my $messages; > my $exceeded_maxreserves; >+my $maxreserves = C4::Context->preference('maxreserves'); > > my $date = output_pref({ dt => dt_from_string, dateformat => 'iso', dateonly => 1 }); > my $action = $input->param('action'); >@@ -150,7 +151,6 @@ if ($borrowernumber_hold && !$action) { > > my $new_reserves_count = scalar( @biblionumbers ); > >- my $maxreserves = C4::Context->preference('maxreserves'); > if ( $maxreserves > && ( $reserves_count + $new_reserves_count > $maxreserves ) ) > { >@@ -225,6 +225,13 @@ foreach my $biblionumber (@biblionumbers) { > } > elsif ( $canReserve eq 'tooManyReserves' ) { > $exceeded_maxreserves = 1; >+ >+ my $rights = GetAllowedReserves($borrowerinfo->{'categorycode'}, $dat->{'itype'}, $borrowerinfo->{'branchcode'} ); >+ >+ if ( $rights ) { >+ $maxreserves = $rights->{reservesallowed} >+ if ( $maxreserves && ( $maxreserves > $rights->{reservesallowed} ) ); >+ } > } > elsif ( $canReserve eq 'ageRestricted' ) { > $template->param( $canReserve => 1 ); >@@ -614,6 +621,7 @@ foreach my $biblionumber (@biblionumbers) { > $template->param( biblioloop => \@biblioloop ); > $template->param( biblionumbers => $biblionumbers ); > $template->param( exceeded_maxreserves => $exceeded_maxreserves ); >+$template->param( maxreserves => $maxreserves ); > > if ($multihold) { > $template->param( multi_hold => 1 ); >diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t >index 8a6ab1c..261519f 100755 >--- a/t/db_dependent/Reserves.t >+++ b/t/db_dependent/Reserves.t >@@ -17,7 +17,7 @@ > > use Modern::Perl; > >-use Test::More tests => 73; >+use Test::More tests => 74; > use Test::Warn; > > use MARC::Record; >@@ -679,6 +679,11 @@ MoveReserve( $itemnumber, $borrowernumber ); > is( $status, 'Reserved', 'MoveReserve did not fill future hold of 3 days'); > $dbh->do('DELETE FROM reserves', undef, ($bibnum)); > >+# Test GetAllowedReserves (Bug 15243) >+C4::Context->set_preference( 'maxreserves', 100 ); >+my $rights = GetAllowedReserves('PT', 'BK', 'CPL' ); >+is ($rights->{reservesallowed}, 25, 'GetAllowedReserves should be 25 (defined in issuingrules'); >+ > # we reached the finish > $dbh->rollback; > >-- >1.7.10.4
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 15243
:
45111
|
45289
|
45290
|
45292