@@ -, +, @@ not just home library --- C4/Circulation.pm | 10 +-- Koha/CirculationRules.pm | 47 +++++++++++ circ/returns.pl | 5 +- .../data/mysql/atomicupdate/bug_25426.pl | 17 ++++ installer/data/mysql/mandatory/sysprefs.sql | 1 + .../admin/preferences/circulation.pref | 8 ++ t/db_dependent/Circulation/Branch.t | 78 +++++++++++++++++-- 7 files changed, 150 insertions(+), 16 deletions(-) create mode 100755 installer/data/mysql/atomicupdate/bug_25426.pl --- a/C4/Circulation.pm +++ a/C4/Circulation.pm @@ -1929,11 +1929,6 @@ holdallowed => Hold policy for this branch and itemtype. Possible values: from_any_library: Holds allowed from any patron. from_local_hold_group: Holds allowed from libraries in hold group -returnbranch => branch to which to return item. Possible values: - noreturn: do not return, let item remain where checked in (floating collections) - homebranch: return to item's home branch - holdingbranch: return to issuer branch - This searches branchitemrules in the following order: * Same branchcode and itemtype @@ -1952,13 +1947,12 @@ sub GetBranchItemRule { my $rules = Koha::CirculationRules->get_effective_rules({ branchcode => $branchcode, itemtype => $itemtype, - rules => ['holdallowed', 'hold_fulfillment_policy', 'returnbranch'] + rules => ['holdallowed', 'hold_fulfillment_policy'] }); # built-in default circulation rule $rules->{holdallowed} //= 'from_any_library'; $rules->{hold_fulfillment_policy} //= 'any'; - $rules->{returnbranch} //= 'homebranch'; return $rules; } @@ -2091,7 +2085,7 @@ sub AddReturn { } # full item data, but no borrowernumber or checkout info (no issue) - my $hbr = GetBranchItemRule($item->homebranch, $itemtype)->{'returnbranch'} || "homebranch"; + my $hbr = Koha::CirculationRules->get_return_branch_policy($item); # get the proper branch to which to return the item my $returnbranch = $hbr ne 'noreturn' ? $item->$hbr : $branch; # if $hbr was "noreturn" or any other non-item table value, then it should 'float' (i.e. stay at this branch) --- a/Koha/CirculationRules.pm +++ a/Koha/CirculationRules.pm @@ -474,6 +474,53 @@ sub clone { } } +=head2 get_return_branch_policy + + my $returnbranch = Koha::CirculationRules->get_return_branch_policy($item); + +Returns the branch to use for returning the item based on the +item type, and a branch selected via CircControlReturnsBrnch. + +The return value is the branch to which to return item. Possible values: + noreturn: do not return, let item remain where checked in (floating collections) + homebranch: return to item's home branch + holdingbranch: return to issuer branch + +This searches branchitemrules in the following order: + * Same branchcode and itemtype + * Same branchcode, itemtype '*' + * branchcode '*', same itemtype + * branchcode and itemtype '*' + +=cut + +sub get_return_branch_policy { + my ( $self, $item ) = @_; + + my $pref = C4::Context->preference('CircControlReturnsBranch'); + + my $branchcode = + $pref eq 'ItemHomeLibrary' ? $item->homebranch + : $pref eq 'ItemHoldingLibrary' ? $item->holdingbranch + : $pref eq 'CheckInLibrary' ? C4::Context->userenv + ? C4::Context->userenv->{branch} + : $item->homebranch + : $item->homebranch; + + my $itemtype = $item->effective_itemtype; + + my $rule = Koha::CirculationRules->get_effective_rule( + { + rule_name => 'returnbranch', + itemtype => $itemtype, + branchcode => $branchcode, + } + ); + + return $rule ? $rule->rule_value : 'homebranch'; +} + + =head3 get_opacitemholds_policy my $can_place_a_hold_at_item_level = Koha::CirculationRules->get_opacitemholds_policy( { patron => $patron, item => $item } ); --- a/circ/returns.pl +++ a/circ/returns.pl @@ -47,10 +47,11 @@ use Koha::AuthorisedValues; use Koha::BiblioFrameworks; use Koha::Calendar; use Koha::Checkouts; +use Koha::CirculationRules; use Koha::DateUtils qw( dt_from_string ); use Koha::Holds; -use Koha::Items; use Koha::Item::Transfers; +use Koha::Items; use Koha::Patrons; use Koha::Recalls; @@ -293,7 +294,7 @@ if ($barcode) { } # make sure return branch respects home branch circulation rules, default to homebranch - my $hbr = GetBranchItemRule($item->homebranch, $itemtype ? $itemtype->itemtype : undef )->{'returnbranch'} || "homebranch"; + my $hbr = Koha::CirculationRules->get_return_branch_policy($item); $returnbranch = $hbr ne 'noreturn' ? $item->$hbr : $userenv_branch; # can be noreturn, homebranch or holdingbranch my $materials = $item->materials; --- a/installer/data/mysql/atomicupdate/bug_25426.pl +++ a/installer/data/mysql/atomicupdate/bug_25426.pl @@ -0,0 +1,17 @@ +use Modern::Perl; + +return { + bug_number => "25426", + description => "Add new syspref CircControlReturnsBranch", + up => sub { + my ($args) = @_; + my ($dbh, $out) = @$args{qw(dbh out)}; + + $dbh->do(q{ + INSERT INTO systempreferences VALUES ( + 'CircControlReturnsBranch','ItemHomeLibrary','ItemHomeLibrary|ItemHoldingLibrary|CheckInLibrary', + 'Specify the agency that controls the return policy','Choice' + ) + }); + }, +}; --- a/installer/data/mysql/mandatory/sysprefs.sql +++ a/installer/data/mysql/mandatory/sysprefs.sql @@ -135,6 +135,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('CircAutoPrintQuickSlip','qslip',NULL,'Choose what should happen when an empty barcode field is submitted in circulation: Display a print quick slip window, Display a print slip window or Clear the screen.','Choice'), ('CircConfirmItemParts', '0', NULL, 'Require staff to confirm that all parts of an item are present at checkin/checkout.', 'Yes/No'), ('CircControl','ItemHomeLibrary','PickupLibrary|PatronLibrary|ItemHomeLibrary','Specify the agency that controls the circulation and fines policy','Choice'), +('CircControlReturnsBranch','ItemHomeLibrary','ItemHomeLibrary|ItemHoldingLibrary|CheckInLibrary','Specify the agency that controls the return policy','Choice'), ('CircSidebar','0',NULL,'Activate or deactivate the navigation sidebar on all Circulation pages','YesNo'), ('CirculateILL','0','','If enabled, it is possible to circulate ILL items from within ILL','YesNo'), ('ClaimsBccCopy','0','','Bcc the ClaimAcquisition and ClaimIssues alerts','YesNo'), --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref +++ a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref @@ -276,6 +276,14 @@ Circulation: PickupLibrary: the library you are logged in at. PatronLibrary: the library the patron is from. ItemHomeLibrary: the library the item is from. + - + - For check in, use the circulation rules of + - pref: CircControlReturnsBranch + type: choice + choices: + ItemHoldingLibrary: the library the item is currently held by. + CheckInLibrary: the library you are logged in at. + ItemHomeLibrary: the library the item is owned by. - - Use the checkout and fines rules of - pref: HomeOrHoldingBranch --- a/t/db_dependent/Circulation/Branch.t +++ a/t/db_dependent/Circulation/Branch.t @@ -25,7 +25,7 @@ use Koha::CirculationRules; use Koha::Patrons; -use Test::More tests => 17; +use Test::More tests => 18; use t::lib::Mocks; use t::lib::TestBuilder; @@ -264,22 +264,22 @@ is_deeply( $samplebranch1->{branchcode}, $sampleitemtype1->{itemtype}, ), - { returnbranch => 'homebranch', holdallowed => 'invalid_value', @lazy_any }, + { holdallowed => 'invalid_value', @lazy_any }, "GetBranchitem returns holdallowed and return branch" ); is_deeply( GetBranchItemRule(), - { returnbranch => 'homebranch', holdallowed => 'from_local_hold_group', @lazy_any }, + { holdallowed => 'from_local_hold_group', @lazy_any }, "Without parameters GetBranchItemRule returns the values in default_circ_rules" ); is_deeply( GetBranchItemRule( $samplebranch2->{branchcode} ), - { returnbranch => 'holdingbranch', holdallowed => 'from_home_library', @lazy_any }, + { holdallowed => 'from_home_library', @lazy_any }, "With only a branchcode GetBranchItemRule returns values in default_branch_circ_rules" ); is_deeply( GetBranchItemRule( -1, -1 ), - { returnbranch => 'homebranch', holdallowed => 'from_local_hold_group', @lazy_any }, + { holdallowed => 'from_local_hold_group', @lazy_any }, "With only one parametern GetBranchItemRule returns default values" ); @@ -329,5 +329,71 @@ t::lib::Mocks::mock_preference( 'item-level_itypes', 1 ); is($messages->{NeedsTransfer},undef,"AddReturn respects branch item return policy - noreturn"); t::lib::Mocks::mock_preference( 'item-level_itypes', 0 ); -$schema->storage->txn_rollback; +subtest "Test GetBranchItemRule" => sub { + plan tests => 3; + + $schema->storage->txn_begin; + + $dbh->do('DELETE FROM circulation_rules'); + + my $homebranch = $builder->build( { source => 'Branch' } )->{branchcode}; + my $holdingbranch = $builder->build( { source => 'Branch' } )->{branchcode}; + my $checkinbranch = $builder->build( { source => 'Branch' } )->{branchcode}; + my $manager = $builder->build_object( { class => "Koha::Patrons" } ); + t::lib::Mocks::mock_userenv( + { + patron => $manager, + branchcode => $checkinbranch, + } + ); + + my $item = Koha::Item->new( + { + biblionumber => $biblionumber, + homebranch => $homebranch, + holdingbranch => $holdingbranch, + itype => $sampleitemtype1->{itemtype} + } + )->store; + + Koha::CirculationRules->set_rule( + { + branchcode => $homebranch, + itemtype => undef, + rule_name => 'returnbranch', + rule_value => 'homebranch', + } + ); + + Koha::CirculationRules->set_rule( + { + branchcode => $holdingbranch, + itemtype => undef, + rule_name => 'returnbranch', + rule_value => 'holdingbranch', + } + ); + + Koha::CirculationRules->set_rule( + { + branchcode => $checkinbranch, + itemtype => undef, + rule_name => 'returnbranch', + rule_value => 'noreturn', + } + ); + + t::lib::Mocks::mock_preference( 'CircControlReturnsBranch', 'ItemHomeLibrary' ); + is( Koha::CirculationRules->get_return_branch_policy($item), 'homebranch' ); + + t::lib::Mocks::mock_preference( 'CircControlReturnsBranch', 'ItemHoldingLibrary' ); + is( Koha::CirculationRules->get_return_branch_policy($item), 'holdingbranch' ); + + t::lib::Mocks::mock_preference( 'CircControlReturnsBranch', 'CheckInLibrary' ); + is( Koha::CirculationRules->get_return_branch_policy($item), 'noreturn' ); + + $schema->storage->txn_rollback; +}; + +$schema->storage->txn_rollback; --