From 9a37568a4d49ab13bb5358a1922da1d7f0bda08b Mon Sep 17 00:00:00 2001 From: Olli-Antti Kivilahti Date: Mon, 20 Oct 2014 19:04:51 +0300 Subject: [PATCH] Bug 13116 - Make it possible to propagate errors from C4::Reserves::CanItemBeReserved() to the web-templates. This patch changes the way CanBookBeReserved() and CanItemBeReserved() return error messages and how they are dealt with in the templates. This change makes it possible to distinguish between different types of reservation failure. Currently only two types of errors are handled, all the way to the user, from the CanItemBeReserved(): -ageRestricted -tooManyReserves which translates to maxreserves ############# - TEST PLAN - ############# ((-- AGE RESTRICTION --)) STAFF CLIENT 1. Find a Record with Items, update the MARC Subfield 521a to "PEGI 16". 2. Get a Borrower who is younger than 16 years. 3. Place a hold for the underage Borrower for the ageRestricted Record. 4. You get a notification, that placing a hold on ageRestricted material is forbidden. (previously you just got a notification about maximum amount of reserves reached) OPAC A. In OPAC go to opac-detail.pl for the ageRestricted Biblio. The "Place Hold" -button is missing on the right sidebar if the underage user is logged in. B. From the opac-search.pl a hold can still be placed, but it will fail giving an error about age restrictions. ((-- MAXIMUM RESERVES REACHED --)) 0. Set the maxreserves -syspref to 3 (or any low value) STAFF CLIENT AND OPAC 1. Make a ton of reserves for one borrower. 2. Observe the notification about maximum reserves reached blocking your reservations. --- C4/Reserves.pm | 30 +++++++++++++------- .../prog/en/modules/reserve/request.tt | 5 +++- .../opac-tmpl/bootstrap/en/modules/opac-reserve.tt | 6 ++++ opac/opac-detail.pl | 4 +-- opac/opac-reserve.pl | 10 ++++++- reserve/request.pl | 19 ++++++++++--- t/db_dependent/Reserves.t | 6 ++-- 7 files changed, 59 insertions(+), 21 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 97990ac..19a8e97 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -448,7 +448,10 @@ sub GetReservesFromBorrowernumber { #------------------------------------------------------------------------------------- =head2 CanBookBeReserved - $error = &CanBookBeReserved($borrowernumber, $biblionumber) + $canReserve = &CanBookBeReserved($borrowernumber, $biblionumber) + if ($canReserve eq 'OK') { #We can reserve this Item! } + +See CanItemBeReserved() for possible return values. =cut @@ -462,17 +465,24 @@ sub CanBookBeReserved{ push (@$items,@hostitems); } + my $canReserve; foreach my $item (@$items){ - return 1 if CanItemBeReserved($borrowernumber, $item); + $canReserve = CanItemBeReserved($borrowernumber, $item); + return 'OK' if $canReserve eq 'OK'; } - return 0; + return $canReserve; } =head2 CanItemBeReserved - $error = &CanItemBeReserved($borrowernumber, $itemnumber) + $canReserve = &CanItemBeReserved($borrowernumber, $itemnumber) + if ($canReserve eq 'OK') { #We can reserve this Item! } -This function return 1 if an item can be issued by this borrower. +@RETURNS OK, if the Item can be reserved. + ageRestricted, if the Item is age restricted for this borrower. + damaged, if the Item is damaged. + cannotReserveFromOtherBranches, if syspref 'canreservefromotherbranches' is OK. + tooManyReserves, if the borrower has exceeded his maximum reserve amount. =cut @@ -490,11 +500,11 @@ sub CanItemBeReserved{ my $borrower = C4::Members::GetMember('borrowernumber'=>$borrowernumber); # If an item is damaged and we don't allow holds on damaged items, we can stop right here - return 0 if ( $item->{damaged} && !C4::Context->preference('AllowHoldsOnDamagedItems') ); + return 'damaged' if ( $item->{damaged} && !C4::Context->preference('AllowHoldsOnDamagedItems') ); #Check for the age restriction my ($ageRestriction, $daysToAgeRestriction) = C4::Circulation::GetAgeRestriction( $biblioData->{agerestriction}, $borrower ); - return 0 if $daysToAgeRestriction && $daysToAgeRestriction > 0; + return 'ageRestricted' if $daysToAgeRestriction && $daysToAgeRestriction > 0; my $controlbranch = C4::Context->preference('ReservesControlBranch'); my $itemtypefield = C4::Context->preference('item-level_itypes') ? "itype" : "itemtype"; @@ -561,7 +571,7 @@ sub CanItemBeReserved{ # we check if it's ok or not if( $reservecount >= $allowedreserves ){ - return 0; + return 'tooManyReserves'; } # If reservecount is ok, we check item branch if IndependentBranches is ON @@ -571,11 +581,11 @@ sub CanItemBeReserved{ { my $itembranch = $item->{homebranch}; if ($itembranch ne $borrower->{branchcode}) { - return 0; + return 'cannotReserveFromOtherBranches'; } } - return 1; + return 'OK'; } =head2 CanReserveBeCanceledFromOpac 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 6c061e4..ddec2b5 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt @@ -241,7 +241,7 @@ function checkMultiHold() { [% ELSE %] -[% IF ( maxreserves || alreadyreserved || none_available || alreadypossession ) %] +[% IF ( maxreserves || alreadyreserved || none_available || alreadypossession || ageRestricted ) %]
[% UNLESS ( multi_hold ) %] @@ -250,6 +250,9 @@ function checkMultiHold() { [% IF ( maxreserves ) %]
  • Too many holds: [% borrowerfirstname %] [% borrowersurname %] has too many holds.
  • [% END %] + [% IF ( ageRestricted ) %] +
  • Age restricted
  • + [% END %] [% IF ( alreadyreserved ) %]
  • [% borrowerfirstname %] [% borrowersurname %] already has a hold on this item
  • [% END %] 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 a0b86e8..e7e4ee7 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt @@ -52,6 +52,12 @@
    [% END %] + [% IF ( ageRestricted ) %] +
    + Sorry, you are too young to reserve this material. +
    + [% END %] + [% IF ( bad_biblionumber ) %]
    ERROR: No biblio record found for biblionumber [% bad_biblionumber %].
    diff --git a/opac/opac-detail.pl b/opac/opac-detail.pl index b856304..a817fbc 100755 --- a/opac/opac-detail.pl +++ b/opac/opac-detail.pl @@ -522,8 +522,8 @@ my $dat = &GetBiblioData($biblionumber); #Check if we can place a hold on this biblio. my $canReserve = C4::Reserves::CanBookBeReserved( $biblionumber, $biblionumber ); -unless ($canReserve ) { - $template->param( cannotReserve => 1 ) ; +unless ($canReserve eq 'OK' ) { + $template->param( cannotReserve => $canReserve ) ; } diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl index 45d03bd..0e2d88c 100755 --- a/opac/opac-reserve.pl +++ b/opac/opac-reserve.pl @@ -548,9 +548,17 @@ foreach my $biblioNum (@biblionumbers) { if ($biblioLoopIter{already_reserved}) { $biblioLoopIter{holdable} = undef; } - if(not CanBookBeReserved($borrowernumber,$biblioNum)){ + my $canReserve = CanBookBeReserved($borrowernumber,$biblioNum); + if ($canReserve eq 'OK') { + #All is OK! + } + elsif ($canReserve eq 'ageRestricted') { $biblioLoopIter{holdable} = undef; + $template->param( message => 1, $canReserve => 1 ); } + else { + $biblioLoopIter{holdable} = undef; + } if(not C4::Context->preference('AllowHoldsOnPatronsPossessions') and CheckIfIssuedToPatron($borrowernumber,$biblioNum)) { $biblioLoopIter{holdable} = undef; $biblioLoopIter{already_patron_possession} = 1; diff --git a/reserve/request.pl b/reserve/request.pl index 14340c2..a679065 100755 --- a/reserve/request.pl +++ b/reserve/request.pl @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/perl -d #writen 2/1/00 by chris@katipo.oc.nz @@ -191,9 +191,20 @@ foreach my $biblionumber (@biblionumbers) { my $dat = GetBiblioData($biblionumber); - unless ( CanBookBeReserved($borrowerinfo->{borrowernumber}, $biblionumber) ) { - $maxreserves = 1; + my $canReserve = CanBookBeReserved($borrowerinfo->{borrowernumber}, $biblionumber); + if ($canReserve eq 'OK') { + #All is OK and we can continue + } + elsif ( $canReserve eq 'tooManyReserves' ) { + $maxreserves = 1; } + elsif ( $canReserve eq 'ageRestricted' ) { + $template->param( $canReserve => 1 ); + $biblioloopiter{ $canReserve } = 1; + } + else { + $biblioloopiter{ $canReserve } = 1; + } my $alreadypossession; if (not C4::Context->preference('AllowHoldsOnPatronsPossessions') and CheckIfIssuedToPatron($borrowerinfo->{borrowernumber},$biblionumber)) { @@ -415,7 +426,7 @@ foreach my $biblionumber (@biblionumbers) { && IsAvailableForItemLevelRequest($itemnumber) && CanItemBeReserved( $borrowerinfo->{borrowernumber}, $itemnumber - ) + ) eq 'OK' ) { $item->{available} = 1; diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t index 0bc763b..b666dfe 100755 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -486,7 +486,7 @@ my ( $ageres_tagid, $ageres_subfieldid ) = GetMarcFromKohaField( "biblioitems.ag $record->append_fields( MARC::Field->new($ageres_tagid, '', '', $ageres_subfieldid => 'PEGI 16') ); C4::Biblio::ModBiblio( $record, $bibnum, '' ); -is( C4::Reserves::CanBookBeReserved($borrowernumber, $biblionumber) , 1, "Reserving an ageRestricted Biblio without a borrower dateofbirth succeeds" ); +is( C4::Reserves::CanBookBeReserved($borrowernumber, $biblionumber) , 'OK', "Reserving an ageRestricted Biblio without a borrower dateofbirth succeeds" ); #Set the dateofbirth for the Borrower making him "too young". my $now = DateTime->now(); @@ -494,14 +494,14 @@ my $duration_15years = DateTime::Duration->new(years => 15); my $past15yearsAgo = DateTime->now()->subtract_duration($duration_15years); C4::Members::ModMember( borrowernumber => $borrowernumber, dateofbirth => $past15yearsAgo->ymd() ); -is( C4::Reserves::CanBookBeReserved($borrowernumber, $biblionumber) , 0, "Reserving a 'PEGI 16' Biblio by a 15 year old borrower fails"); +is( C4::Reserves::CanBookBeReserved($borrowernumber, $biblionumber) , 'ageRestricted', "Reserving a 'PEGI 16' Biblio by a 15 year old borrower fails"); #Set the dateofbirth for the Borrower making him "too old". my $duration_30years = DateTime::Duration->new(years => 30); my $past30yearsAgo = DateTime->now()->subtract_duration($duration_30years); C4::Members::ModMember( borrowernumber => $borrowernumber, dateofbirth => $past30yearsAgo->ymd() ); -is( C4::Reserves::CanBookBeReserved($borrowernumber, $biblionumber) , 1, "Reserving a 'PEGI 16' Biblio by a 30 year old borrower succeeds"); +is( C4::Reserves::CanBookBeReserved($borrowernumber, $biblionumber) , 'OK', "Reserving a 'PEGI 16' Biblio by a 30 year old borrower succeeds"); #### ####### EO Bug 13113 <<< #### -- 1.7.9.5