@@ -, +, @@
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 <<<
####
--