@@ -, +, @@ C4::Reserves::CanItemBeReserved() to the web-templates. -ageRestricted -tooManyReserves which translates to maxreserves ############# - TEST PLAN - ############# forbidden. (previously you just got a notification about maximum amount of reserves reached) (Not individual results "Place hold") reserved just fine. --- C4/ILSDI/Services.pm | 6 ++-- C4/Reserves.pm | 30 +++++++++++++------- .../prog/en/modules/reserve/request.tt | 5 +++- .../opac-tmpl/bootstrap/en/modules/opac-reserve.tt | 3 ++ opac/opac-reserve.pl | 17 +++++++---- reserve/request.pl | 17 +++++++++-- t/db_dependent/Holds.t | 16 +++++------ t/db_dependent/Reserves.t | 6 ++-- 8 files changed, 66 insertions(+), 34 deletions(-) --- a/C4/ILSDI/Services.pm +++ a/C4/ILSDI/Services.pm @@ -489,7 +489,7 @@ sub GetServices { # Reserve level management my $biblionumber = $item->{'biblionumber'}; my $canbookbereserved = CanBookBeReserved( $borrower, $biblionumber ); - if ($canbookbereserved) { + if ($canbookbereserved eq 'OK') { push @availablefor, 'title level hold'; my $canitembereserved = IsAvailableForItemLevelRequest($itemnumber); if ($canitembereserved) { @@ -609,7 +609,7 @@ sub HoldTitle { my $title = $$biblio{title}; # Check if the biblio can be reserved - return { code => 'NotHoldable' } unless CanBookBeReserved( $borrowernumber, $biblionumber ); + return { code => 'NotHoldable' } unless CanBookBeReserved( $borrowernumber, $biblionumber ) eq 'OK'; my $branch; @@ -687,7 +687,7 @@ sub HoldItem { # Check for item disponibility my $canitembereserved = C4::Reserves::CanItemBeReserved( $borrowernumber, $itemnumber ); my $canbookbereserved = C4::Reserves::CanBookBeReserved( $borrowernumber, $biblionumber ); - return { code => 'NotHoldable' } unless $canbookbereserved and $canitembereserved; + return { code => 'NotHoldable' } unless $canbookbereserved eq 'OK' and $canitembereserved eq 'OK'; # Pickup branch management my $branch; --- a/C4/Reserves.pm +++ a/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 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt +++ a/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 %] --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt +++ a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt @@ -143,6 +143,9 @@

    [% UNLESS ( bibitemloo.holdable ) %] + [% IF ( bibitemloo.ageRestricted ) %] +
    Sorry, you are too young to reserve this material.
    + [% END %] [% IF ( bibitemloo.already_reserved ) %]
    You have already requested this title.
    [% ELSE %] --- a/opac/opac-reserve.pl +++ a/opac/opac-reserve.pl @@ -252,7 +252,7 @@ if ( $query->param('place_reserve') ) { # holdingbranch, force the value $rank and $found. my $rank = $biblioData->{rank}; if ( $itemNum ne '' ) { - $canreserve = 1 if CanItemBeReserved( $borrowernumber, $itemNum ); + $canreserve = 1 if CanItemBeReserved( $borrowernumber, $itemNum ) eq 'OK'; $rank = '0' unless C4::Context->preference('ReservesNeedReturns'); my $item = GetItem($itemNum); if ( $item->{'holdingbranch'} eq $branch ) { @@ -261,7 +261,7 @@ if ( $query->param('place_reserve') ) { } } else { - $canreserve = 1 if CanBookBeReserved( $borrowernumber, $biblioNum ); + $canreserve = 1 if CanBookBeReserved( $borrowernumber, $biblioNum ) eq 'OK'; # Inserts a null into the 'itemnumber' field of 'reserves' table. $itemNum = undef; @@ -524,7 +524,7 @@ foreach my $biblioNum (@biblionumbers) { $policy_holdallowed = 0; } - if (IsAvailableForItemLevelRequest($itemNum) and $policy_holdallowed and CanItemBeReserved($borrowernumber,$itemNum) and ($itemLoopIter->{already_reserved} ne 1)) { + if (IsAvailableForItemLevelRequest($itemNum) and $policy_holdallowed and CanItemBeReserved($borrowernumber,$itemNum) eq 'OK' and ($itemLoopIter->{already_reserved} ne 1)) { $itemLoopIter->{available} = 1; $numCopiesAvailable++; } @@ -548,9 +548,14 @@ foreach my $biblioNum (@biblionumbers) { if ($biblioLoopIter{already_reserved}) { $biblioLoopIter{holdable} = undef; } - if(not CanBookBeReserved($borrowernumber,$biblioNum)){ - $biblioLoopIter{holdable} = undef; - } + my $canReserve = CanBookBeReserved($borrowernumber,$biblioNum); + if ($canReserve eq 'OK') { + #All is OK! + } + else { + $biblioLoopIter{holdable} = undef; + $biblioLoopIter{ $canReserve } = 1; + } if(not C4::Context->preference('AllowHoldsOnPatronsPossessions') and CheckIfIssuedToPatron($borrowernumber,$biblioNum)) { $biblioLoopIter{holdable} = undef; $biblioLoopIter{already_patron_possession} = 1; --- a/reserve/request.pl +++ a/reserve/request.pl @@ -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; --- a/t/db_dependent/Holds.t +++ a/t/db_dependent/Holds.t @@ -210,7 +210,7 @@ t::lib::Mocks::mock_preference('item-level_itypes', 1); # if IndependentBranches is OFF, a CPL patron can reserve an MPL item t::lib::Mocks::mock_preference('IndependentBranches', 0); ok( - CanItemBeReserved($borrowernumbers[0], $foreign_itemnumber), + CanItemBeReserved($borrowernumbers[0], $foreign_itemnumber) eq 'OK', 'CPL patron allowed to reserve MPL item with IndependentBranches OFF (bug 2394)' ); @@ -218,14 +218,14 @@ ok( t::lib::Mocks::mock_preference('IndependentBranches', 1); t::lib::Mocks::mock_preference('canreservefromotherbranches', 0); ok( - ! CanItemBeReserved($borrowernumbers[0], $foreign_itemnumber), + CanItemBeReserved($borrowernumbers[0], $foreign_itemnumber) eq 'cannotReserveFromOtherBranches', 'CPL patron NOT allowed to reserve MPL item with IndependentBranches ON ... (bug 2394)' ); # ... unless canreservefromotherbranches is ON t::lib::Mocks::mock_preference('canreservefromotherbranches', 1); ok( - CanItemBeReserved($borrowernumbers[0], $foreign_itemnumber), + CanItemBeReserved($borrowernumbers[0], $foreign_itemnumber) eq 'OK', '... unless canreservefromotherbranches is ON (bug 2394)' ); @@ -294,10 +294,10 @@ 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), "Patron can reserve damaged item with AllowHoldsOnDamagedItems enabled" ); +ok( CanItemBeReserved( $borrowernumbers[0], $itemnumber) eq '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), "Patron cannot reserve damaged item with AllowHoldsOnDamagedItems disabled" ); +ok( CanItemBeReserved( $borrowernumbers[0], $itemnumber) eq 'damaged', "Patron cannot reserve damaged item with AllowHoldsOnDamagedItems disabled" ); ok( !defined( ( CheckReserves($itemnumber) )[1] ), "Hold cannot be trapped for damaged item with AllowHoldsOnDamagedItems disabled" ); # Regression test for bug 9532 @@ -312,19 +312,19 @@ AddReserve( 1, ); ok( - !CanItemBeReserved( $borrowernumbers[0], $itemnumber), + CanItemBeReserved( $borrowernumbers[0], $itemnumber) eq 'tooManyReserves', "cannot request item if policy that matches on item-level item type forbids it" ); ModItem({ itype => 'CAN' }, $item_bibnum, $itemnumber); ok( - CanItemBeReserved( $borrowernumbers[0], $itemnumber), + CanItemBeReserved( $borrowernumbers[0], $itemnumber) eq 'OK', "can request item if policy that matches on item type allows it" ); t::lib::Mocks::mock_preference('item-level_itypes', 0); ModItem({ itype => undef }, $item_bibnum, $itemnumber); ok( - !CanItemBeReserved( $borrowernumbers[0], $itemnumber), + CanItemBeReserved( $borrowernumbers[0], $itemnumber) eq 'tooManyReserves', "cannot request item if policy that matches on bib-level item type forbids it (bug 9532)" ); --- a/t/db_dependent/Reserves.t +++ a/t/db_dependent/Reserves.t @@ -485,20 +485,20 @@ 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(); C4::Members::SetAge( $borrower, '0015-00-00' ); C4::Members::ModMember( borrowernumber => $borrowernumber, dateofbirth => $borrower->{dateofbirth} ); -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". C4::Members::SetAge( $borrower, '0030-00-00' ); C4::Members::ModMember( borrowernumber => $borrowernumber, dateofbirth => $borrower->{dateofbirth} ); -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 <<< #### --