From 59b25e378cc1adc90d57197b7a25361feae65603 Mon Sep 17 00:00:00 2001 From: Lari Taskula Date: Fri, 10 Apr 2020 00:02:24 +0000 Subject: [PATCH] Bug 25089: Add checkout_type to set_rule To test: 1. Find all occurrences of set_rule that are missing checkout_type where required grep --exclude-dir '.git' --exclude-dir 'misc/translator' \ --exclude-dir 'koha-tmpl' -Przo '(?s)(::|->)set_rule(?!s).*?\)' | \ grep -avz 'checkout_type' | 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 checkout_type (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=326=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 checkout_type. 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 | 5 +++++ 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 | 19 +++++++++++++++++-- 5 files changed, 28 insertions(+), 3 deletions(-) diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index 946611222d..4145860ac5 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -667,6 +667,7 @@ subtest "CanBookBeRenewed tests" => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rule_name => 'norenewalbefore', rule_value => '7', } @@ -2156,6 +2157,7 @@ subtest 'AddReturn + suspension_chargeperiod' => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rule_name => 'suspension_chargeperiod', rule_value => '2', } @@ -3488,6 +3490,8 @@ subtest 'Incremented fee tests' => sub { my $library = $builder->build_object( { class => 'Koha::Libraries' } )->store; + my $checkout_type = $Koha::Checkouts::type->{checkout}; + my $module = new Test::MockModule('C4::Context'); $module->mock( 'userenv', sub { { branch => $library->id } } ); @@ -3613,6 +3617,7 @@ subtest 'Incremented fee tests' => sub { categorycode => $patron->categorycode, itemtype => $itemtype->id, branchcode => $library->id, + checkout_type => $checkout_type, rule_name => 'lengthunit', rule_value => 'hours', } diff --git a/t/db_dependent/Circulation/CalcFine.t b/t/db_dependent/Circulation/CalcFine.t index f09a0e011d..2adf88ed75 100644 --- a/t/db_dependent/Circulation/CalcFine.t +++ b/t/db_dependent/Circulation/CalcFine.t @@ -83,6 +83,7 @@ subtest 'Test basic functionality' => sub { branchcode => undef, categorycode => undef, itemtype => undef, + checkout_type => undef, rules => { fine => '1.00', lengthunit => 'days', @@ -123,6 +124,7 @@ subtest 'Test cap_fine_to_replacement_price' => sub { branchcode => undef, categorycode => undef, itemtype => undef, + checkout_type => undef, rules => { fine => '1.00', lengthunit => 'days', @@ -170,6 +172,7 @@ subtest 'Test cap_fine_to_replacement_pricew with overduefinescap' => sub { branchcode => undef, categorycode => undef, itemtype => undef, + checkout_type => undef, rules => { fine => '1.00', lengthunit => 'days', @@ -197,7 +200,7 @@ subtest 'Test cap_fine_to_replacement_pricew with overduefinescap' => sub { my ($amount) = CalcFine( $item, $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, categorycode => undef, itemtype => undef, checkout_type => undef }); ($amount) = CalcFine( $item, $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 464420ead2..3e2dd1e835 100644 --- 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, + checkout_type => undef, rule_name => 'issuelength', rule_value => 1 } diff --git a/t/db_dependent/Circulation/Returns.t b/t/db_dependent/Circulation/Returns.t index f93ff66945..55b78e7d0d 100644 --- 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, + checkout_type => 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..40ddb58e12 100644 --- 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 $checkout_type = $Koha::Checkouts::type->{checkout}; 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, + checkout_type => $checkout_type, rule_name => 'patron_maxissueqty', rule_value => '', } ); @@ -191,10 +193,22 @@ subtest 'set_rule' => sub { branchcode => $branchcode, categorycode => $categorycode, itemtype => $itemtype, + checkout_type => $checkout_type, rule_name => 'fine', rule_value => '', } ); - }, 'setting fine with branch/category/itemtype succeeds' ); + }, 'setting fine with branch/category/itemtype/checkout_type succeeds' ); + + lives_ok( sub { + Koha::CirculationRules->set_rule( { + branchcode => $branchcode, + categorycode => $categorycode, + itemtype => $itemtype, + checkout_type => $checkout_type, + rule_name => 'maxissueqty', + rule_value => 5, + } ); + }, 'setting maxissueqty with branch/category/itemtype/checkout_type succeeds' ); }; subtest 'Call with missing params' => sub { @@ -212,6 +226,7 @@ subtest 'set_rule' => sub { throws_ok( sub { Koha::CirculationRules->set_rule( { branchcode => $branchcode, + checkout_type => $checkout_type, rule_name => 'patron_maxissueqty', rule_value => '', } ); -- 2.17.1