From dabdb39f384d60ec60e91131e130c6c755578d44 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Thu, 24 Sep 2020 18:21:45 +0000 Subject: [PATCH] Bug 26529: Define some rules as not able to be blank This patch uses the existing hash that controls the scope of rules and adds a new parameter 'can_be_blank' To test: 1 - Goto to Admin -> Circulation and fines rules and select a library 2 - Under "Default checkout, hold, and return policy" set: Total current checkouts allowed: 1 Hold policy: Not set Hold pickup library match: Not set Return policay: Not set 3 - Save the rule 4 - Check the DB SELECT * FROM circulation_rules WHERE branchcode = 'BRANCHCODE' using the branchcode for the library you chode 5 - Note holdallowed, returnbranch, and hold_fulfillment_policy rules exist with blank value 6 - Apply patch and restart all the things 7 - Reload the page and save the rules as before 8 - Check the DB and note the rules no longer exist 9 - pr0ve -v t/db_dependent/Koha/CirculationRules.t --- Koha/CirculationRules.pm | 5 +++++ admin/smart-rules.pl | 2 -- t/db_dependent/Koha/CirculationRules.t | 27 ++++++++++++++++++++++++++- 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/Koha/CirculationRules.pm b/Koha/CirculationRules.pm index 87062f541a..5b63998a8b 100644 --- a/Koha/CirculationRules.pm +++ b/Koha/CirculationRules.pm @@ -62,12 +62,15 @@ our $RULE_KINDS = { holdallowed => { scope => [ 'branchcode', 'itemtype' ], + can_be_blank => 0, }, hold_fulfillment_policy => { scope => [ 'branchcode', 'itemtype' ], + can_be_blank => 0, }, returnbranch => { scope => [ 'branchcode', 'itemtype' ], + can_be_blank => 0, }, article_requests => { @@ -278,6 +281,8 @@ sub set_rule { my $itemtype = $params->{itemtype}; my $rule_name = $params->{rule_name}; my $rule_value = $params->{rule_value}; + my $can_be_blank = defined $kind_info->{can_be_blank} ? $kind_info->{can_be_blank} : 1; + $rule_value = undef if $rule_value eq "" && !$can_be_blank; for my $v ( $branchcode, $categorycode, $itemtype ) { $v = undef if $v and $v eq '*'; diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl index 4a4727e5c2..a2e9b41cd8 100755 --- a/admin/smart-rules.pl +++ b/admin/smart-rules.pl @@ -341,8 +341,6 @@ elsif ($op eq "set-branch-defaults") { my $hold_fulfillment_policy = $input->param('hold_fulfillment_policy'); my $returnbranch = $input->param('returnbranch'); my $max_holds = strip_non_numeric( scalar $input->param('max_holds') ); - $holdallowed =~ s/\s//g; - $holdallowed = undef if $holdallowed !~ /^\d+/; if ($branch eq "*") { Koha::CirculationRules->set_rules( diff --git a/t/db_dependent/Koha/CirculationRules.t b/t/db_dependent/Koha/CirculationRules.t index 708168a048..cc5468e7eb 100755 --- a/t/db_dependent/Koha/CirculationRules.t +++ b/t/db_dependent/Koha/CirculationRules.t @@ -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 => 8; + plan tests => 9; $schema->storage->txn_begin; @@ -102,6 +102,31 @@ subtest 'set_rule + get_effective_rule' => sub { is( $rule->rule_value, 1, 'Default rule is returned if there is no rule for this branchcode' ); + subtest 'test rules that cannot be blank' => sub { + plan tests => 3; + foreach my $no_blank_rule ( ('holdallowed','hold_fulfillment_policy','returnbranch') ){ + Koha::CirculationRules->set_rule( + { + branchcode => $branchcode, + itemtype => '*', + rule_name => $no_blank_rule, + rule_value => '', + } + ); + + $rule = Koha::CirculationRules->get_effective_rule( + { + branchcode => $branchcode, + categorycode => undef, + itemtype => undef, + rule_name => $no_blank_rule, + } + ); + is( $rule, undef, 'Rules that cannot be blank are not set when passed blank string' ); + } + }; + + subtest 'test rule matching with different combinations of rule scopes' => sub { my ( $tests, $order ) = prepare_tests_for_rule_scope_combinations( { -- 2.11.0