Bug 25113: Refactor CirculationRules.t when testing scope combinations
authorLari Taskula <lari.taskula@hypernova.fi>
Sat, 4 Apr 2020 15:01:02 +0000 (15:01 +0000)
committerJonathan Druart <jonathan.druart@bugs.koha-community.org>
Mon, 24 Aug 2020 10:00:39 +0000 (12:00 +0200)
We used to test rule scopes by explicitly defining each combination.

When adding new scopes, it is much easier if these tests are auto-
generated for you so that you don't have to repeat similar code.

This patch removes those "duplicates" and adds a method that returns
test cases for each scope as follows:

branchcode categorycode itemtype
__________ ____________ ________
branchcode categorycode itemtype
branchcode categorycode *
branchcode *            itemtype
branchcode *            *
*          categorycode itemtype
*          categorycode *
*          *            itemtype
*          *            *

And automatically extends the test when new scopes are added.

This also obsoletes the "Get effective issuing rule in correct order"
test in t/db_dependent/Koha/IssuingRules.t

To test:
1. prove t/db_dependent/Koha/CirculationRules.t

Sponsored-by: The National Library of Finland
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

JD amended patch: perl tidy

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

t/db_dependent/Koha/CirculationRules.t
t/db_dependent/Koha/IssuingRules.t

