Bugzilla – Attachment 103241 Details for
Bug 25089
Add checkout_type to circulation rules
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 25089: Add checkout_type to Koha::CirculationRules
Bug-25089-Add-checkouttype-to-KohaCirculationRules.patch (text/plain), 15.12 KB, created by
Lari Taskula
on 2020-04-20 09:28:09 UTC
(
hide
)
Description:
Bug 25089: Add checkout_type to Koha::CirculationRules
Filename:
MIME Type:
Creator:
Lari Taskula
Created:
2020-04-20 09:28:09 UTC
Size:
15.12 KB
patch
obsolete
>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
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=103241">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
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