From ef0bf75eed10275b0abf3a134dc392fd77ed507c Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Thu, 25 Aug 2022 13:05:32 +0000 Subject: [PATCH] Bug 25426: Allow return policy to be selected via syspref and not just home library 1) Apply this patch 2) Run updatedatabase.pl 3) Verify CircControlReturnsBranch is set to home library by default 4) Set a Return policy for Branch A to "Item returns home" ( homebranch ) 5) Set a Return polity for Branch B to "Item returns to issuing library" ( holdingbranch ) 6) Set a Return polity for Branch C to "Item floats" ( noreturn ) 7) Create an item with homebranch of Branch A, holding branch of branch B 8) Log in as Branch C 9) Set CircControlReturnsBranch to "the library the item is currently held by" 10) Check the item in, note it should be returned to the holding library 11) Set CircControlReturnsBranch to "the library the item is owned by" 12) Check the item in, note it should be returned to the home library 13) Set CircControlReturnsBranch to "the library you are logged in at" 14) Check the item in, note it should float Signed-off-by: Andrew Fuerste-Henry Signed-off-by: Katrin Fischer Signed-off-by: Tomas Cohen Arazi --- 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 diff --git a/C4/Circulation.pm b/C4/Circulation.pm index d34769f5ed..27351e8127 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1932,11 +1932,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 @@ -1955,13 +1950,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; } @@ -2094,7 +2088,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) diff --git a/Koha/CirculationRules.pm b/Koha/CirculationRules.pm index b9fc11c941..8b5d4807d9 100644 --- a/Koha/CirculationRules.pm +++ b/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 } ); diff --git a/circ/returns.pl b/circ/returns.pl index 12e4d70b05..4493afb297 100755 --- a/circ/returns.pl +++ b/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; @@ -296,7 +297,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; diff --git a/installer/data/mysql/atomicupdate/bug_25426.pl b/installer/data/mysql/atomicupdate/bug_25426.pl new file mode 100755 index 0000000000..aa7d513ac4 --- /dev/null +++ b/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' + ) + }); + }, +}; diff --git a/installer/data/mysql/mandatory/sysprefs.sql b/installer/data/mysql/mandatory/sysprefs.sql index ae3d9e718f..68b6841c12 100644 --- a/installer/data/mysql/mandatory/sysprefs.sql +++ b/installer/data/mysql/mandatory/sysprefs.sql @@ -136,6 +136,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','1',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'), diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref index dd3267b19b..4cd4483b88 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref @@ -270,6 +270,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 diff --git a/t/db_dependent/Circulation/Branch.t b/t/db_dependent/Circulation/Branch.t index 68a029d133..ef6fc31408 100755 --- a/t/db_dependent/Circulation/Branch.t +++ b/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; -- 2.39.5