From b09a825ba419a0f1c34aa4f5344347d79861d536 Mon Sep 17 00:00:00 2001 From: Lari Taskula Date: Sun, 5 Apr 2020 19:17:15 +0000 Subject: [PATCH] Bug 25089: Add checkout_type to Koha::CirculationRules To test: 1. prove t/db_dependent/Koha/CirculationRules.t Sponsored-by: The National Library of Finland --- Koha/CirculationRules.pm | 62 +++++++++++++++----------- t/db_dependent/Koha/CirculationRules.t | 31 ++++++++++--- 2 files changed, 61 insertions(+), 32 deletions(-) diff --git a/Koha/CirculationRules.pm b/Koha/CirculationRules.pm index 92a20e65c8..4b8d8d8675 100644 --- a/Koha/CirculationRules.pm +++ b/Koha/CirculationRules.pm @@ -51,7 +51,7 @@ our $RULE_KINDS = { }, patron_maxissueqty => { - scope => [ 'branchcode', 'categorycode' ], + scope => [ 'branchcode', 'categorycode', 'checkout_type' ], }, patron_maxonsiteissueqty => { scope => [ 'branchcode', 'categorycode' ], @@ -74,31 +74,31 @@ our $RULE_KINDS = { scope => [ 'branchcode', 'categorycode', 'itemtype' ], }, auto_renew => { - scope => [ 'branchcode', 'categorycode', 'itemtype' ], + scope => [ 'branchcode', 'categorycode', 'itemtype', 'checkout_type' ], }, cap_fine_to_replacement_price => { - scope => [ 'branchcode', 'categorycode', 'itemtype' ], + scope => [ 'branchcode', 'categorycode', 'itemtype', 'checkout_type' ], }, chargeperiod => { - scope => [ 'branchcode', 'categorycode', 'itemtype' ], + scope => [ 'branchcode', 'categorycode', 'itemtype', 'checkout_type' ], }, chargeperiod_charge_at => { - scope => [ 'branchcode', 'categorycode', 'itemtype' ], + scope => [ 'branchcode', 'categorycode', 'itemtype', 'checkout_type' ], }, fine => { - scope => [ 'branchcode', 'categorycode', 'itemtype' ], + scope => [ 'branchcode', 'categorycode', 'itemtype', 'checkout_type' ], }, finedays => { - scope => [ 'branchcode', 'categorycode', 'itemtype' ], + scope => [ 'branchcode', 'categorycode', 'itemtype', 'checkout_type' ], }, firstremind => { - scope => [ 'branchcode', 'categorycode', 'itemtype' ], + scope => [ 'branchcode', 'categorycode', 'itemtype', 'checkout_type' ], }, hardduedate => { - scope => [ 'branchcode', 'categorycode', 'itemtype' ], + scope => [ 'branchcode', 'categorycode', 'itemtype', 'checkout_type' ], }, hardduedatecompare => { - scope => [ 'branchcode', 'categorycode', 'itemtype' ], + scope => [ 'branchcode', 'categorycode', 'itemtype', 'checkout_type' ], }, holds_per_day => { scope => [ 'branchcode', 'categorycode', 'itemtype' ], @@ -107,28 +107,28 @@ our $RULE_KINDS = { scope => [ 'branchcode', 'categorycode', 'itemtype' ], }, issuelength => { - scope => [ 'branchcode', 'categorycode', 'itemtype' ], + scope => [ 'branchcode', 'categorycode', 'itemtype', 'checkout_type' ], }, lengthunit => { - scope => [ 'branchcode', 'categorycode', 'itemtype' ], + scope => [ 'branchcode', 'categorycode', 'itemtype', 'checkout_type' ], }, maxissueqty => { - scope => [ 'branchcode', 'categorycode', 'itemtype' ], + scope => [ 'branchcode', 'categorycode', 'itemtype', 'checkout_type' ], }, maxonsiteissueqty => { scope => [ 'branchcode', 'categorycode', 'itemtype' ], }, maxsuspensiondays => { - scope => [ 'branchcode', 'categorycode', 'itemtype' ], + scope => [ 'branchcode', 'categorycode', 'itemtype', 'checkout_type' ], }, no_auto_renewal_after => { - scope => [ 'branchcode', 'categorycode', 'itemtype' ], + scope => [ 'branchcode', 'categorycode', 'itemtype', 'checkout_type' ], }, no_auto_renewal_after_hard_limit => { - scope => [ 'branchcode', 'categorycode', 'itemtype' ], + scope => [ 'branchcode', 'categorycode', 'itemtype', 'checkout_type' ], }, norenewalbefore => { - scope => [ 'branchcode', 'categorycode', 'itemtype' ], + scope => [ 'branchcode', 'categorycode', 'itemtype', 'checkout_type' ], }, onshelfholds => { scope => [ 'branchcode', 'categorycode', 'itemtype' ], @@ -137,25 +137,25 @@ our $RULE_KINDS = { scope => [ 'branchcode', 'categorycode', 'itemtype' ], }, overduefinescap => { - scope => [ 'branchcode', 'categorycode', 'itemtype' ], + scope => [ 'branchcode', 'categorycode', 'itemtype', 'checkout_type' ], }, renewalperiod => { - scope => [ 'branchcode', 'categorycode', 'itemtype' ], + scope => [ 'branchcode', 'categorycode', 'itemtype', 'checkout_type' ], }, renewalsallowed => { - scope => [ 'branchcode', 'categorycode', 'itemtype' ], + scope => [ 'branchcode', 'categorycode', 'itemtype', 'checkout_type' ], }, rentaldiscount => { - scope => [ 'branchcode', 'categorycode', 'itemtype' ], + scope => [ 'branchcode', 'categorycode', 'itemtype', 'checkout_type' ], }, reservesallowed => { scope => [ 'branchcode', 'categorycode', 'itemtype' ], }, suspension_chargeperiod => { - scope => [ 'branchcode', 'categorycode', 'itemtype' ], + scope => [ 'branchcode', 'categorycode', 'itemtype', 'checkout_type' ], }, note => { # This is not really a rule. Maybe we will want to separate this later. - scope => [ 'branchcode', 'categorycode', 'itemtype' ], + scope => [ 'branchcode', 'categorycode', 'itemtype', 'checkout_type' ], }, # Not included (deprecated?): # * accountsent @@ -177,22 +177,24 @@ sub get_effective_rule { $params->{categorycode} //= undef; $params->{branchcode} //= undef; $params->{itemtype} //= undef; + $params->{checkout_type} //= undef; my $rule_name = $params->{rule_name}; my $categorycode = $params->{categorycode}; my $itemtype = $params->{itemtype}; my $branchcode = $params->{branchcode}; + my $checkout_type = $params->{checkout_type}; Koha::Exceptions::MissingParameter->throw( "Required parameter 'rule_name' missing") unless $rule_name; - for my $v ( $branchcode, $categorycode, $itemtype ) { + for my $v ( $branchcode, $categorycode, $itemtype, $checkout_type ) { $v = undef if $v and $v eq '*'; } my $order_by = $params->{order_by} - // { -desc => [ 'branchcode', 'categorycode', 'itemtype' ] }; + // { -desc => [ 'branchcode', 'categorycode', 'itemtype', 'checkout_type' ] }; my $search_params; $search_params->{rule_name} = $rule_name; @@ -200,6 +202,7 @@ sub get_effective_rule { $search_params->{categorycode} = defined $categorycode ? [ $categorycode, undef ] : undef; $search_params->{itemtype} = defined $itemtype ? [ $itemtype, undef ] : undef; $search_params->{branchcode} = defined $branchcode ? [ $branchcode, undef ] : undef; + $search_params->{checkout_type} = defined $checkout_type ? [ $checkout_type, undef ] : undef; my $rule = $self->search( $search_params, @@ -222,6 +225,7 @@ sub get_effective_rules { my $rules = $params->{rules}; my $categorycode = $params->{categorycode}; my $itemtype = $params->{itemtype}; + my $checkout_type = $params->{checkout_type}; my $branchcode = $params->{branchcode}; my $r; @@ -231,6 +235,7 @@ sub get_effective_rules { rule_name => $rule, categorycode => $categorycode, itemtype => $itemtype, + checkout_type => $checkout_type, branchcode => $branchcode, } ); @@ -260,7 +265,7 @@ sub set_rule { unless defined $kind_info; # Enforce scope; a rule should be set for its defined scope, no more, no less. - foreach my $scope_level ( qw( branchcode categorycode itemtype ) ) { + foreach my $scope_level ( qw( branchcode categorycode itemtype checkout_type ) ) { if ( grep /$scope_level/, @{ $kind_info->{scope} } ) { croak "set_rule needs '$scope_level' to set '$params->{rule_name}'!" unless exists $params->{$scope_level}; @@ -273,10 +278,11 @@ sub set_rule { my $branchcode = $params->{branchcode}; my $categorycode = $params->{categorycode}; my $itemtype = $params->{itemtype}; + my $checkout_type = $params->{checkout_type}; my $rule_name = $params->{rule_name}; my $rule_value = $params->{rule_value}; - for my $v ( $branchcode, $categorycode, $itemtype ) { + for my $v ( $branchcode, $categorycode, $itemtype, $checkout_type ) { $v = undef if $v and $v eq '*'; } my $rule = $self->search( @@ -285,6 +291,7 @@ sub set_rule { branchcode => $branchcode, categorycode => $categorycode, itemtype => $itemtype, + checkout_type => $checkout_type, } )->next(); @@ -304,6 +311,7 @@ sub set_rule { branchcode => $branchcode, categorycode => $categorycode, itemtype => $itemtype, + checkout_type => $checkout_type, rule_name => $rule_name, rule_value => $rule_value, } diff --git a/t/db_dependent/Koha/CirculationRules.t b/t/db_dependent/Koha/CirculationRules.t index 14ac30c08b..95e063a6da 100644 --- a/t/db_dependent/Koha/CirculationRules.t +++ b/t/db_dependent/Koha/CirculationRules.t @@ -22,6 +22,7 @@ use Modern::Perl; use Test::More tests => 3; use Test::Exception; +use Koha::Checkouts; use Koha::CirculationRules; use Koha::Database; @@ -35,6 +36,7 @@ subtest 'set_rule + get_effective_rule' => sub { $schema->storage->txn_begin; + my $checkout_type = $Koha::Checkouts::type->{checkout}; my $categorycode = $builder->build_object( { class => 'Koha::Patron::Categories' } )->categorycode; my $itemtype = $builder->build_object( { class => 'Koha::ItemTypes' } )->itemtype; my $branchcode = $builder->build_object( { class => 'Koha::Libraries' } )->branchcode; @@ -54,6 +56,7 @@ subtest 'set_rule + get_effective_rule' => sub { branchcode => $branchcode, categorycode => $categorycode, itemtype => $itemtype, + checkout_type => $checkout_type, rule_name => $rule_name, } ); @@ -64,6 +67,7 @@ subtest 'set_rule + get_effective_rule' => sub { branchcode => '*', categorycode => '*', itemtype => '*', + checkout_type => '*', rule_name => $rule_name, rule_value => $default_rule_value, } @@ -74,6 +78,7 @@ subtest 'set_rule + get_effective_rule' => sub { branchcode => undef, categorycode => undef, itemtype => undef, + checkout_type => undef, rule_name => $rule_name, } ); @@ -83,6 +88,7 @@ subtest 'set_rule + get_effective_rule' => sub { branchcode => '*', categorycode => '*', itemtype => '*', + checkout_type => '*', rule_name => $rule_name, } ); @@ -94,6 +100,7 @@ subtest 'set_rule + get_effective_rule' => sub { branchcode => $branchcode_2, categorycode => '*', itemtype => '*', + checkout_type => '*', rule_name => $rule_name, } ); @@ -106,6 +113,7 @@ subtest 'set_rule + get_effective_rule' => sub { branchcode => $branchcode, categorycode => $categorycode, itemtype => $itemtype, + checkout_type => $checkout_type, }, 'maxissueqty' ); plan tests => 2**scalar @$order; @@ -121,6 +129,7 @@ subtest 'set_rule + get_effective_rule' => sub { branchcode => $branchcode, categorycode => $categorycode, itemtype => $itemtype, + checkout_type => $checkout_type, rule_name => $rule_name, } ); @@ -136,9 +145,9 @@ subtest 'set_rule + get_effective_rule' => sub { }; my $our_branch_rules = Koha::CirculationRules->search({branchcode => $branchcode}); - is( $our_branch_rules->count, 4, "We added 8 rules"); + is( $our_branch_rules->count, 8, "We added 16 rules"); $our_branch_rules->delete; - is( $our_branch_rules->count, 0, "We deleted 8 rules"); + is( $our_branch_rules->count, 0, "We deleted 16 rules"); $schema->storage->txn_rollback; }; @@ -177,7 +186,7 @@ subtest 'set_rules() tests' => sub { plan tests => 1; subtest 'scope validation' => sub { - plan tests => 6; + plan tests => 7; $schema->storage->txn_begin; @@ -187,6 +196,7 @@ subtest 'set_rules() tests' => sub { class => 'Koha::ItemTypes' } )->itemtype; my $branchcode = $builder->build_object( { class => 'Koha::Libraries' } )->branchcode; + my $checkout_type = $Koha::Checkouts::type->{checkout}; my $rule; Koha::CirculationRules->delete; @@ -195,14 +205,16 @@ subtest 'set_rules() tests' => sub { branchcode => $branchcode, categorycode => $categorycode, itemtype => $itemtype, + checkout_type => $checkout_type, rules => { refund => 1, + max_holds => 9001, patron_maxissueqty => 2, holdallowed => 3, article_requests => 4, maxissueqty => 5, } - } )}, 5, 'All rules added' ); + } )}, 6, 'All rules added' ); is( Koha::CirculationRules->get_effective_rule( { branchcode => $branchcode, @@ -212,6 +224,14 @@ subtest 'set_rules() tests' => sub { is( Koha::CirculationRules->get_effective_rule( { branchcode => $branchcode, categorycode => $categorycode, + checkout_type => $checkout_type, + rule_name => 'max_holds' + } )->rule_value, 9001, 'Found rule patron_maxissueqty' ); + + is( Koha::CirculationRules->get_effective_rule( { + branchcode => $branchcode, + categorycode => $categorycode, + checkout_type => $checkout_type, rule_name => 'patron_maxissueqty' } )->rule_value, 2, 'Found rule patron_maxissueqty' ); @@ -232,6 +252,7 @@ subtest 'set_rules() tests' => sub { branchcode => $branchcode, categorycode => $categorycode, itemtype => $itemtype, + checkout_type => $checkout_type, rule_name => 'maxissueqty' } )->rule_value, 5, 'Found rule maxissueqty' ); @@ -267,7 +288,7 @@ sub prepare_tests_for_rule_scope_combinations { # We must maintain the order in order to keep the test valid. This should be # equal to Koha/CirculationRules.pm "order_by" of C sub. # Let's explicitly define the order and fail test if we are missing a scope: - my $order = [ 'branchcode', 'categorycode', 'itemtype' ]; + my $order = [ 'branchcode', 'categorycode', 'itemtype', 'checkout_type' ]; is( join(", ", sort keys %$scope), join(", ", sort @$order), 'Missing a scope!' ) if keys %$scope ne scalar @$order; -- 2.17.1