Attachment 103356 Details for Bug 25089 – Bug 25089: Add checkout_type to set_rule

[patch] Bug 25089: Add checkout_type to set_rule

Bug-25089-Add-checkouttype-to-setrule.patch (text/plain), 8.63 KB, created by Lari Taskula on 2020-04-21 10:51:47 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Lari Taskula

Created: 2020-04-21 10:51:47 UTC

Size: 8.63 KB

patch

obsolete

>From 9a237ea77e19cd892eabd2a90978e6f53fce4556 Mon Sep 17 00:00:00 2001
>From: Lari Taskula <lari.taskula@hypernova.fi>
>Date: Fri, 10 Apr 2020 00:02:24 +0000
>Subject: [PATCH] Bug 25089: Add checkout_type to set_rule
>
>To test:
>1. Find all occurrences of set_rule that are missing checkout_type
>   where required
>
>grep --exclude-dir '.git' --exclude-dir 'misc/translator' \
>--exclude-dir 'koha-tmpl' -Przo '(?s)(::|->)set_rule(?!s).*?\)' | \
>grep -avz 'checkout_type' | grep -Pavz 'hold|reserves|article_requests' \
>&& echo ""
>
>   The only occasions this should return anything are cases where
>   a HASH or HASHref is given to set_rule(), 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_rule()
>
>git grep --no-index -n -p -P 'set_rule\s*\(' | grep -v 'sub {' \
>| grep -P 'sub .*'
>
>   2.1 The only return should be
>       Koha/CirculationRules.pm=326=sub set_rules {
>
>3. The boring step. Find all occurrences of set_rule() with
>   the following command:
>
>grep --exclude-dir='.git' -Prn 'set_rule' | grep -v 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
>---
> t/db_dependent/Circulation.t              |  5 +++++
> t/db_dependent/Circulation/CalcFine.t     |  5 ++++-
> t/db_dependent/Circulation/ReturnClaims.t |  1 +
> t/db_dependent/Circulation/Returns.t      |  1 +
> t/db_dependent/Koha/IssuingRules.t        | 19 +++++++++++++++++--
> 5 files changed, 28 insertions(+), 3 deletions(-)
>
>diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t
>index 946611222d..4145860ac5 100755
>--- a/t/db_dependent/Circulation.t
>+++ b/t/db_dependent/Circulation.t
>@@ -667,6 +667,7 @@ subtest "CanBookBeRenewed tests" => sub {
>             categorycode => undef,
>             branchcode   => undef,
>             itemtype     => undef,
>+            checkout_type => undef,
>             rule_name    => 'norenewalbefore',
>             rule_value   => '7',
>         }
>@@ -2156,6 +2157,7 @@ subtest 'AddReturn + suspension_chargeperiod' => sub {
>             categorycode => undef,
>             branchcode   => undef,
>             itemtype     => undef,
>+            checkout_type => undef,
>             rule_name    => 'suspension_chargeperiod',
>             rule_value   => '2',
>         }
>@@ -3488,6 +3490,8 @@ subtest 'Incremented fee tests' => sub {
>     my $library =
>       $builder->build_object( { class => 'Koha::Libraries' } )->store;
> 
>+    my $checkout_type = $Koha::Checkouts::type->{checkout};
>+
>     my $module = new Test::MockModule('C4::Context');
>     $module->mock( 'userenv', sub { { branch => $library->id } } );
> 
>@@ -3613,6 +3617,7 @@ subtest 'Incremented fee tests' => sub {
>             categorycode => $patron->categorycode,
>             itemtype     => $itemtype->id,
>             branchcode   => $library->id,
>+            checkout_type => $checkout_type,
>             rule_name    => 'lengthunit',
>             rule_value   => 'hours',
>         }
>diff --git a/t/db_dependent/Circulation/CalcFine.t b/t/db_dependent/Circulation/CalcFine.t
>index f09a0e011d..2adf88ed75 100644
>--- a/t/db_dependent/Circulation/CalcFine.t
>+++ b/t/db_dependent/Circulation/CalcFine.t
>@@ -83,6 +83,7 @@ subtest 'Test basic functionality' => sub {
>             branchcode   => undef,
>             categorycode => undef,
>             itemtype     => undef,
>+            checkout_type => undef,
>             rules        => {
>                 fine                          => '1.00',
>                 lengthunit                    => 'days',
>@@ -123,6 +124,7 @@ subtest 'Test cap_fine_to_replacement_price' => sub {
>             branchcode   => undef,
>             categorycode => undef,
>             itemtype     => undef,
>+            checkout_type => undef,
>             rules        => {
>                 fine                          => '1.00',
>                 lengthunit                    => 'days',
>@@ -170,6 +172,7 @@ subtest 'Test cap_fine_to_replacement_pricew with overduefinescap' => sub {
>             branchcode   => undef,
>             categorycode => undef,
>             itemtype     => undef,
>+            checkout_type => undef,
>             rules        => {
>                 fine                          => '1.00',
>                 lengthunit                    => 'days',
>@@ -197,7 +200,7 @@ subtest 'Test cap_fine_to_replacement_pricew with overduefinescap' => sub {
>     my ($amount) = CalcFine( $item, $patron->{categorycode}, $branch->{branchcode}, $start_dt, $end_dt );
>     is( int($amount), 3, 'Got the lesser of overduefinescap and replacement price where overduefinescap < replacement price' );
> 
>-    Koha::CirculationRules->set_rule({ rule_name => 'overduefinescap', rule_value => 6, branchcode => undef, categorycode => undef, itemtype => undef });
>+    Koha::CirculationRules->set_rule({ rule_name => 'overduefinescap', rule_value => 6, branchcode => undef, categorycode => undef, itemtype => undef, checkout_type => undef });
>     ($amount) = CalcFine( $item, $patron->{categorycode}, $branch->{branchcode}, $start_dt, $end_dt );
>     is( int($amount), 5, 'Get the lesser of overduefinescap and replacement price where overduefinescap > replacement price' );
> 
>diff --git a/t/db_dependent/Circulation/ReturnClaims.t b/t/db_dependent/Circulation/ReturnClaims.t
>index 464420ead2..3e2dd1e835 100644
>--- a/t/db_dependent/Circulation/ReturnClaims.t
>+++ b/t/db_dependent/Circulation/ReturnClaims.t
>@@ -51,6 +51,7 @@ Koha::CirculationRules->set_rule(
>         branchcode   => undef,
>         categorycode => undef,
>         itemtype     => undef,
>+        checkout_type => undef,
>         rule_name    => 'issuelength',
>         rule_value   => 1
>     }
>diff --git a/t/db_dependent/Circulation/Returns.t b/t/db_dependent/Circulation/Returns.t
>index f93ff66945..55b78e7d0d 100644
>--- a/t/db_dependent/Circulation/Returns.t
>+++ b/t/db_dependent/Circulation/Returns.t
>@@ -63,6 +63,7 @@ Koha::CirculationRules->set_rule(
>         categorycode => undef,
>         itemtype     => undef,
>         branchcode   => undef,
>+        checkout_type => undef,
>         rule_name    => 'issuelength',
>         rule_value   => 1,
>     }
>diff --git a/t/db_dependent/Koha/IssuingRules.t b/t/db_dependent/Koha/IssuingRules.t
>index 31abe27c4c..40ddb58e12 100644
>--- a/t/db_dependent/Koha/IssuingRules.t
>+++ b/t/db_dependent/Koha/IssuingRules.t
>@@ -154,9 +154,10 @@ subtest 'set_rule' => sub {
>     my $branchcode   = $builder->build({ source => 'Branch' })->{'branchcode'};
>     my $categorycode = $builder->build({ source => 'Category' })->{'categorycode'};
>     my $itemtype     = $builder->build({ source => 'Itemtype' })->{'itemtype'};
>+    my $checkout_type = $Koha::Checkouts::type->{checkout};
> 
>     subtest 'Correct call' => sub {
>-        plan tests => 4;
>+        plan tests => 5;
> 
>         Koha::CirculationRules->delete;
> 
>@@ -172,6 +173,7 @@ subtest 'set_rule' => sub {
>             Koha::CirculationRules->set_rule( {
>                 branchcode => $branchcode,
>                 categorycode => $categorycode,
>+                checkout_type => $checkout_type,
>                 rule_name => 'patron_maxissueqty',
>                 rule_value => '',
>             } );
>@@ -191,10 +193,22 @@ subtest 'set_rule' => sub {
>                 branchcode => $branchcode,
>                 categorycode => $categorycode,
>                 itemtype => $itemtype,
>+                checkout_type => $checkout_type,
>                 rule_name => 'fine',
>                 rule_value => '',
>             } );
>-        }, 'setting fine with branch/category/itemtype succeeds' );
>+        }, 'setting fine with branch/category/itemtype/checkout_type succeeds' );
>+
>+        lives_ok( sub {
>+            Koha::CirculationRules->set_rule( {
>+                branchcode => $branchcode,
>+                categorycode => $categorycode,
>+                itemtype => $itemtype,
>+                checkout_type => $checkout_type,
>+                rule_name => 'maxissueqty',
>+                rule_value => 5,
>+            } );
>+        }, 'setting maxissueqty with branch/category/itemtype/checkout_type succeeds' );
>     };
> 
>     subtest 'Call with missing params' => sub {
>@@ -212,6 +226,7 @@ subtest 'set_rule' => sub {
>         throws_ok( sub {
>             Koha::CirculationRules->set_rule( {
>                 branchcode => $branchcode,
>+                checkout_type => $checkout_type,
>                 rule_name => 'patron_maxissueqty',
>                 rule_value => '',
>             } );
>-- 
>2.17.1

Actions: View | Diff | Splinter Review

Attachments on bug 25089: 103046 | 103047 | 103048 | 103049 | 103050 | 103051 | 103052 | 103053 | 103054 | 103239 | 103240 | 103241 | 103242 | 103243 | 103244 | 103245 | 103246 | 103247 | 103248 | 103283 | 103284 | 103285 | 103286 | 103287 | 103288 | 103289 | 103290 | 103291 | 103292 | 103339 | 103340 | 103341 | 103342 | 103343 | 103344 | 103345 | 103346 | 103347 | 103348 | 103349 | 103351 | 103352 | 103353 | 103354 | 103355 | 103356 | 103357 | 103358 | 103359 | 103360 | 103361 | 103537 | 103538 | 103539 | 103540 | 103541 | 103542 | 103543 | 103544 | 103545 | 103546 | 103547 | 104274 | 104275 | 107523 | 107524 | 107525 | 107526 | 107527 | 107528 | 107529 | 107530 | 107531 | 107532 | 107533