From a8d4fc3b051e4e377b7462f35784b361456d4c9e Mon Sep 17 00:00:00 2001
From: Nick Clemens <nick@bywatersolutions.com>
Date: Thu, 24 Sep 2020 18:21:45 +0000
Subject: [PATCH] Bug 26529: Define some rules as not able to be blank

Currently, the 'Default checkout, hold, and return policy' have a conditional to blank
'holdallowed' when not set, but other rules don't get the same treatment

This patch uses the existing hash that controls the scope of rules and adds a
new parameter 'can_be_blank' Default is assumed to be true, can_be_blank, but if set to '0' then
blank rule values will not be save to the database

To test:
 0 - Set CircControl to 'the library the item is from'
 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 policy: 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 returnbranch, and hold_fulfillment_policy rules exist with blank value
 6 - Set 'All libraries'->'Default checkout, hold and return policy'->'Item floats'
 7 - Checkin an item from the library with blank rules at another library
 8 - It returns home rather than floating
 9 - Apply patch and restart all the things
10 - Reload the page and save the rules as before
11 - Check the DB and note the rules no longer exist
12 - Check in the item, it floats!
13 - prove -v t/db_dependent/Koha/CirculationRules.t

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
---
 Koha/CirculationRules.pm | 5 +++++
 admin/smart-rules.pl     | 2 --
 2 files changed, 5 insertions(+), 2 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(
-- 
2.11.0