From fc583dcb397169155d31447eaa843130e895de72 Mon Sep 17 00:00:00 2001 From: Lari Taskula Date: Fri, 10 Apr 2020 00:02:24 +0000 Subject: [PATCH] Bug 26814: Add onsite_checkout to set_rule To test: 1. Find all occurrences of set_rule that are missing onsite_checkout where required grep --exclude-dir '.git' --exclude-dir 'misc/translator' \ --exclude-dir 'koha-tmpl' -Przo '(?s)(::|->)set_rule(?!s).*?\)' | \ grep -avz 'onsite_checkout' | grep -Pavz 'hold|reserves|article_requests' \ && echo "" The only occasions this should return anything are cases where a HASH or HASHref is given to set_rule(), or that we are explicitly testing missing parameters in an unit test. 1.2 Verify the HASH/HASHref cases. The hash should contain a onsite_checkout (unless the rule is related to holds) 2. Find all subroutines using set_rule() git grep --no-index -n -p -P 'set_rule\s*\(' | grep -v 'sub {' \ | grep -P 'sub .*' 2.1 The only return should be Koha/CirculationRules.pm=331=sub set_rules { 3. The boring step. Find all occurrences of set_rule() with the following command: grep --exclude-dir='.git' -Prn 'set_rule' | grep -v set_rules Go through this list and make sure all neccessary locations are updated. A change is required when the rule scope includes onsite_checkout. See Koha/CirculationRules.pm for scopes. Hold/reserve related rules do not require it. Sponsored-by: The National Library of Finland --- t/db_dependent/Circulation.t | 3 +++ t/db_dependent/Circulation/CalcFine.t | 5 +++- t/db_dependent/Circulation/ReturnClaims.t | 1 + t/db_dependent/Circulation/Returns.t | 1 + t/db_dependent/Koha/IssuingRules.t | 33 ++++++++++++++++++++--- 5 files changed, 39 insertions(+), 4 deletions(-) diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index a51349b186..e7b62033ed 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -680,6 +680,7 @@ subtest "CanBookBeRenewed tests" => sub { categorycode => undef, branchcode => undef, itemtype => undef, + onsite_checkout => undef, rule_name => 'norenewalbefore', rule_value => '7', } @@ -2159,6 +2160,7 @@ subtest 'AddReturn + suspension_chargeperiod' => sub { categorycode => undef, branchcode => undef, itemtype => undef, + onsite_checkout => undef, rule_name => 'suspension_chargeperiod', rule_value => '2', } @@ -3438,6 +3440,7 @@ subtest 'Incremented fee tests' => sub { categorycode => $patron->categorycode, itemtype => $itemtype->id, branchcode => $library->id, + onsite_checkout => undef, rule_name => 'lengthunit', rule_value => 'hours', } diff --git a/t/db_dependent/Circulation/CalcFine.t b/t/db_dependent/Circulation/CalcFine.t index 85f38ea42e..c443cd16a9 100755 --- a/t/db_dependent/Circulation/CalcFine.t +++ b/t/db_dependent/Circulation/CalcFine.t @@ -69,6 +69,7 @@ subtest 'Test basic functionality' => sub { branchcode => undef, categorycode => undef, itemtype => undef, + onsite_checkout => undef, rules => { fine => '1.00', lengthunit => 'days', @@ -109,6 +110,7 @@ subtest 'Test cap_fine_to_replacement_price' => sub { branchcode => undef, categorycode => undef, itemtype => undef, + onsite_checkout => undef, rules => { fine => '1.00', lengthunit => 'days', @@ -162,6 +164,7 @@ subtest 'Test cap_fine_to_replacement_pricew with overduefinescap' => sub { branchcode => undef, categorycode => undef, itemtype => undef, + onsite_checkout => undef, rules => { fine => '1.00', lengthunit => 'days', @@ -189,7 +192,7 @@ subtest 'Test cap_fine_to_replacement_pricew with overduefinescap' => sub { my ($amount) = CalcFine( $item->unblessed, $patron->{categorycode}, $branch->{branchcode}, $start_dt, $end_dt ); is( int($amount), 3, 'Got the lesser of overduefinescap and replacement price where overduefinescap < replacement price' ); - Koha::CirculationRules->set_rule({ rule_name => 'overduefinescap', rule_value => 6, branchcode => undef, categorycode => undef, itemtype => undef }); + Koha::CirculationRules->set_rule({ rule_name => 'overduefinescap', rule_value => 6, branchcode => undef, onsite_checkout => undef, categorycode => undef, itemtype => undef }); ($amount) = CalcFine( $item->unblessed, $patron->{categorycode}, $branch->{branchcode}, $start_dt, $end_dt ); is( int($amount), 5, 'Get the lesser of overduefinescap and replacement price where overduefinescap > replacement price' ); diff --git a/t/db_dependent/Circulation/ReturnClaims.t b/t/db_dependent/Circulation/ReturnClaims.t index 7d3b52128e..c5f6622b6d 100755 --- a/t/db_dependent/Circulation/ReturnClaims.t +++ b/t/db_dependent/Circulation/ReturnClaims.t @@ -51,6 +51,7 @@ Koha::CirculationRules->set_rule( branchcode => undef, categorycode => undef, itemtype => undef, + onsite_checkout => undef, rule_name => 'issuelength', rule_value => 1 } diff --git a/t/db_dependent/Circulation/Returns.t b/t/db_dependent/Circulation/Returns.t index 79ae8555bf..5a2c4ded1b 100755 --- a/t/db_dependent/Circulation/Returns.t +++ b/t/db_dependent/Circulation/Returns.t @@ -63,6 +63,7 @@ Koha::CirculationRules->set_rule( categorycode => undef, itemtype => undef, branchcode => undef, + onsite_checkout => undef, rule_name => 'issuelength', rule_value => 1, } diff --git a/t/db_dependent/Koha/IssuingRules.t b/t/db_dependent/Koha/IssuingRules.t index 31abe27c4c..c9ec5429c9 100755 --- a/t/db_dependent/Koha/IssuingRules.t +++ b/t/db_dependent/Koha/IssuingRules.t @@ -154,9 +154,10 @@ subtest 'set_rule' => sub { my $branchcode = $builder->build({ source => 'Branch' })->{'branchcode'}; my $categorycode = $builder->build({ source => 'Category' })->{'categorycode'}; my $itemtype = $builder->build({ source => 'Itemtype' })->{'itemtype'}; + my $onsite_checkout = 0; subtest 'Correct call' => sub { - plan tests => 4; + plan tests => 5; Koha::CirculationRules->delete; @@ -172,6 +173,7 @@ subtest 'set_rule' => sub { Koha::CirculationRules->set_rule( { branchcode => $branchcode, categorycode => $categorycode, + onsite_checkout => $onsite_checkout, rule_name => 'patron_maxissueqty', rule_value => '', } ); @@ -191,10 +193,22 @@ subtest 'set_rule' => sub { branchcode => $branchcode, categorycode => $categorycode, itemtype => $itemtype, + onsite_checkout => $onsite_checkout, rule_name => 'fine', rule_value => '', } ); - }, 'setting fine with branch/category/itemtype succeeds' ); + }, 'setting fine with branch/category/itemtype/onsite_checkout succeeds' ); + + lives_ok( sub { + Koha::CirculationRules->set_rule( { + branchcode => $branchcode, + categorycode => $categorycode, + itemtype => $itemtype, + onsite_checkout => $onsite_checkout, + rule_name => 'maxissueqty', + rule_value => 5, + } ); + }, 'setting maxissueqty with branch/category/itemtype/onsite_checkout succeeds' ); }; subtest 'Call with missing params' => sub { @@ -212,6 +226,7 @@ subtest 'set_rule' => sub { throws_ok( sub { Koha::CirculationRules->set_rule( { branchcode => $branchcode, + onsite_checkout => $onsite_checkout, rule_name => 'patron_maxissueqty', rule_value => '', } ); @@ -229,6 +244,7 @@ subtest 'set_rule' => sub { Koha::CirculationRules->set_rule( { branchcode => $branchcode, categorycode => $categorycode, + onsite_checkout => undef, rule_name => 'fine', rule_value => '', } ); @@ -236,7 +252,7 @@ subtest 'set_rule' => sub { }; subtest 'Call with extra params' => sub { - plan tests => 3; + plan tests => 4; Koha::CirculationRules->delete; @@ -252,6 +268,7 @@ subtest 'set_rule' => sub { throws_ok( sub { Koha::CirculationRules->set_rule( { branchcode => $branchcode, + onsite_checkout => undef, categorycode => $categorycode, itemtype => $itemtype, rule_name => 'patron_maxissueqty', @@ -268,6 +285,16 @@ subtest 'set_rule' => sub { rule_value => '', } ); }, qr/categorycode/, 'setting holdallowed with categorycode fails' ); + + throws_ok( sub { + Koha::CirculationRules->set_rule( { + branchcode => $branchcode, + onsite_checkout => undef, + rule_name => 'holdallowed', + itemtype => $itemtype, + rule_value => '', + } ); + }, qr/onsite_checkout/, 'setting holdallowed with onsite_checkout fails' ); }; }; -- 2.17.1