From e006d645211f2b5e709ee42aec2d7e9c9d5f0f93 Mon Sep 17 00:00:00 2001 From: Lari Taskula Date: Sat, 11 Apr 2020 17:25:57 +0000 Subject: [PATCH] Bug 25089: Add checkout_type to set_rules To test: 1. Find all occurrences of set_rules that are missing checkout_type where required grep --exclude-dir '.git' --exclude-dir 'misc/translator' \ --exclude-dir 'koha-tmpl' -Przo '(?s)(::|->)set_rules.*?\)' | \ grep -avz 'checkout_type' && echo "" The only occasions this should return anything are cases where a HASH or HASHref is given to set_rules(), 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_rules() git grep --no-index -n -p -P 'set_rules\s*\(' | grep -v 'sub {' \ | grep -P 'sub .*' 2.1 The only return should be t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t=293= sub set_holdallowed_rule { 3. The boring step. Find all occurrences of set_rules() with the following command: grep --exclude-dir='.git' -Prn '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 --- installer/onboarding.pl | 2 ++ .../en/modules/onboarding/onboardingstep5.tt | 10 +++++++ t/db_dependent/Circulation.t | 28 +++++++++++++++++++ t/db_dependent/Circulation/CalcDateDue.t | 4 +++ t/db_dependent/Circulation/GetHardDueDate.t | 4 +++ .../IssuingRules/maxsuspensiondays.t | 2 ++ t/db_dependent/Circulation/Returns.t | 1 + t/db_dependent/Circulation/issue.t | 2 ++ t/db_dependent/DecreaseLoanHighHolds.t | 1 + t/db_dependent/Fines.t | 2 ++ .../Holds/DisallowHoldIfItemsAvailable.t | 2 ++ t/db_dependent/Koha/Item.t | 1 + t/db_dependent/SIP/ILS.t | 1 + t/db_dependent/SIP/Transaction.t | 2 ++ t/db_dependent/api/v1/checkouts.t | 1 + t/db_dependent/selenium/basic_workflow.t | 6 ++++ 16 files changed, 69 insertions(+) diff --git a/installer/onboarding.pl b/installer/onboarding.pl index b1257dec0f..de62244f4e 100755 --- a/installer/onboarding.pl +++ b/installer/onboarding.pl @@ -237,6 +237,7 @@ if ( $step == 5 ) { my $branchcode = $input->param('branch'); my $categorycode = $input->param('categorycode'); my $itemtype = $input->param('itemtype'); + my $checkout_type = $input->param('checkout_type'); my $maxissueqty = $input->param('maxissueqty'); my $issuelength = $input->param('issuelength'); my $lengthunit = $input->param('lengthunit'); @@ -254,6 +255,7 @@ if ( $step == 5 ) { branchcode => $branchcode, categorycode => $categorycode, itemtype => $itemtype, + checkout_type => $checkout_type, rules => { renewalsallowed => $renewalsallowed, renewalperiod => $renewalperiod, diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/onboarding/onboardingstep5.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/onboarding/onboardingstep5.tt index e70870f500..9464c4a08a 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/onboarding/onboardingstep5.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/onboarding/onboardingstep5.tt @@ -67,6 +67,16 @@ Required +
  • + + + Required +
  • diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index 30e5a611ae..f5901c354b 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -261,6 +261,7 @@ Koha::CirculationRules->set_rules( categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { reservesallowed => 25, issuelength => 14, @@ -768,6 +769,7 @@ subtest "CanBookBeRenewed tests" => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { norenewalbefore => '7', no_auto_renewal_after => '9', @@ -784,6 +786,7 @@ subtest "CanBookBeRenewed tests" => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { norenewalbefore => '7', no_auto_renewal_after => '10', @@ -800,6 +803,7 @@ subtest "CanBookBeRenewed tests" => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { norenewalbefore => '7', no_auto_renewal_after => '11', @@ -816,6 +820,7 @@ subtest "CanBookBeRenewed tests" => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { norenewalbefore => '10', no_auto_renewal_after => '11', @@ -832,6 +837,7 @@ subtest "CanBookBeRenewed tests" => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { norenewalbefore => '10', no_auto_renewal_after => undef, @@ -849,6 +855,7 @@ subtest "CanBookBeRenewed tests" => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { norenewalbefore => '7', no_auto_renewal_after => '15', @@ -866,6 +873,7 @@ subtest "CanBookBeRenewed tests" => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { norenewalbefore => '10', no_auto_renewal_after => undef, @@ -899,6 +907,7 @@ subtest "CanBookBeRenewed tests" => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { norenewalbefore => '10', no_auto_renewal_after => '11', @@ -991,6 +1000,7 @@ subtest "CanBookBeRenewed tests" => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { norenewalbefore => 10, no_auto_renewal_after => 11, @@ -1057,6 +1067,7 @@ subtest "CanBookBeRenewed tests" => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { norenewalbefore => '7', no_auto_renewal_after => '', @@ -1072,6 +1083,7 @@ subtest "CanBookBeRenewed tests" => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { norenewalbefore => '10', no_auto_renewal_after => '5', @@ -1093,6 +1105,7 @@ subtest "CanBookBeRenewed tests" => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { norenewalbefore => '10', no_auto_renewal_after => '15', @@ -1111,6 +1124,7 @@ subtest "CanBookBeRenewed tests" => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { norenewalbefore => '10', no_auto_renewal_after => '', @@ -1128,6 +1142,7 @@ subtest "CanBookBeRenewed tests" => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { norenewalbefore => '10', no_auto_renewal_after => '15', @@ -1150,6 +1165,7 @@ subtest "CanBookBeRenewed tests" => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { norenewalbefore => undef, renewalsallowed => 0, @@ -1396,6 +1412,7 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { categorycode => undef, itemtype => undef, branchcode => undef, + checkout_type => undef, rules => { reservesallowed => 25, issuelength => 14, @@ -1463,6 +1480,7 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { categorycode => undef, itemtype => undef, branchcode => undef, + checkout_type => undef, rules => { onshelfholds => 0, } @@ -1477,6 +1495,7 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { categorycode => undef, itemtype => undef, branchcode => undef, + checkout_type => undef, rules => { onshelfholds => 0, } @@ -1491,6 +1510,7 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { categorycode => undef, itemtype => undef, branchcode => undef, + checkout_type => undef, rules => { onshelfholds => 1, } @@ -1505,6 +1525,7 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { categorycode => undef, itemtype => undef, branchcode => undef, + checkout_type => undef, rules => { onshelfholds => 1, } @@ -2025,6 +2046,7 @@ subtest 'AddReturn + CumulativeRestrictionPeriods' => sub { categorycode => undef, itemtype => undef, branchcode => undef, + checkout_type => undef, rules => { issuelength => 1, firstremind => 1, # 1 day of grace @@ -2131,6 +2153,7 @@ subtest 'AddReturn + suspension_chargeperiod' => sub { categorycode => '*', itemtype => '*', branchcode => '*', + checkout_type => '*', rules => { issuelength => 1, firstremind => 0, # 0 day of grace @@ -2187,6 +2210,7 @@ subtest 'AddReturn + suspension_chargeperiod' => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { suspension_chargeperiod => 3, firstremind => 1, @@ -2211,6 +2235,7 @@ subtest 'AddReturn + suspension_chargeperiod' => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { finedays => 2, suspension_chargeperiod => 1, @@ -2380,6 +2405,7 @@ subtest 'AddReturn | is_overdue' => sub { categorycode => undef, itemtype => undef, branchcode => undef, + checkout_type => undef, rules => { issuelength => 6, lengthunit => 'days', @@ -3301,6 +3327,7 @@ subtest 'CanBookBeIssued | is_overdue' => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { maxissueqty => 1, reservesallowed => 25, @@ -3347,6 +3374,7 @@ subtest 'ItemsDeniedRenewal preference' => sub { categorycode => '*', itemtype => '*', branchcode => $idr_lib->branchcode, + checkout_type => '*', rules => { reservesallowed => 25, issuelength => 14, diff --git a/t/db_dependent/Circulation/CalcDateDue.t b/t/db_dependent/Circulation/CalcDateDue.t index 8085dbd486..0e6c9274cf 100644 --- a/t/db_dependent/Circulation/CalcDateDue.t +++ b/t/db_dependent/Circulation/CalcDateDue.t @@ -10,6 +10,7 @@ use t::lib::Mocks; use t::lib::TestBuilder; use C4::Calendar; +use Koha::Checkouts; use Koha::CirculationRules; use_ok('C4::Circulation'); @@ -21,6 +22,7 @@ my $builder = t::lib::TestBuilder->new; my $categorycode = 'B'; my $itemtype = 'MX'; my $branchcode = 'FPL'; +my $checkout_type = $Koha::Checkouts::type->{checkout}; my $issuelength = 10; my $renewalperiod = 5; my $lengthunit = 'days'; @@ -31,6 +33,7 @@ Koha::CirculationRules->set_rules( categorycode => $categorycode, itemtype => $itemtype, branchcode => $branchcode, + checkout_type => $checkout_type, rules => { issuelength => $issuelength, renewalperiod => $renewalperiod, @@ -170,6 +173,7 @@ Koha::CirculationRules->set_rules( categorycode => $dayweek_categorycode, itemtype => $dayweek_itemtype, branchcode => $dayweek_branchcode, + checkout_type => $checkout_type, rules => { issuelength => $dayweek_issuelength, renewalperiod => $dayweek_renewalperiod, diff --git a/t/db_dependent/Circulation/GetHardDueDate.t b/t/db_dependent/Circulation/GetHardDueDate.t index 275ecc426e..6f81ccd30f 100644 --- a/t/db_dependent/Circulation/GetHardDueDate.t +++ b/t/db_dependent/Circulation/GetHardDueDate.t @@ -5,6 +5,7 @@ use C4::Context; use DateTime; use Koha::Database; use Koha::DateUtils; +use Koha::Checkouts; use Koha::CirculationRules; use Koha::Library; @@ -120,6 +121,7 @@ my $sampleissuingrule1 = { branchcode => $samplebranch1->{branchcode}, categorycode => $samplecat->{categorycode}, itemtype => $sampleitemtype1, + checkout_type => $Koha::Checkouts::type->{checkout}, rules => { finedays => 0, lengthunit => 'days', @@ -153,6 +155,7 @@ my $sampleissuingrule2 = { branchcode => $samplebranch2->{branchcode}, categorycode => $samplecat->{categorycode}, itemtype => $sampleitemtype1, + checkout_type => $Koha::Checkouts::type->{checkout}, rules => { renewalsallowed => 0, renewalperiod => 2, @@ -182,6 +185,7 @@ my $sampleissuingrule3 = { branchcode => $samplebranch1->{branchcode}, categorycode => $samplecat->{categorycode}, itemtype => $sampleitemtype2, + checkout_type => $Koha::Checkouts::type->{checkout}, rules => { renewalsallowed => 0, renewalperiod => 3, diff --git a/t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t b/t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t index bdb9d5aee6..0e51662f04 100644 --- a/t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t +++ b/t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t @@ -113,6 +113,7 @@ subtest "suspension_chargeperiod" => sub { categorycode => undef, itemtype => undef, branchcode => undef, + checkout_type => undef, rules => { firstremind => 0, finedays => 7, @@ -139,6 +140,7 @@ subtest "maxsuspensiondays" => sub { categorycode => undef, itemtype => undef, branchcode => undef, + checkout_type => undef, rules => { firstremind => 0, finedays => 15, diff --git a/t/db_dependent/Circulation/Returns.t b/t/db_dependent/Circulation/Returns.t index 5187076bb3..9db57c48c1 100644 --- a/t/db_dependent/Circulation/Returns.t +++ b/t/db_dependent/Circulation/Returns.t @@ -254,6 +254,7 @@ subtest 'Handle ids duplication' => sub { categorycode => undef, itemtype => undef, branchcode => undef, + checkout_type => undef, rules => { chargeperiod => 1, fine => 1, diff --git a/t/db_dependent/Circulation/issue.t b/t/db_dependent/Circulation/issue.t index 2a75da7de4..3337cdcbb8 100644 --- a/t/db_dependent/Circulation/issue.t +++ b/t/db_dependent/Circulation/issue.t @@ -93,6 +93,7 @@ Koha::CirculationRules->set_rules( itemtype => '*', categorycode => '*', branchcode => '*', + checkout_type => '*', rules => { lengthunit => 'days', issuelength => 0, @@ -355,6 +356,7 @@ Koha::CirculationRules->set_rules( categorycode => undef, itemtype => undef, branchcode => undef, + checkout_type => undef, rules => { renewalsallowed => 3, } diff --git a/t/db_dependent/DecreaseLoanHighHolds.t b/t/db_dependent/DecreaseLoanHighHolds.t index 9228f32c93..f0a3b0ef1a 100755 --- a/t/db_dependent/DecreaseLoanHighHolds.t +++ b/t/db_dependent/DecreaseLoanHighHolds.t @@ -104,6 +104,7 @@ Koha::CirculationRules->set_rules( branchcode => undef, categorycode => undef, itemtype => $item->itype, + checkout_type => undef, rules => { issuelength => '14', lengthunit => 'days', diff --git a/t/db_dependent/Fines.t b/t/db_dependent/Fines.t index 76cc0638a9..4968d3b134 100644 --- a/t/db_dependent/Fines.t +++ b/t/db_dependent/Fines.t @@ -20,6 +20,7 @@ my $issuingrule = Koha::CirculationRules->set_rules( categorycode => undef, itemtype => undef, branchcode => undef, + checkout_type => undef, rules => { fine => 1, finedays => 0, @@ -49,6 +50,7 @@ $issuingrule = Koha::CirculationRules->set_rules( categorycode => undef, itemtype => undef, branchcode => undef, + checkout_type => undef, rules => { chargeperiod_charge_at => 1, } diff --git a/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t b/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t index ae5e96cb60..a4382738e0 100755 --- a/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t +++ b/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t @@ -87,6 +87,7 @@ Koha::CirculationRules->set_rules( categorycode => undef, itemtype => $itemtype, branchcode => undef, + checkout_type => undef, rules => { issuelength => 7, lengthunit => 8, @@ -272,6 +273,7 @@ Koha::CirculationRules->set_rules( categorycode => undef, itemtype => $itemtype2, branchcode => undef, + checkout_type => undef, rules => { maxissueqty => 99, onshelfholds => 0, diff --git a/t/db_dependent/Koha/Item.t b/t/db_dependent/Koha/Item.t index 20547d71b4..312c230db1 100644 --- a/t/db_dependent/Koha/Item.t +++ b/t/db_dependent/Koha/Item.t @@ -174,6 +174,7 @@ subtest 'pickup_locations' => sub { categorycode => undef, itemtype => undef, branchcode => undef, + checkout_type => undef, rules => { reservesallowed => 25, } diff --git a/t/db_dependent/SIP/ILS.t b/t/db_dependent/SIP/ILS.t index f08a259621..d93b7f8d30 100755 --- a/t/db_dependent/SIP/ILS.t +++ b/t/db_dependent/SIP/ILS.t @@ -92,6 +92,7 @@ subtest cancel_hold => sub { categorycode => $patron->categorycode, branchcode => $library->branchcode, itemtype => $item->effective_itemtype, + checkout_type => undef, rules => { onshelfholds => 1, reservesallowed => 3, diff --git a/t/db_dependent/SIP/Transaction.t b/t/db_dependent/SIP/Transaction.t index cb10003c42..257024ffd7 100755 --- a/t/db_dependent/SIP/Transaction.t +++ b/t/db_dependent/SIP/Transaction.t @@ -70,6 +70,7 @@ subtest fill_holds_at_checkout => sub { categorycode => $borrower->{categorycode}, branchcode => $branch->{branchcode}, itemtype => $itype->{itemtype}, + checkout_type => undef, rules => { onshelfholds => 1, reservesallowed => 3, @@ -176,6 +177,7 @@ subtest cancel_hold => sub { categorycode => $patron->categorycode, branchcode => $library->branchcode, itemtype => $item->effective_itemtype, + checkout_type => undef, rules => { onshelfholds => 1, reservesallowed => 3, diff --git a/t/db_dependent/api/v1/checkouts.t b/t/db_dependent/api/v1/checkouts.t index 5f7f4ed2fe..99ab202032 100644 --- a/t/db_dependent/api/v1/checkouts.t +++ b/t/db_dependent/api/v1/checkouts.t @@ -76,6 +76,7 @@ Koha::CirculationRules->set_rules( categorycode => undef, itemtype => undef, branchcode => undef, + checkout_type => undef, rules => { renewalperiod => 7, renewalsallowed => 1, diff --git a/t/db_dependent/selenium/basic_workflow.t b/t/db_dependent/selenium/basic_workflow.t index a988bb8683..c7c7cb537d 100644 --- a/t/db_dependent/selenium/basic_workflow.t +++ b/t/db_dependent/selenium/basic_workflow.t @@ -70,6 +70,11 @@ our $sample_data = { rentalcharge => 0, notforloan => 0, }, + checkout_type => { + itemtype => 'CHECKOUT', + description => 'Just a checkout_type for tests', + maxissueqty => 80100, + }, issuingrule => { categorycode => 'test_cat', itemtype => 'IT4test', @@ -159,6 +164,7 @@ SKIP: { categorycode => $issuing_rules->{categorycode}, itemtype => $issuing_rules->{itemtype}, branchcode => $issuing_rules->{branchcode}, + checkout_type => $issuing_rules->{checkout_type}, rules => { maxissueqty => $issuing_rules->{maxissueqty}, issuelength => $issuing_rules->{issuelength}, -- 2.17.1