@@ -, +, @@
and is missing data
message should end "...cannot place holds on some of these titles'
items"
---
C4/Reserves.pm | 22 +-
.../prog/en/modules/reserve/request.tt | 12 +-
reserve/request.pl | 9 +-
t/db_dependent/Holds.t | 225 +++++++++++++++++++--
4 files changed, 244 insertions(+), 24 deletions(-)
--- a/C4/Reserves.pm
+++ a/C4/Reserves.pm
@@ -425,9 +425,13 @@ sub CanItemBeReserved {
$search_params->{found} = undef if $params->{ignore_found_holds};
my $holds = Koha::Holds->search($search_params);
- if ( defined $holds_per_record && $holds_per_record ne ''
- && $holds->count() >= $holds_per_record ) {
- return { status => "tooManyHoldsForThisRecord", limit => $holds_per_record };
+ if ( defined $holds_per_record && $holds_per_record ne '' ){
+ if ( $holds_per_record == 0 ) {
+ return { status => "noReservesAllowed" };
+ }
+ if ( $holds->count() >= $holds_per_record ) {
+ return { status => "tooManyHoldsForThisRecord", limit => $holds_per_record };
+ }
}
my $today_holds = Koha::Holds->search({
@@ -468,9 +472,13 @@ sub CanItemBeReserved {
}
# we check if it's ok or not
- if ( defined $allowedreserves && $allowedreserves ne ''
- && $reservecount >= $allowedreserves ) {
- return { status => 'tooManyReserves', limit => $allowedreserves };
+ if ( defined $allowedreserves && $allowedreserves ne '' ){
+ if( $allowedreserves == 0 ){
+ return { status => 'noReservesAllowed' };
+ }
+ if ( $reservecount >= $allowedreserves ) {
+ return { status => 'tooManyReserves', limit => $allowedreserves };
+ }
}
# Now we need to check hold limits by patron category
@@ -494,7 +502,7 @@ sub CanItemBeReserved {
my $reserves_control_branch =
GetReservesControlBranch( $item->unblessed(), $borrower );
my $branchitemrule =
- C4::Circulation::GetBranchItemRule( $reserves_control_branch, $item->itype ); # FIXME Should not be item->effective_itemtype?
+ C4::Circulation::GetBranchItemRule( $reserves_control_branch, $item->effective_itemtype );
if ( $branchitemrule->{holdallowed} == 0 ) {
return { status => 'notReservable' };
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
+++ a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
@@ -300,13 +300,15 @@
[% END %]
- [% IF ( exceeded_maxreserves || exceeded_holds_per_record || alreadyreserved || none_available || alreadypossession || ageRestricted ) %]
+ [% IF ( no_reserves_allowed || exceeded_maxreserves || exceeded_holds_per_record || alreadyreserved || none_available || alreadypossession || ageRestricted ) %]
[% UNLESS ( multi_hold ) %]
Cannot place hold
[% ELSE # UNLESS multi_hold %]
Cannot place hold on some items
- [% IF ( exceeded_maxreserves ) %]
+ [% IF (no_reserves_allowed ) %]
+
No holds allowed: [% patron.firstname | html %] [% patron.surname | html %] cannot place holds on some of these title's items.
+ [% ELSIF ( exceeded_maxreserves ) %]
Too many holds: [% patron.firstname | html %] [% patron.surname | html %] can place [% new_reserves_allowed | html %] of the requested [% new_reserves_count | html %] holds for a maximum of [% maxreserves | html %] total holds.
[% ELSIF ( exceeded_holds_per_record ) %]
[% FOREACH biblioloo IN biblioloop %]
@@ -577,6 +581,8 @@
Patron already has hold for this item
[% ELSIF itemloo.not_holdable == 'cannotBeTransferred' %]
Cannot be transferred to pickup library
+ [% ELSIF itemloo.not_holdable == 'noReservesAllowed' %]
+ No reserves are allowed on this item
[% ELSE %]
[% itemloo.not_holdable | html %]
[% END %]
--- a/reserve/request.pl
+++ a/reserve/request.pl
@@ -185,6 +185,8 @@ if ($borrowernumber_hold && !$action) {
my $new_reserves_count = scalar( @biblionumbers );
my $maxreserves = C4::Context->preference('maxreserves');
+ $template->param( maxreserves => $maxreserves );
+
if ( $maxreserves
&& ( $reserves_count + $new_reserves_count > $maxreserves ) )
{
@@ -287,6 +289,7 @@ if ($patron) {
my $itemdata_enumchron = 0;
my $itemdata_ccode = 0;
my @biblioloop = ();
+my $no_reserves_allowed = 0;
foreach my $biblionumber (@biblionumbers) {
next unless $biblionumber =~ m|^\d+$|;
@@ -302,7 +305,10 @@ foreach my $biblionumber (@biblionumbers) {
#All is OK and we can continue
}
- elsif ( $canReserve->{status} eq 'tooManyReserves' ) {
+ elsif ( $canReserve eq 'noReservesAllowed') {
+ $no_reserves_allowed = 1;
+ }
+ elsif ( $canReserve eq 'tooManyReserves' ) {
$exceeded_maxreserves = 1;
$template->param( maxreserves => $canReserve->{limit} );
}
@@ -745,6 +751,7 @@ foreach my $biblionumber (@biblionumbers) {
$template->param( biblioloop => \@biblioloop );
$template->param( biblionumbers => $biblionumbers );
+$template->param( no_reserves_allowed => $no_reserves_allowed );
$template->param( exceeded_maxreserves => $exceeded_maxreserves );
$template->param( exceeded_holds_per_record => $exceeded_holds_per_record );
$template->param( subscriptionsnumber => CountSubscriptionFromBiblionumber($biblionumber));
--- a/t/db_dependent/Holds.t
+++ a/t/db_dependent/Holds.t
@@ -7,7 +7,7 @@ use t::lib::TestBuilder;
use C4::Context;
-use Test::More tests => 66;
+use Test::More tests => 65;
use MARC::Record;
use C4::Biblio;
@@ -381,22 +381,221 @@ AddReserve(
}
);
is(
- CanItemBeReserved( $borrowernumbers[0], $item->itemnumber)->{status}, 'tooManyReserves',
+ CanItemBeReserved( $borrowernumbers[0], $item->itemnumber)->{status}, 'noReservesAllowed',
"cannot request item if policy that matches on item-level item type forbids it"
);
-$item->itype('CAN')->store;
-ok(
- CanItemBeReserved( $borrowernumbers[0], $item->itemnumber)->{status} eq 'OK',
- "can request item if policy that matches on item type allows it"
-);
+subtest 'CanItemBeReserved' => sub {
+ plan tests => 2;
-t::lib::Mocks::mock_preference('item-level_itypes', 0);
-$item->itype(undef)->store;
-ok(
- CanItemBeReserved( $borrowernumbers[0], $item->itemnumber)->{status} eq 'tooManyReserves',
- "cannot request item if policy that matches on bib-level item type forbids it (bug 9532)"
-);
+ my $itemtype_can = $builder->build({source => "Itemtype"})->{itemtype};
+ my $itemtype_cant = $builder->build({source => "Itemtype"})->{itemtype};
+ my $itemtype_cant_record = $builder->build({source => "Itemtype"})->{itemtype};
+
+ Koha::CirculationRules->set_rules(
+ {
+ categorycode => undef,
+ branchcode => undef,
+ itemtype => $itemtype_cant,
+ rules => {
+ reservesallowed => 0,
+ holds_per_record => 99,
+ }
+ }
+ );
+ Koha::CirculationRules->set_rules(
+ {
+ categorycode => undef,
+ branchcode => undef,
+ itemtype => $itemtype_can,
+ rules => {
+ reservesallowed => 2,
+ holds_per_record => 2,
+ }
+ }
+ );
+ Koha::CirculationRules->set_rules(
+ {
+ categorycode => undef,
+ branchcode => undef,
+ itemtype => $itemtype_cant_record,
+ rules => {
+ reservesallowed => 0,
+ holds_per_record => 0,
+ }
+ }
+ );
+
+ Koha::CirculationRules->set_rules(
+ {
+ branchcode => $branch_1,
+ itemtype => $itemtype_cant,
+ rules => {
+ holdallowed => 0,
+ returnbranch => 'homebranch',
+ }
+ }
+ );
+ Koha::CirculationRules->set_rules(
+ {
+ branchcode => $branch_1,
+ itemtype => $itemtype_can,
+ rules => {
+ holdallowed => 1,
+ returnbranch => 'homebranch',
+ }
+ }
+ );
+
+ subtest 'noReservesAllowed' => sub {
+ plan tests => 5;
+
+ my $biblionumber_cannot = $builder->build_sample_biblio({ itemtype => $itemtype_cant })->biblionumber;
+ my $biblionumber_can = $builder->build_sample_biblio({ itemtype => $itemtype_can })->biblionumber;
+ my $biblionumber_record_cannot = $builder->build_sample_biblio({ itemtype => $itemtype_cant_record })->biblionumber;
+
+ my $itemnumber_1_can = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can, biblionumber => $biblionumber_cannot })->itemnumber;
+ my $itemnumber_1_cannot = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_cant, biblionumber => $biblionumber_cannot })->itemnumber;
+ my $itemnumber_2_can = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can, biblionumber => $biblionumber_can })->itemnumber;
+ my $itemnumber_2_cannot = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_cant, biblionumber => $biblionumber_can })->itemnumber;
+ my $itemnumber_3_cannot = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_cant_record, biblionumber => $biblionumber_record_cannot })->itemnumber;
+
+ Koha::Holds->search({borrowernumber => $borrowernumbers[0]})->delete;
+
+ t::lib::Mocks::mock_preference('item-level_itypes', 1);
+ is(
+ CanItemBeReserved( $borrowernumbers[0], $itemnumber_2_cannot)->{status}, 'noReservesAllowed',
+ "With item level set, rule from item must be picked (CANNOT)"
+ );
+ is(
+ CanItemBeReserved( $borrowernumbers[0], $itemnumber_1_can)->{status}, 'OK',
+ "With item level set, rule from item must be picked (CAN)"
+ );
+ t::lib::Mocks::mock_preference('item-level_itypes', 0);
+ is(
+ CanItemBeReserved( $borrowernumbers[0], $itemnumber_1_can)->{status}, 'noReservesAllowed',
+ "With biblio level set, rule from biblio must be picked (CANNOT)"
+ );
+ is(
+ CanItemBeReserved( $borrowernumbers[0], $itemnumber_2_cannot)->{status}, 'OK',
+ "With biblio level set, rule from biblio must be picked (CAN)"
+ );
+ is(
+ CanItemBeReserved( $borrowernumbers[0], $itemnumber_3_cannot)->{status}, 'noReservesAllowed',
+ "When no holds allowed and no holds per record allowed should return noReservesAllowed"
+ );
+ };
+
+ subtest 'tooManyHoldsForThisRecord + tooManyReserves + itemAlreadyOnHold' => sub {
+ plan tests => 7;
+
+ my $biblionumber_1 = $builder->build_sample_biblio({ itemtype => $itemtype_can })->biblionumber;
+ my $itemnumber_11 = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can, biblionumber => $biblionumber_1 })->itemnumber;
+ my $itemnumber_12 = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can, biblionumber => $biblionumber_1 })->itemnumber;
+ my $biblionumber_2 = $builder->build_sample_biblio({ itemtype => $itemtype_can })->biblionumber;
+ my $itemnumber_21 = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can, biblionumber => $biblionumber_2 })->itemnumber;
+ my $itemnumber_22 = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can, biblionumber => $biblionumber_2 })->itemnumber;
+
+ Koha::Holds->search({borrowernumber => $borrowernumbers[0]})->delete;
+
+ # Biblio-level hold
+ AddReserve({
+ branch => $branch_1,
+ borrowernumber => $borrowernumbers[0],
+ biblionumber => $biblionumber_1,
+ });
+ for my $item_level ( 0..1 ) {
+ t::lib::Mocks::mock_preference('item-level_itypes', $item_level);
+ is(
+ # FIXME This is not really correct, but CanItemBeReserved does not check if biblio-level holds already exist
+ CanItemBeReserved( $borrowernumbers[0], $itemnumber_11)->{status}, 'OK',
+ "A biblio-level hold already exists - another hold can be placed on a specific item item"
+ );
+ }
+
+ Koha::Holds->search({borrowernumber => $borrowernumbers[0]})->delete;
+ # Item-level hold
+ AddReserve({
+ branch => $branch_1,
+ borrowernumber => $borrowernumbers[0],
+ biblionumber => $biblionumber_1,
+ itemnumber => $itemnumber_11,
+ });
+
+ $dbh->do('DELETE FROM circulation_rules');
+ Koha::CirculationRules->set_rules(
+ {
+ categorycode => undef,
+ branchcode => undef,
+ itemtype => undef,
+ rules => {
+ reservesallowed => 5,
+ holds_per_record => 1,
+ }
+ }
+ );
+ is(
+ CanItemBeReserved( $borrowernumbers[0], $itemnumber_12)->{status}, 'tooManyHoldsForThisRecord',
+ "A item-level hold already exists and holds_per_record=1, another hold cannot be placed on this record"
+ );
+ Koha::CirculationRules->set_rules(
+ {
+ categorycode => undef,
+ branchcode => undef,
+ itemtype => undef,
+ rules => {
+ reservesallowed => 1,
+ holds_per_record => 1,
+ }
+ }
+ );
+ is(
+ CanItemBeReserved( $borrowernumbers[0], $itemnumber_12)->{status}, 'tooManyHoldsForThisRecord',
+ "A item-level hold already exists and holds_per_record=1 - tooManyHoldsForThisRecord has priority over tooManyReserves"
+ );
+ Koha::CirculationRules->set_rules(
+ {
+ categorycode => undef,
+ branchcode => undef,
+ itemtype => undef,
+ rules => {
+ reservesallowed => 5,
+ holds_per_record => 2,
+ }
+ }
+ );
+ is(
+ CanItemBeReserved( $borrowernumbers[0], $itemnumber_12)->{status}, 'OK',
+ "A item-level hold already exists but holds_per_record=2- another item-level hold can be placed on this record"
+ );
+
+ AddReserve({
+ branch => $branch_1,
+ borrowernumber => $borrowernumbers[0],
+ biblionumber => $biblionumber_2,
+ itemnumber => $itemnumber_21
+ });
+ Koha::CirculationRules->set_rules(
+ {
+ categorycode => undef,
+ branchcode => undef,
+ itemtype => undef,
+ rules => {
+ reservesallowed => 2,
+ holds_per_record => 2,
+ }
+ }
+ );
+ is(
+ CanItemBeReserved( $borrowernumbers[0], $itemnumber_21)->{status}, 'itemAlreadyOnHold',
+ "A item-level holds already exists on this item, itemAlreadyOnHold should be raised"
+ );
+ is(
+ CanItemBeReserved( $borrowernumbers[0], $itemnumber_22)->{status}, 'tooManyReserves',
+ "This patron has already placed reservesallowed holds, tooManyReserves should be raised"
+ );
+ };
+};
# Test branch item rules
--