@@ -, +, @@ policy set to 'from_home_library' 1 - Have a record with an item from library A and library B 2 - Set the 'Default checkout, hold and return policy'->Hold policy->From home library for all libraries and ensure you have no branch specific/itemtype specific rules set 3 - Attempt to place a hold on the record for a patron from library B 4 - Note that only the library B item is holdable - place a title level hold (do not choose an item) 5 - Check in the item from library A 6 - It fills the hold - This is incorrect - ignore the hold 7 - Apply patch 8 - Restart all the things 9 - Check in the item from library A --- C4/Reserves.pm | 2 +- .../admin/preferences/circulation.pref | 2 +- misc/cronjobs/update_patrons_category.pl | 1 + t/db_dependent/Circulation.t | 2 +- t/db_dependent/Reserves.t | 74 ++++++++++++++++++- 5 files changed, 74 insertions(+), 7 deletions(-) --- a/C4/Reserves.pm +++ a/C4/Reserves.pm @@ -894,7 +894,7 @@ sub CheckReserves { my $branch = GetReservesControlBranch( $item->unblessed, $patron->unblessed ); my $branchitemrule = C4::Circulation::GetBranchItemRule($branch,$item->effective_itemtype); next if ($branchitemrule->{'holdallowed'} == 0); - next if (($branchitemrule->{'holdallowed'} == 1) && ($branch ne $patron->branchcode)); + next if (($branchitemrule->{'holdallowed'} == 1) && ($item->homebranch ne $patron->branchcode)); my $library = Koha::Libraries->find({branchcode=>$item->homebranch}); next if (($branchitemrule->{'holdallowed'} == 3) && (!$library->validate_hold_sibling({branchcode => $patron->branchcode}) )); my $hold_fulfillment_policy = $branchitemrule->{hold_fulfillment_policy}; --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref +++ a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref @@ -716,7 +716,7 @@ Circulation: class: integer - days from now) at checkin time. Note that this number of days will be used too in calculating the default end date for the Holds to pull-report. But it does not interfere with issuing, renewing or transferring items. - - - Check the + - Check the rule from the - pref: ReservesControlBranch choices: ItemHomeLibrary: "item's home library" --- a/misc/cronjobs/update_patrons_category.pl +++ a/misc/cronjobs/update_patrons_category.pl @@ -47,6 +47,7 @@ update_patrons_category.pl -f=categorycode -t=categorycode update_patrons_category.pl --help | --man Options: + --help brief help message --man full documentation -too_old update if over maximum age for current category --- a/t/db_dependent/Circulation.t +++ a/t/db_dependent/Circulation.t @@ -291,7 +291,7 @@ subtest "CanBookBeRenewed AllowRenewalIfOtherItemsAvailable multiple borrowers a branchcode => undef, itemtype => undef, rule_name => 'holdallowed', - rule_value => 1 + rule_value => 2 } ); Koha::CirculationRules->set_rule( --- a/t/db_dependent/Reserves.t +++ a/t/db_dependent/Reserves.t @@ -1076,9 +1076,9 @@ subtest 'RevertWaitingStatus' => sub { ); }; -subtest 'CheckReserves additional test' => sub { +subtest 'CheckReserves additional tests' => sub { - plan tests => 3; + plan tests => 8; my $item = $builder->build_sample_item; my $reserve1 = $builder->build_object( @@ -1151,6 +1151,68 @@ subtest 'CheckReserves additional test' => sub { is( $matched_reserve->{reserve_id}, $reserve1->reserve_id, "We got the Transit reserve" ); is( scalar @$possible_reserves, 2, 'We do get both reserves' ); + + my $patron_B = $builder->build_object({ class => "Koha::Patrons" }); + my $item_A = $builder->build_sample_item; + my $item_B = $builder->build_sample_item({ + homebranch => $patron_B->branchcode, + biblionumber => $item_A->biblionumber, + itype => $item_A->itype + }); + Koha::CirculationRules->set_rules( + { + branchcode => undef, + categorycode => undef, + itemtype => $item_A->itype, + rules => { + reservesallowed => 25, + holds_per_record => 1, + } + } + ); + Koha::CirculationRules->set_rule({ + branchcode => undef, + itemtype => $item_A->itype, + rule_name => 'holdallowed', + rule_value => 1 + }); + my $reserve_id = AddReserve( + { + branchcode => $patron_B->branchcode, + borrowernumber => $patron_B->borrowernumber, + biblionumber => $item_A->biblionumber, + priority => 1, + itemnumber => undef, + } + ); + + ok( $reserve_id, "We can place a record level hold because one item is owned by patron's home library"); + t::lib::Mocks::mock_preference('ReservesControlBranch', 'ItemHomeLibrary'); + ( $status, $matched_reserve, $possible_reserves ) = CheckReserves( $item_A->itemnumber ); + is( $status, "", "We do not fill the hold with item A because it is not from the patron's homebranch"); + Koha::CirculationRules->set_rule({ + branchcode => $item_A->homebranch, + itemtype => $item_A->itype, + rule_name => 'holdallowed', + rule_value => 2 + }); + ( $status, $matched_reserve, $possible_reserves ) = CheckReserves( $item_A->itemnumber ); + is( $status, "Reserved", "We fill the hold with item A because item's branch rule says allow any"); + + + # Changing the control branch should change only the rule we get + t::lib::Mocks::mock_preference('ReservesControlBranch', 'PatronLibrary'); + ( $status, $matched_reserve, $possible_reserves ) = CheckReserves( $item_A->itemnumber ); + is( $status, "", "We do not fill the hold with item A because it is not from the patron's homebranch"); + Koha::CirculationRules->set_rule({ + branchcode => $patron_B->branchcode, + itemtype => $item_A->itype, + rule_name => 'holdallowed', + rule_value => 2 + }); + ( $status, $matched_reserve, $possible_reserves ) = CheckReserves( $item_A->itemnumber ); + is( $status, "Reserved", "We fill the hold with item A because patron's branch rule says allow any"); + }; subtest 'AllowHoldOnPatronPossession test' => sub { @@ -1250,11 +1312,15 @@ subtest 'ModReserveAffect logging' => sub { $hold = Koha::Holds->find($reserve_id); is( $hold->timestamp, $previous_timestamp, 'Make sure the previous timestamp has been used' ); + # Avoid warnings + my $reserve_mock = Test::MockModule->new('C4::Reserves'); + $reserve_mock->mock( '_koha_notify_reserve', undef ); + # Mark it waiting ModReserveAffect( $item->itemnumber, $patron->borrowernumber ); - $hold = Koha::Holds->find($reserve_id); - is( $hold->found, 'W', 'Hold has been set waiting' ); + $hold->discard_changes; + ok( $hold->is_waiting, 'Hold has been set waiting' ); isnt( $hold->timestamp, $previous_timestamp, 'The timestamp has been modified' ); my $log = Koha::ActionLogs->search({ module => 'HOLDS', action => 'MODIFY', object => $hold->reserve_id })->next; --