From 4dbb5e934961b5747239e6ff42acf0c2cf7b15d5 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 | 4 +++ .../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, 71 insertions(+) diff --git a/installer/onboarding.pl b/installer/onboarding.pl index dab7980ee9..ad54beac4a 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, @@ -288,6 +290,7 @@ if ( $step == 5 ) { my $params_2 = { branchcode => $branchcode, categorycode => $categorycode, + checkout_type => $checkout_type, rules => { patron_maxissueqty => "", max_holds => "", @@ -321,6 +324,7 @@ if ( $step == 5 ) { categorycode => $categorycode, itemtype => $itemtype, branchcode => $branchcode, + checkout_type => $checkout_type, rules => { maxissueqty => $maxissueqty, } 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 b559255591..28bacb56af 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 4145860ac5..86ed9fc1d0 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -255,6 +255,7 @@ Koha::CirculationRules->set_rules( categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { reservesallowed => 25, issuelength => 14, @@ -762,6 +763,7 @@ subtest "CanBookBeRenewed tests" => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { norenewalbefore => '7', no_auto_renewal_after => '9', @@ -778,6 +780,7 @@ subtest "CanBookBeRenewed tests" => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { norenewalbefore => '7', no_auto_renewal_after => '10', @@ -794,6 +797,7 @@ subtest "CanBookBeRenewed tests" => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { norenewalbefore => '7', no_auto_renewal_after => '11', @@ -810,6 +814,7 @@ subtest "CanBookBeRenewed tests" => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { norenewalbefore => '10', no_auto_renewal_after => '11', @@ -826,6 +831,7 @@ subtest "CanBookBeRenewed tests" => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { norenewalbefore => '10', no_auto_renewal_after => undef, @@ -843,6 +849,7 @@ subtest "CanBookBeRenewed tests" => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { norenewalbefore => '7', no_auto_renewal_after => '15', @@ -860,6 +867,7 @@ subtest "CanBookBeRenewed tests" => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { norenewalbefore => '10', no_auto_renewal_after => undef, @@ -893,6 +901,7 @@ subtest "CanBookBeRenewed tests" => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { norenewalbefore => '10', no_auto_renewal_after => '11', @@ -985,6 +994,7 @@ subtest "CanBookBeRenewed tests" => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { norenewalbefore => 10, no_auto_renewal_after => 11, @@ -1051,6 +1061,7 @@ subtest "CanBookBeRenewed tests" => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { norenewalbefore => '7', no_auto_renewal_after => '', @@ -1066,6 +1077,7 @@ subtest "CanBookBeRenewed tests" => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { norenewalbefore => '10', no_auto_renewal_after => '5', @@ -1087,6 +1099,7 @@ subtest "CanBookBeRenewed tests" => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { norenewalbefore => '10', no_auto_renewal_after => '15', @@ -1105,6 +1118,7 @@ subtest "CanBookBeRenewed tests" => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { norenewalbefore => '10', no_auto_renewal_after => '', @@ -1122,6 +1136,7 @@ subtest "CanBookBeRenewed tests" => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { norenewalbefore => '10', no_auto_renewal_after => '15', @@ -1144,6 +1159,7 @@ subtest "CanBookBeRenewed tests" => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { norenewalbefore => undef, renewalsallowed => 0, @@ -1390,6 +1406,7 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { categorycode => undef, itemtype => undef, branchcode => undef, + checkout_type => undef, rules => { reservesallowed => 25, issuelength => 14, @@ -1457,6 +1474,7 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { categorycode => undef, itemtype => undef, branchcode => undef, + checkout_type => undef, rules => { onshelfholds => 0, } @@ -1471,6 +1489,7 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { categorycode => undef, itemtype => undef, branchcode => undef, + checkout_type => undef, rules => { onshelfholds => 0, } @@ -1485,6 +1504,7 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { categorycode => undef, itemtype => undef, branchcode => undef, + checkout_type => undef, rules => { onshelfholds => 1, } @@ -1499,6 +1519,7 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub { categorycode => undef, itemtype => undef, branchcode => undef, + checkout_type => undef, rules => { onshelfholds => 1, } @@ -2019,6 +2040,7 @@ subtest 'AddReturn + CumulativeRestrictionPeriods' => sub { categorycode => undef, itemtype => undef, branchcode => undef, + checkout_type => undef, rules => { issuelength => 1, firstremind => 1, # 1 day of grace @@ -2125,6 +2147,7 @@ subtest 'AddReturn + suspension_chargeperiod' => sub { categorycode => '*', itemtype => '*', branchcode => '*', + checkout_type => '*', rules => { issuelength => 1, firstremind => 0, # 0 day of grace @@ -2181,6 +2204,7 @@ subtest 'AddReturn + suspension_chargeperiod' => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { suspension_chargeperiod => 3, firstremind => 1, @@ -2205,6 +2229,7 @@ subtest 'AddReturn + suspension_chargeperiod' => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { finedays => 2, suspension_chargeperiod => 1, @@ -2362,6 +2387,7 @@ subtest 'AddReturn | is_overdue' => sub { categorycode => undef, itemtype => undef, branchcode => undef, + checkout_type => undef, rules => { issuelength => 6, lengthunit => 'days', @@ -3048,6 +3074,7 @@ subtest 'CanBookBeIssued | is_overdue' => sub { categorycode => undef, branchcode => undef, itemtype => undef, + checkout_type => undef, rules => { maxissueqty => 1, reservesallowed => 25, @@ -3094,6 +3121,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 4c0999ef0d..219bf4524d 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, @@ -169,6 +172,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 55b78e7d0d..de24206ad3 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 36c56bb427..0331f3fa99 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 f0bf9b61d0..fd0acdf064 100755 --- a/t/db_dependent/DecreaseLoanHighHolds.t +++ b/t/db_dependent/DecreaseLoanHighHolds.t @@ -101,6 +101,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 55709643b7..386ea132d1 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 c6d2abd65f..566d9b9d7a 100755 --- a/t/db_dependent/SIP/Transaction.t +++ b/t/db_dependent/SIP/Transaction.t @@ -71,6 +71,7 @@ subtest fill_holds_at_checkout => sub { categorycode => $borrower->{categorycode}, branchcode => $branch->{branchcode}, itemtype => $itype->{itemtype}, + checkout_type => undef, rules => { onshelfholds => 1, reservesallowed => 3, @@ -177,6 +178,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