From 3d43a452b32e86d94ecda01a996e847b2dce0ab6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc=20V=C3=A9ron?= 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 ) %]

Hold found for ([% nextreservtitle %]), please transfer

-

Hold placed by : [% nextreservsurname %] [% nextreservfirstname %] at : [% branchname %] , Please transfer this item. +

Hold placed by: [% nextreservsurname %] [% nextreservfirstname %] at : [% branchname %] . Please transfer this item.

@@ -305,23 +305,23 @@ function checkMultiHold() {

Cannot place hold

[% ELSE %]

Cannot place hold on some items

[% IF ( exceeded_maxreserves ) %] -
  • Too many holds: [% borrowerfirstname %] [% borrowersurname %] can place [% new_reserves_allowed %] of the requested [% new_reserves_count %] holds for a maximum of [% maxreserves %] total holds.
  • +
  • Too many holds: [% borrowerfirstname %] [% borrowersurname %] can place [% new_reserves_allowed %] of the requested [% new_reserves_count %] holds for a maximum of [% maxreserves %] total holds.
  • [% END %] [% END %] @@ -331,19 +331,19 @@ function checkMultiHold() { [% IF ( expiry || diffbranch ) %]
    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