@@ -, +, @@ tests --- C4/Reserves.pm | 3 -- t/db_dependent/.Reserves.t.swp | Bin 0 -> 16384 bytes t/db_dependent/Circulation.t | 22 ++++++++++ t/db_dependent/Holds.t | 5 ++- t/db_dependent/Reserves.t | 93 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 119 insertions(+), 4 deletions(-) create mode 100644 t/db_dependent/.Reserves.t.swp --- a/C4/Reserves.pm +++ a/C4/Reserves.pm @@ -853,7 +853,6 @@ sub CheckReserves { my $priority = 10000000; foreach my $res (@reserves) { - #Before allocating to bib-level hold check if reserve respects hold rule, i.e. can patron category/item type combination allow reserves my $checkreserve = Koha::Hold->check_if_existing_hold_matches_issuingrules( $res->{'borrowernumber'}, $itemnumber ); next unless ($checkreserve eq 'OK'); @@ -910,14 +909,12 @@ sub CheckReserves { } } } - # If we get this far, then no exact match was found. # We return the most important (i.e. next) reservation. if ($highest) { $highest->{'itemnumber'} = $item; return ( "Reserved", $highest, \@reserves ); } - return ( '' ); } --- a/t/db_dependent/Circulation.t +++ a/t/db_dependent/Circulation.t @@ -278,6 +278,8 @@ Koha::CirculationRules->set_rules( auto_renew => 0, fine => .10, chargeperiod => 1, + holds_per_day => 12, + holds_per_record => 1, } } ); @@ -1559,6 +1561,7 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { branchcode => undef, rules => { onshelfholds => 0, + holds_per_day => 1 } } ); @@ -3339,6 +3342,22 @@ subtest 'Set waiting flag' => sub { my $library_2 = $builder->build( { source => 'Branch' } ); my $patron_2 = $builder->build( { source => 'Borrower', value => { branchcode => $library_2->{branchcode}, categorycode => $patron_category->{categorycode} } } ); + my $biblio = $builder->build( { source => 'Biblio' } ); + my $biblioitem = $builder->build( { source => 'Biblioitem', value => { biblionumber => $biblio->{biblionumber} } } ); + + Koha::CirculationRules->set_rules( + { + categorycode => undef, + itemtype => undef, + branchcode => undef, + rules => { + reservesallowed => 1, + holds_per_day => 1, + holds_per_record => 1, + } + } + ); + my $item = $builder->build_sample_item( { library => $library_1->{branchcode}, @@ -3366,6 +3385,9 @@ subtest 'Set waiting flag' => sub { my ( $status ) = CheckReserves($item->itemnumber); is( $status, 'Transferred', 'Hold is not waiting yet'); + my ( $status ) = CheckReserves($item->{itemnumber}); + is( $status, 'Reserved', 'Hold is not waiting yet'); + set_userenv( $library_2 ); $do_transfer = 0; AddReturn( $item->barcode, $library_2->{branchcode} ); --- a/t/db_dependent/Holds.t +++ a/t/db_dependent/Holds.t @@ -242,6 +242,7 @@ Koha::CirculationRules->set_rules( rules => { reservesallowed => 25, holds_per_record => 99, + holds_per_day => 99, } } ); @@ -251,8 +252,9 @@ Koha::CirculationRules->set_rules( branchcode => undef, itemtype => 'CANNOT', rules => { - reservesallowed => 0, + reservesallowed => 1, holds_per_record => 99, + holds_per_day => 99, } } ); @@ -382,6 +384,7 @@ AddReserve( priority => 1, } ); + is( CanItemBeReserved( $borrowernumbers[0], $item->itemnumber)->{status}, 'tooManyReserves', "cannot request item if policy that matches on item-level item type forbids it" --- a/t/db_dependent/Reserves.t +++ a/t/db_dependent/Reserves.t @@ -187,6 +187,7 @@ Koha::CirculationRules->set_rules( rules => { reservesallowed => 25, holds_per_record => 1, + holds_per_day => 1, } } ); @@ -311,6 +312,7 @@ my $messages; # the one placed by the CPL patron, as the other two patron's hold # requests cannot be filled by that item per policy. (undef, $messages, undef, undef) = AddReturn('bug10272_CPL', $branch_2); + is( $messages->{ResFound}->{borrowernumber}, $requesters{$branch_1}, 'restrictive library\'s items only fill requests by own patrons (bug 10272)'); @@ -722,6 +724,97 @@ MoveReserve( $item->itemnumber, $borrowernumber ); is( $status, 'Reserved', 'MoveReserve did not fill future hold of 3 days'); $dbh->do('DELETE FROM reserves', undef, ($bibnum)); + + + + + +#Test bug 23172. Check when returning an item that any patron category/item type combinations that forbid reservation are skipped over +#when allocating the item to a bib-level hold +$dbh->do('DELETE FROM issuingrules'); + +#Create two new patrons +my $patronnumcat1 = Koha::Patron->new({branchcode => $branch_1, categorycode => $category_1})->store->borrowernumber; +my $patronnumcat2 = Koha::Patron->new({branchcode => $branch_1, categorycode => $category_2})->store->borrowernumber; + +#Create a new biblio record +my $bibnum3 = $builder->build_sample_biblio({frameworkcode => $frameworkcode})->biblionumber; + +#Create a second item type +my $itemtype2 = $builder->build({ source => 'Itemtype', value => { notforloan => undef } } )->{itemtype}; + +# Create an item from new itemtype +my ( $item_bibnum2, $item_bibitemnum2, $itemnumber2 ) = AddItem( + { homebranch => $branch_1, + holdingbranch => $branch_1, + itype => $itemtype2, + barcode => 'bug23172_CPL' + }, + $bibnum3 +); + +#Make a general All patron category/all item type rule +$dbh->do( + q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_day, holds_per_record) + VALUES (?, ?, ?, ?, ?, ?)}, + {}, + '*', '*', '*', 25, 1, 1 +); + +#When patron category = $category_1 and itemtype = $itemtype1 then reservesallowed, holds_per_day and holds_per_record are 0 +$dbh->do( + q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_day, holds_per_record) + VALUES (?, ?, ?, ?, ?, ?)}, + {}, + $category_1, $branch_1, $itemtype2, 0, 1, 1 +); + +#When patron category = $category_2 and itemtype = $itemtype2 then reservesallowed, holds_per_day and holds_per_record are 1 +$dbh->do( + q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_day, holds_per_record) + VALUES (?, ?, ?, ?, ?, ?)}, + {}, + $category_2, $branch_1, $itemtype2, 1, 1, 1 +); + +#Remove existing reserves +$dbh->do("DELETE FROM reserves"); + +#Create Bib-level hold for borrower who has a patron category of $category_2 +my $reserveid2 = AddReserve($branch_1, $patronnumcat2, $bibnum3); + +#Bib-level hold for borrower who has a patron category of $category_1 +my $reserveid1 = AddReserve($branch_1, $patronnumcat1, $bibnum3 ); + +#Check bib-level hold priority and confirm hold made by patronnumcat1 (who fits into issuingrule with reserveallowed = 0) has first priority +my $reserve1 = Koha::Holds->find($reserveid1); +is( $reserve1->priority, 1, 'patronnumcat1 reserve is first priority'); + +my $reserve2 = Koha::Holds->find($reserveid2); +is( $reserve2->priority, 2, 'patronnumcat2 reserve is second priority'); + +#Return the item +(undef, $messages, undef, undef) = AddReturn('bug23172_CPL', $branch_1); + +#First priority hold is skipped as issuingrule is checked and the first hold does not respect issuingrules (as one of the three fields: reservesallowed, holds_per_day is 0) therefore not allocating item to this bib-level hold +is( $messages->{ResFound}->{borrowernumber}, $patronnumcat2,'Skip allocating item to first priority hold as it does not respect issuingrules instead allocate to next valid hold (bug 23172)'); + +#Remove and re-add the issuing rule that prevented patronnumcat1 bib-level hold from being allocated upon item return making reservesallowed, holds_per_day and holds_per_record > 0 +$dbh->do(q{DELETE FROM issuingrules WHERE categorycode = ? AND branchcode = ? AND itemtype = ?}, + {}, + $category_1, $branch_1, $itemtype2 +); +$dbh->do( + q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_day, holds_per_record) + VALUES (?, ?, ?, ?, ?, ?)}, + {}, + $category_1, $branch_1, $itemtype2, 1, 1, 1 +); + +#Return the item again and notice this time it is allocated to the bib-level hold made by patronnumcat1 +(undef, $messages, undef, undef) = AddReturn('bug23172_CPL', $branch_1); +is( $messages->{ResFound}->{borrowernumber}, $patronnumcat1,'Skip allocating item to first priority hold as it does not respect issuingrules instead allocate to next valid hold (bug 23172)'); + $cache->clear_from_cache("MarcStructure-0-$frameworkcode"); $cache->clear_from_cache("MarcStructure-1-$frameworkcode"); $cache->clear_from_cache("default_value_for_mod_marc-"); --