index 9e3fb44..708168a 100644 (file)
@@ -33,7 +33,7 @@ my $schema = Koha::Database->new->schema;
 my $builder = t::lib::TestBuilder->new;
 
 subtest 'set_rule + get_effective_rule' => sub {
-    plan tests => 14;
+    plan tests => 8;
 
     $schema->storage->txn_begin;
 
@@ -91,27 +91,6 @@ subtest 'set_rule + get_effective_rule' => sub {
 
     is( $rule->rule_value, $default_rule_value, '* means default' );
 
-    Koha::CirculationRules->set_rule(
-        {
-            branchcode   => '*',
-            categorycode => '*',
-            itemtype     => $itemtype,
-            rule_name    => $rule_name,
-            rule_value   => 2,
-        }
-    );
-
-    $rule = Koha::CirculationRules->get_effective_rule(
-        {
-            branchcode   => $branchcode,
-            categorycode => $categorycode,
-            itemtype     => $itemtype,
-            rule_name    => $rule_name,
-        }
-    );
-    is( $rule->rule_value, 2,
-        'More specific rule is returned when itemtype is given' );
-
     $rule = Koha::CirculationRules->get_effective_rule(
         {
             branchcode   => $branchcode_2,
@@ -123,129 +102,44 @@ subtest 'set_rule + get_effective_rule' => sub {
     is( $rule->rule_value, 1,
         'Default rule is returned if there is no rule for this branchcode' );
 
-    Koha::CirculationRules->set_rule(
-        {
-            branchcode   => '*',
-            categorycode => $categorycode,
-            itemtype     => '*',
-            rule_name    => $rule_name,
-            rule_value   => 3,
-        }
-    );
-
-    $rule = Koha::CirculationRules->get_effective_rule(
-        {
-
-            branchcode   => $branchcode,
-            categorycode => $categorycode,
-            itemtype     => $itemtype,
-            rule_name    => $rule_name,
-        }
-    );
-    is( $rule->rule_value, 3,
-        'More specific rule is returned when categorycode exists' );
-
-    Koha::CirculationRules->set_rule(
-        {
-            branchcode   => '*',
-            categorycode => $categorycode,
-            itemtype     => $itemtype,
-            rule_name    => $rule_name,
-            rule_value   => 4,
-        }
-    );
-    $rule = Koha::CirculationRules->get_effective_rule(
-        {
-            branchcode   => $branchcode,
-            categorycode => $categorycode,
-            itemtype     => $itemtype,
-            rule_name    => $rule_name,
-        }
-    );
-    is( $rule->rule_value, 4,
-        'More specific rule is returned when categorycode and itemtype exist' );
-
-    Koha::CirculationRules->set_rule(
-        {
-            branchcode   => $branchcode,
-            categorycode => '*',
-            itemtype     => '*',
-            rule_name    => $rule_name,
-            rule_value   => 5,
-        }
-    );
-    $rule = Koha::CirculationRules->get_effective_rule(
-        {
-            branchcode   => $branchcode,
-            categorycode => $categorycode,
-            itemtype     => $itemtype,
-            rule_name    => $rule_name,
-        }
-    );
-    is( $rule->rule_value, 5,
-        'More specific rule is returned when branchcode exists' );
-
-    Koha::CirculationRules->set_rule(
-        {
-            branchcode   => $branchcode,
-            categorycode => '*',
-            itemtype     => $itemtype,
-            rule_name    => $rule_name,
-            rule_value   => 6,
-        }
-    );
-    $rule = Koha::CirculationRules->get_effective_rule(
-        {
-            branchcode   => $branchcode,
-            categorycode => $categorycode,
-            itemtype     => $itemtype,
-            rule_name    => $rule_name,
-        }
-    );
-    is( $rule->rule_value, 6,
-        'More specific rule is returned when branchcode and itemtype exists' );
-
-    Koha::CirculationRules->set_rule(
-        {
-            branchcode   => $branchcode,
-            categorycode => $categorycode,
-            itemtype     => '*',
-            rule_name    => $rule_name,
-            rule_value   => 7,
-        }
-    );
-    $rule = Koha::CirculationRules->get_effective_rule(
-        {
-            branchcode   => $branchcode,
-            categorycode => $categorycode,
-            itemtype     => $itemtype,
-            rule_name    => $rule_name,
-        }
-    );
-    is( $rule->rule_value, 7,
-        'More specific rule is returned when branchcode and categorycode exist'
-    );
+    subtest 'test rule matching with different combinations of rule scopes' => sub {
+        my ( $tests, $order ) = prepare_tests_for_rule_scope_combinations(
+            {
+                branchcode   => $branchcode,
+                categorycode => $categorycode,
+                itemtype     => $itemtype,
+            },
+            'maxissueqty'
+        );
+
+        plan tests => 2**scalar @$order;
+
+        foreach my $test (@$tests) {
+            my $rule_params = {%$test};
+            $rule_params->{rule_name} = $rule_name;
+            my $rule_value = $rule_params->{rule_value} = int( rand(10) );
+
+            Koha::CirculationRules->set_rule($rule_params);
+
+            my $rule = Koha::CirculationRules->get_effective_rule(
+                {
+                    branchcode   => $branchcode,
+                    categorycode => $categorycode,
+                    itemtype     => $itemtype,
+                    rule_name    => $rule_name,
+                }
+            );
+
+            my $scope_output = '';
+            foreach my $key ( values @$order ) {
+                $scope_output .= " $key" if $test->{$key} ne '*';
+            }
 
-    Koha::CirculationRules->set_rule(
-        {
-            branchcode   => $branchcode,
-            categorycode => $categorycode,
-            itemtype     => $itemtype,
-            rule_name    => $rule_name,
-            rule_value   => 8,
-        }
-    );
-    $rule = Koha::CirculationRules->get_effective_rule(
-        {
-            branchcode   => $branchcode,
-            categorycode => $categorycode,
-            itemtype     => $itemtype,
-            rule_name    => $rule_name,
+            is( $rule->rule_value, $rule_value,
+                'Explicitly scoped'
+                  . ( $scope_output ? $scope_output : ' nothing' ) );
         }
-    );
-    is( $rule->rule_value, 8,
-        'More specific rule is returned when branchcode, categorycode and itemtype exist'
-    );
+    };
 
     my $our_branch_rules = Koha::CirculationRules->search({branchcode => $branchcode});
     is( $our_branch_rules->count, 4, "We added 8 rules");
@@ -506,3 +400,47 @@ subtest 'get_lostreturn_policy() tests' => sub {
 
     $schema->storage->txn_rollback;
 };
+
+sub prepare_tests_for_rule_scope_combinations {
+    my ( $scope, $rule_name ) = @_;
+
+    # Here we create a combinations of 1s and 0s the following way
+    #
+    # 000...
+    # 001...
+    # 010...
+    # 011...
+    # 100...
+    # 101...
+    # 110...
+    # 111...
+    #
+    # (the number of columns equals to the amount of rule scopes)
+    # The ... symbolizes possible future scopes.
+    #
+    # - 0 equals to circulation rule scope with any value (aka. *)
+    # - 1 equals to circulation rule scope exact value, e.g.
+    #     "CPL" (for branchcode).
+    #
+    # The order is the same as the weight of scopes when sorting circulation
+    # rules. So the first column of numbers is the scope with most weight.
+    # This is defined by C<$order> which will be assigned next.
+    #
+    # 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' ];
+    is( join(", ", sort keys %$scope),
+       join(", ", sort @$order), 'Missing a scope!' ) if keys %$scope ne scalar @$order;
+
+    my @tests = ();
+    foreach my $value ( glob( "{0,1}" x keys %$scope || 1 ) ) {
+        my $test = { %$scope };
+        for ( my $i=0; $i < keys %$scope; $i++ ) {
+            $test->{$order->[$i]} = '*' unless substr( $value, $i, 1 );
+        }
+        push @tests, $test;
+    }
+
+    return \@tests, $order;
+}
index 4d37fcf..31abe27 100644 (file)
@@ -36,7 +36,7 @@ $schema->storage->txn_begin;
 my $builder      = t::lib::TestBuilder->new;
 
 subtest 'get_effective_issuing_rule' => sub {
-    plan tests => 3;
+    plan tests => 2;
 
     my $categorycode = $builder->build({ source => 'Category' })->{'categorycode'};
     my $itemtype     = $builder->build({ source => 'Itemtype' })->{'itemtype'};
@@ -94,301 +94,6 @@ subtest 'get_effective_issuing_rule' => sub {
         );
     };
 
-    subtest 'Get effective issuing rule in correct order' => sub {
-        plan tests => 26;
-
-        my $rule;
-        Koha::CirculationRules->delete;
-        is(Koha::CirculationRules->search->count, 0, 'There are no issuing rules.');
-        $rule = Koha::CirculationRules->get_effective_rule({
-            branchcode   => $branchcode,
-            categorycode => $categorycode,
-            itemtype     => $itemtype,
-            rule_name    => 'fine',
-        });
-        is($rule, undef, 'When I attempt to get effective issuing rule, then undef'
-                        .' is returned.');
-
-        # undef, undef, undef => 5
-        ok(Koha::CirculationRule->new({
-            branchcode => undef,
-            categorycode => undef,
-            itemtype => undef,
-            rule_name => 'fine',
-            rule_value   => 5,
-        })->store, 'Given I added an issuing rule branchcode => undef, categorycode => undef, itemtype => undef,');
-        $rule = Koha::CirculationRules->get_effective_rule({
-            branchcode   => $branchcode,
-            categorycode => $categorycode,
-            itemtype     => $itemtype,
-            rule_name    => 'fine',
-        });
-        _is_row_match(
-            $rule,
-            {
-                branchcode   => undef,
-                categorycode => undef,
-                itemtype     => undef,
-                rule_name    => 'fine',
-                rule_value   => 5,
-            },
-            'When I attempt to get effective issuing rule,'
-           .' then the above one is returned.'
-        );
-
-        # undef, undef, undef     => 5
-        # undef, undef, $itemtype => 7
-        ok(
-            Koha::CirculationRule->new(
-                {
-                    branchcode   => undef,
-                    categorycode => undef,
-                    itemtype     => $itemtype,
-                    rule_name    => 'fine',
-                    rule_value   => 7,
-                }
-              )->store,
-            "Given I added an issuing rule branchcode => undef, categorycode => undef, itemtype => $itemtype,"
-        );
-        $rule = Koha::CirculationRules->get_effective_rule({
-            branchcode   => $branchcode,
-            categorycode => $categorycode,
-            itemtype     => $itemtype,
-            rule_name    => 'fine',
-        });
-        _is_row_match(
-            $rule,
-            {
-                branchcode   => undef,
-                categorycode => undef,
-                itemtype     => $itemtype,
-                rule_name    => 'fine',
-                rule_value   => 7,
-            },
-            'When I attempt to get effective issuing rule,'
-           .' then the above one is returned.'
-        );
-
-        # undef, undef,         undef     => 5
-        # undef, undef,         $itemtype => 7
-        # undef, $categorycode, undef     => 9
-        ok(
-            Koha::CirculationRule->new(
-                {
-                    branchcode   => undef,
-                    categorycode => $categorycode,
-                    itemtype     => undef,
-                    rule_name    => 'fine',
-                    rule_value   => 9,
-                }
-              )->store,
-            "Given I added an issuing rule branchcode => undef, categorycode => $categorycode, itemtype => undef,"
-        );
-        $rule = Koha::CirculationRules->get_effective_rule({
-            branchcode   => $branchcode,
-            categorycode => $categorycode,
-            itemtype     => $itemtype,
-            rule_name    => 'fine',
-        });
-        _is_row_match(
-            $rule,
-            {
-                branchcode   => undef,
-                categorycode => $categorycode,
-                itemtype     => undef,
-                rule_name    => 'fine',
-                rule_value   => 9,
-            },
-            'When I attempt to get effective issuing rule,'
-           .' then the above one is returned.'
-        );
-
-        # undef, undef,         undef     => 5
-        # undef, undef,         $itemtype => 7
-        # undef, $categorycode, undef     => 9
-        # undef, $categorycode, $itemtype => 11
-        ok(
-            Koha::CirculationRule->new(
-                {
-                    branchcode   => undef,
-                    categorycode => $categorycode,
-                    itemtype     => $itemtype,
-                    rule_name    => 'fine',
-                    rule_value   => 11,
-                }
-              )->store,
-            "Given I added an issuing rule branchcode => undef, categorycode => $categorycode, itemtype => $itemtype,"
-        );
-        $rule = Koha::CirculationRules->get_effective_rule({
-            branchcode   => $branchcode,
-            categorycode => $categorycode,
-            itemtype     => $itemtype,
-            rule_name    => 'fine',
-        });
-        _is_row_match(
-            $rule,
-            {
-                branchcode   => undef,
-                categorycode => $categorycode,
-                itemtype     => $itemtype,
-                rule_name    => 'fine',
-                rule_value   => 11,
-            },
-            'When I attempt to get effective issuing rule,'
-           .' then the above one is returned.'
-        );
-
-        # undef,       undef,         undef     => 5
-        # undef,       undef,         $itemtype => 7
-        # undef,       $categorycode, undef     => 9
-        # undef,       $categorycode, $itemtype => 11
-        # $branchcode, undef,         undef     => 13
-        ok(
-            Koha::CirculationRule->new(
-                {
-                    branchcode   => $branchcode,
-                    categorycode => undef,
-                    itemtype     => undef,
-                    rule_name    => 'fine',
-                    rule_value   => 13,
-                }
-              )->store,
-            "Given I added an issuing rule branchcode => $branchcode, categorycode => undef, itemtype => undef,"
-        );
-        $rule = Koha::CirculationRules->get_effective_rule({
-            branchcode   => $branchcode,
-            categorycode => $categorycode,
-            itemtype     => $itemtype,
-            rule_name    => 'fine',
-        });
-        _is_row_match(
-            $rule,
-            {
-                branchcode   => $branchcode,
-                categorycode => undef,
-                itemtype     => undef,
-                rule_name    => 'fine',
-                rule_value   => 13,
-            },
-            'When I attempt to get effective issuing rule,'
-           .' then the above one is returned.'
-        );
-
-        # undef,       undef,         undef     => 5
-        # undef,       undef,         $itemtype => 7
-        # undef,       $categorycode, undef     => 9
-        # undef,       $categorycode, $itemtype => 11
-        # $branchcode, undef,         undef     => 13
-        # $branchcode, undef,         $itemtype => 15
-        ok(
-            Koha::CirculationRule->new(
-                {
-                    branchcode   => $branchcode,
-                    categorycode => undef,
-                    itemtype     => $itemtype,
-                    rule_name    => 'fine',
-                    rule_value   => 15,
-                }
-              )->store,
-            "Given I added an issuing rule branchcode => $branchcode, categorycode => undef, itemtype => $itemtype,"
-        );
-        $rule = Koha::CirculationRules->get_effective_rule({
-            branchcode   => $branchcode,
-            categorycode => $categorycode,
-            itemtype     => $itemtype,
-            rule_name    => 'fine',
-        });
-        _is_row_match(
-            $rule,
-            {
-                branchcode   => $branchcode,
-                categorycode => undef,
-                itemtype     => $itemtype,
-                rule_name    => 'fine',
-                rule_value   => 15,
-            },
-            'When I attempt to get effective issuing rule,'
-           .' then the above one is returned.'
-        );
-
-        # undef,       undef,         undef     => 5
-        # undef,       undef,         $itemtype => 7
-        # undef,       $categorycode, undef     => 9
-        # undef,       $categorycode, $itemtype => 11
-        # $branchcode, undef,         undef     => 13
-        # $branchcode, undef,         $itemtype => 15
-        # $branchcode, $categorycode, undef     => 17
-        ok(
-            Koha::CirculationRule->new(
-                {
-                    branchcode   => $branchcode,
-                    categorycode => $categorycode,
-                    itemtype     => undef,
-                    rule_name    => 'fine',
-                    rule_value   => 17,
-                }
-              )->store,
-            "Given I added an issuing rule branchcode => $branchcode, categorycode => $categorycode, itemtype => undef,"
-        );
-        $rule = Koha::CirculationRules->get_effective_rule({
-            branchcode   => $branchcode,
-            categorycode => $categorycode,
-            itemtype     => $itemtype,
-            rule_name    => 'fine',
-        });
-        _is_row_match(
-            $rule,
-            {
-                branchcode   => $branchcode,
-                categorycode => $categorycode,
-                itemtype     => undef,
-                rule_name    => 'fine',
-                rule_value   => 17,
-            },
-            'When I attempt to get effective issuing rule,'
-           .' then the above one is returned.'
-        );
-
-        # undef,       undef,         undef     => 5
-        # undef,       undef,         $itemtype => 7
-        # undef,       $categorycode, undef     => 9
-        # undef,       $categorycode, $itemtype => 11
-        # $branchcode, undef,         undef     => 13
-        # $branchcode, undef,         $itemtype => 15
-        # $branchcode, $categorycode, undef     => 17
-        # $branchcode, $categorycode, $itemtype => 19
-        ok(
-            Koha::CirculationRule->new(
-                {
-                    branchcode   => $branchcode,
-                    categorycode => $categorycode,
-                    itemtype     => $itemtype,
-                    rule_name    => 'fine',
-                    rule_value   => 19,
-                }
-              )->store,
-            "Given I added an issuing rule branchcode => $branchcode, categorycode => $categorycode, itemtype => $itemtype,"
-        );
-        $rule = Koha::CirculationRules->get_effective_rule({
-            branchcode   => $branchcode,
-            categorycode => $categorycode,
-            itemtype     => $itemtype,
-            rule_name    => 'fine',
-        });
-        _is_row_match(
-            $rule,
-            {
-                branchcode   => $branchcode,
-                categorycode => $categorycode,
-                itemtype     => $itemtype,
-                rule_name    => 'fine',
-                rule_value   => 19,
-            },
-            'When I attempt to get effective issuing rule,'
-           .' then the above one is returned.'
-        );
-    };
-
     subtest 'Performance' => sub {
         plan tests => 4;