From d9572d8d3b17412854756e1dea2652c57a862163 Mon Sep 17 00:00:00 2001
From: Lari Taskula <lari.taskula@hypernova.fi>
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 | 30 ++++++++++---
 2 files changed, 60 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 2e6a334826..ba6e1c4775 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;
@@ -131,9 +139,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;
 };
@@ -172,7 +180,7 @@ subtest 'set_rules() tests' => sub {
     plan tests => 1;
 
     subtest 'scope validation' => sub {
-        plan tests => 6;
+        plan tests => 7;
 
         $schema->storage->txn_begin;
 
@@ -182,6 +190,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;
@@ -190,14 +199,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,
@@ -207,6 +218,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' );
 
@@ -227,6 +246,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' );
 
@@ -262,7 +282,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<get_effective_rule> 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