@@ -, +, @@ --- C4/Reserves.pm | 11 +- Koha/Hold.pm | 43 +++++--- t/db_dependent/Circulation.t | 3 - t/db_dependent/Hold.t | 235 +++++++++++++++++++++++++++---------------- t/db_dependent/Reserves.t | 186 +++++++++++++++++++++++----------- 5 files changed, 309 insertions(+), 169 deletions(-) --- a/C4/Reserves.pm +++ a/C4/Reserves.pm @@ -595,6 +595,10 @@ sub CanReserveBeCanceledFromOpac { return 0 unless $reserve->borrowernumber == $borrowernumber; return $reserve->is_cancelable_from_opac; + return 0 if ( $reserve->found && $reserve->found eq 'W' ) or ( $reserve->found && $reserve->found eq 'T' ); + + return 1; + } =head2 GetOtherReserves @@ -854,7 +858,12 @@ 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 ); + my $checkreserve; + if ( $item ){ + $checkreserve = Koha::Holds->find( $res->{reserve_id} )->matches_circulation_rules({ itemnumber => $item }); + } else { + $checkreserve = Koha::Holds->find( $res->{reserve_id} )->matches_circulation_rules({ barcode => $barcode }); + } next unless ($checkreserve eq 'OK'); if ($res->{'found'} && $res->{'found'} eq 'W') { return ( "Waiting", $res, \@reserves ); # Found it, it is waiting --- a/Koha/Hold.pm +++ a/Koha/Hold.pm @@ -36,7 +36,6 @@ use Koha::Items; use Koha::Libraries; use Koha::Old::Holds; use Koha::Calendar; - use Koha::Exceptions::Hold; use base qw(Koha::Object); @@ -476,41 +475,53 @@ sub is_suspended { return $self->suspend(); } -=head3 check_if_existing_hold_matches_issuingrules +=head3 matches_circulation_rules -my $checkreserve = Koha::Hold->check_if_existing_hold_matches_issuingrules($res->{'borrowernumber'}, $itemnumber ); + my $checkreserve = Koha::Hold->matches_circulation_rules({ itemnumber => $itemnumber }); + my $checkreserve = Koha::Hold->matches_circulation_rules({ barcode => $barcode }); Upon checking in an item of a bibliographic record with a biblio-level hold on it, check if a bib-level hold can be allocated to a patron based on the current values in the issuingrules table +Parameter is an attribute of the item that is being checked in + =cut -sub check_if_existing_hold_matches_issuingrules { - my ( $self, $borrowernumber, $itemnumber ) = @_; +sub matches_circulation_rules { + my ( $self, $params ) = @_; #Get patron and item objects - my $patron = Koha::Patrons->find( $borrowernumber ); - my $item = Koha::Items->find( $itemnumber ); - $item = $item->unblessed(); + my $patron = $self->borrower; + my $item = Koha::Items->find( $params->{itemnumber} ); + if ( !defined $item ){ + $item = Koha::Items->find({ barcode => $params->{barcode} }); + } #Get branchcode my $branchcode; my $controlbranch = C4::Context->preference('ReservesControlBranch'); - - if ( $controlbranch eq "ItemHomeLibrary" ) { - $branchcode = $item->{homebranch}; - } - elsif ( $controlbranch eq "PatronLibrary" ) { + if ( $controlbranch eq "ItemHomeLibrary" && defined $item ) { + $branchcode = $item->homebranch; + } else { $branchcode = $patron->branchcode; } - my $issuingrule = Koha::IssuingRules->get_effective_issuing_rule({ + my $issuingrule = Koha::CirculationRules->get_effective_rules({ branchcode => $branchcode, categorycode => $patron->categorycode, - itemtype => $item->{itype}, + itemtype => $item->effective_itemtype, + rules => [ + 'reservesallowed', + 'holds_per_record', + 'holds_per_day', + ], }); #Check if the patron category/item type combination is valid - if ( ($issuingrule->reservesallowed == 0 || $issuingrule->holds_per_record == 0 || $issuingrule->holds_per_day == 0 )) { + if ( + ( defined $issuingrule->{reservesallowed} && $issuingrule->{reservesallowed} == 0 ) || + ( defined $issuingrule->{holds_per_record} && $issuingrule->{holds_per_record} == 0 ) || + ( defined $issuingrule->{holds_per_day} && $issuingrule->{holds_per_day} == 0 ) + ) { return 'InvalidHold'; } else { return 'OK'; --- a/t/db_dependent/Circulation.t +++ a/t/db_dependent/Circulation.t @@ -3385,9 +3385,6 @@ 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/Hold.t +++ a/t/db_dependent/Hold.t @@ -28,7 +28,8 @@ use Koha::Holds; use Koha::Item; use Koha::DateUtils; use t::lib::TestBuilder; -use Koha::IssuingRules; +use Koha::CirculationRules; +use C4::Reserves; use Test::More tests => 33; use Test::Exception; @@ -288,99 +289,155 @@ subtest 'suspend() tests' => sub { $schema->storage->txn_rollback; }; -subtest "check_if_existing_hold_matches_issuingrules tests" => sub { +subtest "matches_circulation_rules tests" => sub { plan tests => 9; $schema->storage->txn_begin(); #Create test data - my $branchcode = $builder->build( { source => 'Branch' } )->{ branchcode }; - my $categorycode1 = $builder->build({ source => 'Category' })->{categorycode}; - my $categorycode2 = $builder->build({ source => 'Category' })->{categorycode}; - my $patron1 = $builder->build({source => 'Borrower', value => { branchcode => $branchcode }}); - - my $item1 = $builder->build({ source => 'Item', value => { homebranch => $branchcode }}); - my $item2 = $builder->build({ source => 'Item', value => { homebranch => $branchcode }}); - - my $itemtype1 = $item1->{'itype'}; - my $itemtype2 = $item2->{'itype'}; - - Koha::IssuingRules->delete; - - #Get branchcode - my $controlbranch = C4::Context->preference('ReservesControlBranch'); - - if ( $controlbranch eq "ItemHomeLibrary" ) { - $branchcode = $item->{homebranch}; - } - elsif ( $controlbranch eq "PatronLibrary" ) { - $branchcode = $patron1->{branchcode}; - } - - #Test all cases when the check_return - #check_if_existing_hold_matches_issuingrules() returns 'InvalidHold' when reservesallowed, holds_per_record and holds_per_day are all 0 - my $rule1 = Koha::IssuingRule->new({ - branchcode => $branchcode, - categorycode => $patron1->{'categorycode'}, - itemtype => $item1->{'itype'}, - reservesallowed => 0, - holds_per_record => 0, - holds_per_day => 0, - })->store; - my $checkreserve1 = Koha::Hold->check_if_existing_hold_matches_issuingrules($patron1->{'borrowernumber'}, $item1->{'itemnumber'} ); - is( $checkreserve1, 'InvalidHold', 'Allocating item1 to a bib-reserve is not allowed as the issuingrules reservesallowed, holds_per_record and holds_per_day is 0 for this patron category/itemtype combination' ); - - #Confirm that when any of the 4 reserves related issuingrules fields (reservesallowed, holds_per_record, holds_per_day) are set to 0 then check_if_existing_hold_matches_issuingrules() returns 'InvalidHold' and so a newly returned item is not allocated to a specific bib-level reserve - - #check_if_existing_hold_matches_issuingrules() returns 'InvalidHold' when reservesallowed is 0 for the patron category/itemtype combination - $rule1->set({ reservesallowed => 0, holds_per_record => 1, holds_per_day => 1 })->store; - my $checkreserve2 = Koha::Hold->check_if_existing_hold_matches_issuingrules($patron1->{'borrowernumber'}, $item1->{'itemnumber'} ); - is( $checkreserve2, 'InvalidHold', 'Allocating item1 to a bib-reserve is not allowed as the issuingrules reservesallowed is 0 for this patron category/itemtype combination' ); - - #Now change to reservesallowed = 1, holds_per_record = 0, holds_per_day = 1 - $rule1->set({ reservesallowed => 1, holds_per_record => 0, holds_per_day => 1 })->store; - my $checkreserve3 = Koha::Hold->check_if_existing_hold_matches_issuingrules($patron1->{'borrowernumber'}, $item1->{'itemnumber'} ); - is( $checkreserve3, 'InvalidHold', 'Allocating item1 to a bib-reserve is not allowed as the issuingrules holds_per_record is set to 0 for this patron category/itemtype combination' ); - - #Now change to reservesallowed = 1, holds_per_record = 1, holds_per_day = 0 - $rule1->set({ reservesallowed => 1, holds_per_record => 1, holds_per_day => 0 })->store; - my $checkreserve4 = Koha::Hold->check_if_existing_hold_matches_issuingrules($patron1->{'borrowernumber'}, $item1->{'itemnumber'} ); - is( $checkreserve4, 'InvalidHold', 'Allocating item1 to a bib-reserve is not allowed as the issuingrules holds_per_record is set to0 for this patron category/itemtype combination' ); - - #Now change to reservesallowed = 1, holds_per_record = 1, holds_per_day = 1 and confirm 'OK' is returned - $rule1->set({ reservesallowed => 1, holds_per_record => 1, holds_per_day => 1 })->store; - my $checkreserve5 = Koha::Hold->check_if_existing_hold_matches_issuingrules($patron1->{'borrowernumber'}, $item1->{'itemnumber'} ); - is( $checkreserve5, 'OK', 'Allocating item2 to a bib-reserve is allowed as there is no specific issuingrule for this patron category/itemtype combination and so we default to the all patron category/all item type rule which allows reserves' ); - - #Create a default rule (patron categories: ALL, itemtypes: ALL) - my $rule3 = Koha::IssuingRule->new({ - branchcode => $branchcode, - categorycode => '*', - itemtype => '*', - reservesallowed => 1, - holds_per_record => 1, - holds_per_day => 1, - })->store; - - #Check that when no specific patron category/item type issuingrule combo exists we revert to using the default ALL categories/ALL - #itemtypes issuingrule. When generic rule is reservesallowed, holds_per_record and holds_per_day = 1 then 'OK' is returned - my $checkreserve6 = Koha::Hold->check_if_existing_hold_matches_issuingrules($patron1->{'borrowernumber'}, $item2->{'itemnumber'} ); - is( $checkreserve6, 'OK', 'Allocating item2 to a bib-reserve is allowed as there is no specific issuingrule for this patron category/itemtype combination and so we default to the all patron category/all item type rule which allows reserves' ); - - #When default rule is reservesallowed = 0, holds_per_record = 1, holds_per_day = 1 then 'InvalidHold is returned - $rule3->set({ reservesallowed => 0, holds_per_record => 1, holds_per_day => 1 })->store; - my $checkreserve7 = Koha::Hold->check_if_existing_hold_matches_issuingrules($patron1->{'borrowernumber'}, $item2->{'itemnumber'} ); - is( $checkreserve7, 'InvalidHold', 'Allocating item2 to a bib-reserve is not allowed as there is no specific issuingrule for this patron category/itemtype combination and so we default to the all patron category/all item type rule which doesnt allows reserves as reservesallowed is 0' ); - - #When default rule is reservesallowed = 1, holds_per_record = 0, holds_per_day = 1 then 'InvalidHold' is returned - $rule3->set({ reservesallowed => 1, holds_per_record => 0, holds_per_day => 1 })->store; - my $checkreserve8 = Koha::Hold->check_if_existing_hold_matches_issuingrules($patron1->{'borrowernumber'}, $item2->{'itemnumber'} ); - is( $checkreserve8, 'InvalidHold', 'Allocating item2 to a bib-reserve is not allowed as there is no specific issuingrule for this patron category/itemtype combination and so we default to the all patron category/all item type rule which doesnt allows reserves as holds_per_record is 0' ); - - #When default rule is reservesallowed = 1, holds_per_record = 1, holds_per_day = 0 then 'InvalidHold' is returned - $rule3->set({ reservesallowed => 1, holds_per_record => 1, holds_per_day => 0 })->store; - my $checkreserve9 = Koha::Hold->check_if_existing_hold_matches_issuingrules($patron1->{'borrowernumber'}, $item2->{'itemnumber'} ); - is( $checkreserve9, 'InvalidHold', 'Allocating item2 to a bib-reserve is not allowed as there is no specific issuingrule for this patron category/itemtype combination and so we default to the all patron category/all item type rule which doesnt allows reserves as holds_per_day is 0' ); + my $branchcode = $builder->build({ source => 'Branch' })->{ branchcode }; + my $categorycode = $builder->build({ source => 'Category' })->{ categorycode }; + my $patron = $builder->build({ source => 'Borrower', value => { categorycode => $categorycode, branchcode => $branchcode }}); + my $biblionumber = $builder->build({ source => 'Biblio' })->{biblionumber}; + my $itemtype1 = $builder->build({ source => 'Itemtype' })->{itemtype}; + my $itemtype2 = $builder->build({ source => 'Itemtype' })->{itemtype}; + my $item1 = $builder->build({ source => 'Item', value => { homebranch => $branchcode, biblionumber => $biblionumber, itype => $itemtype1 }}); + my $item2 = $builder->build({ source => 'Item', value => { homebranch => $branchcode, biblionumber => $biblionumber, itype => $itemtype2 }}); + + Koha::CirculationRules->set_rules({ + branchcode => $branchcode, + categorycode => $categorycode, + itemtype => $itemtype1, + rules => { + reservesallowed => 0, + holds_per_record => 0, + holds_per_day => 0, + } + }); + + my $reserveid1 = AddReserve({ + branchcode => $branchcode, + borrowernumber => $patron->{borrowernumber}, + biblionumber => $biblionumber, + }); + my $reserve1 = Koha::Holds->find($reserveid1); + + my $checkreserve1 = $reserve1->matches_circulation_rules({ itemnumber => $item1->{itemnumber} }); + is( $checkreserve1, 'InvalidHold', 'Circulation rules disallow item being allocated to the hold' ); + + #Confirm that when any of the 3 reserves-related circulaion rules fields (reservesallowed, holds_per_record, holds_per_day) are set to 0 then matches_circulation_rules returns 'InvalidHold' and so a newly returned item is not allocated to a specific bib-level reserve + + Koha::CirculationRules->set_rules({ + branchcode => $branchcode, + categorycode => $categorycode, + itemtype => $itemtype1, + rules => { + reservesallowed => 0, + holds_per_record => 1, + holds_per_day => 1, + } + }); + my $checkreserve2 = $reserve1->matches_circulation_rules({ itemnumber => $item1->{itemnumber} }); + is( $checkreserve2, 'InvalidHold', 'Reserves allowed is 0' ); + + Koha::CirculationRules->set_rules({ + branchcode => $branchcode, + categorycode => $categorycode, + itemtype => $itemtype1, + rules => { + reservesallowed => 1, + holds_per_record => 0, + holds_per_day => 1, + } + }); + my $checkreserve3 = $reserve1->matches_circulation_rules({ itemnumber => $item1->{itemnumber} }); + is( $checkreserve3, 'InvalidHold', 'Holds per record is 0' ); + + Koha::CirculationRules->set_rules({ + branchcode => $branchcode, + categorycode => $categorycode, + itemtype => $itemtype1, + rules => { + reservesallowed => 1, + holds_per_record => 1, + holds_per_day => 0, + } + }); + my $checkreserve4 = $reserve1->matches_circulation_rules({ itemnumber => $item1->{itemnumber} }); + is( $checkreserve4, 'InvalidHold', 'Holds per day is 0' ); + + Koha::CirculationRules->set_rules({ + branchcode => $branchcode, + categorycode => $categorycode, + itemtype => $itemtype1, + rules => { + reservesallowed => 1, + holds_per_record => 1, + holds_per_day => 1, + } + }); + my $checkreserve5 = $reserve1->matches_circulation_rules({ itemnumber => $item1->{itemnumber} }); + is( $checkreserve5, 'OK', 'Hold is allowed, item allocated to hold' ); + + #Check that when no specific patron category/item type circulation rule combo exists we revert to using the default ALL categories/ALL + my $reserveid2 = AddReserve({ + branchcode => $branchcode, + borrowernumber => $patron->{borrowernumber}, + biblionumber => $biblionumber, + }); + my $reserve2 = Koha::Holds->find($reserveid2); + + Koha::CirculationRules->set_rules({ + branchcode => undef, + categorycode => undef, + itemtype => undef, + rules => { + reservesallowed => 0, + holds_per_record => 1, + holds_per_day => 1, + } + }); + my $checkreserve6 = $reserve2->matches_circulation_rules({ itemnumber => $item2->{itemnumber} }); + is( $checkreserve6, 'InvalidHold', 'Reserves allowed is 0' ); + + Koha::CirculationRules->set_rules({ + branchcode => undef, + categorycode => undef, + itemtype => undef, + rules => { + reservesallowed => 1, + holds_per_record => 0, + holds_per_day => 1, + } + }); + my $checkreserve7 = $reserve2->matches_circulation_rules({ itemnumber => $item2->{itemnumber} }); + is( $checkreserve7, 'InvalidHold', 'Holds per record is 0' ); + + Koha::CirculationRules->set_rules({ + branchcode => undef, + categorycode => undef, + itemtype => undef, + rules => { + reservesallowed => 1, + holds_per_record => 1, + holds_per_day => 0, + } + }); + my $checkreserve8 = $reserve2->matches_circulation_rules({ itemnumber => $item2->{itemnumber} }); + is( $checkreserve8, 'InvalidHold', 'Holds per day is 0' ); + + Koha::CirculationRules->set_rules({ + branchcode => undef, + categorycode => undef, + itemtype => undef, + rules => { + reservesallowed => 1, + holds_per_record => 1, + holds_per_day => 1, + } + }); + my $checkreserve9 = $reserve2->matches_circulation_rules({ itemnumber => $item2->{itemnumber} }); + is( $checkreserve9, 'OK', 'Hold is allowed, item allocated to hold' ); $schema->storage->txn_rollback; }; --- a/t/db_dependent/Reserves.t +++ a/t/db_dependent/Reserves.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 66; +use Test::More tests => 71; use Test::MockModule; use Test::Warn; @@ -529,13 +529,14 @@ AddReserve( priority => 1, } ); + my (undef, $canres, undef) = CheckReserves($item->itemnumber); is( CanReserveBeCanceledFromOpac(), undef, 'CanReserveBeCanceledFromOpac should return undef if called without any parameter' ); is( - CanReserveBeCanceledFromOpac( $canres->{resserve_id} ), + CanReserveBeCanceledFromOpac( $canres->{reserve_id} ), undef, 'CanReserveBeCanceledFromOpac should return undef if called without the reserve_id' ); @@ -724,67 +725,101 @@ 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; +#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 circulation_rules'); #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 +Koha::CirculationRules->set_rules( + { + branchcode => undef, + categorycode => undef, + itemtype => undef, + rules => { + reservesallowed => 25, + holds_per_record => 1, + holds_per_day => 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_1 and itemtype = $itemtype2 then reservesallowed, holds_per_day and holds_per_record are 0 +Koha::CirculationRules->set_rules( + { + branchcode => $branch_1, + categorycode => $category_1, + itemtype => $itemtype2, + rules => { + reservesallowed => 0, + holds_per_record => 0, + holds_per_day => 0, + } + } ); #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 +Koha::CirculationRules->set_rules( + { + branchcode => $branch_1, + categorycode => $category_2, + itemtype => $itemtype2, + rules => { + reservesallowed => 1, + holds_per_record => 1, + holds_per_day => 1, + } + } +); + +#When patron category = $category_1 and itemtype = $itemtype2 AND different branch, then reservesallowed, holds_per_day and holds_per_record are 0 +Koha::CirculationRules->set_rules( + { + branchcode => $branch_2, + categorycode => $category_1, + itemtype => $itemtype2, + rules => { + reservesallowed => 0, + holds_per_record => 0, + holds_per_day => 0, + } + } ); +#Create a new biblio record +my $bibnum3 = $builder->build_sample_biblio({frameworkcode => $frameworkcode})->biblionumber; + +# Create an item from new itemtype +my $item2 = $builder->build_sample_item({ + homebranch => $branch_1, + holdingbranch => $branch_1, + itype => $itemtype2, + barcode => 'bug23172_CPL', + biblionumber => $bibnum3 +}); + +#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; +my $patronnumcat3 = Koha::Patron->new({branchcode => $branch_2, categorycode => $category_1})->store->borrowernumber; + #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 ); +my $reserveid1 = AddReserve({ + branchcode => $branch_1, + borrowernumber => $patronnumcat1, + biblionumber => $bibnum3 +}); + +#Create Bib-level hold for borrower who has a patron category of $category_2 +my $reserveid2 = AddReserve({ + branchcode => $branch_1, + borrowernumber => $patronnumcat2, + biblionumber => $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); @@ -793,27 +828,58 @@ 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 +#Test with ReservesControlBranch system preference set to ItemHomeLibrary +t::lib::Mocks::mock_preference('ReservesControlBranch', 'ItemHomeLibrary'); + +# Reserve 1 should be skipped because the circ rule for that patron's category does not allow reserves (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)'); +is( $messages->{ResFound}->{borrowernumber}, $patronnumcat2,'First priority hold is skipped because circulation rules disallow holds for this patron category, so item is allocated to next hold.'); -#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 +#Remove existing reserves +$dbh->do("DELETE FROM reserves"); + +#Bib-level hold for borrower who has a patron category of $category_1 +my $reserveid3 = AddReserve({ + branchcode => $branch_1, + borrowernumber => $patronnumcat3, + biblionumber => $bibnum3 +}); + +#Create Bib-level hold for borrower who has a patron category of $category_2 +my $reserveid4 = AddReserve({ + branchcode => $branch_1, + borrowernumber => $patronnumcat2, + biblionumber => $bibnum3 +}); + +#Test with ReservesControlBranch system preference set to PatronLibrary +t::lib::Mocks::mock_preference('ReservesControlBranch', 'PatronLibrary'); + +# Reserve 1 should be skipped because the circ rule for that patron's library does not allow reserves +(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,'First priority hold is skipped because circulation rules disallow holds for this branch, so item is allocated to next hold.'); + +# Update the circulation rule that prevented patronnumcat3 bib-level hold from being allocated upon item return making reservesallowed, holds_per_day and holds_per_record > 0 +Koha::CirculationRules->set_rules( + { + branchcode => $branch_2, + categorycode => $category_1, + itemtype => $itemtype2, + rules => { + reservesallowed => 1, + holds_per_record => 1, + holds_per_day => 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)'); +is( $messages->{ResFound}->{borrowernumber}, $patronnumcat3,'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"); --