From 30af01938cb7a038a81a19f2aa75c94929d29c98 Mon Sep 17 00:00:00 2001
From: Katrin Fischer <katrin.fischer@bsz-bw.de>
Date: Fri, 16 Jun 2023 17:01:52 +0000
Subject: [PATCH] Bug 33028: (follow-up) Rewrite database update
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

This rewrite the database update with some things in mind:

* We now use a positive value list of allowed characters to check
  This makes sure that all of those are recognized:
  1,00
  1.00€
  abc
* Instead of dying after finding one wrong value, we loop through
  all values first, building up an error string
* When we have errors... we die and print the full list of things
  that need fixing.

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
(cherry picked from commit 0199f5fbbb50cbc4656cbeb73c269a7cc6718e55)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 00fc50a1f6af3b94e60b06c7edb0588974d6b1fd)
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
---
 ...ix_calc_fines_fr-currency_21-11-MT39815.pl | 54 ++++++-------------
 1 file changed, 17 insertions(+), 37 deletions(-)

diff --git a/installer/data/mysql/atomicupdate/fix_calc_fines_fr-currency_21-11-MT39815.pl b/installer/data/mysql/atomicupdate/fix_calc_fines_fr-currency_21-11-MT39815.pl
index 637b0f18f4..f3203dc764 100755
--- a/installer/data/mysql/atomicupdate/fix_calc_fines_fr-currency_21-11-MT39815.pl
+++ b/installer/data/mysql/atomicupdate/fix_calc_fines_fr-currency_21-11-MT39815.pl
@@ -1,52 +1,32 @@
 use Modern::Perl;
 
 return {
-    bug_number => "BUG_33028",
-    description =>
-"Fix calculations around fines and values with comma as decimal separator",
+    bug_number => "33028",
+    description => "Fix wrongly formatted values for monetary values in circulation rules",
     up => sub {
         my ($args) = @_;
-        my ( $dbh, $out ) = @$args{qw(dbh out)};
+        my ($dbh, $out) = @$args{qw(dbh out)};
 
         my $rules = $dbh->selectall_arrayref(
-q|select * from circulation_rules where rule_name IN ('fine', 'overduefinescap')|,
+            q|SELECT * FROM circulation_rules WHERE rule_name IN ('fine', 'overduefinescap', 'recall_overdue_fine', 'article_request_fee')|,
             { Slice => {} }
         );
 
         my $query = $dbh->prepare(
-            "UPDATE circulation_rules SET rule_value = ? where id = ?");
+            "UPDATE circulation_rules SET rule_value = ? WHERE id = ?");
 
-        foreach my $rule ( @{$rules} ) {
-            my $rule_id    = $rule->{'id'};
-            my $rule_value = $rule->{'rule_value'};
-            if ( $rule_value =~ /[a-zA-Z]/ ) {
-                die(
-                    sprintf(
-                        'No only numbers in rule id %s ("%s") - fix it before restart this update',
-                        $rule_id, $rule_value
-                    )
-                );
+        my $error;
+        for my $rule ( @{$rules} ) {
+            my $library = defined($rule->{'branchcode'}) ? $rule->{'branchcode'} : "All";
+            my $category = defined($rule->{'categorycode'}) ? $rule->{'categorycode'} : "All";
+            my $itemtype = defined($rule->{'itemtype'}) ? $rule->{'itemtype'} : "All";
+            if ( !( $rule->{'rule_value'} =~ /^[0-9.]*$/ )) {
+                $error .= "Rule ID: $rule->{'id'} ($library-$category-$itemtype) \tRule: $rule->{'rule_name'}\tValue: $rule->{'rule_value'}\n";
             }
-            else {
-                if ( $rule_value =~ /,/ ) {
-                    if ( $rule_value !~ /,.*?,/ ) {
-                        $rule_value =~ s/,/./;
-                        $rule_value =~ s/\.0+$//;
-                        $query->execute( $rule_value, $rule_id );
-                    }
-                    else {
-                        die(
-                            sprintf(
-                                'Many commas in rule id %s ("%s") - fix it before restart this update',
-                                $rule_id, $rule_value
-                            )
-                        );
-                    }
-                }
-            }
-
         }
-        say $out
-        "BUG_33028 - Patch applied";
+        if ( $error ) {
+            die("Circulation rules contain invalid monetary values:\n$error\nPlease fix these before you restart the update.");
+        }
+        say $out "Circulation rules have been validated. All circulation rule values are correctly formatted.";
     },
-  }
+  };
-- 
2.39.5