From 70c19d2d609a57b0cf103f946eec23d26497a97b Mon Sep 17 00:00:00 2001
From: Nick Clemens <nick@bywatersolutions.com>
Date: Wed, 28 Jul 2021 12:23:50 +0000
Subject: [PATCH] Bug 28774: Don't store blank values for rental discount

This patch adds 'can_be_blank => 0' for the rentaldiscount rule to prevent
storing blank values in the database

Additionally, if there is no charge we do not need to check for a discount
and can simply return

To test:
1 - Set rental discount to "" to a rule in circulation rules
2 - Checkout an item that will follow this rule
3 - Check the intranet log:
    [WARN] Argument "" isn't numeric in subtraction (-) at /kohadevbox/koha/C4/Circulation.pm line 3385.
4 - Apply patch and restart all
5 - Update database
6 - Set the rule to "" again
7 - Check the DB, no rule is stored
    SELECT * FROM circulation_rules WHERE rule_name = 'rentaldiscount';
8 - Checkout the item again
9 - No warns in log
---
 C4/Circulation.pm                             | 24 +++++++++----------
 Koha/CirculationRules.pm                      |  1 +
 ...bug_28774_remove_blank_rentaldiscount.perl |  8 +++++++
 3 files changed, 21 insertions(+), 12 deletions(-)
 create mode 100644 installer/data/mysql/atomicupdate/bug_28774_remove_blank_rentaldiscount.perl

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index 10e17c045d..937e66f2a8 100644
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -3372,19 +3372,19 @@ sub GetIssuingCharges {
     if ( my $item_data = $sth->fetchrow_hashref ) {
         $item_type = $item_data->{itemtype};
         $charge    = $item_data->{rentalcharge};
-        # FIXME This should follow CircControl
-        my $branch = C4::Context::mybranch();
-        my $patron = Koha::Patrons->find( $borrowernumber );
-        my $discount = Koha::CirculationRules->get_effective_rule({
-            categorycode => $patron->categorycode,
-            branchcode   => $branch,
-            itemtype     => $item_type,
-            rule_name    => 'rentaldiscount'
-        });
-        if ($discount) {
-            $charge = ( $charge * ( 100 - $discount->rule_value ) ) / 100;
-        }
         if ($charge) {
+            # FIXME This should follow CircControl
+            my $branch = C4::Context::mybranch();
+            my $patron = Koha::Patrons->find( $borrowernumber );
+            my $discount = Koha::CirculationRules->get_effective_rule({
+                categorycode => $patron->categorycode,
+                branchcode   => $branch,
+                itemtype     => $item_type,
+                rule_name    => 'rentaldiscount'
+            });
+            if ($discount) {
+                $charge = ( $charge * ( 100 - $discount->rule_value ) ) / 100;
+            }
             $charge = sprintf '%.2f', $charge; # ensure no fractions of a penny returned
         }
     }
diff --git a/Koha/CirculationRules.pm b/Koha/CirculationRules.pm
index d1c5648ff3..9e23449ac1 100644
--- a/Koha/CirculationRules.pm
+++ b/Koha/CirculationRules.pm
@@ -156,6 +156,7 @@ our $RULE_KINDS = {
     },
     rentaldiscount => {
         scope => [ 'branchcode', 'categorycode', 'itemtype' ],
+        can_be_blank => 0,
     },
     reservesallowed => {
         scope => [ 'branchcode', 'categorycode', 'itemtype' ],
diff --git a/installer/data/mysql/atomicupdate/bug_28774_remove_blank_rentaldiscount.perl b/installer/data/mysql/atomicupdate/bug_28774_remove_blank_rentaldiscount.perl
new file mode 100644
index 0000000000..1fdca905d2
--- /dev/null
+++ b/installer/data/mysql/atomicupdate/bug_28774_remove_blank_rentaldiscount.perl
@@ -0,0 +1,8 @@
+$DBversion = 'XXX';
+if( CheckVersion( $DBversion ) ) {
+    $dbh->do(q{
+        DELETE FROM circulation_rules 
+        WHERE rule_name = 'rentaldiscount' AND rule_value=''
+    });
+    NewVersion( $DBversion, 28774, "Delete blank rental discounts");
+}
-- 
2.20.1